diff mbox series

[-next,12/15] fs: dcache: move the sysctl into its own file

Message ID 20240826120449.1666461-13-yukaixiong@huawei.com (mailing list archive)
State New
Headers show
Series sysctl: move sysctls from vm_table into its own files | expand

Commit Message

yukaixiong Aug. 26, 2024, 12:04 p.m. UTC
The sysctl_vfs_cache_pressure belongs to fs/dcache.c, move it to
its own file from kernel/sysctl.c. As a part of fs/dcache.c cleaning,
sysctl_vfs_cache_pressure is changed to a static variable, and export
vfs_pressure_ratio with EXPORT_SYMBOL_GPL to be used by other files.
And move the unneeded include(linux/dcache.h).

Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
---
 fs/dcache.c            | 21 +++++++++++++++++++--
 include/linux/dcache.h |  7 +------
 kernel/sysctl.c        |  9 ---------
 3 files changed, 20 insertions(+), 17 deletions(-)

Comments

Kees Cook Aug. 26, 2024, 7:56 p.m. UTC | #1
On Mon, Aug 26, 2024 at 08:04:46PM +0800, Kaixiong Yu wrote:
> The sysctl_vfs_cache_pressure belongs to fs/dcache.c, move it to
> its own file from kernel/sysctl.c. As a part of fs/dcache.c cleaning,
> sysctl_vfs_cache_pressure is changed to a static variable, and export
> vfs_pressure_ratio with EXPORT_SYMBOL_GPL to be used by other files.
> And move the unneeded include(linux/dcache.h).
> 
> Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
> ---
>  fs/dcache.c            | 21 +++++++++++++++++++--
>  include/linux/dcache.h |  7 +------
>  kernel/sysctl.c        |  9 ---------
>  3 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 1af75fa68638..8717d5026cda 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -73,8 +73,13 @@
>   * If no ancestor relationship:
>   * arbitrary, since it's serialized on rename_lock
>   */
> -int sysctl_vfs_cache_pressure __read_mostly = 100;
> -EXPORT_SYMBOL_GPL(sysctl_vfs_cache_pressure);
> +static int sysctl_vfs_cache_pressure __read_mostly = 100;
> +
> +unsigned long vfs_pressure_ratio(unsigned long val)
> +{
> +	return mult_frac(val, sysctl_vfs_cache_pressure, 100);
> +}
> +EXPORT_SYMBOL_GPL(vfs_pressure_ratio);

This was a static inline, but AFAICT it's only called through
alloc_super() which is hardly "fast path". If this series gets another
version it may be worth calling out this inline->out-of-line change in
the commit log.

I don't think it's a blocker, but I'm not a VFS maintainer. :)
Jan Kara Sept. 2, 2024, 11:04 a.m. UTC | #2
On Mon 26-08-24 12:56:00, Kees Cook wrote:
> On Mon, Aug 26, 2024 at 08:04:46PM +0800, Kaixiong Yu wrote:
> > The sysctl_vfs_cache_pressure belongs to fs/dcache.c, move it to
> > its own file from kernel/sysctl.c. As a part of fs/dcache.c cleaning,
> > sysctl_vfs_cache_pressure is changed to a static variable, and export
> > vfs_pressure_ratio with EXPORT_SYMBOL_GPL to be used by other files.
> > And move the unneeded include(linux/dcache.h).
> > 
> > Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
> > ---
> >  fs/dcache.c            | 21 +++++++++++++++++++--
> >  include/linux/dcache.h |  7 +------
> >  kernel/sysctl.c        |  9 ---------
> >  3 files changed, 20 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/dcache.c b/fs/dcache.c
> > index 1af75fa68638..8717d5026cda 100644
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -73,8 +73,13 @@
> >   * If no ancestor relationship:
> >   * arbitrary, since it's serialized on rename_lock
> >   */
> > -int sysctl_vfs_cache_pressure __read_mostly = 100;
> > -EXPORT_SYMBOL_GPL(sysctl_vfs_cache_pressure);
> > +static int sysctl_vfs_cache_pressure __read_mostly = 100;
> > +
> > +unsigned long vfs_pressure_ratio(unsigned long val)
> > +{
> > +	return mult_frac(val, sysctl_vfs_cache_pressure, 100);
> > +}
> > +EXPORT_SYMBOL_GPL(vfs_pressure_ratio);
> 
> This was a static inline, but AFAICT it's only called through
> alloc_super() which is hardly "fast path". If this series gets another
> version it may be worth calling out this inline->out-of-line change in
> the commit log.
> 
> I don't think it's a blocker, but I'm not a VFS maintainer. :)

It's actually called from about 7 shrinkers of filesystem objects. They get
called relatively frequently during memory reclaim but I don't think a
function call is *that* expensive to matter in this case. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>


								Honza
Christian Brauner Sept. 2, 2024, 11:17 a.m. UTC | #3
On Mon, Aug 26, 2024 at 08:04:46PM GMT, Kaixiong Yu wrote:
> The sysctl_vfs_cache_pressure belongs to fs/dcache.c, move it to
> its own file from kernel/sysctl.c. As a part of fs/dcache.c cleaning,
> sysctl_vfs_cache_pressure is changed to a static variable, and export
> vfs_pressure_ratio with EXPORT_SYMBOL_GPL to be used by other files.
> And move the unneeded include(linux/dcache.h).
> 
> Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>
diff mbox series

Patch

diff --git a/fs/dcache.c b/fs/dcache.c
index 1af75fa68638..8717d5026cda 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -73,8 +73,13 @@ 
  * If no ancestor relationship:
  * arbitrary, since it's serialized on rename_lock
  */
-int sysctl_vfs_cache_pressure __read_mostly = 100;
-EXPORT_SYMBOL_GPL(sysctl_vfs_cache_pressure);
+static int sysctl_vfs_cache_pressure __read_mostly = 100;
+
+unsigned long vfs_pressure_ratio(unsigned long val)
+{
+	return mult_frac(val, sysctl_vfs_cache_pressure, 100);
+}
+EXPORT_SYMBOL_GPL(vfs_pressure_ratio);
 
 __cacheline_aligned_in_smp DEFINE_SEQLOCK(rename_lock);
 
@@ -196,8 +201,20 @@  static struct ctl_table fs_dcache_sysctls[] = {
 	},
 };
 
+static struct ctl_table vm_dcache_sysctls[] = {
+	{
+		.procname	= "vfs_cache_pressure",
+		.data		= &sysctl_vfs_cache_pressure,
+		.maxlen		= sizeof(sysctl_vfs_cache_pressure),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+	},
+};
+
 static int __init init_fs_dcache_sysctls(void)
 {
+	register_sysctl_init("vm", vm_dcache_sysctls);
 	register_sysctl_init("fs", fs_dcache_sysctls);
 	return 0;
 }
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index bff956f7b2b9..c81c2e9e13df 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -508,12 +508,7 @@  static inline int simple_positive(const struct dentry *dentry)
 	return d_really_is_positive(dentry) && !d_unhashed(dentry);
 }
 
-extern int sysctl_vfs_cache_pressure;
-
-static inline unsigned long vfs_pressure_ratio(unsigned long val)
-{
-	return mult_frac(val, sysctl_vfs_cache_pressure, 100);
-}
+unsigned long vfs_pressure_ratio(unsigned long val);
 
 /**
  * d_inode - Get the actual inode of this dentry
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index d638a1bac9af..6f03fc749794 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -46,7 +46,6 @@ 
 #include <linux/key.h>
 #include <linux/times.h>
 #include <linux/limits.h>
-#include <linux/dcache.h>
 #include <linux/syscalls.h>
 #include <linux/nfs_fs.h>
 #include <linux/acpi.h>
@@ -2024,14 +2023,6 @@  static struct ctl_table kern_table[] = {
 };
 
 static struct ctl_table vm_table[] = {
-	{
-		.procname	= "vfs_cache_pressure",
-		.data		= &sysctl_vfs_cache_pressure,
-		.maxlen		= sizeof(sysctl_vfs_cache_pressure),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= SYSCTL_ZERO,
-	},
 #if (defined(CONFIG_X86_32) && !defined(CONFIG_UML))|| \
    (defined(CONFIG_SUPERH) && defined(CONFIG_VSYSCALL))
 	{