[PATCH v1 0/3] Cleanup and fix tools/mm/slabinfo utility

[PATCH v1 0/3] Cleanup and fix tools/mm/slabinfo utility

[wangxuewen]

This series fixes one bug and cleans up two code quality issues in tools/mm/slabinfo:

1. Fix trace disable logic inversion: when the user intends to disable
   tracing (!tracing) and it is currently enabled (s->trace),
   set_obj() was called with 1 instead of 0, which is opposite to
   the intended behavior. All other options (sanity_checks, red_zone,
   poison, store_user) in the same function use 0 for the disable
   case.

2. Remove dead assignment in get_obj_and_str(): `x = NULL` sets the
   local parameter variable instead of `*x`, which is a no-op since
   `*x` was already set to NULL on the line above.

3. Remove redundant slab->partial assignment in read_slab_dir():
   slab->partial is assigned by get_obj("partial") and then
   immediately overwritten by get_obj_and_str("partial", &t).

wangxuewen (3):
  tools/mm/slabinfo: Fix trace disable logic inversion
  tools/mm/slabinfo: remove dead assignment in get_obj_and_str()
  tools/mm/slabinfo: remove redundant slab->partial assignment

 tools/mm/slabinfo.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

-- 
2.25.1

[PATCH v1 1/3] tools/mm/slabinfo: Fix trace disable logic inversion

[wangxuewen]

The disable trace path in slab_debug() had a logic error where it would
set trace=1 instead of trace=0. This made trace functionality permanently
enabled once turned on for any slab cache.

Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>
---
 tools/mm/slabinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
index 54c7265ab52d..39f7eae7eecd 100644
--- a/tools/mm/slabinfo.c
+++ b/tools/mm/slabinfo.c
@@ -798,7 +798,7 @@ static void slab_debug(struct slabinfo *s)
 			fprintf(stderr, "%s can only enable trace for one slab at a time\n", s->name);
 	}
 	if (!tracing && s->trace)
-		set_obj(s, "trace", 1);
+		set_obj(s, "trace", 0);
 }
 
 static void totals(void)
-- 
2.25.1

Re: [PATCH v1 1/3] tools/mm/slabinfo: Fix trace disable logic inversion

[SeongJae Park]

Hello wangxuewen,

On Fri, 15 May 2026 14:53:23 +0800 wangxuewen <18810879172@163.com> wrote:

> The disable trace path in slab_debug() had a logic error where it would
> set trace=1 instead of trace=0. This made trace functionality permanently
> enabled once turned on for any slab cache.

Nice catch.

> 
> Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>

checkpatch.pl gives below warning.  I think that should be fixed:

    WARNING: From:/Signed-off-by: email address mismatch: 'From: wangxuewen <18810879172@163.com>' != 'Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>'

Also, should we add Fixes: and Cc: stable@ to this patch?

Assuming the email address mismatch will be fixed (my second question is not a
blocking question),

Reviewed-by: SeongJae Park <sj@kernel.org>


> ---
>  tools/mm/slabinfo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 54c7265ab52d..39f7eae7eecd 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -798,7 +798,7 @@ static void slab_debug(struct slabinfo *s)
>  			fprintf(stderr, "%s can only enable trace for one slab at a time\n", s->name);
>  	}
>  	if (!tracing && s->trace)
> -		set_obj(s, "trace", 1);
> +		set_obj(s, "trace", 0);
>  }

I think it might make sense to use 'tracing' as the third argument.  Just a
loud thought of my personal taste.  I don't think that's what this patch should
do.


Thanks,
SJ

[...]

[PATCH v1 2/3] tools/mm/slabinfo: remove dead assignment in get_obj_and_str()

[wangxuewen]

The assignment `x = NULL` sets the local parameter variable instead of
`*x`, which is a no-op since `*x` was already set to NULL on the line
above. Remove the dead assignment.

Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>
---
 tools/mm/slabinfo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
index 39f7eae7eecd..ac00daee41d1 100644
--- a/tools/mm/slabinfo.c
+++ b/tools/mm/slabinfo.c
@@ -194,7 +194,6 @@ static unsigned long get_obj_and_str(const char *name, char **x)
 	*x = NULL;
 
 	if (!read_obj(name)) {
-		x = NULL;
 		return 0;
 	}
 	result = strtoul(buffer, &p, 10);
-- 
2.25.1

Re: [PATCH v1 2/3] tools/mm/slabinfo: remove dead assignment in get_obj_and_str()

[SeongJae Park]

On Fri, 15 May 2026 14:53:24 +0800 wangxuewen <18810879172@163.com> wrote:

> The assignment `x = NULL` sets the local parameter variable instead of
> `*x`, which is a no-op since `*x` was already set to NULL on the line
> above. Remove the dead assignment.
> 
> Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>

The email address mismatches.

> ---
>  tools/mm/slabinfo.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 39f7eae7eecd..ac00daee41d1 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -194,7 +194,6 @@ static unsigned long get_obj_and_str(const char *name, char **x)
>  	*x = NULL;
>  
>  	if (!read_obj(name)) {
> -		x = NULL;
>  		return 0;
>  	}

You could further remove the braces?

>  	result = strtoul(buffer, &p, 10);
> -- 
> 2.25.1


Thanks,
SJ

[PATCH v1 3/3] tools/mm/slabinfo: remove redundant slab->partial assignment

[wangxuewen]

slab->partial is assigned by get_obj("partial") and then immediately
overwritten by get_obj_and_str("partial", &t). Remove the first
redundant assignment.

Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>
---
 tools/mm/slabinfo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
index ac00daee41d1..e52d718ba5f2 100644
--- a/tools/mm/slabinfo.c
+++ b/tools/mm/slabinfo.c
@@ -1265,7 +1265,6 @@ static void read_slab_dir(void)
 			slab->objects_total = get_obj("objects_total");
 			slab->objs_per_slab = get_obj("objs_per_slab");
 			slab->order = get_obj("order");
-			slab->partial = get_obj("partial");
 			slab->partial = get_obj_and_str("partial", &t);
 			decode_numa_list(slab->numa_partial, t);
 			free(t);
-- 
2.25.1

Re: [PATCH v1 3/3] tools/mm/slabinfo: remove redundant slab->partial assignment

[SeongJae Park]

On Fri, 15 May 2026 14:53:25 +0800 wangxuewen <18810879172@163.com> wrote:

> slab->partial is assigned by get_obj("partial") and then immediately
> overwritten by get_obj_and_str("partial", &t). Remove the first
> redundant assignment.
> 
> Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>

The singer's email mismatches with the author's one.

Assuming that will be fixed,

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]