Patchwork fs: allow d_instantiate to be called with negative parent dentry

login
register
mail settings
Submitter Jeremy Kerr
Date June 29, 2009, 6:31 a.m.
Message ID <1246257118.265649.273423097532.1.gpush@pingu>
Download mbox | patch
Permalink /patch/29261/
State Accepted
Headers show

Comments

Jeremy Kerr - June 29, 2009, 6:31 a.m.
The new fsnotify infrastructure (starting at 90586523) causes an oops in
spufs, where we populate a directory with files before instantiating the
directory itself. The new changes seem to have introduced an assumption
that a dentry's parent will be positive when instantiating.

This change makes it once again possible to d_instantiate a dentry
with a negative parent, and brings __fsnotify_d_instantiate() into
line with inotify_d_instantiate(), which already has this NULL check.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
Apologies for the resend, fixed lkml address

---
 include/linux/fsnotify_backend.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Eric Paris - June 29, 2009, 1:45 p.m.
On Mon, 2009-06-29 at 14:31 +0800, Jeremy Kerr wrote:
> The new fsnotify infrastructure (starting at 90586523) causes an oops in
> spufs, where we populate a directory with files before instantiating the
> directory itself. The new changes seem to have introduced an assumption
> that a dentry's parent will be positive when instantiating.
> 
> This change makes it once again possible to d_instantiate a dentry
> with a negative parent, and brings __fsnotify_d_instantiate() into
> line with inotify_d_instantiate(), which already has this NULL check.
> 
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

This problem was introduced based on a private off list comment from Al
Viro back on Feb 17th when he was reviewing my code:

fsnotify_d_instantiate(): check for NULL inode in parent?  Really?
BTW, where's the codepath without that stuff?  I mean, for sane
boxen that have *notify configured out.  Idiotify is at least
configurable away...

>From that comment I dropped the check for dentry->d_parent->d_inode.
The old behavior (which inotify had) is exactly what you propose and
would cause no problems for anything.  I'll leave it up to Al if he
wants to argue that the spufs logic is illegal but I'll put this patch
into my tree and send it toward Linus.

-Eric

Patch

diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 44848aa..6c3de99 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -280,7 +280,7 @@  static inline void __fsnotify_update_dcache_flags(struct dentry *dentry)
 	assert_spin_locked(&dentry->d_lock);
 
 	parent = dentry->d_parent;
-	if (fsnotify_inode_watches_children(parent->d_inode))
+	if (parent->d_inode && fsnotify_inode_watches_children(parent->d_inode))
 		dentry->d_flags |= DCACHE_FSNOTIFY_PARENT_WATCHED;
 	else
 		dentry->d_flags &= ~DCACHE_FSNOTIFY_PARENT_WATCHED;