[RFC,0/4] Sort out fsnotify_nameremove() mess

Message

Amir Goldstein May 14, 2019, 10:18 p.m. UTC

Jan,

I started out working on your suggestion [1] of annotating
simple_unlink/rmdir_notify() callers when I realized we could
do better (IMO).

Please see this RFC. If you like the idea, I can split patches 3-4
to per filesystem patches and a final patch to make the switch from
fsnotify_nameremove() to fsnotify_remove().

I audited all the d_delete() call sites that will NOT generate
fsnotify events after these changes and noted to myself why that
makes sense.  I will include those notes in next posting if this
works out for you.

Note that configfs got a special treatment, because its helpers
that call simple_unlink/rmdir() are called from both vfs_XXX code
path and non vfs_XXX code path.

Thanks,
Amir.

[1] https://lore.kernel.org/linux-fsdevel/[e-mail]/

Amir Goldstein (4):
  fs: create simple_remove() helper
  fsnotify: add empty fsnotify_remove() hook
  fs: convert filesystems to use simple_remove() helper
  fsnotify: move fsnotify_nameremove() hook out of d_delete()

 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                         | 25 ++++++++++++++++++
 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           | 18 +++++++++++++
 include/linux/fsnotify_backend.h   |  4 ---
 net/sunrpc/rpc_pipe.c              | 16 ++----------
 security/apparmor/apparmorfs.c     |  6 +----
 18 files changed, 67 insertions(+), 122 deletions(-)

Comments

Jan Kara May 15, 2019, 8:36 a.m. UTC | #1

Hi Amir!

On Wed 15-05-19 01:18:57, Amir Goldstein wrote:
> I started out working on your suggestion [1] of annotating
> simple_unlink/rmdir_notify() callers when I realized we could
> do better (IMO).
> 
> Please see this RFC. If you like the idea, I can split patches 3-4
> to per filesystem patches and a final patch to make the switch from
> fsnotify_nameremove() to fsnotify_remove().
> 
> I audited all the d_delete() call sites that will NOT generate
> fsnotify events after these changes and noted to myself why that
> makes sense.  I will include those notes in next posting if this
> works out for you.
> 
> Note that configfs got a special treatment, because its helpers
> that call simple_unlink/rmdir() are called from both vfs_XXX code
> path and non vfs_XXX code path.

Thanks for working on this! I like the series. I would structure it
somewhat differently - see my comments to individual patches - but the end
result looks OK to me. And yes, notes about d_delete() call sites would
make it easier to verify that we didn't miss anything :) so I'd be happy if
you posted them.

								Honza

> 
> Thanks,
> Amir.
> 
> [1] https://lore.kernel.org/linux-fsdevel/[e-mail]/
> 
> Amir Goldstein (4):
>   fs: create simple_remove() helper
>   fsnotify: add empty fsnotify_remove() hook
>   fs: convert filesystems to use simple_remove() helper
>   fsnotify: move fsnotify_nameremove() hook out of d_delete()
> 
>  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                         | 25 ++++++++++++++++++
>  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           | 18 +++++++++++++
>  include/linux/fsnotify_backend.h   |  4 ---
>  net/sunrpc/rpc_pipe.c              | 16 ++----------
>  security/apparmor/apparmorfs.c     |  6 +----
>  18 files changed, 67 insertions(+), 122 deletions(-)
> 
> -- 
> 2.17.1
>