On Tue 18-01-22 00:02:38, Amir Goldstein wrote: > > > 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. Agreed. I'm for simplicity as well. > > 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()? No, that's what I wanted to suggest :) ihold() should be pretty cheap as, as you have noticed, the cacheline should be dirty and hot anyway. I think this is by far cheaper than trying to find out whether the event will be sent or not. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
