On Wed, Jan 15, 2020 at 2:09 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > On Wed, Jan 15, 2020 at 02:03:35AM +0200, Amir Goldstein wrote: > > > Only problem I forgot about is that I need to propagate name_snapshot > > into fsnotify_move() instead of qstr (in order to snapshot it). > > That means I will need to snapshot also new_dentry in vfs_rename(), so > > I have a name_snapshot to pass into the RENAME_EXCHANGE > > fsnotify_move() hook. > > > > My current patch passes the cute &old_dentry->d_name_snap abstract > > to fsnotify_move(). > > > > What shall I do about that? > > > > take_dentry_name_snapshot(&new_name, new_dentry); > > ??? > > Wait a sec... How long is that thing going to be using the snapshot? > And I bloody well hope that this is _not_ going to be unconditional - > having each rename() pay for *notify is a bad idea, for obvious > reasons. Details, please... Well, if nobody asked to get notifications about entry name changes, the snapshot is going to stay alive until fsnotify(), same as old_name with current code. The addition of new_name snapshot is only needed for the RENAME_EXCHANGE case and will be conditional to the flag. If anyone asked for notifications about entry name changes via the new FAN_DIR_MODIFY [1][2] event, then event will be queued with a clone of the name_snapshot that will live until user reads the event. This is similar to the way that fanotify events keep the dentry refcount elevated until user reads an event (for reporting object fd) The advantage of snapshotting the name instead of copy, beyond the obvious is that events can be allocated from a dedicated kmem_cache and avoid spurious small variable sized allocations. An argument in favor of allocating a variable size buffer by fanotify to copy the name is that fanotify accounts event allocations to the group->memcg of the watcher, so the risk of DoS on the system gets smaller. One of the less advertised improvements of FAN_REPORT_FID in v5.1 is that events do not keep an elevated refcount on dentries, the way that they do with 'legacy' fanotify (reporting fd as object id). FAN_REPORT_FID_NAME will be a little step back in that sense if it holds a reference on the name. As you can see, I am not fixed into snapshot or copy the name. Looking for opinions is one of the reasons I posted this early bird. Thanks, Amir. [1] https://lore.kernel.org/linux-fsdevel/20200108120422.GD20521@xxxxxxxxxxxxxx/ [2] https://github.com/amir73il/linux/commits/fanotify_name
