Re: [PATCH 0/6] refine and rename slub sysfs

[Wei Yang <richard.weiyang@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3497 bytes --]

On Tue, May 23, 2017 at 08:39:11AM +0200, Michal Hocko wrote:
>On Tue 23-05-17 11:27:05, Wei Yang wrote:
>> On Thu, May 18, 2017 at 11:06:37AM +0200, Michal Hocko wrote:
>> >On Wed 17-05-17 22:11:40, Wei Yang wrote:
>> >> This patch serial could be divided into two parts.
>> >> 
>> >> First three patches refine and adds slab sysfs.
>> >> Second three patches rename slab sysfs.
>> >> 
>> >> 1. Refine slab sysfs
>> >> 
>> >> There are four level slabs:
>> >> 
>> >>     CPU
>> >>     CPU_PARTIAL
>> >>     PARTIAL
>> >>     FULL
>> >> 
>> >> And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
>> >> reflect the statistics.
>> >> 
>> >> In patch 2, it splits some function in show_slab_objects() which makes sure
>> >> only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.
>> >> 
>> >> After doing so, it would be more clear that show_slab_objects() has totally 9
>> >> statistic combinations for three level of slabs. Each slab has three cases
>> >> statistic.
>> >> 
>> >>     slabs
>> >>     objects
>> >>     total_objects
>> >> 
>> >> And when we look at current implementation, some of them are missing. So patch
>> >> 2 & 3 add them up.
>> >> 
>> >> 2. Rename sysfs
>> >> 
>> >> The slab statistics in sysfs are
>> >> 
>> >>     slabs
>> >>     objects
>> >>     total_objects
>> >>     cpu_slabs
>> >>     partial
>> >>     partial_objects
>> >>     cpu_partial_slabs
>> >> 
>> >> which is a little bit hard for users to understand. The second three patches
>> >> rename sysfs file in this pattern.
>> >> 
>> >>     xxx_slabs[[_total]_objects]
>> >> 
>> >> Finally it looks Like
>> >> 
>> >>     slabs
>> >>     slabs_objects
>> >>     slabs_total_objects
>> >>     cpu_slabs
>> >>     cpu_slabs_objects
>> >>     cpu_slabs_total_objects
>> >>     partial_slabs
>> >>     partial_slabs_objects
>> >>     partial_slabs_total_objects
>> >>     cpu_partial_slabs
>> >
>> >_Why_ do we need all this?
>> 
>> To have a clear statistics for each slab level.
>
>Is this worth risking breakage of the userspace which consume this data
>now? Do you have any user space code which will greatly benefit from the
>new data and which couldn't do the same with the current format/output?
>
>If yes this all should be in the changelog.

The answer is no.

I have the same concern as yours. So this patch set could be divided into two
parts: 1. add some new entry with current name convention, 2. change the name
convention.

If there are many userspace tools use these entries, the changing is really
risky, I agree. Hmm, I still send this out, since current name convention is a
little difficult for users to understand, especially after we have several
levels slabs. Is it possible to use the name convention I proposed and add
link to them to keep the userspace interface?

And the second part is to fully utilize current functions. In function
show_slab_objects(), we have 9 combinations of slab statistics. 3 for each
slab level. And currently code just enable 6 of them. So the first three tries
to enable the missing 3 to make it a more complete statistics.

BTW, I found we don't have any entry for full slabs statistics. Not sure this
is omitted intendedly or not. If the community agrees, I still have a path to
enable the statistics for full slabs.

Thanks for your comments~ Michal

>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]