2009-06-29 20:02:01

by Eric Paris

[permalink] [raw]
Subject: [GIT PULL] notification tree - fsnotify assumes incorrectly positive parent dentry

Linus please consider pulling from the notification tree. There is one
patch in the tree which should fix an issue in which fsnotify assumed
that dentry->d_parent->d_inode != NULL. This turns out to be false for
spufs and causes a panic.

-Eric

The following changes since commit 52989765629e7d182b4f146050ebba0abf2cb0b7:
Linus Torvalds (1):
Merge git://git.kernel.org/.../davem/net-2.6

are available in the git repository at:

git://git.infradead.org/users/eparis/notify.git for-linus

Jeremy Kerr (1):
fs: allow d_instantiate to be called with negative parent dentry

include/linux/fsnotify_backend.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)


2009-06-29 20:10:17

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] notification tree - fsnotify assumes incorrectly positive parent dentry



On Mon, 29 Jun 2009, Eric Paris wrote:
>
> Linus please consider pulling from the notification tree. There is one
> patch in the tree which should fix an issue in which fsnotify assumed
> that dentry->d_parent->d_inode != NULL. This turns out to be false for
> spufs and causes a panic.

Hmm. It does sound like a reasonable assumption, though. Maybe spufs
should be fixed to have an inode for all directories?

A NULL d_inode means that something is a negative dentry, and a negative
dentry shouldn't have children.

Added a few ppc people to the cc.

I'll pull the fix, but I really think it sounds like spufs is doing
something wrong here.

Linus

2009-06-30 01:02:46

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] notification tree - fsnotify assumes incorrectly positive parent dentry



On Tue, 30 Jun 2009, Jeremy Kerr wrote:
>
> > Hmm. It does sound like a reasonable assumption, though. Maybe spufs
> > should be fixed to have an inode for all directories?
>
> We have inodes for all directories, it's just the order which we set
> things up. When a new 'spu context' (ie, a directory with a bunch of
> files) is created, we add the parent dentry, populate it with files
> (positive dentries), then instantiate the parent.
>
> There's no specific need to do it in this order, it just makes the code
> a little simpler - we just 'stitch the parent in' once everything else
> has completed successfully, so less stuff to do in the error path.
>
> > A NULL d_inode means that something is a negative dentry, and a
> > negative dentry shouldn't have children.
>
> OK. If this is a general invariant, then I'll get a change to spufs
> going to do the setup in the right order.

Hmm. I doubt it matters a ton, but if it's easy to change it so that it
populates the parent inode first, I think that would be a good thing.
Thanks,

Linus

2009-06-30 00:49:53

by Jeremy Kerr

[permalink] [raw]
Subject: Re: [GIT PULL] notification tree - fsnotify assumes incorrectly positive parent dentry

Linus,

> Hmm. It does sound like a reasonable assumption, though. Maybe spufs
> should be fixed to have an inode for all directories?

We have inodes for all directories, it's just the order which we set
things up. When a new 'spu context' (ie, a directory with a bunch of
files) is created, we add the parent dentry, populate it with files
(positive dentries), then instantiate the parent.

There's no specific need to do it in this order, it just makes the code
a little simpler - we just 'stitch the parent in' once everything else
has completed successfully, so less stuff to do in the error path.

> A NULL d_inode means that something is a negative dentry, and a
> negative dentry shouldn't have children.

OK. If this is a general invariant, then I'll get a change to spufs
going to do the setup in the right order.

Cheers,


Jeremy