linux-kernel - Re: [PATCH v7] mm: slub: move sysfs slab alloc/free interfaces to debugfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <544e7eaf-ceaa-0ff0-15c3-b615db7e1b00@codeaurora.org>
Date:   Wed, 26 May 2021 20:36:18 +0530
From:   Faiyaz Mohammed <faiyazm@...eaurora.org>
To:     Vlastimil Babka <vbabka@...e.cz>,
        Greg KH <gregkh@...uxfoundation.org>
Cc:     cl@...ux.com, penberg@...nel.org, rientjes@...gle.com,
        iamjoonsoo.kim@....com, akpm@...ux-foundation.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        glittao@...il.com, vinmenon@...eaurora.org
Subject: Re: [PATCH v7] mm: slub: move sysfs slab alloc/free interfaces to
 debugfs



On 5/26/2021 4:33 PM, Vlastimil Babka wrote:
> On 5/25/21 1:54 PM, Greg KH wrote:
>> On Tue, May 25, 2021 at 02:27:15PM +0530, Faiyaz Mohammed wrote:
>>>> --- a/mm/slab_common.c
>>>>> +++ b/mm/slab_common.c
>>>>> @@ -455,6 +455,9 @@ static void slab_caches_to_rcu_destroy_workfn(struct work_struct *work)
>>>>>  #else
>>>>>  		slab_kmem_cache_release(s);
>>>>>  #endif
>>>>> +#ifdef SLAB_SUPPORTS_DEBUGFS
>>>>> +		debugfs_slab_release(s);
>>>>> +#endif
>>>>
>>>> Why do you need these #ifdef if your slub_dev.h file already provides an
>>>> "empty" function for this?
>>>>
>>> We are not including slub_def.h directly. mm/slab.h includes the
>>> slub_def.h if CONFIG_SLUB enable,
>>>
>>> from mm/slab.h
>>> #ifdef CONFIG_SLAB
>>> #include <linux/slab_def.h>
>>> #endif
>>>
>>> #ifdef CONFIG_SLUB
>>> #include <linux/slub_def.h>
>>> #endif
>>>
>>> so if CONFIG_SLAB is enable then mm/slab.h includes slab_def.h, to avoid
>>> undefined reference error added SLAB_SUPPORTS_DEBUGFS like
>>> SLAB_SUPPORTS_SYSFS.
>>
>> Ick, ok, messy code, I'll stop complaining now if this really is the
>> only way to do it (still feels wrong to me...)
> 
> How about simply replicating the empty function in
> include/linux/slab_def.h
> 
Yes, we can add empty function in include/linux/slab_def.h.
I will add in next patch version.

> We could do the same with SYSFS, except the SLAB (and SLUB w/o SYSFS) versions
> of sysfs_slab_release() would not be empty, but just call
> slab_kmem_cache_release(s);
> Then we could get rid of the #ifdef's completely?
> 
Is it okay, if I raise separate patch for sysfs by adding empty function
in slab_def.h?

Thanks and regards,
Mohammed Faiyaz
>> greg k-h
>>
>