| Message ID | 20190526143411.11244-9-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Sort out fsnotify_nameremove() mess | expand |
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 >
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 --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);
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(+)