Re: [patch] stop inotify from sending random DELETE_SELF event under load

[Al Viro <viro@ftp.linux.org.uk>]

On Mon, Sep 19, 2005 at 06:37:43PM -0700, Linus Torvalds wrote:
> 
> 
> On Mon, 19 Sep 2005, John McCutchan wrote:
> > 
> > Below is a patch that fixes the random DELETE_SELF events when the
> > system is under load. The problem is that the DELETE_SELF event is sent
> > from dentry_iput, which is called in two code paths,
> > 
> > 1) When a dentry is being deleted
> > 2) When the dcache is being pruned.
> 
> No no.
> 
> The problem is that you put the "fsnotify_inoderemove(inode);" in the 
> wrong place, and I and Al never noticed.
> 
> iput() doesn't have anything to do with delete at all, and adding a flag 
> to it would be wrong. The inode may stay around _after_ the unlink() for 
> as long as it has users (or much longer, if you have hardlinks ;). 
> 
> You should probably move the "fsnotify_inoderemove(inode);" call into
> generic_delete_inode() instead, just after "security_inode_delete()".  No
> new flags, just a new place.
> 
> (Oh, I think you need to add it to "hugetlbfs_delete_inode()" too).
> 
> There's still a potential problem there: some network filesystems seem to
> use "generic_delete_inode()" as their "drop_inode" thing. Which may mean 
> that you get spurious delete messages when the reference is dropped. I 
> don't see how to avoid that, though - we fundamentally don't _know_ when 
> the inode actually gets deleted.
> 
> Al, do you have any comments? Anything stupid I missed?

One fundamentally stupid thing is exposing to userland events that
are none of its fscking business.  Link removal - sure.  Last link
removal - perhaps, but that's not obvious; in any case it should be
tied to notification of link removal.  But inode getting freed or
last dentry going away is none of the userland concern.  

IOW, let the notification of link removal check ->i_nlink and add
"that was the last one" event if it had hit zero.  Or, better yet,
pass i_mode and i_nlink in the event itself and let the recepient
do obvious checks.