On Thu, Dec 3, 2020 at 11:51 AM Jan Kara <jack@xxxxxxx> wrote:
>
> On Wed 02-12-20 14:07:07, Amir Goldstein wrote:
> > The handle_inode_event() interface was added as (quoting comment):
> > "a simple variant of handle_event() for groups that only have inode
> > marks and don't have ignore mask".
> >
> > In other words, all backends except fanotify. The inotify backend
> > also falls under this category, but because it required extra arguments
> > it was left out of the initial pass of backends conversion to the
> > simple interface.
> >
> > This results in code duplication between the generic helper
> > fsnotify_handle_event() and the inotify_handle_event() callback
> > which also happen to be buggy code.
> >
> > Generalize the handle_inode_event() arguments and add the check for
> > FS_EXCL_UNLINK flag to the generic helper, so inotify backend could
> > be converted to use the simple interface.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> The patch looks good to me. Just one curious question below.
>
> > +static int fsnotify_handle_inode_event(struct fsnotify_group *group,
> > + struct fsnotify_mark *inode_mark,
> > + u32 mask, const void *data, int data_type,
> > + struct inode *dir, const struct qstr *name,
> > + u32 cookie)
> > +{
> > + const struct path *path = fsnotify_data_path(data, data_type);
> > + struct inode *inode = fsnotify_data_inode(data, data_type);
> > + const struct fsnotify_ops *ops = group->ops;
> > +
> > + if (WARN_ON_ONCE(!ops->handle_inode_event))
> > + return 0;
> > +
> > + if ((inode_mark->mask & FS_EXCL_UNLINK) &&
> > + path && d_unlinked(path->dentry))
> > + return 0;
>
> When I was looking at this condition I was wondering why do we check
> d_unlinked() and not inode->i_nlink? When is there a difference?
When a hardlink has been unlinked.
inotify gets the filename and it doesn't want to get events with unlinked
names (although another name could still be linked) I suppose...
Thanks,
Amir.
