Re: [PATCH] mm, slab: add kerneldocs for common SLAB_ flags

[Vlastimil Babka <vbabka@suse.cz>]

On 10/9/24 17:08, Jonathan Corbet wrote:
> Vlastimil Babka <vbabka@suse.cz> writes:
> 
>> We have many SLAB_ flags but many are used only internally, by kunit
>> tests or debugging subsystems cooperating with slab, or are set
>> according to slab_debug boot parameter.
>>
>> Create kerneldocs for the commonly used flags that may be passed to
>> kmem_cache_create(). SLAB_TYPESAFE_BY_RCU already had a detailed
>> description, so turn it to a kerneldoc. Add some details for
>> SLAB_ACCOUNT, SLAB_RECLAIM_ACCOUNT and SLAB_HWCACHE_ALIGN. Reference
>> them from the __kmem_cache_create_args() kerneldoc.
>>
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>> ---
>> I plan to take this in the slab tree, but a question for Jon/linux-doc:
>>
>> I think I'm doing properly the "Object-like macro documentation" for
>> parameter-less macros from the doc-guide. Yet I can see in the htmldocs
>> things like "SLAB_TYPESAFE_BY_RCU ()" and "Parameters". Is there a bug
>> in the sphinx machinery? Thanks.
> 
> No, it's totally bug-free and any appearance to the contrary is entirely
> in your imagination :)

:)

> I don't think anybody has tried to do kerneldoc for macros that don't
> look like functions; it doesn't surprise me that it doesn't work right. 

I tried to follow the Documentation/doc-guide/kernel-doc.rst section
"Object-like macro documentation" here. Looks like it's been added only in
January this year via commit 91a3d6be99e63 by Randy and references commit
cbb4d3e6510b implementing the support for that from 2014 (was it even sphinx
based back then?)

The DRM_GEM_VRAM_PLANE_HELPER_FUNCS from the example there still exists
(include/drm/drm_gem_vram_helper.h) and seems to have the same rendering
issue here. Somewhat weirdly it doesn't use the "define" keyword that the
example does. DRM_GEM_VRAM_DRIVER in the same file does have the "define"
keyword and with the same result.

>>  include/linux/slab.h | 60 ++++++++++++++++++++++++++++++--------------
>>  mm/slab_common.c     | 14 ++++++++++-
>>  2 files changed, 54 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/linux/slab.h b/include/linux/slab.h
>> index b35e2db7eb0e..49e9fb93e864 100644
>> --- a/include/linux/slab.h
>> +++ b/include/linux/slab.h
>> @@ -77,7 +77,17 @@ enum _slab_flag_bits {
>>  #define SLAB_POISON		__SLAB_FLAG_BIT(_SLAB_POISON)
>>  /* Indicate a kmalloc slab */
>>  #define SLAB_KMALLOC		__SLAB_FLAG_BIT(_SLAB_KMALLOC)
>> -/* Align objs on cache lines */
>> +/**
>> + * define SLAB_HWCACHE_ALIGN - Align objects on cache line boundaries.
>> + *
>> + * Sufficiently large objects are aligned on cache line boundary. For object
>> + * size smaller than a half of cache line size, the alignment is on the half of
>> + * cache line size. In general, if object size is smaller than 1/2^n of cache
>> + * line size, the alignment is adjusted to 1/2^n.
> 
> I'm kind of surprised that kernel-doc doesn't complain about that; it's
> definitely not something that was ever envisioned, as far as I know.
> 
> Making it work properly probably requires somebody to wander into Perl
> regex hell.  In the short term, if you want to get this text into the
> rendered docs, the usual approach is to make a DOC: block out of it and
> include it explicitly.

Thanks for the hints. I hope if we can agree that documenting the macros was
intended to be supported, doesn't break the build (there are users already)
and has only those minor rendering issues, it can be used?

> Thanks,
> 
> jon