The for-loop might re-read the content of the memory the si_bits_global
points to on each iteration. Instead, just capture it for the sake of
consistency and use that instead.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/sys_info.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/sys_info.c b/lib/sys_info.c
index 44bc6d96b702..5d98560f3f53 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -58,11 +58,11 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
 	char names[sizeof(sys_info_avail) + 1];
 	struct ctl_table table;
 	unsigned long *si_bits_global;
+	unsigned long si_bits;
 
 	si_bits_global = ro_table->data;
 
 	if (write) {
-		unsigned long si_bits;
 		int ret;
 
 		table = *ro_table;
@@ -81,8 +81,11 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
 		char *delim = "";
 		int i, len = 0;
 
+		/* The access to the global value is not synchronized. */
+		si_bits = READ_ONCE(*si_bits_global);
+
 		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
-			if (*si_bits_global & si_names[i].bit) {
+			if (si_bits & si_names[i].bit) {
 				len += scnprintf(names + len, sizeof(names) - len,
 					"%s%s", delim, si_names[i].name);
 				delim = ",";
-- 
2.47.2

Hi Andy,

Thanks for the patch! please cc Petr Mladek <pmladek@suse.com> for changes
as I mentioned in the cover letter, he contributed a lot to the code and arch
from RFC to v3.

On Fri, Jul 11, 2025 at 12:51:09PM +0300, Andy Shevchenko wrote:
> The for-loop might re-read the content of the memory the si_bits_global
> points to on each iteration. Instead, just capture it for the sake of
> consistency and use that instead.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  lib/sys_info.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/sys_info.c b/lib/sys_info.c
> index 44bc6d96b702..5d98560f3f53 100644
> --- a/lib/sys_info.c
> +++ b/lib/sys_info.c
> @@ -58,11 +58,11 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
>  	char names[sizeof(sys_info_avail) + 1];
>  	struct ctl_table table;
>  	unsigned long *si_bits_global;
> +	unsigned long si_bits;
>  
>  	si_bits_global = ro_table->data;
>  
>  	if (write) {
> -		unsigned long si_bits;
>  		int ret;
>  
>  		table = *ro_table;
> @@ -81,8 +81,11 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
>  		char *delim = "";
>  		int i, len = 0;
>  
> +		/* The access to the global value is not synchronized. */
> +		si_bits = READ_ONCE(*si_bits_global);

Good catch!

Thanks,
Feng

> +
>  		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> -			if (*si_bits_global & si_names[i].bit) {
> +			if (si_bits & si_names[i].bit) {
>  				len += scnprintf(names + len, sizeof(names) - len,
>  					"%s%s", delim, si_names[i].name);
>  				delim = ",";
> -- 
> 2.47.2
>

+Cc: Petr (as requested by Feng).

On Fri, Jul 11, 2025 at 10:56:33PM +0800, Feng Tang wrote:
> 
> Thanks for the patch! please cc Petr Mladek <pmladek@suse.com> for changes
> as I mentioned in the cover letter, he contributed a lot to the code and arch
> from RFC to v3.

Sure, I can do that in next version if that one will be needed (otherwise I
would wait for your new version, depending of what we decide to do, because it
seems to me the 7 patches on a brand new feature which even doesn't compile is
too many).

-- 
With Best Regards,
Andy Shevchenko