[PATCH] mm/slab: Add slabreclaim flag to slabinfo

Fangzheng Zhang posted 1 patch 1 year, 10 months ago
mm/slab_common.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
[PATCH] mm/slab: Add slabreclaim flag to slabinfo
Posted by Fangzheng Zhang 1 year, 10 months ago
In order to enhance slab debugging, we add slabreclaim flag to
slabinfo. Slab type is also an important analysis point in slabinfo
for per slab, when various problems such as memory leaks or memory
statistics occur.

Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
---
 mm/slab_common.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 238293b1dbe1..aeeb2bfe6dda 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1038,7 +1038,7 @@ static void print_slabinfo_header(struct seq_file *m)
 	seq_puts(m, "slabinfo - version: 2.1\n");
 	seq_puts(m, "# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab>");
 	seq_puts(m, " : tunables <limit> <batchcount> <sharedfactor>");
-	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail>");
+	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail> <slabreclaim>");
 	seq_putc(m, '\n');
 }
 
@@ -1071,8 +1071,9 @@ static void cache_show(struct kmem_cache *s, struct seq_file *m)
 
 	seq_printf(m, " : tunables %4u %4u %4u",
 		   sinfo.limit, sinfo.batchcount, sinfo.shared);
-	seq_printf(m, " : slabdata %6lu %6lu %6lu",
-		   sinfo.active_slabs, sinfo.num_slabs, sinfo.shared_avail);
+	seq_printf(m, " : slabdata %6lu %6lu %6lu %6u",
+		   sinfo.active_slabs, sinfo.num_slabs, sinfo.shared_avail,
+		   !!(s->flags & SLAB_RECLAIM_ACCOUNT));
 	slabinfo_show_stats(m, s);
 	seq_putc(m, '\n');
 }
-- 
2.43.0
Re: [PATCH] mm/slab: Add slabreclaim flag to slabinfo
Posted by Greg KH 1 year, 10 months ago
On Wed, Jan 31, 2024 at 05:44:42PM +0800, Fangzheng Zhang wrote:
> In order to enhance slab debugging, we add slabreclaim flag to
> slabinfo. Slab type is also an important analysis point in slabinfo
> for per slab, when various problems such as memory leaks or memory
> statistics occur.
> 
> Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
> ---
>  mm/slab_common.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 238293b1dbe1..aeeb2bfe6dda 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1038,7 +1038,7 @@ static void print_slabinfo_header(struct seq_file *m)
>  	seq_puts(m, "slabinfo - version: 2.1\n");
>  	seq_puts(m, "# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab>");
>  	seq_puts(m, " : tunables <limit> <batchcount> <sharedfactor>");
> -	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail>");
> +	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail> <slabreclaim>");

Doesn't this change the slabinfo version number above?  Where is this
change documented so that userspace knows about it?

thanks,

greg k-h
Re: [PATCH] mm/slab: Add slabreclaim flag to slabinfo
Posted by Vlastimil Babka 1 year, 10 months ago
On 2/4/24 14:09, Greg KH wrote:
> On Wed, Jan 31, 2024 at 05:44:42PM +0800, Fangzheng Zhang wrote:
>> In order to enhance slab debugging, we add slabreclaim flag to
>> slabinfo. Slab type is also an important analysis point in slabinfo
>> for per slab, when various problems such as memory leaks or memory
>> statistics occur.
>> 
>> Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
>> ---
>>  mm/slab_common.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 238293b1dbe1..aeeb2bfe6dda 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -1038,7 +1038,7 @@ static void print_slabinfo_header(struct seq_file *m)
>>  	seq_puts(m, "slabinfo - version: 2.1\n");
>>  	seq_puts(m, "# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab>");
>>  	seq_puts(m, " : tunables <limit> <batchcount> <sharedfactor>");
>> -	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail>");
>> +	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail> <slabreclaim>");
> 
> Doesn't this change the slabinfo version number above?  Where is this
> change documented so that userspace knows about it?

Yeah I was vary about this. Do the other longer-time-than-me slab
maintainers recall how we handled this in the past?
Also the information is already available, even if harder to gather, in the
file /sys/kernel/slab/$cache/reclaim_account

> thanks,
> 
> greg k-h
Re: [PATCH] mm/slab: Add slabreclaim flag to slabinfo
Posted by zhang fangzheng 1 year, 10 months ago
On Mon, Feb 5, 2024 at 4:50 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 2/4/24 14:09, Greg KH wrote:
> > On Wed, Jan 31, 2024 at 05:44:42PM +0800, Fangzheng Zhang wrote:
> >> In order to enhance slab debugging, we add slabreclaim flag to
> >> slabinfo. Slab type is also an important analysis point in slabinfo
> >> for per slab, when various problems such as memory leaks or memory
> >> statistics occur.
> >>
> >> Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
> >> ---
> >>  mm/slab_common.c | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/mm/slab_common.c b/mm/slab_common.c
> >> index 238293b1dbe1..aeeb2bfe6dda 100644
> >> --- a/mm/slab_common.c
> >> +++ b/mm/slab_common.c
> >> @@ -1038,7 +1038,7 @@ static void print_slabinfo_header(struct seq_file *m)
> >>      seq_puts(m, "slabinfo - version: 2.1\n");
> >>      seq_puts(m, "# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab>");
> >>      seq_puts(m, " : tunables <limit> <batchcount> <sharedfactor>");
> >> -    seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail>");
> >> +    seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail> <slabreclaim>");
> >
> > Doesn't this change the slabinfo version number above?  Where is this
> > change documented so that userspace knows about it?
>
Ok, I will  modify the slabinfo version number to 2.2 and I find that
there is no corresponding slabinfo output example in the proc.rst
document. Can we add an output example so that user space knows about
it?

> Yeah I was vary about this. Do the other longer-time-than-me slab
> maintainers recall how we handled this in the past?
> Also the information is already available, even if harder to gather, in the
> file /sys/kernel/slab/$cache/reclaim_account
>
First  of all, thank you very much for your comments.

I would like to say, when performing slab memory information
maintenance, people often hope to see more detailed information
through a simple slabinfo command. As you mentioned the method, but it
is very unintuitive to the status of the entire slab, so we add the
slabreclaim column to slabinfo and directly output it using cmdline '
> cat proc/slabinfo'. And I think this approach will also be helpful
for future work on memory statistics.

> > thanks,
> >
> > greg k-h
>
Thanks.
Re: [PATCH] mm/slab: Add slabreclaim flag to slabinfo
Posted by Christoph Lameter (Ampere) 1 year, 10 months ago
On Mon, 5 Feb 2024, Vlastimil Babka wrote:

> On 2/4/24 14:09, Greg KH wrote:
>> On Wed, Jan 31, 2024 at 05:44:42PM +0800, Fangzheng Zhang wrote:
>>> In order to enhance slab debugging, we add slabreclaim flag to
>>> slabinfo. Slab type is also an important analysis point in slabinfo
>>> for per slab, when various problems such as memory leaks or memory
>>> statistics occur.
>>>
>>> Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
>>> ---
>>>  mm/slab_common.c | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>>> index 238293b1dbe1..aeeb2bfe6dda 100644
>>> --- a/mm/slab_common.c
>>> +++ b/mm/slab_common.c
>>> @@ -1038,7 +1038,7 @@ static void print_slabinfo_header(struct seq_file *m)
>>>  	seq_puts(m, "slabinfo - version: 2.1\n");
>>>  	seq_puts(m, "# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab>");
>>>  	seq_puts(m, " : tunables <limit> <batchcount> <sharedfactor>");
>>> -	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail>");
>>> +	seq_puts(m, " : slabdata <active_slabs> <num_slabs> <sharedavail> <slabreclaim>");
>>
>> Doesn't this change the slabinfo version number above?  Where is this
>> change documented so that userspace knows about it?

I have never seen such a document. I would suggest incrementing the 
version to 2.2 since there is a minor extension of the format.

I tried to remove /proc/slabinfo in the past and have people use the more 
versatile /sys/kernel/slab interface. But /proc/slabinfo has a long 
legacy.

Could we mark /proc/slabinfo as deprecated and recommend the use of
either sysfs directly or use of the "slabinfo" tool that we have 
been providing in linux/tools/mm for a long time?