On Thu 03-12-20 12:41:41, Amir Goldstein wrote:
> 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...
OK, fair enough. I didn't think about this case.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
