Em Tue, 9 Sep 2025 11:20:31 -0700 Randy Dunlap <rdunlap@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx/T/#m947c25d95cb1d96a394410ab1131dc8e9e5013f1 > >>>> Suggested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> > >>>> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> > >>>> --- > >>>> 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
