Re: [PATCH v4 -next 08/15] mm: nommu: move sysctl to mm/nommu.c

[yukaixiong <yukaixiong@xxxxxxxxxx>]

On 2025/1/2 22:09, Lorenzo Stoakes wrote:
> On Mon, Dec 23, 2024 at 10:15:27PM +0800, Kaixiong Yu wrote:
> > The sysctl_nr_trim_pages belongs to nommu.c, move it to mm/nommu.c
> > from /kernel/sysctl.c. And remove the useless extern variable declaration
> > from include/linux/mm.h
> >
> > Signed-off-by: Kaixiong Yu <yukaixiong@xxxxxxxxxx>
> Looks good to me,
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
Thanks for your review !

Best ...
> > ---
> > v4:
> >   - const qualify struct ctl_table nommu_table
> > v3:
> >   - change the title
> > v2:
> >   - fix the build error: expected ';' after top level declarator
> >   - fix the build error: call to undeclared function 'register_syscall_init',
> >     use 'register_sysctl_init' to replace it.
> > ---
> > ---
> >   include/linux/mm.h |  2 --
> >   kernel/sysctl.c    | 10 ----------
> >   mm/nommu.c         | 15 ++++++++++++++-
> >   3 files changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index b3b87c1dc1e4..9813b5b9c093 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -4080,8 +4080,6 @@ unsigned long wp_shared_mapping_range(struct address_space *mapping,
> >   				      pgoff_t first_index, pgoff_t nr);
> >   #endif
> >
> > -extern int sysctl_nr_trim_pages;
> > -
> >   #ifdef CONFIG_PRINTK
> >   void mem_dump_obj(void *object);
> >   #else
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 62a58e417c40..97f9abffff0f 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -2031,16 +2031,6 @@ static struct ctl_table vm_table[] = {
> >   		.extra1		= SYSCTL_ONE,
> >   		.extra2		= SYSCTL_FOUR,
> >   	},
> > -#ifndef CONFIG_MMU
> > -	{
> > -		.procname	= "nr_trim_pages",
> > -		.data		= &sysctl_nr_trim_pages,
> > -		.maxlen		= sizeof(sysctl_nr_trim_pages),
> > -		.mode		= 0644,
> > -		.proc_handler	= proc_dointvec_minmax,
> > -		.extra1		= SYSCTL_ZERO,
> > -	},
> > -#endif
> Of course later on in the series you do what I asked in a previous commit :P Nice.
>
> >   	{
> >   		.procname	= "vfs_cache_pressure",
> >   		.data		= &sysctl_vfs_cache_pressure,
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index baa79abdaf03..3c32f8b1eb54 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -48,7 +48,6 @@ struct page *mem_map;
> >   unsigned long max_mapnr;
> >   EXPORT_SYMBOL(max_mapnr);
> >   unsigned long highest_memmap_pfn;
> > -int sysctl_nr_trim_pages = CONFIG_NOMMU_INITIAL_TRIM_EXCESS;
> >   int heap_stack_gap = 0;
> >
> >   atomic_long_t mmap_pages_allocated;
> > @@ -392,6 +391,19 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
> >   	return mm->brk = brk;
> >   }
> >
> > +static int sysctl_nr_trim_pages = CONFIG_NOMMU_INITIAL_TRIM_EXCESS;
> > +
> > +static const struct ctl_table nommu_table[] = {
> > +	{
> > +		.procname	= "nr_trim_pages",
> > +		.data		= &sysctl_nr_trim_pages,
> > +		.maxlen		= sizeof(sysctl_nr_trim_pages),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ZERO,
> > +	},
> > +};
> > +
> >   /*
> >    * initialise the percpu counter for VM and region record slabs
> >    */
> > @@ -402,6 +414,7 @@ void __init mmap_init(void)
> >   	ret = percpu_counter_init(&vm_committed_as, 0, GFP_KERNEL);
> >   	VM_BUG_ON(ret);
> >   	vm_region_jar = KMEM_CACHE(vm_region, SLAB_PANIC|SLAB_ACCOUNT);
> > +	register_sysctl_init("vm", nommu_table);
> >   }
> >
> >   /*
> > --
> > 2.34.1
> .