Re: [PATCH v2] mm: slub: Convert sys slab alloc_calls, free_calls to bin attribute

[Vlastimil Babka <vbabka@suse.cz>]

On 3/19/21 6:28 AM, Faiyaz Mohammed wrote:
> Hi,
> 
> Sorry for late response!
> 
> On 3/2/2021 6:41 PM, Greg Kroah-Hartman wrote:
>> On Tue, Mar 02, 2021 at 01:59:46PM +0100, Vlastimil Babka wrote:
>>> On 2/17/21 7:31 AM, Faiyaz Mohammed wrote:
>>>> Reading the sys slab alloc_calls, free_calls returns the available object
>>>> owners, but the size of this file is limited to PAGE_SIZE
>>>> because of the limitation of sysfs attributes, it is returning the
>>>> partial owner info, which is not sufficient to debug/account the slab
>>>> memory and alloc_calls output is not matching with /proc/slabinfo.
>>>>
>>>> To remove the PAGE_SIZE limitation converted the sys slab
>>>> alloc_calls, free_calls to bin attribut
>>>>
>>>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org>
>>>
>>> After discussing this with Greg and PeterZ, sysfs should be one value per file,
>>> and is just not proper API for this kind of info. We should reimplement at least
>>> these clearly debugging "attributes" in debugfs, where they belong, instead of
>>> trying to hack around the limitation in sysfs.
>> 
>> sysfs is _REQUIRED_ to be "one value per file", any kernel code that
>> abuses this needs to be fixed up.
>> 
>> Why are these slab files in sysfs at all anyway?  They all feel like
>> debugging stuff to me, why not move everything to debugfs?  Would make
>> for a lot less code overall.
>> 
> Yes, we can move the /sys/kernel/slab/kmemcache/alloc_calls and
> /sys/kernel/slab/kmemcache/free_calls implementation to debugfs but is
> it okay to move only alloc_calls and free_calls? or we have to move
> whole sysfs interface to debugfs?.

I don't think we need to move everything, just files where it makes sense.

> If we are moving only alloc_calls/free_calls interface to debugfs then I
> think we can add all slab objects data into single file.
> 
> For example: /sys/kernel/debugfs/slab/alloc_calls, which will print all
> slab objects data.
> 
> Example Output: alloc_list: call_site=__request_region+0xb4/0x2f0
> count=228 object_size=128 slab_size=640 slab_name=kmalloc-128

I wouldn't do this, as processing all caches will have a large overhead and then
somebody interested in single cache would throw most of the info away.

> or
> 
> We can have just like current sysfs interface, have separate
> alloc_calls/free_calls traces per kmem cache.
> 
> Which one would be better?.
> 
>> 
> 
> Thanks and regards,
> Mohammed Faiyaz
>