On Sun 11-06-23 15:24:29, Amir Goldstein wrote: > fsnotify_open() hook is called only from high level system calls > context and not called for the very many helpers to open files. > > This may makes sense for many of the special file open cases, but it is > inconsistent with fsnotify_close() hook that is called for every last > fput() of on a file object with FMODE_OPENED. > > As a result, it is possible to observe ACCESS, MODIFY and CLOSE events > without ever observing an OPEN event. > > Fix this inconsistency by replacing all the fsnotify_open() hooks with > a single hook inside do_dentry_open(). > > If there are special cases that would like to opt-out of the possible > overhead of fsnotify() call in fsnotify_open(), they would probably also > want to avoid the overhead of fsnotify() call in the rest of the fsnotify > hooks, so they should be opening that file with the __FMODE_NONOTIFY flag. > > However, in the majority of those cases, the s_fsnotify_connectors > optimization in fsnotify_parent() would be sufficient to avoid the > overhead of fsnotify() call anyway. > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> Thanks! The cleanup looks nice so I've applied it with the typo fixup from Christian. I have a slight worry this might break something subtle somewhere but after searching for a while I didn't find anything and the machine boots and ltp tests pass so it's worth a try :) Honza > --- > > Jan, > > I found out about this problem when I tested the work to remove > FMODE_NONOTIFY from overlayfs internal files - even after I enabled > notifications on the underlying fs, the LTS tests [2] did not observe > the OPEN events. > > Because this change is independent of the ovl work and has implications > on other subsystems as well (e.g. cachefiles), I think it is better > if the change came through your tree. > > This change has a potential to regress some micro-benchmarks, so if > you could queue it up for soaking in linux-next that would be great. > > Thanks, > Amir. > > > [1] https://lore.kernel.org/linux-fsdevel/20230609073239.957184-1-amir73il@xxxxxxxxx/ > [2] https://github.com/amir73il/ltp/commits/ovl_encode_fid > > fs/exec.c | 5 ----- > fs/fhandle.c | 1 - > fs/open.c | 6 +++++- > io_uring/openclose.c | 1 - > 4 files changed, 5 insertions(+), 8 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index a466e797c8e2..238473de1ec5 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -152,8 +152,6 @@ SYSCALL_DEFINE1(uselib, const char __user *, library) > path_noexec(&file->f_path))) > goto exit; > > - fsnotify_open(file); > - > error = -ENOEXEC; > > read_lock(&binfmt_lock); > @@ -934,9 +932,6 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) > if (err) > goto exit; > > - if (name->name[0] != '\0') > - fsnotify_open(file); > - > out: > return file; > > diff --git a/fs/fhandle.c b/fs/fhandle.c > index fd0d6a3b3699..6ea8d35a9382 100644 > --- a/fs/fhandle.c > +++ b/fs/fhandle.c > @@ -242,7 +242,6 @@ static long do_handle_open(int mountdirfd, struct file_handle __user *ufh, > retval = PTR_ERR(file); > } else { > retval = fd; > - fsnotify_open(file); > fd_install(fd, file); > } > path_put(&path); > diff --git a/fs/open.c b/fs/open.c > index 4478adcc4f3a..81444ebf6091 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -969,6 +969,11 @@ static int do_dentry_open(struct file *f, > } > } > > + /* > + * Once we return a file with FMODE_OPENED, __fput() will call > + * fsnotify_close(), so we need fsnotify_open() here for symetry. > + */ > + fsnotify_open(f); > return 0; > > cleanup_all: > @@ -1358,7 +1363,6 @@ static long do_sys_openat2(int dfd, const char __user *filename, > put_unused_fd(fd); > fd = PTR_ERR(f); > } else { > - fsnotify_open(f); > fd_install(fd, f); > } > } > diff --git a/io_uring/openclose.c b/io_uring/openclose.c > index a1b98c81a52d..10ca57f5bd24 100644 > --- a/io_uring/openclose.c > +++ b/io_uring/openclose.c > @@ -150,7 +150,6 @@ int io_openat2(struct io_kiocb *req, unsigned int issue_flags) > > if ((issue_flags & IO_URING_F_NONBLOCK) && !nonblock_set) > file->f_flags &= ~O_NONBLOCK; > - fsnotify_open(file); > > if (!fixed) > fd_install(ret, file); > -- > 2.34.1 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
