[PATCH] vfs: Add a sysctl for automated deletion of dentry

[PATCH] vfs: Add a sysctl for automated deletion of dentry

[Yafang Shao]

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>
---
 fs/dcache.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3d8daaecb6d1..ffd2cae2ba8d 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -130,6 +130,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. */
@@ -194,6 +195,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)
@@ -2394,6 +2404,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 {
-- 
2.43.5

Re: [PATCH] vfs: Add a sysctl for automated deletion of dentry

[Yafang Shao]

On Fri, Sep 13, 2024 at 4:32 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> 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>
> ---
>  fs/dcache.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 3d8daaecb6d1..ffd2cae2ba8d 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -130,6 +130,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. */
> @@ -194,6 +195,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)
> @@ -2394,6 +2404,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 {
> --
> 2.43.5
>

Gently ping.  Any feedback on this change?

--
Regards
Yafang