| Message ID | 20240929122831.92515-1-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] vfs: Add a sysctl for automated deletion of dentry | expand |
On Sun, 29 Sept 2024 at 05:29, Yafang Shao <laoar.shao@gmail.com> wrote: > > This patch seeks to reintroduce the concept conditionally, where the > associated dentry is deleted only when the user explicitly opts for it > during file removal. A new sysctl fs.automated_deletion_of_dentry is > added for this purpose. Its default value is set to 0. I look at this patch, and part of me goes "I think we should make it a mount option", but at the same time I'm not sure it's worth the extra complexity, since this is such a special case. So Ack. While I'm not convinced a sysctl is the way to go, I also don't think it's worth bikeshedding any further, at least until we have other cases that care. Christian, I assume this will come through you? Or should I just pick it up directly? Linus
On Sun, Sep 29, 2024 at 10:07:31AM GMT, Linus Torvalds wrote: > On Sun, 29 Sept 2024 at 05:29, Yafang Shao <laoar.shao@gmail.com> wrote: > > > > This patch seeks to reintroduce the concept conditionally, where the > > associated dentry is deleted only when the user explicitly opts for it > > during file removal. A new sysctl fs.automated_deletion_of_dentry is > > added for this purpose. Its default value is set to 0. > > I look at this patch, and part of me goes "I think we should make it a > mount option", but at the same time I'm not sure it's worth the extra > complexity, since this is such a special case. > > So Ack. While I'm not convinced a sysctl is the way to go, I also > don't think it's worth bikeshedding any further, at least until we > have other cases that care. > > Christian, I assume this will come through you? Or should I just pick > it up directly? Yes. I'm just digging myself out from the mail avalance since coming back from Vienna. I'll get to it today.
diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst index 47499a1742bd..d8e1db4f2a29 100644 --- a/Documentation/admin-guide/sysctl/fs.rst +++ b/Documentation/admin-guide/sysctl/fs.rst @@ -38,6 +38,11 @@ requests. ``aio-max-nr`` allows you to change the maximum value ``aio-max-nr`` does not result in the pre-allocation or re-sizing of any kernel data structures. +automated_deletion_of_dentry +---------------------------- + +Deletes the associated dentry when a file is removed. Set to 1 to enable +this behavior, and 0 to disable it. By default, this behavior is disabled. dentry-state ------------ diff --git a/fs/dcache.c b/fs/dcache.c index 0f6b16ba30d0..498b2e261237 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -135,6 +135,7 @@ struct dentry_stat_t { static DEFINE_PER_CPU(long, nr_dentry); static DEFINE_PER_CPU(long, nr_dentry_unused); static DEFINE_PER_CPU(long, nr_dentry_negative); +static int automated_deletion_of_dentry; #if defined(CONFIG_SYSCTL) && defined(CONFIG_PROC_FS) /* Statistics gathering. */ @@ -199,6 +200,15 @@ static struct ctl_table fs_dcache_sysctls[] = { .mode = 0444, .proc_handler = proc_nr_dentry, }, + { + .procname = "automated_deletion_of_dentry", + .data = &automated_deletion_of_dentry, + .maxlen = sizeof(automated_deletion_of_dentry), + .mode = 0644, + .proc_handler = proc_dointvec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_ONE, + }, }; static int __init init_fs_dcache_sysctls(void) @@ -2401,6 +2411,8 @@ void d_delete(struct dentry * dentry) * Are we the only user? */ if (dentry->d_lockref.count == 1) { + if (automated_deletion_of_dentry) + __d_drop(dentry); dentry->d_flags &= ~DCACHE_CANT_MOUNT; dentry_unlink_inode(dentry); } else {
Commit 681ce8623567 ("vfs: Delete the associated dentry when deleting a file") introduced an unconditional deletion of the associated dentry when a file is removed. However, this led to performance regressions in specific benchmarks, such as ilebench.sum_operations/s [0], prompting a revert in commit 4a4be1ad3a6e ("Revert "vfs: Delete the associated dentry when deleting a file""). This patch seeks to reintroduce the concept conditionally, where the associated dentry is deleted only when the user explicitly opts for it during file removal. A new sysctl fs.automated_deletion_of_dentry is added for this purpose. Its default value is set to 0. There are practical use cases for this proactive dentry reclamation. Besides the Elasticsearch use case mentioned in commit 681ce8623567, additional examples have surfaced in our production environment. For instance, in video rendering services that continuously generate temporary files, upload them to persistent storage servers, and then delete them, a large number of negative dentries—serving no useful purpose—accumulate. Users in such cases would benefit from proactively reclaiming these negative dentries. Link: https://lore.kernel.org/linux-fsdevel/202405291318.4dfbb352-oliver.sang@intel.com [0] Link: https://lore.kernel.org/all/20240912-programm-umgibt-a1145fa73bb6@brauner/ Suggested-by: Christian Brauner <brauner@kernel.org> Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Christian Brauner <brauner@kernel.org> Cc: Jan Kara <jack@suse.cz> Cc: Mateusz Guzik <mjguzik@gmail.com> --- Documentation/admin-guide/sysctl/fs.rst | 5 +++++ fs/dcache.c | 12 ++++++++++++ 2 files changed, 17 insertions(+) --- v2: Add doc for this new sysctl