diff mbox series

[v3,08/10] configfs: call fsnotify_rmdir() hook

Message ID 20190526143411.11244-9-amir73il@gmail.com
State New, archived
Headers show
Series Sort out fsnotify_nameremove() mess | expand

Commit Message

Amir Goldstein May 26, 2019, 2:34 p.m. UTC
This will allow generating fsnotify delete events on unregister
of group/subsystem after the fsnotify_nameremove() hook is removed
from d_delete().

The rest of the d_delete() calls from this filesystem are either
called recursively from within debugfs_unregister_{group,subsystem},
called from a vfs function that already has delete hooks or are
called from shutdown/cleanup code.

Cc: Joel Becker <jlbec@evilplan.org>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/configfs/dir.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Amir Goldstein May 30, 2019, 6:06 a.m. UTC | #1
On Sun, May 26, 2019 at 5:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> This will allow generating fsnotify delete events on unregister
> of group/subsystem after the fsnotify_nameremove() hook is removed
> from d_delete().
>
> The rest of the d_delete() calls from this filesystem are either
> called recursively from within debugfs_unregister_{group,subsystem},
> called from a vfs function that already has delete hooks or are
> called from shutdown/cleanup code.
>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Christoph Hellwig <hch@lst.de>

Hi Christoph and Joel,

Per Christoph's request, I cc'ed you guys on the entire patch series
for context,
so my discussion with Greg [1] about the special status of configfs in
this patch set
should already be somewhere in your mailboxes...

Could I ask you to provide an ACK on this patch and on the chosen
policy. To recap:
Before patch set:
1. User gets create/delete events when files/dirs created/removed via vfs_*()
2. User does *not* get create events when files/dirs created via
debugfs_register_*()
3. User *does* get delete events when files/dirs removed via
debugfs_unregister_*()

After patch set:
1. No change
2. No change
3. User will get delete events only on the root group/subsystem dir
when tree is removed via debugfs_unregister_*()

For symmetry, we could also add create events for  root group/subsystem dir
when tree is created via debugfs_unregister_*(), but that would be a
followup patch.
For users though, it may be that delete events are more important than
create events
(i.e. for user cleanup tasks).

Thanks,
Amir.

[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxjyg5AVPrcR4bPm4zMY9BKmgV8g7TAuH--cfKNJv8pRYQ@mail.gmail.com/

> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/configfs/dir.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index 5e7932d668ab..ba17881a8d84 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -27,6 +27,7 @@
>  #undef DEBUG
>
>  #include <linux/fs.h>
> +#include <linux/fsnotify.h>
>  #include <linux/mount.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> @@ -1804,6 +1805,7 @@ void configfs_unregister_group(struct config_group *group)
>         configfs_detach_group(&group->cg_item);
>         d_inode(dentry)->i_flags |= S_DEAD;
>         dont_mount(dentry);
> +       fsnotify_rmdir(d_inode(parent), dentry);
>         d_delete(dentry);
>         inode_unlock(d_inode(parent));
>
> @@ -1932,6 +1934,7 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys)
>         configfs_detach_group(&group->cg_item);
>         d_inode(dentry)->i_flags |= S_DEAD;
>         dont_mount(dentry);
> +       fsnotify_rmdir(d_inode(root), dentry);
>         inode_unlock(d_inode(dentry));
>
>         d_delete(dentry);
> --
> 2.17.1
>
Amir Goldstein June 13, 2019, 4:57 p.m. UTC | #2
On Thu, May 30, 2019 at 9:06 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Sun, May 26, 2019 at 5:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > This will allow generating fsnotify delete events on unregister
> > of group/subsystem after the fsnotify_nameremove() hook is removed
> > from d_delete().
> >
> > The rest of the d_delete() calls from this filesystem are either
> > called recursively from within debugfs_unregister_{group,subsystem},
> > called from a vfs function that already has delete hooks or are
> > called from shutdown/cleanup code.
> >
> > Cc: Joel Becker <jlbec@evilplan.org>
> > Cc: Christoph Hellwig <hch@lst.de>
>
> Hi Christoph and Joel,
>
> Per Christoph's request, I cc'ed you guys on the entire patch series
> for context,
> so my discussion with Greg [1] about the special status of configfs in
> this patch set
> should already be somewhere in your mailboxes...
>
> Could I ask you to provide an ACK on this patch and on the chosen

PING.
Any comment from configfs maintainers?

Jan,

We can also drop this patch from the series given that configs has no
explicit fsnotify create hooks. If users come shouting, we can add
the missing create and delete hooks.

Thanks,
Amir.

> policy. To recap:
> Before patch set:
> 1. User gets create/delete events when files/dirs created/removed via vfs_*()
> 2. User does *not* get create events when files/dirs created via
> debugfs_register_*()
> 3. User *does* get delete events when files/dirs removed via
> debugfs_unregister_*()
>
> After patch set:
> 1. No change
> 2. No change
> 3. User will get delete events only on the root group/subsystem dir
> when tree is removed via debugfs_unregister_*()
>
> For symmetry, we could also add create events for  root group/subsystem dir
> when tree is created via debugfs_unregister_*(), but that would be a
> followup patch.
> For users though, it may be that delete events are more important than
> create events
> (i.e. for user cleanup tasks).
>
> Thanks,
> Amir.
>
> [1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxjyg5AVPrcR4bPm4zMY9BKmgV8g7TAuH--cfKNJv8pRYQ@mail.gmail.com/
>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/configfs/dir.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> > index 5e7932d668ab..ba17881a8d84 100644
> > --- a/fs/configfs/dir.c
> > +++ b/fs/configfs/dir.c
> > @@ -27,6 +27,7 @@
> >  #undef DEBUG
> >
> >  #include <linux/fs.h>
> > +#include <linux/fsnotify.h>
> >  #include <linux/mount.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> > @@ -1804,6 +1805,7 @@ void configfs_unregister_group(struct config_group *group)
> >         configfs_detach_group(&group->cg_item);
> >         d_inode(dentry)->i_flags |= S_DEAD;
> >         dont_mount(dentry);
> > +       fsnotify_rmdir(d_inode(parent), dentry);
> >         d_delete(dentry);
> >         inode_unlock(d_inode(parent));
> >
> > @@ -1932,6 +1934,7 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys)
> >         configfs_detach_group(&group->cg_item);
> >         d_inode(dentry)->i_flags |= S_DEAD;
> >         dont_mount(dentry);
> > +       fsnotify_rmdir(d_inode(root), dentry);
> >         inode_unlock(d_inode(dentry));
> >
> >         d_delete(dentry);
> > --
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 5e7932d668ab..ba17881a8d84 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -27,6 +27,7 @@ 
 #undef DEBUG
 
 #include <linux/fs.h>
+#include <linux/fsnotify.h>
 #include <linux/mount.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -1804,6 +1805,7 @@  void configfs_unregister_group(struct config_group *group)
 	configfs_detach_group(&group->cg_item);
 	d_inode(dentry)->i_flags |= S_DEAD;
 	dont_mount(dentry);
+	fsnotify_rmdir(d_inode(parent), dentry);
 	d_delete(dentry);
 	inode_unlock(d_inode(parent));
 
@@ -1932,6 +1934,7 @@  void configfs_unregister_subsystem(struct configfs_subsystem *subsys)
 	configfs_detach_group(&group->cg_item);
 	d_inode(dentry)->i_flags |= S_DEAD;
 	dont_mount(dentry);
+	fsnotify_rmdir(d_inode(root), dentry);
 	inode_unlock(d_inode(dentry));
 
 	d_delete(dentry);