Message-ID: <20250606012817.27f16dd0@foz.lan>
Date: Fri, 6 Jun 2025 01:28:17 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Jonathan Corbet <corbet@....net>
Cc: Matthew Wilcox <willy@...radead.org>, Randy Dunlap
 <rdunlap@...radead.org>, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>, Suren
 Baghdasaryan <surenb@...gle.com>, Mauro Carvalho Chehab
 <mchehab@...nel.org>
Subject: Re: [PATCH] scripts/kernel-doc: drop "_noprof" on function
 prototypes

Em Thu, 05 Jun 2025 13:18:50 -0600
Jonathan Corbet <corbet@....net> escreveu:

> Matthew Wilcox <willy@...radead.org> writes:
> 
> > On Mon, Mar 25, 2024 at 10:41:49PM -0700, Randy Dunlap wrote:  
> >> Memory profiling introduces macros as hooks for function-level
> >> allocation profiling[1]. Memory allocation functions that are profiled
> >> are named like xyz_alloc() for API access to the function. xyz_alloc()
> >> then calls xyz_alloc_noprof() to do the allocation work.
> >> 
> >> The kernel-doc comments for the memory allocation functions are
> >> introduced with the xyz_alloc() function names but the function
> >> implementations are the xyz_alloc_noprof() names.
> >> This causes kernel-doc warnings for mismatched documentation and
> >> function prototype names.
> >> By dropping the "_noprof" part of the function name, the kernel-doc
> >> function name matches the function prototype name, so the warnings
> >> are resolved.  
> >
> > This turns out not to be enough.  For example, krealloc() is
> > currently undocumented.  This is because we match the function name
> > in EXPORT_SYMBOL() against the function name in the comment, and they
> > don't match.  This patch restores the documentation, although only
> > for the python version of kernel-doc, and I'm pretty sure there's a
> > better way to do it (eg building it into the export_symbol* regexes).
> > I can turn this into a proper patch if this is the way to go, but for
> > now it's just to illustrate the problem.  
> 
> FWIW, I have no problem with leaving the perl version behind, I expect
> we'll drop it in 6.17.
> 
> We see other variants of this problem out there, where we want to
> document foo(), but that's really just a macro calling _foo(), where the
> real code is.
> 
> I wonder if we could add some sort of a marker to the kerneldoc comment
> saying "we are documenting foo(), but do you checks against _foo()"
> instead?  That would be more general than trying to keep a list of
> suffixes to hack off.
> 
> I'll try to ponder on this...
> 
> (Meanwhile I don't object to your fix as a short-term workaround)

If we willing to place hacks like that, better to bold it:

	# FIXME: this is not what we should do in long term

> > diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> > index 062453eefc7a..bdfa698d5570 100644
> > --- a/scripts/lib/kdoc/kdoc_parser.py
> > +++ b/scripts/lib/kdoc/kdoc_parser.py
> > @@ -1176,11 +1176,15 @@ class KernelDoc:
> >
> >          if export_symbol.search(line):
> >              symbol = export_symbol.group(2)
> > +            # See alloc_tags.h
> > +            symbol = symbol.removesuffix('_noprof')

If we're willing to do that, I would prefer to place "_noprof" into an array, 
as we may have other similar cases. Also, please comment why we need it and
where we have those "_noprof". We tent to forget why rules are added. As the
code churns, we may end dropping things without updating kernel-doc.

---

for a more long term solution, maybe one option for cases like that would
be to have something like:

/**
  * foo(), foo_noprof() - common function description (is it possible to have 
  *      a single description for both - as they're semantically different?)
  * @_size: size description
  * @_flags: flags description
  *
  * some description, including an explanation what are the differences
  * between both
  */
#define foo(_size, _flags)                    foo_node(_size, _flags, NUMA_NO_NODE)
#define foo_noprof(_size, _flags)             foo_node_noprof(_size, _flags, NUMA_NO_NODE)

Still, another kernel-doc markup will be needed for foo_node variants, as 
the parameters are different anyway.

Please notice that this is easier said than done as the above may break the
kernel-doc's sequential state machine at the parser if not done with care,
specially since one might eventually modify the arguments on just one of
the variants, like:

#define foo(_size, _flags, _bar)              foo_node(_size, _flags, bar, NUMA_NO_NODE)
#define foo_noprof(_size, _flags)             foo_node_noprof(_size, _flags, NUMA_NO_NODE)
	
Btw, we do have things like that: there are several register 
functions/macros that have THIS_MODULE on one of their variants,
like this:

	#define acpi_bus_register_driver(drv) \
	        __acpi_bus_register_driver(drv, THIS_MODULE)

I didn't find yet a good way to have a single kernel-doc markup
that would fill both cases and won't add too much complexity on
both kernel-doc syntax and at the kernel-doc code.

At the above, we probably don't want to document the __foo
variant, as all kAPI calls should use the variant that doesn't
have THIS_MODULE, but there are other similar cases where the
__foo variant, for instance, don't have some mutex or semaphore,
and we may still want both documented.


Thanks,
Mauro

Powered by blists - more mailing lists