Re: Potential regression after fsnotify_nameremove() rework in 5.3

[Amir Goldstein <amir73il@gmail.com>]

> > One possibility I can see is: Add fsnotify primitive to create the event,
> > just not queue it in the notification queue yet (essentially we would
> > cut-short the event handling before calling fsnotify_insert_event() /
> > fsnotify_add_event()), only return it. Then another primitive would be for
> > queueing already prepared event. Then the sequence for unlink would be:
> >
> >         LIST_HEAD(event_list);
> >
> >         fsnotify_events_prepare(&event_list, ...);
> >         d_delete(dentry);
> >         fsnotify_events_report(&event_list);
> >
> > And we can optionally wrap this inside d_delete_notify() to make it easier
> > on the callers. What do you think?
> >
>
> I think it sounds like the "correct" design, but also sounds like a
> big change that
> is not so practical for backporting.
>
> Given that this is a regression that goes way back, backportability
> plays a role.
> Also, a big change like this needs developer time, which I myself don't have
> at the moment.
>
> For a simpler backportable solution, instead of preparing the event
> perhaps it is enough that we ihold() the inode until after fsnotify_unlink()
> and pass it as an argument very similar to fsnotify_link().
>
> The question is how to ihold() the inode only if we are going to queue
> an IN_DELETE event? Maybe send an internal FS_PRE_DELETE
> event?
>

Actually, seeing that do_unlinkat() already iholds the inode outside
vfs_unlink()
anyway, is it so bad that vfs_unlink() will elevate refcount as well, so we can
call fsnotify_unlink() with the inode arg after d_delete()?

Thanks,
Amir.