Re: [PATCH v2 00/14] Sort out fsnotify_nameremove() mess

[Amir Goldstein <amir73il@gmail.com>]

On Thu, May 16, 2019 at 1:26 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Jan,
>
> What do you think will be the best merge strategy for this patch series?
>
> How about sending first 3 prep patches to Linus for applying at the end
> of v5.2 merge window, so individual maintainers can pick up per fs
> patches for the v5.3 development cycle?

Doh! I was going to CC Al on the entire series and forgot.

Al,

We could definitely use your input on this series in general and about
merge strategy in particular.
If you would agree to take the entire series through your tree, that could
make things considerably easier.
After all it, this patch set is more a vfs work than it is an fsnotify work.

Thanks,
Amir.

>
> The following d_delete() call sites have been audited and will no longer
> generate fsnotify event after this series:
>
> drivers/usb/gadget/function/f_fs.c:ffs_epfiles_destroy() - cleanup? (*)
> fs/ceph/dir.c:ceph_unlink() - from vfs_unlink()
> fs/ceph/inode.c:ceph_fill_trace() - invalidate (**)
> fs/configfs/dir.c:detach_groups() - cleanup (*)
> fs/configfs/dir.c:configfs_attach_item() - cleanup (*)
> fs/configfs/dir.c:configfs_attach_group() - cleanup (*)
> fs/efivarfs/file.c:efivarfs_file_write() - invalidate (**)
> fs/fuse/dir.c:fuse_reverse_inval_entry() - invalidate (**)
> fs/nfs/dir.c:nfs_dentry_handle_enoent() - invalidate (**)
> fs/nsfs.c:__ns_get_path() - invalidate (**)
> fs/ocfs2/dlmglue.c:ocfs2_dentry_convert_worker() - invalidate? (**)
> fs/reiserfs/xattr.c:xattr_{unlink,rmdir}() - hidden xattr inode
>
> (*) There are 2 "cleanup" use cases:
>   - Create;init;delte if init failed
>   - Batch delete of files within dir before removing dir
>   Both those cases are not interesting for users that wish to observe
>   configuration changes on pseudo filesystems.  Often, there is already
>   an fsnotify event generated on the directory removal which is what
>   users should find interesting, for example:
>   configfs_unregister_{group,subsystem}().
>
> (**) The different "invalidate" use cases differ, but they all share
>   one thing in common - user is not guarantied to get an event with
>   current kernel.  For example, when a file is deleted remotely on
>   nfs server, nfs client is not guarantied to get an fsnotify delete
>   event.  On current kernel, nfs client could generate an fsnotify
>   delete event if the local entry happens to be in cache and client
>   finds out that entry is deleted on server during another user
>   operation.  Incidentally, this group of use cases is where most of
>   the call sites are with "unstable" d_name, which is the reason for
>   this patch series to begin with.
>
> Thanks,
> Amir.
>
> Changes since v1:
> - Split up per filesystem patches
> - New hook names fsnotify_{unlink,rmdir}()
> - Simplify fsnotify code in separate final patch
>
> Amir Goldstein (14):
>   ASoC: rename functions that pollute the simple_xxx namespace
>   fs: create simple_remove() helper
>   fsnotify: add empty fsnotify_{unlink,rmdir}() hooks
>   fs: convert hypfs to use simple_remove() helper
>   fs: convert qibfs/ipathfs to use simple_remove() helper
>   fs: convert debugfs to use simple_remove() helper
>   fs: convert tracefs to use simple_remove() helper
>   fs: convert rpc_pipefs to use simple_remove() helper
>   fs: convert apparmorfs to use simple_remove() helper
>   fsnotify: call fsnotify_rmdir() hook from btrfs
>   fsnotify: call fsnotify_rmdir() hook from configfs
>   fsnotify: call fsnotify_unlink() hook from devpts
>   fsnotify: move fsnotify_nameremove() hook out of d_delete()
>   fsnotify: get rid of fsnotify_nameremove()
>
>  arch/s390/hypfs/inode.c            |  9 ++-----
>  drivers/infiniband/hw/qib/qib_fs.c |  3 +--
>  fs/afs/dir_silly.c                 |  5 ----
>  fs/btrfs/ioctl.c                   |  4 ++-
>  fs/configfs/dir.c                  |  3 +++
>  fs/dcache.c                        |  2 --
>  fs/debugfs/inode.c                 | 20 +++------------
>  fs/devpts/inode.c                  |  1 +
>  fs/libfs.c                         | 32 +++++++++++++++++++++++
>  fs/namei.c                         |  2 ++
>  fs/nfs/unlink.c                    |  6 -----
>  fs/notify/fsnotify.c               | 41 ------------------------------
>  fs/tracefs/inode.c                 | 23 +++--------------
>  include/linux/fs.h                 |  1 +
>  include/linux/fsnotify.h           | 26 +++++++++++++++++++
>  include/linux/fsnotify_backend.h   |  4 ---
>  net/sunrpc/rpc_pipe.c              | 16 ++----------
>  security/apparmor/apparmorfs.c     |  6 +----
>  sound/soc/generic/simple-card.c    |  8 +++---
>  19 files changed, 86 insertions(+), 126 deletions(-)
>
> --
> 2.17.1
>