[PATCH V2 0/2] Introduce slabinfo version 2.2

[PATCH V2 0/2] Introduce slabinfo version 2.2

[Fangzheng Zhang]

Hi all,

This series introduces slabinfo version 2.2 to users.
In slabinfo V2.2, we added a slabreclaim column to
record whether each slab pool is of reclaim type.
This will be more conducive for users to obtain
the type of each slabdata more intuitively than through
the interface /sys/kernel/slab/$cache/reclaim_account.
And we have added an example of the output result
executing '> cat proc/slabinfo' in the file
Documentation/filesystems/proc.rst.

Changes in v2:
- Modify the slabinfo version number to 2.2.
- Add an example of slabinfo output and future works.

Changes in v1:
- Add a slabreclaim column to record type of each slab
  in file proc/slabinfo.

[1] https://lore.kernel.org/linux-mm/20240131094442.28834-1-fangzheng.zhang@unisoc.com/

Fangzheng Zhang (2):
  mm/slab: Add slabreclaim flag to slabinfo
  Documentation: filesystems: introduce proc/slabinfo to users

 Documentation/filesystems/proc.rst | 33 ++++++++++++++++++++++++++++++
 mm/slab_common.c                   |  9 ++++----
 2 files changed, 38 insertions(+), 4 deletions(-)

-- 
2.17.1

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

[Fangzheng Zhang]

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 | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 238293b1dbe1..fd865ca335ea 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1035,10 +1035,10 @@ static void print_slabinfo_header(struct seq_file *m)
 	 * Output format version, so at least we can change it
 	 * without _too_ many complaints.
 	 */
-	seq_puts(m, "slabinfo - version: 2.1\n");
+	seq_puts(m, "slabinfo - version: 2.2\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.17.1

[PATCH V2 2/2] Documentation: filesystems: introduce proc/slabinfo to users

[Fangzheng Zhang]

Supplement slabinfo-version 2.2 details in proc.rst, so that
users can have the status of slabinfo at a glance. And mark
the optimization work that will be performed on proc/slabinfo
in the next step.

Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
---
 Documentation/filesystems/proc.rst | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 104c6d047d9b..89ab92f6be2d 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -892,6 +892,39 @@ Linux uses  slab  pools for memory management above page level in version 2.2.
 Commonly used  objects  have  their  own  slab  pool (such as network buffers,
 directory cache, and so on).
 
+Example output. You can have all of these fields in slabinfo - version: 2.2.
+
+::
+
+    > cat /proc/slabinfo
+
+    slabinfo - version: 2.2
+    # name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab> : tunables <limit> <batchcount> <sharedfactor> : slabdata <active_slabs> <num_slabs> <sharedavail> <slabreclaim>
+    zspage              2240   2240     72   56    1 : tunables    0    0    0 : slabdata     40     40      0      0
+    zs_handle          17408  17408      8  512    1 : tunables    0    0    0 : slabdata     34     34      0      0
+    f2fs_xattr_entry-254:48    312    312    208   39    2 : tunables    0    0    0 : slabdata      8      8      0      1
+    imsbr_flow           102    102     80   51    1 : tunables    0    0    0 : slabdata      2      2      0      0
+    ......
+    ext4_groupinfo_4k    312    312    208   39    2 : tunables    0    0    0 : slabdata      8      8      0      1
+    dm_verity_fec_buffers      8      8   4048    8    8 : tunables    0    0    0 : slabdata      1      1      0      0
+    dm_bufio_buffer       28     28    144   28    1 : tunables    0    0    0 : slabdata      1      1      0      1
+    ......
+    kernfs_iattrs_cache   4010   4116     96   42    1 : tunables    0    0    0 : slabdata     98     98      0      0
+    kernfs_node_cache  67169  67232    128   32    1 : tunables    0    0    0 : slabdata   2101   2101      0      0
+    mnt_cache           5624   5700    320   25    2 : tunables    0    0    0 : slabdata    228    228      0      0
+    filp               15840  17400    320   25    2 : tunables    0    0    0 : slabdata    696    696      0      0
+    ......
+    kmalloc-32         30398  32384     32  128    1 : tunables    0    0    0 : slabdata    253    253      0      0
+    kmalloc-16         31566  31744     16  256    1 : tunables    0    0    0 : slabdata    124    124      0      0
+    kmalloc-8          51623  51712      8  512    1 : tunables    0    0    0 : slabdata    101    101      0      0
+    kmem_cache_node      416    416    128   32    1 : tunables    0    0    0 : slabdata     13     13      0      0
+    kmem_cache           416    416    256   32    2 : tunables    0    0    0 : slabdata     13     13      0      0
+
+Note, <slabreclaim> comes from the collected results in the file
+/sys/kernel/slab/$cache/reclaim_account. Next, we will 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.
+
 ::
 
     > cat /proc/buddyinfo
-- 
2.17.1

Re: [PATCH V2 2/2] Documentation: filesystems: introduce proc/slabinfo to users

[Matthew Wilcox]

On Mon, Feb 19, 2024 at 11:19:11AM +0800, Fangzheng Zhang wrote:
> +Note, <slabreclaim> comes from the collected results in the file
> +/sys/kernel/slab/$cache/reclaim_account. Next, we will 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.

Wait, so you're going to all of the trouble of changing the format of
slabinfo (with the associated costs of updating every tool that currently
parses it), only to recommend that we stop using it and start using
tools/mm/slabinfo instead?

How about we simply do nothing?

Re: [PATCH V2 2/2] Documentation: filesystems: introduce proc/slabinfo to users

[Vlastimil Babka]

On 2/19/24 07:23, zhang fangzheng wrote:
> On Mon, Feb 19, 2024 at 12:24 PM Matthew Wilcox <willy@infradead.org> wrote:
>>
>> On Mon, Feb 19, 2024 at 11:19:11AM +0800, Fangzheng Zhang wrote:
>> > +Note, <slabreclaim> comes from the collected results in the file
>> > +/sys/kernel/slab/$cache/reclaim_account. Next, we will 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.
>>
>> Wait, so you're going to all of the trouble of changing the format of
>> slabinfo (with the associated costs of updating every tool that currently
>> parses it), only to recommend that we stop using it and start using
>> tools/mm/slabinfo instead?
>>

Hi,

> The initial purpose was to obtain the type of each slab through
> a simple command 'cat proc/slabinfo'. So here, my intention is not to
> update all slabinfo-related tools for the time being, but to modify
> the version number of proc/slabinfo and further display the results
> of using the command.

I'm not sure you understand the concern. There are existing consumers of
/proc/slabinfo, that might become broken by patch 1/2. We don't even know
them all, they might not be all opensource etc. So we can't even make sure
all of them are updated. What can happen after patch 1/2:
- they keep working and ignore the new column (good)
- they include a version check and notice a new unsupported version and
refuse to work
- confused by the new column they start throwing error, or report wrong
stats (that's worse)

>> How about we simply do nothing?

Agreed wrt modifying /proc/slabinfo

> The note here means what changes will occur after
> we modify the version number of proc/slabinfo to 2.2.
> As for the replacement of tools/mm/slabinfo (that inspired
> by Christoph’s suggestions), it will be implemented in the next version
> or even the later version.

So what is your motivation for all this in the first place? You have some
monitoring tool that relies on /proc/slabinfo and want to distinguish
reclaimable caches? So you can change it to parse the /sys directories. Is
it more work? Yes, but you only have to do that once per boot, because
unlike the object/memory stats in /proc/slabinfo, the reclaimable flag will
not change for a cache.

Would tools/mm/slabinfo almost work for you, but you're missing something?
Then send patches for that in the first place. Changing /proc/slabinfo (and
breaking other consumers) for a quick and easy fix with a different solution
planned for the future is simply not feasible.

HTH,
Vlastimil

> Thanks!

Re: [PATCH V2 0/2] Introduce slabinfo version 2.2

[Chengming Zhou]

On 2024/2/19 11:19, Fangzheng Zhang wrote:
> Hi all,
> 
> This series introduces slabinfo version 2.2 to users.
> In slabinfo V2.2, we added a slabreclaim column to
> record whether each slab pool is of reclaim type.
> This will be more conducive for users to obtain
> the type of each slabdata more intuitively than through
> the interface /sys/kernel/slab/$cache/reclaim_account.

I want to recommend a better tool: drgn[1] for these tasks, instead of changing
the output format of /proc/slabinfo, which may break existing userspace tools.

[1] https://drgn.readthedocs.io/en/latest/index.html#

> And we have added an example of the output result
> executing '> cat proc/slabinfo' in the file
> Documentation/filesystems/proc.rst.
> 
> Changes in v2:
> - Modify the slabinfo version number to 2.2.
> - Add an example of slabinfo output and future works.
> 
> Changes in v1:
> - Add a slabreclaim column to record type of each slab
>   in file proc/slabinfo.
> 
> [1] https://lore.kernel.org/linux-mm/20240131094442.28834-1-fangzheng.zhang@unisoc.com/
> 
> Fangzheng Zhang (2):
>   mm/slab: Add slabreclaim flag to slabinfo
>   Documentation: filesystems: introduce proc/slabinfo to users
> 
>  Documentation/filesystems/proc.rst | 33 ++++++++++++++++++++++++++++++
>  mm/slab_common.c                   |  9 ++++----
>  2 files changed, 38 insertions(+), 4 deletions(-)
> 

Re: [PATCH V2 0/2] Introduce slabinfo version 2.2

[zhang fangzheng]

On Mon, Feb 19, 2024 at 7:29 PM Chengming Zhou
<zhouchengming@bytedance.com> wrote:
>
> On 2024/2/19 11:19, Fangzheng Zhang wrote:
> > Hi all,
> >
> > This series introduces slabinfo version 2.2 to users.
> > In slabinfo V2.2, we added a slabreclaim column to
> > record whether each slab pool is of reclaim type.
> > This will be more conducive for users to obtain
> > the type of each slabdata more intuitively than through
> > the interface /sys/kernel/slab/$cache/reclaim_account.
>
> I want to recommend a better tool: drgn[1] for these tasks, instead of changing
> the output format of /proc/slabinfo, which may break existing userspace tools.
>
> [1] https://drgn.readthedocs.io/en/latest/index.html#
>

Thank you very much for providing a new way.
I have the following three questions about the new tool you provided:
---- 1. From the introduction, the tool is described as an alternative
to the crash utility.
          Will the permission requirements have different effects when
used, user or userdebug?
----  2. The 'Helpers' chapter introduces the simple use of
common.memory, but there is no output example.
           It involves the use of slab objects, but it also needs to
provide a specific slab_cache_name,
           which cannot give an intuitive overall information like
proc/slabinfo.
           I guess it is difficult to achieve direct output of slab
type (reclaim or unreclaim). I don’t know, right?
---- 3. Regarding the supported versions, is it supported for both
arm/arm64? I don't seem to have seen any similar instructions.
Finally, I would like to express my gratitude again. This tool will be
very helpful for me in other future work.

> > And we have added an example of the output result
> > executing '> cat proc/slabinfo' in the file
> > Documentation/filesystems/proc.rst.
> >
> > Changes in v2:
> > - Modify the slabinfo version number to 2.2.
> > - Add an example of slabinfo output and future works.
> >
> > Changes in v1:
> > - Add a slabreclaim column to record type of each slab
> >   in file proc/slabinfo.
> >
> > [1] https://lore.kernel.org/linux-mm/20240131094442.28834-1-fangzheng.zhang@unisoc.com/
> >
> > Fangzheng Zhang (2):
> >   mm/slab: Add slabreclaim flag to slabinfo
> >   Documentation: filesystems: introduce proc/slabinfo to users
> >
> >  Documentation/filesystems/proc.rst | 33 ++++++++++++++++++++++++++++++
> >  mm/slab_common.c                   |  9 ++++----
> >  2 files changed, 38 insertions(+), 4 deletions(-)
> >

Re: [PATCH V2 0/2] Introduce slabinfo version 2.2

[Chengming Zhou]

On 2024/2/20 14:25, zhang fangzheng wrote:
> On Mon, Feb 19, 2024 at 7:29 PM Chengming Zhou
> <zhouchengming@bytedance.com> wrote:
>>
>> On 2024/2/19 11:19, Fangzheng Zhang wrote:
>>> Hi all,
>>>
>>> This series introduces slabinfo version 2.2 to users.
>>> In slabinfo V2.2, we added a slabreclaim column to
>>> record whether each slab pool is of reclaim type.
>>> This will be more conducive for users to obtain
>>> the type of each slabdata more intuitively than through
>>> the interface /sys/kernel/slab/$cache/reclaim_account.
>>
>> I want to recommend a better tool: drgn[1] for these tasks, instead of changing
>> the output format of /proc/slabinfo, which may break existing userspace tools.
>>
>> [1] https://drgn.readthedocs.io/en/latest/index.html#
>>
> 
> Thank you very much for providing a new way.
> I have the following three questions about the new tool you provided:
> ---- 1. From the introduction, the tool is described as an alternative
> to the crash utility.
>           Will the permission requirements have different effects when
> used, user or userdebug?
> ----  2. The 'Helpers' chapter introduces the simple use of
> common.memory, but there is no output example.
>            It involves the use of slab objects, but it also needs to
> provide a specific slab_cache_name,
>            which cannot give an intuitive overall information like
> proc/slabinfo.

You can of course use drgn to iterate over all slabs by using the global
"slab_caches" list. (All kernel space is at your hand.)

>            I guess it is difficult to achieve direct output of slab
> type (reclaim or unreclaim). I don’t know, right?

It's easy for drgn to inspect each slab's information.

> ---- 3. Regarding the supported versions, is it supported for both
> arm/arm64? I don't seem to have seen any similar instructions.
> Finally, I would like to express my gratitude again. This tool will be
> very helpful for me in other future work.

Please check https://github.com/osandov/drgn for details.

Thanks.

Re: [PATCH V2 2/2] Documentation: filesystems: introduce proc/slabinfo to users

[zhang fangzheng]

On Mon, Feb 19, 2024 at 4:09 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 2/19/24 07:23, zhang fangzheng wrote:
> > On Mon, Feb 19, 2024 at 12:24 PM Matthew Wilcox <willy@infradead.org> wrote:
> >>
> >> On Mon, Feb 19, 2024 at 11:19:11AM +0800, Fangzheng Zhang wrote:
> >> > +Note, <slabreclaim> comes from the collected results in the file
> >> > +/sys/kernel/slab/$cache/reclaim_account. Next, we will 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.
> >>
> >> Wait, so you're going to all of the trouble of changing the format of
> >> slabinfo (with the associated costs of updating every tool that currently
> >> parses it), only to recommend that we stop using it and start using
> >> tools/mm/slabinfo instead?
> >>
>
> Hi,
>
> > The initial purpose was to obtain the type of each slab through
> > a simple command 'cat proc/slabinfo'. So here, my intention is not to
> > update all slabinfo-related tools for the time being, but to modify
> > the version number of proc/slabinfo and further display the results
> > of using the command.
>
> I'm not sure you understand the concern. There are existing consumers of
> /proc/slabinfo, that might become broken by patch 1/2. We don't even know
> them all, they might not be all opensource etc. So we can't even make sure
> all of them are updated. What can happen after patch 1/2:
> - they keep working and ignore the new column (good)
> - they include a version check and notice a new unsupported version and
> refuse to work
> - confused by the new column they start throwing error, or report wrong
> stats (that's worse)
>
I generally understand your concerns about modifying patch 1/2.

But judging from my modifications, this worry does not seem to be valid.
Because the “/proc/slabinfo” is not used in related slabinfo debugging tools
(such as tools/mm/slabinfo), but "/sys/kernel/slab/<slab_name>/" (in
Documentation/mm/slub.rst) or "/ sys/kernel/debug/slab" (in
tools/mm/slabinfo.c).

Furthermore, the current modification only involves optimizing the output
of proc/slabinfo, and does not modify the  struct slabinfo or struct kmem_cache.
So there is no need to adapt other modifications.

> >> How about we simply do nothing?
>
> Agreed wrt modifying /proc/slabinfo
>
> > The note here means what changes will occur after
> > we modify the version number of proc/slabinfo to 2.2.
> > As for the replacement of tools/mm/slabinfo (that inspired
> > by Christoph’s suggestions), it will be implemented in the next version
> > or even the later version.
>
> So what is your motivation for all this in the first place? You have some
> monitoring tool that relies on /proc/slabinfo and want to distinguish
> reclaimable caches? So you can change it to parse the /sys directories. Is
> it more work? Yes, but you only have to do that once per boot, because
> unlike the object/memory stats in /proc/slabinfo, the reclaimable flag will
> not change for a cache.
>
The situation as you mentioned is very suitable for my current needs.
My original intention is just to get an intuitive slab screen through a
simple ‘cat proc/slabinfo’ command. As for the description "<slabreclaim>
comes from the collected results in the file
/sys/kernel/slab/$cache/reclaim_account"
may not be appropriate. Here I want to express that the column <slabreclaim> has
the same effect as traversing "/sys/kernel/slab/$ cache/reclaim_account".

> Would tools/mm/slabinfo almost work for you, but you're missing something?
> Then send patches for that in the first place. Changing /proc/slabinfo (and
> breaking other consumers) for a quick and easy fix with a different solution
> planned for the future is simply not feasible.
>
Using the slabinfo tool to parse /sys/kernel/slab/$cache/reclaim_account
is what I think about optimizing future tools during the discussion.
It will not affect the current patch 1/2, and patch 2/2 is mainly to
supplement the output examples of proc/slabinfo.

If the community is willing to accept it, I will only modify
patch 1/2 to implement it.

Thanks very much!

> HTH,
> Vlastimil
>
> > Thanks!
>

Re: [PATCH V2 2/2] Documentation: filesystems: introduce proc/slabinfo to users

[Vlastimil Babka]

On 2/20/24 09:49, zhang fangzheng wrote:
> On Mon, Feb 19, 2024 at 4:09 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> On 2/19/24 07:23, zhang fangzheng wrote:
>> > On Mon, Feb 19, 2024 at 12:24 PM Matthew Wilcox <willy@infradead.org> wrote:
>> >>
>> >> On Mon, Feb 19, 2024 at 11:19:11AM +0800, Fangzheng Zhang wrote:
>> >> > +Note, <slabreclaim> comes from the collected results in the file
>> >> > +/sys/kernel/slab/$cache/reclaim_account. Next, we will 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.
>> >>
>> >> Wait, so you're going to all of the trouble of changing the format of
>> >> slabinfo (with the associated costs of updating every tool that currently
>> >> parses it), only to recommend that we stop using it and start using
>> >> tools/mm/slabinfo instead?
>> >>
>>
>> Hi,
>>
>> > The initial purpose was to obtain the type of each slab through
>> > a simple command 'cat proc/slabinfo'. So here, my intention is not to
>> > update all slabinfo-related tools for the time being, but to modify
>> > the version number of proc/slabinfo and further display the results
>> > of using the command.
>>
>> I'm not sure you understand the concern. There are existing consumers of
>> /proc/slabinfo, that might become broken by patch 1/2. We don't even know
>> them all, they might not be all opensource etc. So we can't even make sure
>> all of them are updated. What can happen after patch 1/2:
>> - they keep working and ignore the new column (good)
>> - they include a version check and notice a new unsupported version and
>> refuse to work
>> - confused by the new column they start throwing error, or report wrong
>> stats (that's worse)
>>
> I generally understand your concerns about modifying patch 1/2.
> 
> But judging from my modifications, this worry does not seem to be valid.
> Because the “/proc/slabinfo” is not used in related slabinfo debugging tools
> (such as tools/mm/slabinfo),

Hi,

we are not concerned about slabinfo debugging tools that are part of kernel
source tree, but about those outside, including those created privately and
we don't even know they exist.

> but "/sys/kernel/slab/<slab_name>/" (in
> Documentation/mm/slub.rst) or "/ sys/kernel/debug/slab" (in
> tools/mm/slabinfo.c).
> 
> Furthermore, the current modification only involves optimizing the output
> of proc/slabinfo,

It's not "only", the output of /proc/slabinfo is what those tools consume,
so that's what concerns us the most.

> and does not modify the  struct slabinfo or struct kmem_cache.
> So there is no need to adapt other modifications.

These on the other hand are internal details of the kernel which we can
modify as much we want

>> >> How about we simply do nothing?
>>
>> Agreed wrt modifying /proc/slabinfo
>>
>> > The note here means what changes will occur after
>> > we modify the version number of proc/slabinfo to 2.2.
>> > As for the replacement of tools/mm/slabinfo (that inspired
>> > by Christoph’s suggestions), it will be implemented in the next version
>> > or even the later version.
>>
>> So what is your motivation for all this in the first place? You have some
>> monitoring tool that relies on /proc/slabinfo and want to distinguish
>> reclaimable caches? So you can change it to parse the /sys directories. Is
>> it more work? Yes, but you only have to do that once per boot, because
>> unlike the object/memory stats in /proc/slabinfo, the reclaimable flag will
>> not change for a cache.
>>
> The situation as you mentioned is very suitable for my current needs.
> My original intention is just to get an intuitive slab screen through a
> simple ‘cat proc/slabinfo’ command. As for the description "<slabreclaim>

That would be nice, but again we must be careful about existing consumers of
/proc/slabinfo so we can't always have nice things.

> comes from the collected results in the file
> /sys/kernel/slab/$cache/reclaim_account"
> may not be appropriate. Here I want to express that the column <slabreclaim> has
> the same effect as traversing "/sys/kernel/slab/$ cache/reclaim_account".
> 
>> Would tools/mm/slabinfo almost work for you, but you're missing something?
>> Then send patches for that in the first place. Changing /proc/slabinfo (and
>> breaking other consumers) for a quick and easy fix with a different solution
>> planned for the future is simply not feasible.
>>
> Using the slabinfo tool to parse /sys/kernel/slab/$cache/reclaim_account
> is what I think about optimizing future tools during the discussion.
> It will not affect the current patch 1/2, and patch 2/2 is mainly to
> supplement the output examples of proc/slabinfo.
> 
> If the community is willing to accept it, I will only modify
> patch 1/2 to implement it.
> 
> Thanks very much!
> 
>> HTH,
>> Vlastimil
>>
>> > Thanks!
>>

Re: [PATCH V2 2/2] Documentation: filesystems: introduce proc/slabinfo to users

[zhang fangzheng]

On Tue, Feb 20, 2024 at 5:21 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 2/20/24 09:49, zhang fangzheng wrote:
> > On Mon, Feb 19, 2024 at 4:09 PM Vlastimil Babka <vbabka@suse.cz> wrote:
> >>
> >> On 2/19/24 07:23, zhang fangzheng wrote:
> >> > On Mon, Feb 19, 2024 at 12:24 PM Matthew Wilcox <willy@infradead.org> wrote:
> >> >>
> >> >> On Mon, Feb 19, 2024 at 11:19:11AM +0800, Fangzheng Zhang wrote:
> >> >> > +Note, <slabreclaim> comes from the collected results in the file
> >> >> > +/sys/kernel/slab/$cache/reclaim_account. Next, we will 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.
> >> >>
> >> >> Wait, so you're going to all of the trouble of changing the format of
> >> >> slabinfo (with the associated costs of updating every tool that currently
> >> >> parses it), only to recommend that we stop using it and start using
> >> >> tools/mm/slabinfo instead?
> >> >>
> >>
> >> Hi,
> >>
> >> > The initial purpose was to obtain the type of each slab through
> >> > a simple command 'cat proc/slabinfo'. So here, my intention is not to
> >> > update all slabinfo-related tools for the time being, but to modify
> >> > the version number of proc/slabinfo and further display the results
> >> > of using the command.
> >>
> >> I'm not sure you understand the concern. There are existing consumers of
> >> /proc/slabinfo, that might become broken by patch 1/2. We don't even know
> >> them all, they might not be all opensource etc. So we can't even make sure
> >> all of them are updated. What can happen after patch 1/2:
> >> - they keep working and ignore the new column (good)
> >> - they include a version check and notice a new unsupported version and
> >> refuse to work
> >> - confused by the new column they start throwing error, or report wrong
> >> stats (that's worse)
> >>
> > I generally understand your concerns about modifying patch 1/2.
> >
> > But judging from my modifications, this worry does not seem to be valid.
> > Because the “/proc/slabinfo” is not used in related slabinfo debugging tools
> > (such as tools/mm/slabinfo),
>
> Hi,
>
> we are not concerned about slabinfo debugging tools that are part of kernel
> source tree, but about those outside, including those created privately and
> we don't even know they exist.
>
For your concerns, I think the supplementary introduction that new
output results
of slabinfo v2.2  in patch 2/2 will be necessary. This can help them
optimize their tools
more quickly to adapt to proc/slabinfo. Is this more friendly?

> > but "/sys/kernel/slab/<slab_name>/" (in
> > Documentation/mm/slub.rst) or "/ sys/kernel/debug/slab" (in
> > tools/mm/slabinfo.c).
> >
> > Furthermore, the current modification only involves optimizing the output
> > of proc/slabinfo,
>
> It's not "only", the output of /proc/slabinfo is what those tools consume,
> so that's what concerns us the most.
>
> > and does not modify the  struct slabinfo or struct kmem_cache.
> > So there is no need to adapt other modifications.
>
> These on the other hand are internal details of the kernel which we can
> modify as much we want
>
> >> >> How about we simply do nothing?
> >>
> >> Agreed wrt modifying /proc/slabinfo
> >>
> >> > The note here means what changes will occur after
> >> > we modify the version number of proc/slabinfo to 2.2.
> >> > As for the replacement of tools/mm/slabinfo (that inspired
> >> > by Christoph’s suggestions), it will be implemented in the next version
> >> > or even the later version.
> >>
> >> So what is your motivation for all this in the first place? You have some
> >> monitoring tool that relies on /proc/slabinfo and want to distinguish
> >> reclaimable caches? So you can change it to parse the /sys directories. Is
> >> it more work? Yes, but you only have to do that once per boot, because
> >> unlike the object/memory stats in /proc/slabinfo, the reclaimable flag will
> >> not change for a cache.
> >>
> > The situation as you mentioned is very suitable for my current needs.
> > My original intention is just to get an intuitive slab screen through a
> > simple ‘cat proc/slabinfo’ command. As for the description "<slabreclaim>
>
> That would be nice, but again we must be careful about existing consumers of
> /proc/slabinfo so we can't always have nice things.
>
> > comes from the collected results in the file
> > /sys/kernel/slab/$cache/reclaim_account"
> > may not be appropriate. Here I want to express that the column <slabreclaim> has
> > the same effect as traversing "/sys/kernel/slab/$ cache/reclaim_account".
> >
> >> Would tools/mm/slabinfo almost work for you, but you're missing something?
> >> Then send patches for that in the first place. Changing /proc/slabinfo (and
> >> breaking other consumers) for a quick and easy fix with a different solution
> >> planned for the future is simply not feasible.
> >>
> > Using the slabinfo tool to parse /sys/kernel/slab/$cache/reclaim_account
> > is what I think about optimizing future tools during the discussion.
> > It will not affect the current patch 1/2, and patch 2/2 is mainly to
> > supplement the output examples of proc/slabinfo.
> >
> > If the community is willing to accept it, I will only modify
> > patch 1/2 to implement it.
> >
> > Thanks very much!
> >
> >> HTH,
> >> Vlastimil
> >>
> >> > Thanks!
> >>
>