[PATCH] mm/slab: Increase width of first /proc/slabinfo column

[PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Tobin C. Harding]

Currently when displaying /proc/slabinfo if any cache names are too long
then the output columns are not aligned.  We could do something fancy to
get the maximum length of any cache name in the system or we could just
increase the hardcoded width.  Currently it is 17 characters.  Monitors
are wide these days so lets just increase it to 30 characters.

On one running kernel, with this choice of width, the increase is
sufficient to align the columns and total line width is increased from
112 to 119 characters (excluding the heading row).  Admittedly there may
be cache names in the wild which are longer than the cache names on this
machine, in which case the columns would still be unaligned.

Increase the width of the first column (cache name) in the output of
/proc/slabinfo from 17 to 30 characters.

Signed-off-by: Tobin C. Harding <tobin@kernel.org>
---

This patch does not touch the heading row, and discussion of column
width excludes this row.  Please note that the second column labeled by
the heading row is now *not* above the second column.

### Before patch is applied sample output of `cat /proc/slabinfo` (max column width == 112):

slabinfo - version: 2.1
# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab> : tunables <limit> <batchcount> <sharedfactor> : slabdata <active_slabs> <num_slabs> <sharedavail>
kcopyd_job             0      0   3312    9    8 : tunables    0    0    0 : slabdata      0      0      0
dm_uevent              0      0   2632   12    8 : tunables    0    0    0 : slabdata      0      0      0
fuse_request          60     60    392   20    2 : tunables    0    0    0 : slabdata      3      3      0
fuse_inode            21     21    768   21    4 : tunables    0    0    0 : slabdata      1      1      0
kvm_async_pf          90     90    136   30    1 : tunables    0    0    0 : slabdata      3      3      0
kvm_vcpu               4      4  24192    1    8 : tunables    0    0    0 : slabdata      4      4      0
kvm_mmu_page_header    100    150    160   25    1 : tunables    0    0    0 : slabdata      6      6      0
i915_request         100    100    640   25    4 : tunables    0    0    0 : slabdata      4      4      0
i915_vma             316    336    576   28    4 : tunables    0    0    0 : slabdata     12     12      0
fat_inode_cache       22     22    728   22    4 : tunables    0    0    0 : slabdata      1      1      0
fat_cache              0      0     40  102    1 : tunables    0    0    0 : slabdata      0      0      0
ext4_groupinfo_4k   3780   3780    144   28    1 : tunables    0    0    0 : slabdata    135    135      0
ext4_inode_cache  255633 258480   1080   30    8 : tunables    0    0    0 : slabdata   8616   8616      0
ext4_allocation_context    128    128    128   32    1 : tunables    0    0    0 : slabdata      4      4      0
ext4_io_end          256    256     64   64    1 : tunables    0    0    0 : slabdata      4      4      0
ext4_extent_status 197111 197778     40  102    1 : tunables    0    0    0 : slabdata   1939   1939      0
mbcache              294    584     56   73    1 : tunables    0    0    0 : slabdata      8      8      0
jbd2_journal_head    364    476    120   34    1 : tunables    0    0    0 : slabdata     14     14      0
jbd2_revoke_table_s    512    512     16  256    1 : tunables    0    0    0 : slabdata      2      2      0
fscrypt_info         512   1024     32  128    1 : tunables    0    0    0 : slabdata      8      8      0
...


### With patch applied output of `cat /proc/slabinfo` (max column width == 119):

slabinfo - version: 2.1
# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab> : tunables <limit> <batchcount> <sharedfactor> : slabdata <active_slabs> <num_slabs> <share>
PINGv6                              0      0   1152   14    4 : tunables    0    0    0 : slabdata      0      0      0
RAWv6                              14     14   1152   14    4 : tunables    0    0    0 : slabdata      1      1      0
UDPv6                               0      0   1280   12    4 : tunables    0    0    0 : slabdata      0      0      0
tw_sock_TCPv6                       0      0    240   17    1 : tunables    0    0    0 : slabdata      0      0      0
request_sock_TCPv6                  0      0    304   13    1 : tunables    0    0    0 : slabdata      0      0      0
TCPv6                               0      0   2304   14    8 : tunables    0    0    0 : slabdata      0      0      0
sgpool-128                          8      8   4096    8    8 : tunables    0    0    0 : slabdata      1      1      0
bfq_io_cq                           0      0    160   25    1 : tunables    0    0    0 : slabdata      0      0      0
bfq_queue                           0      0    464   17    2 : tunables    0    0    0 : slabdata      0      0      0
mqueue_inode_cache                  9      9    896    9    2 : tunables    0    0    0 : slabdata      1      1      0
dnotify_struct                      0      0     32  128    1 : tunables    0    0    0 : slabdata      0      0      0
posix_timers_cache                  0      0    240   17    1 : tunables    0    0    0 : slabdata      0      0      0
UNIX                                0      0   1024    8    2 : tunables    0    0    0 : slabdata      0      0      0
ip4-frags                           0      0    208   19    1 : tunables    0    0    0 : slabdata      0      0      0
tcp_bind_bucket                     0      0    128   32    1 : tunables    0    0    0 : slabdata      0      0      0
PING                                0      0    960    8    2 : tunables    0    0    0 : slabdata      0      0      0
RAW                                 8      8    960    8    2 : tunables    0    0    0 : slabdata      1      1      0
tw_sock_TCP                         0      0    240   17    1 : tunables    0    0    0 : slabdata      0      0      0
request_sock_TCP                    0      0    304   13    1 : tunables    0    0    0 : slabdata      0      0      0
TCP                                 0      0   2176   15    8 : tunables    0    0    0 : slabdata      0      0      0
hugetlbfs_inode_cache              13     13    616   13    2 : tunables    0    0    0 : slabdata      1      1      0
dquot                               0      0    256   16    1 : tunables    0    0    0 : slabdata      0      0      0
eventpoll_pwq                       0      0     72   56    1 : tunables    0    0    0 : slabdata      0      0      0
dax_cache                          10     10    768   10    2 : tunables    0    0    0 : slabdata      1      1      0
request_queue                       0      0   2056   15    8 : tunables    0    0    0 : slabdata      0      0      0
biovec-max                          8      8   8192    4    8 : tunables    0    0    0 : slabdata      2      2      0
biovec-128                          8      8   2048    8    4 : tunables    0    0    0 : slabdata      1      1      0
biovec-64                           8      8   1024    8    2 : tunables    0    0    0 : slabdata      1      1      0
user_namespace                      8      8    512    8    1 : tunables    0    0    0 : slabdata      1      1      0
uid_cache                          21     21    192   21    1 : tunables    0    0    0 : slabdata      1      1      0
dmaengine-unmap-2                  64     64     64   64    1 : tunables    0    0    0 : slabdata      1      1      0
sock_inode_cache                   24     24    640   12    2 : tunables    0    0    0 : slabdata      2      2      0
skbuff_fclone_cache                 0      0    448    9    1 : tunables    0    0    0 : slabdata      0      0      0
skbuff_head_cache                  16     16    256   16    1 : tunables    0    0    0 : slabdata      1      1      0
file_lock_cache                     0      0    216   18    1 : tunables    0    0    0 : slabdata      0      0      0
net_namespace                       0      0   3392    9    8 : tunables    0    0    0 : slabdata      0      0      0


 mm/slab_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 81732d05e74a..a339f1361164 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1365,7 +1365,7 @@ static void cache_show(struct kmem_cache *s, struct seq_file *m)
 
 	memcg_accumulate_slabinfo(s, &sinfo);
 
-	seq_printf(m, "%-17s %6lu %6lu %6u %4u %4d",
+	seq_printf(m, "%-30s %6lu %6lu %6u %4u %4d",
 		   cache_name(s), sinfo.active_objs, sinfo.num_objs, s->size,
 		   sinfo.objects_per_slab, (1 << sinfo.cache_order));
 
-- 
2.20.1

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Tobin C. Harding]

On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
[snip]

This applies on top of Linus' tree

	commit e74c98ca2d6a ('gfs2: Revert "Fix loop in gfs2_rbm_find"')

For this patch I doubt very much that it matters but for the record I
can't find mention in MAINTAINERS which tree to base work on for slab
patches.  Are mm patches usually based of an mm tree or do you guys work
off linux-next?

thanks,
Tobin.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Andrew Morton]

On Fri, 1 Feb 2019 11:58:38 +1100 "Tobin C. Harding" <me@tobin.cc> wrote:

> On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
> [snip]
> 
> This applies on top of Linus' tree
> 
> 	commit e74c98ca2d6a ('gfs2: Revert "Fix loop in gfs2_rbm_find"')
> 
> For this patch I doubt very much that it matters but for the record I
> can't find mention in MAINTAINERS which tree to base work on for slab
> patches.  Are mm patches usually based of an mm tree or do you guys work
> off linux-next?

It's usually best to work off current mainline and I handle the
integration stuff.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

On Fri, 1 Feb 2019, Tobin C. Harding wrote:

> Currently when displaying /proc/slabinfo if any cache names are too long
> then the output columns are not aligned.  We could do something fancy to
> get the maximum length of any cache name in the system or we could just
> increase the hardcoded width.  Currently it is 17 characters.  Monitors
> are wide these days so lets just increase it to 30 characters.

Hmm.. I wonder if there are any tools that depend on the field width here?

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Christopher Lameter]

On Fri, 1 Feb 2019, Tobin C. Harding wrote:

> Currently when displaying /proc/slabinfo if any cache names are too long
> then the output columns are not aligned.  We could do something fancy to
> get the maximum length of any cache name in the system or we could just
> increase the hardcoded width.  Currently it is 17 characters.  Monitors
> are wide these days so lets just increase it to 30 characters.

Hmm.. I wonder if there are any tools that depend on the field width here?

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Matthew Wilcox]

On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
> Currently when displaying /proc/slabinfo if any cache names are too long
> then the output columns are not aligned.  We could do something fancy to
> get the maximum length of any cache name in the system or we could just
> increase the hardcoded width.  Currently it is 17 characters.  Monitors
> are wide these days so lets just increase it to 30 characters.

I had a proposal some time ago to turn the slab name from being kmalloced
to being an inline 16 bytes (with some fun hacks for cgroups).  I think
that's a better approach than permitting such long names.  For example,
ext4_allocation_context could be shortened to ext4_alloc_ctx without
losing any expressivity.

Let me know if you can't find that and I'll try to dig it up.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Tobin C. Harding]

On Thu, Jan 31, 2019 at 05:13:06PM -0800, Andrew Morton wrote:
> On Fri, 1 Feb 2019 11:58:38 +1100 "Tobin C. Harding" <me@tobin.cc> wrote:
> 
> > On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
> > [snip]
> > 
> > This applies on top of Linus' tree
> > 
> > 	commit e74c98ca2d6a ('gfs2: Revert "Fix loop in gfs2_rbm_find"')
> > 
> > For this patch I doubt very much that it matters but for the record I
> > can't find mention in MAINTAINERS which tree to base work on for slab
> > patches.  Are mm patches usually based of an mm tree or do you guys work
> > off linux-next?
> 
> It's usually best to work off current mainline and I handle the
> integration stuff.

Awesome, thanks.


	 Tobin

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Tobin C. Harding]

On Thu, Jan 31, 2019 at 06:43:10PM -0800, Matthew Wilcox wrote:
> On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
> > Currently when displaying /proc/slabinfo if any cache names are too long
> > then the output columns are not aligned.  We could do something fancy to
> > get the maximum length of any cache name in the system or we could just
> > increase the hardcoded width.  Currently it is 17 characters.  Monitors
> > are wide these days so lets just increase it to 30 characters.
> 
> I had a proposal some time ago to turn the slab name from being kmalloced
> to being an inline 16 bytes (with some fun hacks for cgroups).  I think
> that's a better approach than permitting such long names.  For example,
> ext4_allocation_context could be shortened to ext4_alloc_ctx without
> losing any expressivity.
> 
> Let me know if you can't find that and I'll try to dig it up.

Thanks Willy, I'll try and find it and bring it back to life.

Cheers,
Tobin.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Andrew Morton]

On Thu, 31 Jan 2019 18:43:10 -0800 Matthew Wilcox <willy@infradead.org> wrote:

> On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
> > Currently when displaying /proc/slabinfo if any cache names are too long
> > then the output columns are not aligned.  We could do something fancy to
> > get the maximum length of any cache name in the system or we could just
> > increase the hardcoded width.  Currently it is 17 characters.  Monitors
> > are wide these days so lets just increase it to 30 characters.
> 
> I had a proposal some time ago to turn the slab name from being kmalloced
> to being an inline 16 bytes (with some fun hacks for cgroups).  I think
> that's a better approach than permitting such long names.  For example,
> ext4_allocation_context could be shortened to ext4_alloc_ctx without
> losing any expressivity.
> 

There are some back-compatibility concerns here.  And truncating long
names might result in duplicates.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Joe Perches]

On Fri, 2019-02-01 at 11:42 +1100, Tobin C. Harding wrote:
> Increase the width of the first column (cache name) in the output of
> /proc/slabinfo from 17 to 30 characters.

Do you care if this breaks any parsing of /proc/slabinfo?

I don't but someone might.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Joe Perches]

On Fri, 2019-02-01 at 11:42 +1100, Tobin C. Harding wrote:
> Increase the width of the first column (cache name) in the output of
> /proc/slabinfo from 17 to 30 characters.

Do you care if this breaks any parsing of /proc/slabinfo?

I don't but someone might.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Pekka Enberg]

Hi,

On 01/02/2019 4.34, Christopher Lameter wrote:
> On Fri, 1 Feb 2019, Tobin C. Harding wrote:
> 
>> Currently when displaying /proc/slabinfo if any cache names are too long
>> then the output columns are not aligned.  We could do something fancy to
>> get the maximum length of any cache name in the system or we could just
>> increase the hardcoded width.  Currently it is 17 characters.  Monitors
>> are wide these days so lets just increase it to 30 characters.
> 
> Hmm.. I wonder if there are any tools that depend on the field width here?
> 

It's possible, but it's more likely that userspace parses by whitespace 
because it's easier to write it that way.

At least procps, which is used by slabtop, is prepared to parse a cache 
name of 128 characters. See the scanf() call in parse_slabinfo20() 
function in proc/slab.c of procps:

   http://procps.sourceforge.net/

Of course, testing with slabtop would make sense.

- Pekka

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Tobin C. Harding]

On Fri, Feb 01, 2019 at 07:27:24PM -0800, Joe Perches wrote:
> On Fri, 2019-02-01 at 11:42 +1100, Tobin C. Harding wrote:
> > Increase the width of the first column (cache name) in the output of
> > /proc/slabinfo from 17 to 30 characters.
> 
> Do you care if this breaks any parsing of /proc/slabinfo?
> 
> I don't but someone might.

Thanks for looking at the patch Joe, Christoph pointed this out also.
Solution is going to take a different approach and not touch the column
width in /proc/slabinfo for the record, although it does not really
matter now, I think that anyone parsing /proc/slabinfo would be using
whitespace because the name field is already variable length.

Anyways, new patch to come.

thanks,
Tobin.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Tobin C. Harding]

On Thu, Jan 31, 2019 at 06:43:10PM -0800, Matthew Wilcox wrote:
> On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
> > Currently when displaying /proc/slabinfo if any cache names are too long
> > then the output columns are not aligned.  We could do something fancy to
> > get the maximum length of any cache name in the system or we could just
> > increase the hardcoded width.  Currently it is 17 characters.  Monitors
> > are wide these days so lets just increase it to 30 characters.
> 
> I had a proposal some time ago to turn the slab name from being kmalloced
> to being an inline 16 bytes (with some fun hacks for cgroups).  I think
> that's a better approach than permitting such long names.  For example,
> ext4_allocation_context could be shortened to ext4_alloc_ctx without
> losing any expressivity.
> 
> Let me know if you can't find that and I'll try to dig it up.

Hi Willy,

I haven't managed to find the patch, I grep'ed LKML using a bunch of
search terms via the google group linux.kernel.  Then I tried a bunch of
different search strings in google prefixed with `site:kernel.org`.  All
to no avail.

I have an idea how to fix it without making life less convenient for
developers *or* for users, I know we don't discuss changes without a
patch so I'll hack it up.

I'm sure your solution contains things I don't understand yet (read: the
cgroups bit) so I'd love to bring your patch back to life but am happy
to work on another solution as well in the name of education.


thanks,
Tobin.

Re: [PATCH] mm/slab: Increase width of first /proc/slabinfo column

[Tobin C. Harding]

On Fri, Feb 01, 2019 at 02:03:46PM -0800, Andrew Morton wrote:
> On Thu, 31 Jan 2019 18:43:10 -0800 Matthew Wilcox <willy@infradead.org> wrote:
> 
> > On Fri, Feb 01, 2019 at 11:42:42AM +1100, Tobin C. Harding wrote:
> > > Currently when displaying /proc/slabinfo if any cache names are too long
> > > then the output columns are not aligned.  We could do something fancy to
> > > get the maximum length of any cache name in the system or we could just
> > > increase the hardcoded width.  Currently it is 17 characters.  Monitors
> > > are wide these days so lets just increase it to 30 characters.
> > 
> > I had a proposal some time ago to turn the slab name from being kmalloced
> > to being an inline 16 bytes (with some fun hacks for cgroups).  I think
> > that's a better approach than permitting such long names.  For example,
> > ext4_allocation_context could be shortened to ext4_alloc_ctx without
> > losing any expressivity.
> > 
> 
> There are some back-compatibility concerns here.

I'm don't understand sorry what back-compatibility concerns (please see
sentiment at end of email :)

> And truncating long names might result in duplicates.

So I thought I had a good idea - add a pr_warn() if cache name > 16 and
patch all current intree calls to kmem_cache_create() called as such.

This process very kindly lead me to the fact that this does *not* work
because of the macro KMEM_CACHE (which uses the struct name as the cache
name).

So, back to the drawing board.  I'm concerned that this may be a waste
of peoples time, if so please say so and I'll move on to something else.

thanks,
Tobin.