[PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Geert Uytterhoeven]

sh/migor_defconfig:

    mm/slab.c: In function ‘slab_memory_callback’:
    mm/slab.c:1127:23: error: implicit declaration of function ‘init_cache_node_node’; did you mean ‘drain_cache_node_node’? [-Werror=implicit-function-declaration]
     1127 |                 ret = init_cache_node_node(nid);
	  |                       ^~~~~~~~~~~~~~~~~~~~
	  |                       drain_cache_node_node

The #ifdef condition protecting the definition of init_cache_node_node()
no longer matches the conditions protecting the (multiple) users.

Fix this by syncing the conditions.

Fixes: 76af6a054da40553 ("mm/migrate: add CPU hotplug to demotion #ifdef")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Link: https://lore.kernel.org/r/b5bdea22-ed2f-3187-6efe-0c72330270a4@infradead.org
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 mm/slab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slab.c b/mm/slab.c
index ba454246ee13dd4d..de1523a78f2e7367 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -839,7 +839,7 @@ static int init_cache_node(struct kmem_cache *cachep, int node, gfp_t gfp)
 	return 0;
 }
 
-#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
+#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
 /*
  * Allocates and initializes node for a node on each slab cache, used for
  * either memory or cpu hotplug.  If memory is being hot-added, the kmem_cache_node
-- 
2.34.1

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[John Paul Adrian Glaubitz]

Hi Geert!

On Tue, 2023-03-21 at 09:30 +0100, Geert Uytterhoeven wrote:
> sh/migor_defconfig:
> 
>     mm/slab.c: In function ‘slab_memory_callback’:
>     mm/slab.c:1127:23: error: implicit declaration of function ‘init_cache_node_node’; did you mean ‘drain_cache_node_node’? [-Werror=implicit-function-declaration]
>      1127 |                 ret = init_cache_node_node(nid);
> 	  |                       ^~~~~~~~~~~~~~~~~~~~
> 	  |                       drain_cache_node_node
> 
> The #ifdef condition protecting the definition of init_cache_node_node()
> no longer matches the conditions protecting the (multiple) users.
> 
> Fix this by syncing the conditions.
> 
> Fixes: 76af6a054da40553 ("mm/migrate: add CPU hotplug to demotion #ifdef")
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Link: https://lore.kernel.org/r/b5bdea22-ed2f-3187-6efe-0c72330270a4@infradead.org
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  mm/slab.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index ba454246ee13dd4d..de1523a78f2e7367 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -839,7 +839,7 @@ static int init_cache_node(struct kmem_cache *cachep, int node, gfp_t gfp)
>  	return 0;
>  }
>  
> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
>  /*
>   * Allocates and initializes node for a node on each slab cache, used for
>   * either memory or cpu hotplug.  If memory is being hot-added, the kmem_cache_node

FWIW, the other #ifdef starting at drain_cache_node_node() closes with "#endif /* CONFIG_NUMA */",
while this #ifdef just ends with "#endif". Just in case you want to make this consistent.

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Geert Uytterhoeven]

Hi Adrian,

On Tue, Mar 21, 2023 at 9:47 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Tue, 2023-03-21 at 09:30 +0100, Geert Uytterhoeven wrote:
> > sh/migor_defconfig:
> >
> >     mm/slab.c: In function ‘slab_memory_callback’:
> >     mm/slab.c:1127:23: error: implicit declaration of function ‘init_cache_node_node’; did you mean ‘drain_cache_node_node’? [-Werror=implicit-function-declaration]
> >      1127 |                 ret = init_cache_node_node(nid);
> >         |                       ^~~~~~~~~~~~~~~~~~~~
> >         |                       drain_cache_node_node
> >
> > The #ifdef condition protecting the definition of init_cache_node_node()
> > no longer matches the conditions protecting the (multiple) users.
> >
> > Fix this by syncing the conditions.
> >
> > Fixes: 76af6a054da40553 ("mm/migrate: add CPU hotplug to demotion #ifdef")
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Link: https://lore.kernel.org/r/b5bdea22-ed2f-3187-6efe-0c72330270a4@infradead.org
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  mm/slab.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/slab.c b/mm/slab.c
> > index ba454246ee13dd4d..de1523a78f2e7367 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -839,7 +839,7 @@ static int init_cache_node(struct kmem_cache *cachep, int node, gfp_t gfp)
> >       return 0;
> >  }
> >
> > -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
> > +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
> >  /*
> >   * Allocates and initializes node for a node on each slab cache, used for
> >   * either memory or cpu hotplug.  If memory is being hot-added, the kmem_cache_node
>
> FWIW, the other #ifdef starting at drain_cache_node_node() closes with "#endif /* CONFIG_NUMA */",
> while this #ifdef just ends with "#endif". Just in case you want to make this consistent.

I guess that's fine, as init_cache_node_node() is a small function.
#endif comments are typically used when the start and end markers
do not fit on your (80x25 ;-) screen.

> Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Vlastimil Babka]

On 3/21/23 09:50, Geert Uytterhoeven wrote:
> Hi Adrian,
> 
> On Tue, Mar 21, 2023 at 9:47 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
>> On Tue, 2023-03-21 at 09:30 +0100, Geert Uytterhoeven wrote:
>> > sh/migor_defconfig:
>> >
>> >     mm/slab.c: In function ‘slab_memory_callback’:
>> >     mm/slab.c:1127:23: error: implicit declaration of function ‘init_cache_node_node’; did you mean ‘drain_cache_node_node’? [-Werror=implicit-function-declaration]
>> >      1127 |                 ret = init_cache_node_node(nid);
>> >         |                       ^~~~~~~~~~~~~~~~~~~~
>> >         |                       drain_cache_node_node
>> >
>> > The #ifdef condition protecting the definition of init_cache_node_node()
>> > no longer matches the conditions protecting the (multiple) users.
>> >
>> > Fix this by syncing the conditions.
>> >
>> > Fixes: 76af6a054da40553 ("mm/migrate: add CPU hotplug to demotion #ifdef")
>> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
>> > Link: https://lore.kernel.org/r/b5bdea22-ed2f-3187-6efe-0c72330270a4@infradead.org
>> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> > ---
>> >  mm/slab.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/mm/slab.c b/mm/slab.c
>> > index ba454246ee13dd4d..de1523a78f2e7367 100644
>> > --- a/mm/slab.c
>> > +++ b/mm/slab.c
>> > @@ -839,7 +839,7 @@ static int init_cache_node(struct kmem_cache *cachep, int node, gfp_t gfp)
>> >       return 0;
>> >  }
>> >
>> > -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
>> > +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
>> >  /*
>> >   * Allocates and initializes node for a node on each slab cache, used for
>> >   * either memory or cpu hotplug.  If memory is being hot-added, the kmem_cache_node
>>
>> FWIW, the other #ifdef starting at drain_cache_node_node() closes with "#endif /* CONFIG_NUMA */",
>> while this #ifdef just ends with "#endif". Just in case you want to make this consistent.
> 
> I guess that's fine, as init_cache_node_node() is a small function.
> #endif comments are typically used when the start and end markers
> do not fit on your (80x25 ;-) screen.

Agreed with this reasoning.

>> Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> 
> Thanks!

Applied to slab/for-6.3-rc4/fixes, thanks!

> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Randy Dunlap]

On 3/21/23 01:30, Geert Uytterhoeven wrote:
> sh/migor_defconfig:
> 
>     mm/slab.c: In function ‘slab_memory_callback’:
>     mm/slab.c:1127:23: error: implicit declaration of function ‘init_cache_node_node’; did you mean ‘drain_cache_node_node’? [-Werror=implicit-function-declaration]
>      1127 |                 ret = init_cache_node_node(nid);
> 	  |                       ^~~~~~~~~~~~~~~~~~~~
> 	  |                       drain_cache_node_node
> 
> The #ifdef condition protecting the definition of init_cache_node_node()
> no longer matches the conditions protecting the (multiple) users.
> 
> Fix this by syncing the conditions.
> 
> Fixes: 76af6a054da40553 ("mm/migrate: add CPU hotplug to demotion #ifdef")
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Link: https://lore.kernel.org/r/b5bdea22-ed2f-3187-6efe-0c72330270a4@infradead.org
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.

> ---
>  mm/slab.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index ba454246ee13dd4d..de1523a78f2e7367 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -839,7 +839,7 @@ static int init_cache_node(struct kmem_cache *cachep, int node, gfp_t gfp)
>  	return 0;
>  }
>  
> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
>  /*
>   * Allocates and initializes node for a node on each slab cache, used for
>   * either memory or cpu hotplug.  If memory is being hot-added, the kmem_cache_node

-- 
~Randy

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Matthew Wilcox]

On Tue, Mar 21, 2023 at 09:30:59AM +0100, Geert Uytterhoeven wrote:
> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)

I'm amused by the thought of CONFIG_NUMA without CONFIG_SMP.
Is it possible to have one node with memory and a single CPU, then
another node with memory and no CPU?

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Randy Dunlap]

On 3/21/23 09:40, Matthew Wilcox wrote:
> On Tue, Mar 21, 2023 at 09:30:59AM +0100, Geert Uytterhoeven wrote:
>> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
>> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
> 
> I'm amused by the thought of CONFIG_NUMA without CONFIG_SMP.
> Is it possible to have one node with memory and a single CPU, then
> another node with memory and no CPU?

More likely 1 with CPU+memory, 1 with memory only.

I've been told that that are also I/O-only nodes.

-- 
~Randy

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Dave Hansen]

On 3/21/23 09:40, Matthew Wilcox wrote:
> On Tue, Mar 21, 2023 at 09:30:59AM +0100, Geert Uytterhoeven wrote:
>> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
>> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
> I'm amused by the thought of CONFIG_NUMA without CONFIG_SMP.
> Is it possible to have one node with memory and a single CPU, then
> another node with memory and no CPU?

It's _possible_ for sure, just unlikely.  The most likely place these
days is probably a teensy tiny VM that just happens to have some
performance-differentiated memory exposed to it for some reason.  Maybe
it's got a slice of slow PMEM or fast High-Bandwidth memory for whatever
reason.

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Matthew Wilcox]

On Wed, Mar 22, 2023 at 09:16:55AM -0700, Dave Hansen wrote:
> On 3/21/23 09:40, Matthew Wilcox wrote:
> > On Tue, Mar 21, 2023 at 09:30:59AM +0100, Geert Uytterhoeven wrote:
> >> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
> >> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
> > I'm amused by the thought of CONFIG_NUMA without CONFIG_SMP.
> > Is it possible to have one node with memory and a single CPU, then
> > another node with memory and no CPU?
> 
> It's _possible_ for sure, just unlikely.  The most likely place these
> days is probably a teensy tiny VM that just happens to have some
> performance-differentiated memory exposed to it for some reason.  Maybe
> it's got a slice of slow PMEM or fast High-Bandwidth memory for whatever
> reason.

Right, you can construct such a system, but do we support the CONFIG
options of NUMA enabled and SMP disabled?  It seems so niche that we
shouldn't be spending time testing that combination.

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Dave Hansen]

On 3/22/23 09:46, Matthew Wilcox wrote:
> On Wed, Mar 22, 2023 at 09:16:55AM -0700, Dave Hansen wrote:
>> On 3/21/23 09:40, Matthew Wilcox wrote:
>>> On Tue, Mar 21, 2023 at 09:30:59AM +0100, Geert Uytterhoeven wrote:
>>>> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
>>>> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
>>> I'm amused by the thought of CONFIG_NUMA without CONFIG_SMP.
>>> Is it possible to have one node with memory and a single CPU, then
>>> another node with memory and no CPU?
>> It's _possible_ for sure, just unlikely.  The most likely place these
>> days is probably a teensy tiny VM that just happens to have some
>> performance-differentiated memory exposed to it for some reason.  Maybe
>> it's got a slice of slow PMEM or fast High-Bandwidth memory for whatever
>> reason.
> Right, you can construct such a system, but do we support the CONFIG
> options of NUMA enabled and SMP disabled?  It seems so niche that we
> shouldn't be spending time testing that combination.

On x86 we don't:

> config NUMA
>         bool "NUMA Memory Allocation and Scheduler Support"
>         depends on SMP
>         depends on X86_64 || (X86_32 && HIGHMEM64G && X86_BIGSMP)

... which I think is fine.  I totally agree that NUMA without SMP is too
niche to care about.  Heck, !SMP is almost too niche to care about these
days.

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Geert Uytterhoeven]

Hi Matthew,

On Wed, Mar 22, 2023 at 5:47 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Wed, Mar 22, 2023 at 09:16:55AM -0700, Dave Hansen wrote:
> > On 3/21/23 09:40, Matthew Wilcox wrote:
> > > On Tue, Mar 21, 2023 at 09:30:59AM +0100, Geert Uytterhoeven wrote:
> > >> -#if (defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)) || defined(CONFIG_SMP)
> > >> +#if defined(CONFIG_NUMA) || defined(CONFIG_SMP)
> > > I'm amused by the thought of CONFIG_NUMA without CONFIG_SMP.
> > > Is it possible to have one node with memory and a single CPU, then
> > > another node with memory and no CPU?
> >
> > It's _possible_ for sure, just unlikely.  The most likely place these
> > days is probably a teensy tiny VM that just happens to have some
> > performance-differentiated memory exposed to it for some reason.  Maybe
> > it's got a slice of slow PMEM or fast High-Bandwidth memory for whatever
> > reason.
>
> Right, you can construct such a system, but do we support the CONFIG
> options of NUMA enabled and SMP disabled?  It seems so niche that we
> shouldn't be spending time testing that combination.

SH has been using this for a long time.

It's supported. Dave just forgot to update the #ifdef around the
definition of init_cache_node_node() when updating an #ifdef around
a code block that contains one of the callers.

P.S. To me, this discussion reminds me of the old discussion about
     discontigmem without NUMA. Yes, not all systems are PCs with
     contiguous memory on a single fast bus ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[John Paul Adrian Glaubitz]

Hi Geert!

On Thu, 2023-03-23 at 09:25 +0100, Geert Uytterhoeven wrote:
> It's supported. Dave just forgot to update the #ifdef around the
> definition of init_cache_node_node() when updating an #ifdef around
> a code block that contains one of the callers.
> 
> P.S. To me, this discussion reminds me of the old discussion about
>      discontigmem without NUMA. Yes, not all systems are PCs with
>      contiguous memory on a single fast bus ;-)

I'm wondering: Could the NUMA code be used to work with the different
memory types found on the Amiga, i.e. chip RAM, fast RAM etc?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] mm/slab: Fix undefined init_cache_node_node() for NUMA and !SMP

[Geert Uytterhoeven]

Hi Adrian,

On Thu, Mar 23, 2023 at 9:28 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Thu, 2023-03-23 at 09:25 +0100, Geert Uytterhoeven wrote:
> > It's supported. Dave just forgot to update the #ifdef around the
> > definition of init_cache_node_node() when updating an #ifdef around
> > a code block that contains one of the callers.
> >
> > P.S. To me, this discussion reminds me of the old discussion about
> >      discontigmem without NUMA. Yes, not all systems are PCs with
> >      contiguous memory on a single fast bus ;-)
>
> I'm wondering: Could the NUMA code be used to work with the different
> memory types found on the Amiga, i.e. chip RAM, fast RAM etc?

I guess so, but only for 32-bit motherboard RAM on A3000/A4000
vs. RAM on an accelerator card vs. Zorro-III RAM on e.g. BigRamPlus.
Chip RAM and Zorro-II RAM do not support RMW-cycles on
Zorro-III capable machines.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds