Re: [PATCH] kernel-doc: add support for handling global variables

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Tue, 9 Sep 2025 11:20:31 -0700
Randy Dunlap <rdunlap@infradead.org> escreveu:

> On 9/9/25 9:18 AM, Mauro Carvalho Chehab wrote:
> > On Tue, Sep 09, 2025 at 08:57:07AM -0700, Randy Dunlap wrote:  
> >> Hi Mauro,
> >>
> >> On 9/9/25 12:27 AM, Randy Dunlap wrote:  
> >>> Hi Mauro,
> >>>
> >>> I have a few patch nits below, then some testing info.
> >>>
> >>>
> >>> On 9/7/25 9:22 AM, Mauro Carvalho Chehab wrote:  
> >>>> Specially on kAPI, sometimes it is desirable to be able to
> >>>> describe global variables that are part of kAPI.
> >>>>
> >>>> Documenting vars with Sphinx is simple, as we don't need
> >>>> to parse a data struct. All we need is the variable
> >>>> declaration and use natice C domain ::c:var: to format it
> >>>> for us.
> >>>>
> >>>> Add support for it.
> >>>>
> >>>> Link: https://lore.kernel.org/linux-doc/491c3022-cef8-4860-a945-c9c4a3b63c09@infradead.org/T/#m947c25d95cb1d96a394410ab1131dc8e9e5013f1
> >>>> Suggested-by: Randy Dunlap <rdunlap@infradead.org>
> >>>> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> >>>> ---
> >>>>  scripts/lib/kdoc/kdoc_output.py | 31 +++++++++++++++++++++++++++++++
> >>>>  scripts/lib/kdoc/kdoc_parser.py | 25 ++++++++++++++++++++++++-
> >>>>  2 files changed, 55 insertions(+), 1 deletion(-)
> >>>>  
> >>
> >>  
> >>> So, I grabbed some global data from 6-8 places in the kernel and put them intoinit/kdoc-globals-test.c. Then I modified Documentation/core-api/kernel-api.rst
> >>> like this at the end of that file:
> >>>
> >>> +
> >>> +Kernel Globals
> >>> +==========================
> >>> +
> >>> +.. kernel-doc:: init/kdoc-globals-test.c
> >>> +   :identifiers:
> >>>
> >>> The html output says
> >>> "Kernel Globals"
> >>> but nothing else.
> >>>
> >>> My test files are attached. I dumbed down (simplified) a few
> >>> of the globals from fancy types to just unsigned long, but that
> >>> didn't help the output results any.
> >>>
> >>> What's happening?
> >>> Thanks.
> >>>  
> >>
> >> My problems here could be from a patch mis-merge.
> >> Maybe your patch was against a tree or previous patches that I don't have.
> >>
> >> You could supply an updated patch or I can just wait until all
> >> the patches are synchronized for further testing.
> >> Or you could just take my sample and keep testing it.  
> > 
> > I applied it after my sphinx-build-wrapper patch series,
> > but it doesn't touch kernel-doc. I did a rebase just to make
> > sure, on the top of docs-next branch from Jon's tree, e.g. 
> > on the top of:
> > 
> >     git://git.lwn.net/linux.git docs-next
> > 
> > e.g. applying it after:
> > 
> >     7e5a0fe4e8ae ("doc: filesystems: proc: remove stale information from intro")
> > 
> > Patch applied cleanly.
> > 
> > Notice that it probably depends on some changes that Jon
> > applied for kernel-doc after -rc1.
> > 
> > If you prefer, the patch is here at global_vars branch:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-docs.git/log/?h=global_vars  
> 
> Yes, this is much better.
> 
> For the simplified global data, it's very good. It produces
> 2 complaints but the html output is still good:
> 
> linux-next-20250909/Documentation/core-api/kernel-api:435: ../init/kdoc-globals-test.c:10: WARNING: Invalid C declaration: Expected end of definition. [error at 32]
>   enum system_states system_state __read_mostly;
>   --------------------------------^
> linux-next-20250909/Documentation/core-api/kernel-api:435: ../init/kdoc-globals-test.c:20: WARNING: Invalid C declaration: Expected end of definition. [error at 25]
>   char *saved_command_line __ro_after_init;
>   -------------------------^
> 
> I suspect that this is not a surprise to you.

Not a surprise. The C domain parser is very strict with regards to 
C syntax.

On the above examples, __read_mostly and __ro_after_init are
macros. Sphinx has no clue about what to do with them. 

We'll need something similar to what we do on structs and functions
to strip things like this:

	sub_prefixes = [
...
            (r"__deprecated +", "", 0),
            (r"__flatten +", "", 0),
            (r"__meminit +", "", 0),
...
        ]

        for search, sub, flags in sub_prefixes:
            prototype = KernRe(search, flags).sub(sub, prototype)

to strip them for the prototype that will be used for .. :c::var.

I guess we have three alternatives here:

1. use the simplified version only, after being converted into a pure
   C code without macros;

2. use simplified version for :c::var: and print the complete one
   ourselves (this is how structs are printed);

3. use another c domain type that would just get a name. Then
   output ourselves the var prototype, captured as-is.

IMHO, from the above, (3) is better, but looking at:

	https://www.sphinx-doc.org/en/master/usage/domains/c.html

It would likely mean we'll need to use :c:macro:

> For the non-simplified global data, a few of the global items are
> completely omitted from the html output. This is the html production:
> 
> Kernel Globals
> dev_t ROOT_DEV;
> system root device
> 
> enum system_states system_state __read_mostly;
> system state used during boot or suspend/hibernate/resume
> 
> char *saved_command_line __ro_after_init;
> kernel’s command line, saved from use at any later time in the kernel.
> 
> unsigned long preset_lpj;
> lpj (loops per jiffy) value set from kernel command line using “lpj=VALUE”
> 
> static atomic64_t diskseq;
> unique sequence number for block device instances
> 
> 
> so these are completely missing/dropped: (they have
> initializers or use DEFINE_MUTEX())

Yeah, the regex is not capturing initializers nor handling macros.
We'll need to improve it.

things like DEFINE_MUTEX() would require either a sub pattern or
some regexes to detect them.

> 
> /**
>  * global loop_per_jiffy - calculated loop count needed to consume one jiffy
>  * of time
>  */
> unsigned long loops_per_jiffy = (1<<12);
> 
> // from init/version.c:
> /**
>  * global linux_proc_banner - text used from /proc/version file
>  *
>  * * first %s is sysname (e.g., "Linux")
>  * * second %s is release
>  * * third %s is version
>  */
> const char linux_proc_banner[] =
> 	"%s version %s"
> 	" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
> 	" (" LINUX_COMPILER ") %s\n";
> //char linux_proc_banner[];
> 
> // from init/version-timestamp.c:
> /**
>  * global linux_banner - Linux boot banner, usually printed at boot time
>  */
> const char linux_banner[] =
> 	"Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> 	LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
> //const char linux_banner[];
> 
> // from net/core/rtnetlink.c:
> /**
>  * global rtnl_mutex - historical global lock for networking control operations.
>  *
>  * @rtnl_mutex is used to serialize rtnetlink requests
>  * and protect all kernel internal data structures related to networking.
>  *
>  * See Documentation/networking/netdevices.rst for details.
>  * Often known as the rtnl_lock, although rtnl_lock is a kernel function.
>  */
> static DEFINE_MUTEX(rtnl_mutex);
> 
> 
> It's looking good. Thanks.

Agreed.

Thanks,
Mauro