mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Randy Dunlap <rdunlap@infradead.org>,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Subject: Re: [PATCH] scripts/kernel-doc: drop "_noprof" on function prototypes
Date: Fri, 6 Jun 2025 00:50:51 +0200	[thread overview]
Message-ID: <20250606005051.60a48cb4@foz.lan> (raw)
In-Reply-To: <aEHwdoNCrF4-KY_i@casper.infradead.org>

Em Thu, 5 Jun 2025 20:31:02 +0100
Matthew Wilcox <willy@infradead.org> escreveu:

> On Thu, Jun 05, 2025 at 01:18:50PM -0600, Jonathan Corbet wrote:
> > Matthew Wilcox <willy@infradead.org> writes:  
> > > 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.

I agree with Jon: it is time to retire the perl version. 

> > 
> > (Meanwhile I don't object to your fix as a short-term workaround)  
> 
> OK, will give Mauro 24 hours to comment, then resend as a patch if
> there are no objections.
> 
> > 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.

The problem is that one may want to document both _foo() and foo(),
if they have different arguments. I'm pretty sure we have cases like
that.

> > 
> > 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.  
> 
> kernel-doc is our own format, so sure, we can add whatever marker
> we want to it.  I think it's not quite general enough because we have
> situations like:
> 
> static inline void foo(int x)
> {
> 	numa_foo(x, NUMA_NO_NODE);
> };
> 
> /**
>  * foo - Frobnicate
>  * @x: How many
>  * @nid: Which node
>  */
> void numa_foo(int x, int node)
> { .. }

If I"m not mistaken, if you do things like that, kernel-doc.py will
complain that "foo" is not "numa_foo". It will also complain that
"nid" doesn't exist and "node" is not documented.

Basically, there is a strict check there (if it got properly 
backported from the Perl version) which checks if kernel-doc
is documenting the next function prototype name and argument
names.

The rationale is that we caught several cases where a function was
removed, renamed and/or have their parameters renamed without the
corresponding kernel-doc change. So, the verification is now
stricter (*). When we enabled such check, we fixed several bad
kernel-doc markups.

(*) Also, kernel-doc handles files in one pass at read time,
    sequentially. It could be possible to change it, but kernel-doc 
    is already complex enough, and placing the markup just before
    the function is a good practice anyway.

> and now we're documenting a parameter that doesn't exist.  The only
> solution is to move the kdoc to the header file, which is annoying for
> everyone.  Or replicate the declaration in the C file and kdoc it there.

Heh, for my taste, having kernel-docs at header files look better, as
this is where the kAPIs should be defined anyway. 

Ok, one may change the behavior of a function without touching the
arguments and forget to update kernel-doc at the header files to tell
about such change, but this is problematic anyway, as, if someone is
relying on a certain behavior of a kAPI function, changing its behavior
may result on unexpected results at the current callers - and even
future callers.

Thanks,
Mauro

  parent reply	other threads:[~2025-06-05 22:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-26  5:41 Randy Dunlap
2024-03-26 20:35 ` Suren Baghdasaryan
2025-06-05 19:07 ` Matthew Wilcox
2025-06-05 19:18   ` Jonathan Corbet
2025-06-05 19:31     ` Matthew Wilcox
2025-06-05 19:52       ` Jonathan Corbet
2025-06-05 22:50       ` Mauro Carvalho Chehab [this message]
2025-06-05 23:28     ` Mauro Carvalho Chehab

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250606005051.60a48cb4@foz.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=surenb@google.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®