Andrew Morton wrote: > John McCutchan wrote: > >> > > @@ -538,7 +537,7 @@ >> > > struct dentry *parent; >> > > struct inode *inode; >> > > >> > > - if (!atomic_read (&inotify_watches)) >> > > + if (!atomic_read (&dentry->d_sb->s_inotify_watches)) >> > > return; >> > > >> > >> > What happens here if we're watching a mountpoint - the parent is on a >> > different fs? >> >> There are four cases to consider here. >> >> Case 1: parent fs watched and child fs watched >> correct results >> Case 2: parent fs watched and child fs not watched >> We may not deliver an event that should be delivered. >> Case 3: parent fs not watched and child fs watched >> We take d_lock when we don't need to >> Case 4: parent fs not watched and child fs not watched >> correct results >> >> Case 2 screws us. We have to take the lock to even look at the parent's >> dentry->d_sb->s_inotify_watches. I don't know of a way around this one. > > > Yeah. There are a lot of "screw"s in this thread. > > I wonder if RCU can save us - if we do an rcu_read_lock() we at least know > that the dentries won't get deallocated. Then we can take a look at > d_parent (which might not be the parent any more). Once in a million years > we might send a false event or miss sending an event, depending on where > our dentry suddenly got moved to. Not very nice, but at least it won't > oops. > > (hopefully cc's Dipankar) I saw this problem when testing my lockless pagecache a while back. Attached is a first implementation of what was my idea then of how to solve it... note it is pretty rough and I never got around to doing much testing of it. Basically: moves work out of inotify event time and to inotify attach /detach time while staying out of the core VFS. -- SUSE Labs, Novell Inc.