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

Al Viro
2005-09-21T01:02:00Z

On Tue, Sep 20, 2005 at 06:53:34PM -0400, John McCutchan wrote:
> 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?

Playing with references to struct inode means playing dirty tricks
behind the filesystem's back. Doing that in a way that really changes
inode lifetime means asking for trouble. Combined with a dirty trick
*already* pulled by sys_unlink() to postpone the final iput until after
we unlock the parent, it means breakage (and aforementioned dirty trick
took some rather interesting logics to compensate for in the first place).

Moreover, your suggestion would do that to _everyone_, whether they use
inotify or not. NAK.

> 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);
> }

Assumes that filesystem treats ->i_nlink on final iput() in usual way.
It doesn't have to.

BTW, what happens if one uses inotify on procfs? Or sysfs, for that matter?
Fundamental problem with that sucker is that you are playing games with
lifetime rules of inodes in a way that might be OK for some filesystems,
but violates a lot of assumptions made by other...

BTW^2, what guarantees that inotify_unmount_inodes() will not happen while we
are in inotify_release()? That would happily keep watch refcount bumped,
so it would outlive inotify_unmount_inodes(). Sure, it would be dropped.
And call iput() on a pinned inode that had outlived the umount(). Oops...