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

John McCutchan
2005-09-20T22:53:07Z

On Tue, 2005-09-20 at 12:37 -0700, Linus Torvalds wrote:
>
> On Tue, 20 Sep 2005, Al Viro wrote:
> > >
> > > I really think that the patch I sent out yesterday is as good as it gets.
> > > If you want immediate notification, you should ask for notification about
> > > name changes in a particular directory. IN_DELETE_SELF notification on a
> > > file simple is _not_ going to be immediate.
> >
> > But then it's too early. Note that with your patch we still get removal
> > of _any_ link to our inode (even though it's alive and well and we'd never
> > heard about the sodding link in the first place) terminating all events
> > on it.
>
> Yes. What is in the current 2.6.14-rc2 tree doesn't do that. It considers
> inodes "global". But it won't work reliably on networked filesystems, I
> think.
>
> Anyway, I do believe that IN_DELETE_SELF is stupid, but that you migth
> re-arm it if you get it.

Is there some reason we can't just do this from vfs_unlink

inode = dentry->inode;
iget (inode);
d_delete (dentry);
fsnotify_inoderemove (inode);
iput (inode);

This would allow us to have immediate event notification, and avoid a
race with the inode going away, right?

I think the path below will make link handling as good as it can get, by
sending IN_DELETE_SELF every time inode->i_nlink goes down, and when
inode->i_nlink == 0, send the IN_IGNORE event. Also, it stuffs
inode->i_nlink into the cookie giving user space a clue about the status
of the inode.

Index: linux/include/linux/fsnotify.h
===================================================================
--- linux.orig/include/linux/fsnotify.h 2005-08-28 19:41:01.000000000 -0400
+++ linux/include/linux/fsnotify.h 2005-09-20 18:46:15.000000000 -0400
@@ -63,8 +63,9 @@
*/
static inline void fsnotify_inoderemove(struct inode *inode)
{
- inotify_inode_queue_event(inode, IN_DELETE_SELF, 0, NULL);
- inotify_inode_is_dead(inode);
+ inotify_inode_queue_event(inode, IN_DELETE_SELF, inode->i_nlink, NULL);
+ if (inode->i_nlink == 0)
+ inotify_inode_is_dead(inode);
}

/*