Skip to contentSkip to conversation

mm/slab: Increase width of first /proc/slabinfo column

PATCH In review Tobin C. Harding 7 participants 13 messages Feb 1, 2019 – Feb 4, 2019

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

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.

---

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(-)
mm/slab_common.c +1 -1
index 81732d05e74a..a339f1361164 100644
@@ -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));
 
Signature · 1 line
2.20.1
#1 lore raw

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.

maintainer
#2 lore raw
Quoted 11 lines · On Fri, 1 Feb 2019 11:58:38 +1100 "Tobin C. Harding" <[email protected]> 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.

#4 lore raw
In reply to Tobin C. Harding
Quoted 5 lines · 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.

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

#6 lore raw
In reply to Tobin C. Harding
Quoted 5 lines · 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.

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.

#3 lore raw
In reply to Andrew Morton
Quoted 9 lines · Andrew Morton
Quoted 7 lines · On Fri, 1 Feb 2019 11:58:38 +1100 "Tobin C. Harding" <[email protected]> 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"')
Quoted 7 lines
Quoted 4 lines

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

#7 lore raw
In reply to Matthew Wilcox
Quoted 7 lines · Matthew Wilcox
Quoted 5 lines · 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.

Quoted 7 lines

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.

maintainer
#8 lore raw
In reply to Matthew Wilcox
Quoted 7 lines · On Thu, 31 Jan 2019 18:43:10 -0800 Matthew Wilcox <[email protected]> wrote:
Quoted 5 lines · 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.

Quoted 6 lines

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.

#11 lore raw
In reply to Tobin C. Harding
Quoted 2 lines · Tobin C. Harding

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.

#5 lore raw
In reply to Christopher Lameter

Hi,

Quoted 10 lines · Christopher Lameter
Quoted 5 lines · 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.

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

#12 lore raw
In reply to Joe Perches
Quoted 7 lines · Joe Perches
Quoted 2 lines · Tobin C. Harding

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.

#10 lore raw
In reply to Matthew Wilcox
Quoted 7 lines · Matthew Wilcox
Quoted 5 lines · 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.

Quoted 7 lines

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.

#9 lore raw
In reply to Andrew Morton
Quoted 9 lines · Andrew Morton
Quoted 7 lines · On Thu, 31 Jan 2019 18:43:10 -0800 Matthew Wilcox <[email protected]> wrote:
Quoted 5 lines · 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.

Quoted 8 lines
Quoted 6 lines

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 :)

Quoted 1 line

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.