All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-07-31 16:09 ` Siddh Raman Pant
  0 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant via Linux-kernel-mentees @ 2022-07-31 16:09 UTC (permalink / raw)
  To: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
  Cc: linux-kernel-mentees, linux-kernel

node_to_cpumask_map is of type cpumask_var_t[].

When CONFIG_CPUMASK_OFFSTACK is set, cpumask_var_t is typedef'd to a
pointer for dynamic allocation, else to an array of one element. The
"wicked game" can be checked on line 700 of include/linux/cpumask.h.

The lines changed in this commit were probably written by the original
authors with CONFIG_CPUMASK_OFFSTACK=y (i.e. dynamic allocation) in mind,
checking if the cpumask was available via a direct NULL check.

When CONFIG_CPUMASK_OFFSTACK is not set, GCC gives the below given warning
while compiling the kernel.

Fix that by using cpumask_available(), which does the NULL check when
CONFIG_CPUMASK_OFFSTACK is set, otherwise returns true. Use it wherever
such checks are made.

Conditional definitions of cpumask_available() can be found along with
the definition of cpumask_var_t. Check the cpumask.h reference mentioned
above.

GCC warning log:
===========================================================================

arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
arch/x86/mm/numa.c:916:39: warning: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Waddress]
  916 |         if (node_to_cpumask_map[node] == NULL) {
      |                                       ^~
In file included from ./include/linux/linkage.h:7,
                 from ./include/linux/kernel.h:17,
                 from ./arch/x86/include/asm/percpu.h:27,
                 from ./arch/x86/include/asm/current.h:6,
                 from ./include/linux/mutex.h:14,
                 from ./include/linux/kernfs.h:11,
                 from ./include/linux/sysfs.h:16,
                 from ./include/linux/kobject.h:20,
                 from ./include/linux/of.h:17,
                 from ./include/linux/irqdomain.h:35,
                 from ./include/linux/acpi.h:13,
                 from arch/x86/mm/numa.c:3:
arch/x86/mm/numa.c:67:15: note: ‘node_to_cpumask_map’ declared here
   67 | EXPORT_SYMBOL(node_to_cpumask_map);
      |               ^~~~~~~~~~~~~~~~~~~
./include/linux/export.h:87:28: note: in definition of macro ‘___EXPORT_SYMBOL’
   87 |         extern typeof(sym) sym;                                                 \
      |                            ^~~
./include/linux/export.h:147:41: note: in expansion of macro ‘__EXPORT_SYMBOL’
  147 | #define _EXPORT_SYMBOL(sym, sec)        __EXPORT_SYMBOL(sym, sec, "")
      |                                         ^~~~~~~~~~~~~~~
./include/linux/export.h:150:41: note: in expansion of macro ‘_EXPORT_SYMBOL’
  150 | #define EXPORT_SYMBOL(sym)              _EXPORT_SYMBOL(sym, "")
      |                                         ^~~~~~~~~~~~~~
arch/x86/mm/numa.c:67:1: note: in expansion of macro ‘EXPORT_SYMBOL’
   67 | EXPORT_SYMBOL(node_to_cpumask_map);
      | ^~~~~~~~~~~~~

===========================================================================

Fixes: c032ef60d1aa ("cpumask: convert node_to_cpumask_map[] to cpumask_var_t")
Fixes: de2d9445f162 ("x86: Unify node_to_cpumask_map handling between 32 and 64bit")

Signed-off-by: Siddh Raman Pant <code@siddh.me>
---
 arch/x86/mm/numa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index e8b061557887..2aadb2019b4f 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -867,7 +867,7 @@ void debug_cpumask_set_cpu(int cpu, int node, bool enable)
 		return;
 	}
 	mask = node_to_cpumask_map[node];
-	if (!mask) {
+	if (!cpumask_available(mask)) {
 		pr_err("node_to_cpumask_map[%i] NULL\n", node);
 		dump_stack();
 		return;
@@ -913,7 +913,7 @@ const struct cpumask *cpumask_of_node(int node)
 		dump_stack();
 		return cpu_none_mask;
 	}
-	if (node_to_cpumask_map[node] == NULL) {
+	if (!cpumask_available(node_to_cpumask_map[node])) {
 		printk(KERN_WARNING
 			"cpumask_of_node(%d): no node_to_cpumask_map!\n",
 			node);
-- 
2.35.1


_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-07-31 16:09 ` Siddh Raman Pant
  0 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant @ 2022-07-31 16:09 UTC (permalink / raw)
  To: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
  Cc: linux-kernel, linux-kernel-mentees

node_to_cpumask_map is of type cpumask_var_t[].

When CONFIG_CPUMASK_OFFSTACK is set, cpumask_var_t is typedef'd to a
pointer for dynamic allocation, else to an array of one element. The
"wicked game" can be checked on line 700 of include/linux/cpumask.h.

The lines changed in this commit were probably written by the original
authors with CONFIG_CPUMASK_OFFSTACK=y (i.e. dynamic allocation) in mind,
checking if the cpumask was available via a direct NULL check.

When CONFIG_CPUMASK_OFFSTACK is not set, GCC gives the below given warning
while compiling the kernel.

Fix that by using cpumask_available(), which does the NULL check when
CONFIG_CPUMASK_OFFSTACK is set, otherwise returns true. Use it wherever
such checks are made.

Conditional definitions of cpumask_available() can be found along with
the definition of cpumask_var_t. Check the cpumask.h reference mentioned
above.

GCC warning log:
===========================================================================

arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
arch/x86/mm/numa.c:916:39: warning: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Waddress]
  916 |         if (node_to_cpumask_map[node] == NULL) {
      |                                       ^~
In file included from ./include/linux/linkage.h:7,
                 from ./include/linux/kernel.h:17,
                 from ./arch/x86/include/asm/percpu.h:27,
                 from ./arch/x86/include/asm/current.h:6,
                 from ./include/linux/mutex.h:14,
                 from ./include/linux/kernfs.h:11,
                 from ./include/linux/sysfs.h:16,
                 from ./include/linux/kobject.h:20,
                 from ./include/linux/of.h:17,
                 from ./include/linux/irqdomain.h:35,
                 from ./include/linux/acpi.h:13,
                 from arch/x86/mm/numa.c:3:
arch/x86/mm/numa.c:67:15: note: ‘node_to_cpumask_map’ declared here
   67 | EXPORT_SYMBOL(node_to_cpumask_map);
      |               ^~~~~~~~~~~~~~~~~~~
./include/linux/export.h:87:28: note: in definition of macro ‘___EXPORT_SYMBOL’
   87 |         extern typeof(sym) sym;                                                 \
      |                            ^~~
./include/linux/export.h:147:41: note: in expansion of macro ‘__EXPORT_SYMBOL’
  147 | #define _EXPORT_SYMBOL(sym, sec)        __EXPORT_SYMBOL(sym, sec, "")
      |                                         ^~~~~~~~~~~~~~~
./include/linux/export.h:150:41: note: in expansion of macro ‘_EXPORT_SYMBOL’
  150 | #define EXPORT_SYMBOL(sym)              _EXPORT_SYMBOL(sym, "")
      |                                         ^~~~~~~~~~~~~~
arch/x86/mm/numa.c:67:1: note: in expansion of macro ‘EXPORT_SYMBOL’
   67 | EXPORT_SYMBOL(node_to_cpumask_map);
      | ^~~~~~~~~~~~~

===========================================================================

Fixes: c032ef60d1aa ("cpumask: convert node_to_cpumask_map[] to cpumask_var_t")
Fixes: de2d9445f162 ("x86: Unify node_to_cpumask_map handling between 32 and 64bit")

Signed-off-by: Siddh Raman Pant <code@siddh.me>
---
 arch/x86/mm/numa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index e8b061557887..2aadb2019b4f 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -867,7 +867,7 @@ void debug_cpumask_set_cpu(int cpu, int node, bool enable)
 		return;
 	}
 	mask = node_to_cpumask_map[node];
-	if (!mask) {
+	if (!cpumask_available(mask)) {
 		pr_err("node_to_cpumask_map[%i] NULL\n", node);
 		dump_stack();
 		return;
@@ -913,7 +913,7 @@ const struct cpumask *cpumask_of_node(int node)
 		dump_stack();
 		return cpu_none_mask;
 	}
-	if (node_to_cpumask_map[node] == NULL) {
+	if (!cpumask_available(node_to_cpumask_map[node])) {
 		printk(KERN_WARNING
 			"cpumask_of_node(%d): no node_to_cpumask_map!\n",
 			node);
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-07-31 16:09 ` Siddh Raman Pant
@ 2022-08-02 11:07   ` Ingo Molnar
  -1 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-02 11:07 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> GCC warning log:
> ===========================================================================
> 
> arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
> arch/x86/mm/numa.c:916:39: warning: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Waddress]
>   916 |         if (node_to_cpumask_map[node] == NULL) {
>       |                                       ^~

Your fix makes sense I suppose, but I'm wondering how testing didn't 
trigger this warning.

Off-stack isn't a rare config option:

  kepler:~/tip> make allmodconfig
  #
  # No change to .config
  #
  kepler:~/tip> grep CPUMASK_OFFSTACK .config
  CONFIG_CPUMASK_OFFSTACK=y
  kepler:~/tip> 

What am I missing?

> Fixes: c032ef60d1aa ("cpumask: convert node_to_cpumask_map[] to cpumask_var_t")
> Fixes: de2d9445f162 ("x86: Unify node_to_cpumask_map handling between 32 and 64bit")

These are ancient commits from 2009 & 2011.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-02 11:07   ` Ingo Molnar
  0 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-02 11:07 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> GCC warning log:
> ===========================================================================
> 
> arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
> arch/x86/mm/numa.c:916:39: warning: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Waddress]
>   916 |         if (node_to_cpumask_map[node] == NULL) {
>       |                                       ^~

Your fix makes sense I suppose, but I'm wondering how testing didn't 
trigger this warning.

Off-stack isn't a rare config option:

  kepler:~/tip> make allmodconfig
  #
  # No change to .config
  #
  kepler:~/tip> grep CPUMASK_OFFSTACK .config
  CONFIG_CPUMASK_OFFSTACK=y
  kepler:~/tip> 

What am I missing?

> Fixes: c032ef60d1aa ("cpumask: convert node_to_cpumask_map[] to cpumask_var_t")
> Fixes: de2d9445f162 ("x86: Unify node_to_cpumask_map handling between 32 and 64bit")

These are ancient commits from 2009 & 2011.

Thanks,

	Ingo
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-08-02 11:07   ` Ingo Molnar
@ 2022-08-02 16:29     ` Siddh Raman Pant
  -1 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant via Linux-kernel-mentees @ 2022-08-02 16:29 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees

On Tue, 02 Aug 2022 16:37:44 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> Your fix makes sense I suppose, but I'm wondering how testing didn't 
> trigger this warning.
> 
> Off-stack isn't a rare config option:
> 
>   kepler:~/tip> make allmodconfig
>   #
>   # No change to .config
>   #
>   kepler:~/tip> grep CPUMASK_OFFSTACK .config
>   CONFIG_CPUMASK_OFFSTACK=y
>   kepler:~/tip> 
> 
> What am I missing?

Maybe this triggers on certain config options set, or maybe due to new
gcc version? (I'm using gcc-12, I also likely saw while on gcc-11.)
It nevertheless is a helpful warning.

I just now tried `make defconfig` (default configuration based on
'x86_64_defconfig') and compiling with `make -j13 all`, and gcc doesn't
give any warning. (CONFIG_CPUMASK_OFFSTACK isn't even listed in the
.config file produced, grep fails.)

The config on which I can reproduce the warning can be found here:
https://gist.github.com/siddhpant/0197ea2b9873e8719d5d7ef991e2cd89
(It has 8969 lines, thus uploaded as a gist.)

This is a modification of a config found on syzkaller, which I was using
to compile and test some bug. I had noticed the gcc warning earlier while
on similar detours and usually ignored it, but now I finally took a look.

I tested compiling with it 5 times (`make clean` and `make -j13 all`),
and gcc gave the warning in all attempts. I also tried `make -j1 all`,
which also had gcc spitting out the warning, so it cannot be any race.

> > Fixes: c032ef60d1aa ("cpumask: convert node_to_cpumask_map[] to cpumask_var_t")
> > Fixes: de2d9445f162 ("x86: Unify node_to_cpumask_map handling between 32 and 64bit")
> 
> These are ancient commits from 2009 & 2011.

Yes, that's where blaming the file leads me to.

Thanks,
Siddh
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-02 16:29     ` Siddh Raman Pant
  0 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant @ 2022-08-02 16:29 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees

On Tue, 02 Aug 2022 16:37:44 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> Your fix makes sense I suppose, but I'm wondering how testing didn't 
> trigger this warning.
> 
> Off-stack isn't a rare config option:
> 
>   kepler:~/tip> make allmodconfig
>   #
>   # No change to .config
>   #
>   kepler:~/tip> grep CPUMASK_OFFSTACK .config
>   CONFIG_CPUMASK_OFFSTACK=y
>   kepler:~/tip> 
> 
> What am I missing?

Maybe this triggers on certain config options set, or maybe due to new
gcc version? (I'm using gcc-12, I also likely saw while on gcc-11.)
It nevertheless is a helpful warning.

I just now tried `make defconfig` (default configuration based on
'x86_64_defconfig') and compiling with `make -j13 all`, and gcc doesn't
give any warning. (CONFIG_CPUMASK_OFFSTACK isn't even listed in the
.config file produced, grep fails.)

The config on which I can reproduce the warning can be found here:
https://gist.github.com/siddhpant/0197ea2b9873e8719d5d7ef991e2cd89
(It has 8969 lines, thus uploaded as a gist.)

This is a modification of a config found on syzkaller, which I was using
to compile and test some bug. I had noticed the gcc warning earlier while
on similar detours and usually ignored it, but now I finally took a look.

I tested compiling with it 5 times (`make clean` and `make -j13 all`),
and gcc gave the warning in all attempts. I also tried `make -j1 all`,
which also had gcc spitting out the warning, so it cannot be any race.

> > Fixes: c032ef60d1aa ("cpumask: convert node_to_cpumask_map[] to cpumask_var_t")
> > Fixes: de2d9445f162 ("x86: Unify node_to_cpumask_map handling between 32 and 64bit")
> 
> These are ancient commits from 2009 & 2011.

Yes, that's where blaming the file leads me to.

Thanks,
Siddh

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-08-02 16:29     ` Siddh Raman Pant
@ 2022-08-03  8:48       ` Ingo Molnar
  -1 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-03  8:48 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> On Tue, 02 Aug 2022 16:37:44 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> > Your fix makes sense I suppose, but I'm wondering how testing didn't 
> > trigger this warning.
> > 
> > Off-stack isn't a rare config option:
> > 
> >   kepler:~/tip> make allmodconfig
> >   #
> >   # No change to .config
> >   #
> >   kepler:~/tip> grep CPUMASK_OFFSTACK .config
> >   CONFIG_CPUMASK_OFFSTACK=y
> >   kepler:~/tip> 
> > 
> > What am I missing?
> 
> Maybe this triggers on certain config options set, or maybe due to new
> gcc version? (I'm using gcc-12, I also likely saw while on gcc-11.)
> It nevertheless is a helpful warning.
> 
> I just now tried `make defconfig` (default configuration based on
> 'x86_64_defconfig') and compiling with `make -j13 all`, and gcc doesn't
> give any warning. (CONFIG_CPUMASK_OFFSTACK isn't even listed in the
> .config file produced, grep fails.)

Does 'allmodconfig' reproduce the warning for you:

  $ make allmodconfig
  $ make arch/x86/mm/numa.o

?

If yes, then this could be due to gcc-12, as it doesn't reproduce with 
gcc-11 for me:

   gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1) 

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-03  8:48       ` Ingo Molnar
  0 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-03  8:48 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> On Tue, 02 Aug 2022 16:37:44 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> > Your fix makes sense I suppose, but I'm wondering how testing didn't 
> > trigger this warning.
> > 
> > Off-stack isn't a rare config option:
> > 
> >   kepler:~/tip> make allmodconfig
> >   #
> >   # No change to .config
> >   #
> >   kepler:~/tip> grep CPUMASK_OFFSTACK .config
> >   CONFIG_CPUMASK_OFFSTACK=y
> >   kepler:~/tip> 
> > 
> > What am I missing?
> 
> Maybe this triggers on certain config options set, or maybe due to new
> gcc version? (I'm using gcc-12, I also likely saw while on gcc-11.)
> It nevertheless is a helpful warning.
> 
> I just now tried `make defconfig` (default configuration based on
> 'x86_64_defconfig') and compiling with `make -j13 all`, and gcc doesn't
> give any warning. (CONFIG_CPUMASK_OFFSTACK isn't even listed in the
> .config file produced, grep fails.)

Does 'allmodconfig' reproduce the warning for you:

  $ make allmodconfig
  $ make arch/x86/mm/numa.o

?

If yes, then this could be due to gcc-12, as it doesn't reproduce with 
gcc-11 for me:

   gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1) 

Thanks,

	Ingo
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-08-03  8:48       ` Ingo Molnar
@ 2022-08-03  8:58         ` Siddh Raman Pant
  -1 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant via Linux-kernel-mentees @ 2022-08-03  8:58 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees

On Wed, 03 Aug 2022 14:18:18 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> 
> * Siddh Raman Pant <code@siddh.me> wrote:
> 
> > On Tue, 02 Aug 2022 16:37:44 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> > > Your fix makes sense I suppose, but I'm wondering how testing didn't 
> > > trigger this warning.
> > > 
> > > Off-stack isn't a rare config option:
> > > 
> > >   kepler:~/tip> make allmodconfig
> > >   #
> > >   # No change to .config
> > >   #
> > >   kepler:~/tip> grep CPUMASK_OFFSTACK .config
> > >   CONFIG_CPUMASK_OFFSTACK=y
> > >   kepler:~/tip> 
> > > 
> > > What am I missing?
> > 
> > Maybe this triggers on certain config options set, or maybe due to new
> > gcc version? (I'm using gcc-12, I also likely saw while on gcc-11.)
> > It nevertheless is a helpful warning.
> > 
> > I just now tried `make defconfig` (default configuration based on
> > 'x86_64_defconfig') and compiling with `make -j13 all`, and gcc doesn't
> > give any warning. (CONFIG_CPUMASK_OFFSTACK isn't even listed in the
> > .config file produced, grep fails.)
> 
> Does 'allmodconfig' reproduce the warning for you:
> 
>   $ make allmodconfig
>   $ make arch/x86/mm/numa.o
> 
> ?
> 
> If yes, then this could be due to gcc-12, as it doesn't reproduce with 
> gcc-11 for me:
> 
>    gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1) 
> 
> Thanks,
> 
>     Ingo
> 

There is no reason why allmodconfig would trigger the warning, as it has
CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the other case.
Still, I tried that, and as expected there was no warning.

Did you try the config file I had linked to earlier?

Thanks,
Siddh
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-03  8:58         ` Siddh Raman Pant
  0 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant @ 2022-08-03  8:58 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees

On Wed, 03 Aug 2022 14:18:18 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> 
> * Siddh Raman Pant <code@siddh.me> wrote:
> 
> > On Tue, 02 Aug 2022 16:37:44 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> > > Your fix makes sense I suppose, but I'm wondering how testing didn't 
> > > trigger this warning.
> > > 
> > > Off-stack isn't a rare config option:
> > > 
> > >   kepler:~/tip> make allmodconfig
> > >   #
> > >   # No change to .config
> > >   #
> > >   kepler:~/tip> grep CPUMASK_OFFSTACK .config
> > >   CONFIG_CPUMASK_OFFSTACK=y
> > >   kepler:~/tip> 
> > > 
> > > What am I missing?
> > 
> > Maybe this triggers on certain config options set, or maybe due to new
> > gcc version? (I'm using gcc-12, I also likely saw while on gcc-11.)
> > It nevertheless is a helpful warning.
> > 
> > I just now tried `make defconfig` (default configuration based on
> > 'x86_64_defconfig') and compiling with `make -j13 all`, and gcc doesn't
> > give any warning. (CONFIG_CPUMASK_OFFSTACK isn't even listed in the
> > .config file produced, grep fails.)
> 
> Does 'allmodconfig' reproduce the warning for you:
> 
>   $ make allmodconfig
>   $ make arch/x86/mm/numa.o
> 
> ?
> 
> If yes, then this could be due to gcc-12, as it doesn't reproduce with 
> gcc-11 for me:
> 
>    gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1) 
> 
> Thanks,
> 
>     Ingo
> 

There is no reason why allmodconfig would trigger the warning, as it has
CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the other case.
Still, I tried that, and as expected there was no warning.

Did you try the config file I had linked to earlier?

Thanks,
Siddh

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-08-03  8:58         ` Siddh Raman Pant
@ 2022-08-03  9:08           ` Ingo Molnar
  -1 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-03  9:08 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> > Does 'allmodconfig' reproduce the warning for you:
> > 
> >   $ make allmodconfig
> >   $ make arch/x86/mm/numa.o
> > 
> > ?
> > 
> > If yes, then this could be due to gcc-12, as it doesn't reproduce with 
> > gcc-11 for me:
> > 
> >    gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1) 
> > 
> > Thanks,
> > 
> >     Ingo
> > 
> 
> There is no reason why allmodconfig would trigger the warning, [...]

Well, unless I'm misreading your changelog, all the warning needs to 
trigger is CONFIG_CPUMASK_OFFSTACK=y.

> as it has CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the 
> other case.

What 'other case'? I've re-read the discussion and don't see it, but maybe 
I'm a bit daft this morning ...

> Did you try the config file I had linked to earlier?

Yes, and it didn't trigger the warning.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-03  9:08           ` Ingo Molnar
  0 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-03  9:08 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> > Does 'allmodconfig' reproduce the warning for you:
> > 
> >   $ make allmodconfig
> >   $ make arch/x86/mm/numa.o
> > 
> > ?
> > 
> > If yes, then this could be due to gcc-12, as it doesn't reproduce with 
> > gcc-11 for me:
> > 
> >    gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1) 
> > 
> > Thanks,
> > 
> >     Ingo
> > 
> 
> There is no reason why allmodconfig would trigger the warning, [...]

Well, unless I'm misreading your changelog, all the warning needs to 
trigger is CONFIG_CPUMASK_OFFSTACK=y.

> as it has CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the 
> other case.

What 'other case'? I've re-read the discussion and don't see it, but maybe 
I'm a bit daft this morning ...

> Did you try the config file I had linked to earlier?

Yes, and it didn't trigger the warning.

Thanks,

	Ingo
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-08-03  9:08           ` Ingo Molnar
@ 2022-08-03  9:21             ` Siddh Raman Pant via Linux-kernel-mentees
  -1 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant @ 2022-08-03  9:21 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees

On Wed, 03 Aug 2022 14:38:19 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> 
> * Siddh Raman Pant <code@siddh.me> wrote:
> > There is no reason why allmodconfig would trigger the warning, [...]
> 
> Well, unless I'm misreading your changelog, all the warning needs to 
> trigger is CONFIG_CPUMASK_OFFSTACK=y.
> 
> > as it has CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the 
> > other case.
> 
> What 'other case'? I've re-read the discussion and don't see it, but maybe 
> I'm a bit daft this morning ...
 
No, the warning is happening because CONFIG_CPUMASK_OFFSTACK is not set. This
is because in that case cpumask_var_t type is not a pointer, and thus the var
can never be NULL, which leads gcc to warn us when comparing with NULL.

The chain of events are like:

        #ifdef CONFIG_CPUMASK_OFFSTACK
                typedef struct cpumask *cpumask_var_t;
        #else
                typedef struct cpumask cpumask_var_t[1];
        endif

        cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
        ...
        if (node_to_cpumask_map[node] == NULL) {


The fix works because:

        #ifdef CONFIG_CPUMASK_OFFSTACK
                static inline bool cpumask_available(cpumask_var_t mask)
                {
                        return mask != NULL;
                }
        #else
                static inline bool cpumask_available(cpumask_var_t mask)
                {
                        return true;
                }
        endif


The conditional definitions, as pointed out earlier, can be seen from line 700
of include/linux/cpumask.h file.

> > Did you try the config file I had linked to earlier?
> 
> Yes, and it didn't trigger the warning.
> 
> Thanks,
> 
>     Ingo
> 

 Thanks,
 Siddh

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-03  9:21             ` Siddh Raman Pant via Linux-kernel-mentees
  0 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant via Linux-kernel-mentees @ 2022-08-03  9:21 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees

On Wed, 03 Aug 2022 14:38:19 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> 
> * Siddh Raman Pant <code@siddh.me> wrote:
> > There is no reason why allmodconfig would trigger the warning, [...]
> 
> Well, unless I'm misreading your changelog, all the warning needs to 
> trigger is CONFIG_CPUMASK_OFFSTACK=y.
> 
> > as it has CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the 
> > other case.
> 
> What 'other case'? I've re-read the discussion and don't see it, but maybe 
> I'm a bit daft this morning ...
 
No, the warning is happening because CONFIG_CPUMASK_OFFSTACK is not set. This
is because in that case cpumask_var_t type is not a pointer, and thus the var
can never be NULL, which leads gcc to warn us when comparing with NULL.

The chain of events are like:

        #ifdef CONFIG_CPUMASK_OFFSTACK
                typedef struct cpumask *cpumask_var_t;
        #else
                typedef struct cpumask cpumask_var_t[1];
        endif

        cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
        ...
        if (node_to_cpumask_map[node] == NULL) {


The fix works because:

        #ifdef CONFIG_CPUMASK_OFFSTACK
                static inline bool cpumask_available(cpumask_var_t mask)
                {
                        return mask != NULL;
                }
        #else
                static inline bool cpumask_available(cpumask_var_t mask)
                {
                        return true;
                }
        endif


The conditional definitions, as pointed out earlier, can be seen from line 700
of include/linux/cpumask.h file.

> > Did you try the config file I had linked to earlier?
> 
> Yes, and it didn't trigger the warning.
> 
> Thanks,
> 
>     Ingo
> 

 Thanks,
 Siddh
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-08-03  9:21             ` Siddh Raman Pant via Linux-kernel-mentees
@ 2022-08-03  9:42               ` Ingo Molnar
  -1 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-03  9:42 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> On Wed, 03 Aug 2022 14:38:19 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> > 
> > * Siddh Raman Pant <code@siddh.me> wrote:
> > > There is no reason why allmodconfig would trigger the warning, [...]
> > 
> > Well, unless I'm misreading your changelog, all the warning needs to 
> > trigger is CONFIG_CPUMASK_OFFSTACK=y.
> > 
> > > as it has CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the 
> > > other case.
> > 
> > What 'other case'? I've re-read the discussion and don't see it, but maybe 
> > I'm a bit daft this morning ...
>  
> No, the warning is happening because CONFIG_CPUMASK_OFFSTACK is not set.

Oh. So I was daft, as suspected. ;-)

And I can confirm that while gcc-11 doesn't trigger the warning, gcc-12 
does:

  $ make ARCH=x86_64 CC=gcc-11 arch/x86/mm/numa.o
    CC      arch/x86/mm/numa.o
  $

  $ rm -f arch/x86/mm/numa.o
  $
  $ make ARCH=x86_64 CC=gcc-12 arch/x86/mm/numa.o

    CC      arch/x86/mm/numa.o
  arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
  arch/x86/mm/numa.c:916:39: error: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Werror=address]
    916 |         if (node_to_cpumask_map[node] == NULL) {


> [...] This is because in that case cpumask_var_t type is not a pointer, 
> and thus the var can never be NULL, which leads gcc to warn us when 
> comparing with NULL.
> 
> The chain of events are like:
> 
>         #ifdef CONFIG_CPUMASK_OFFSTACK
>                 typedef struct cpumask *cpumask_var_t;
>         #else
>                 typedef struct cpumask cpumask_var_t[1];
>         endif
> 
>         cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
>         ...
>         if (node_to_cpumask_map[node] == NULL) {
> 
> 
> The fix works because:
> 
>         #ifdef CONFIG_CPUMASK_OFFSTACK
>                 static inline bool cpumask_available(cpumask_var_t mask)
>                 {
>                         return mask != NULL;
>                 }
>         #else
>                 static inline bool cpumask_available(cpumask_var_t mask)
>                 {
>                         return true;
>                 }
>         endif
> 
> 
> The conditional definitions, as pointed out earlier, can be seen from line 700
> of include/linux/cpumask.h file.

Yeah - and I agree with your fix - will apply it.

Thanks for your patience :-)

	Ingo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-03  9:42               ` Ingo Molnar
  0 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2022-08-03  9:42 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees


* Siddh Raman Pant <code@siddh.me> wrote:

> On Wed, 03 Aug 2022 14:38:19 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> > 
> > * Siddh Raman Pant <code@siddh.me> wrote:
> > > There is no reason why allmodconfig would trigger the warning, [...]
> > 
> > Well, unless I'm misreading your changelog, all the warning needs to 
> > trigger is CONFIG_CPUMASK_OFFSTACK=y.
> > 
> > > as it has CONFIG_CPUMASK_OFFSTACK=y, but the warning is because of the 
> > > other case.
> > 
> > What 'other case'? I've re-read the discussion and don't see it, but maybe 
> > I'm a bit daft this morning ...
>  
> No, the warning is happening because CONFIG_CPUMASK_OFFSTACK is not set.

Oh. So I was daft, as suspected. ;-)

And I can confirm that while gcc-11 doesn't trigger the warning, gcc-12 
does:

  $ make ARCH=x86_64 CC=gcc-11 arch/x86/mm/numa.o
    CC      arch/x86/mm/numa.o
  $

  $ rm -f arch/x86/mm/numa.o
  $
  $ make ARCH=x86_64 CC=gcc-12 arch/x86/mm/numa.o

    CC      arch/x86/mm/numa.o
  arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
  arch/x86/mm/numa.c:916:39: error: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Werror=address]
    916 |         if (node_to_cpumask_map[node] == NULL) {


> [...] This is because in that case cpumask_var_t type is not a pointer, 
> and thus the var can never be NULL, which leads gcc to warn us when 
> comparing with NULL.
> 
> The chain of events are like:
> 
>         #ifdef CONFIG_CPUMASK_OFFSTACK
>                 typedef struct cpumask *cpumask_var_t;
>         #else
>                 typedef struct cpumask cpumask_var_t[1];
>         endif
> 
>         cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
>         ...
>         if (node_to_cpumask_map[node] == NULL) {
> 
> 
> The fix works because:
> 
>         #ifdef CONFIG_CPUMASK_OFFSTACK
>                 static inline bool cpumask_available(cpumask_var_t mask)
>                 {
>                         return mask != NULL;
>                 }
>         #else
>                 static inline bool cpumask_available(cpumask_var_t mask)
>                 {
>                         return true;
>                 }
>         endif
> 
> 
> The conditional definitions, as pointed out earlier, can be seen from line 700
> of include/linux/cpumask.h file.

Yeah - and I agree with your fix - will apply it.

Thanks for your patience :-)

	Ingo
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-08-03  9:42               ` Ingo Molnar
@ 2022-08-03  9:46                 ` Siddh Raman Pant
  -1 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant via Linux-kernel-mentees @ 2022-08-03  9:46 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dave Hansen, Peter Zijlstra, x86, linux-kernel, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Thomas Gleixner, linux-kernel-mentees

On Wed, 03 Aug 2022 15:12:11 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> Oh. So I was daft, as suspected. ;-)
> 
> And I can confirm that while gcc-11 doesn't trigger the warning, gcc-12 
> does:
> 
>   $ make ARCH=x86_64 CC=gcc-11 arch/x86/mm/numa.o
>     CC      arch/x86/mm/numa.o
>   $
> 
>   $ rm -f arch/x86/mm/numa.o
>   $
>   $ make ARCH=x86_64 CC=gcc-12 arch/x86/mm/numa.o
> 
>     CC      arch/x86/mm/numa.o
>   arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
>   arch/x86/mm/numa.c:916:39: error: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Werror=address]
>     916 |         if (node_to_cpumask_map[node] == NULL) {
> 
> 
> > [...]
> 
> Yeah - and I agree with your fix - will apply it.
> 
> Thanks for your patience :-)
> 
>     Ingo
> 

No worries, and thanks! :)
Siddh
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check
@ 2022-08-03  9:46                 ` Siddh Raman Pant
  0 siblings, 0 replies; 19+ messages in thread
From: Siddh Raman Pant @ 2022-08-03  9:46 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: x86, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-kernel, linux-kernel-mentees

On Wed, 03 Aug 2022 15:12:11 +0530  Ingo Molnar <mingo@kernel.org> wrote:
> Oh. So I was daft, as suspected. ;-)
> 
> And I can confirm that while gcc-11 doesn't trigger the warning, gcc-12 
> does:
> 
>   $ make ARCH=x86_64 CC=gcc-11 arch/x86/mm/numa.o
>     CC      arch/x86/mm/numa.o
>   $
> 
>   $ rm -f arch/x86/mm/numa.o
>   $
>   $ make ARCH=x86_64 CC=gcc-12 arch/x86/mm/numa.o
> 
>     CC      arch/x86/mm/numa.o
>   arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
>   arch/x86/mm/numa.c:916:39: error: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Werror=address]
>     916 |         if (node_to_cpumask_map[node] == NULL) {
> 
> 
> > [...]
> 
> Yeah - and I agree with your fix - will apply it.
> 
> Thanks for your patience :-)
> 
>     Ingo
> 

No worries, and thanks! :)
Siddh

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip: x86/urgent] x86/numa: Use cpumask_available instead of hardcoded NULL check
  2022-07-31 16:09 ` Siddh Raman Pant
  (?)
  (?)
@ 2022-08-03 15:41 ` tip-bot2 for Siddh Raman Pant
  -1 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Siddh Raman Pant @ 2022-08-03 15:41 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Siddh Raman Pant, Ingo Molnar, x86, linux-kernel

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     625395c4a0f4775e0fe00f616888d2e6c1ba49db
Gitweb:        https://git.kernel.org/tip/625395c4a0f4775e0fe00f616888d2e6c1ba49db
Author:        Siddh Raman Pant <code@siddh.me>
AuthorDate:    Sun, 31 Jul 2022 21:39:13 +05:30
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 03 Aug 2022 11:44:57 +02:00

x86/numa: Use cpumask_available instead of hardcoded NULL check

GCC-12 started triggering a new warning:

  arch/x86/mm/numa.c: In function ‘cpumask_of_node’:
  arch/x86/mm/numa.c:916:39: warning: the comparison will always evaluate as ‘false’ for the address of ‘node_to_cpumask_map’ will never be NULL [-Waddress]
    916 |         if (node_to_cpumask_map[node] == NULL) {
        |                                       ^~

node_to_cpumask_map is of type cpumask_var_t[].

When CONFIG_CPUMASK_OFFSTACK is set, cpumask_var_t is typedef'd to a
pointer for dynamic allocation, else to an array of one element. The
"wicked game" can be checked on line 700 of include/linux/cpumask.h.

The original code in debug_cpumask_set_cpu() and cpumask_of_node() were
probably written by the original authors with CONFIG_CPUMASK_OFFSTACK=y
(i.e. dynamic allocation) in mind, checking if the cpumask was available
via a direct NULL check.

When CONFIG_CPUMASK_OFFSTACK is not set, GCC gives the above warning
while compiling the kernel.

Fix that by using cpumask_available(), which does the NULL check when
CONFIG_CPUMASK_OFFSTACK is set, otherwise returns true. Use it wherever
such checks are made.

Conditional definitions of cpumask_available() can be found along with
the definition of cpumask_var_t. Check the cpumask.h reference mentioned
above.

Fixes: c032ef60d1aa ("cpumask: convert node_to_cpumask_map[] to cpumask_var_t")
Fixes: de2d9445f162 ("x86: Unify node_to_cpumask_map handling between 32 and 64bit")
Signed-off-by: Siddh Raman Pant <code@siddh.me>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20220731160913.632092-1-code@siddh.me
---
 arch/x86/mm/numa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index e8b0615..2aadb20 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -867,7 +867,7 @@ void debug_cpumask_set_cpu(int cpu, int node, bool enable)
 		return;
 	}
 	mask = node_to_cpumask_map[node];
-	if (!mask) {
+	if (!cpumask_available(mask)) {
 		pr_err("node_to_cpumask_map[%i] NULL\n", node);
 		dump_stack();
 		return;
@@ -913,7 +913,7 @@ const struct cpumask *cpumask_of_node(int node)
 		dump_stack();
 		return cpu_none_mask;
 	}
-	if (node_to_cpumask_map[node] == NULL) {
+	if (!cpumask_available(node_to_cpumask_map[node])) {
 		printk(KERN_WARNING
 			"cpumask_of_node(%d): no node_to_cpumask_map!\n",
 			node);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-08-03 15:42 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-31 16:09 [PATCH] x86/numa: Use cpumask_available instead of hardcoded NULL check Siddh Raman Pant via Linux-kernel-mentees
2022-07-31 16:09 ` Siddh Raman Pant
2022-08-02 11:07 ` Ingo Molnar
2022-08-02 11:07   ` Ingo Molnar
2022-08-02 16:29   ` Siddh Raman Pant via Linux-kernel-mentees
2022-08-02 16:29     ` Siddh Raman Pant
2022-08-03  8:48     ` Ingo Molnar
2022-08-03  8:48       ` Ingo Molnar
2022-08-03  8:58       ` Siddh Raman Pant via Linux-kernel-mentees
2022-08-03  8:58         ` Siddh Raman Pant
2022-08-03  9:08         ` Ingo Molnar
2022-08-03  9:08           ` Ingo Molnar
2022-08-03  9:21           ` Siddh Raman Pant
2022-08-03  9:21             ` Siddh Raman Pant via Linux-kernel-mentees
2022-08-03  9:42             ` Ingo Molnar
2022-08-03  9:42               ` Ingo Molnar
2022-08-03  9:46               ` Siddh Raman Pant via Linux-kernel-mentees
2022-08-03  9:46                 ` Siddh Raman Pant
2022-08-03 15:41 ` [tip: x86/urgent] " tip-bot2 for Siddh Raman Pant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.