All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: Nicolas Schier <nsc@kernel.org>
Cc: Sasha Levin <sashal@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	linux-api@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kbuild@vger.kernel.org, linux-kselftest@vger.kernel.org,
	workflows@vger.kernel.org, tools@kernel.org, x86@kernel.org,
	Thomas Gleixner <tglx@kernel.org>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Dmitry Vyukov <dvyukov@google.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Cyril Hrubis <chrubis@suse.cz>, Kees Cook <kees@kernel.org>,
	Jake Edge <jake@lwn.net>,
	David Laight <david.laight.linux@gmail.com>,
	Askar Safin <safinaskar@zohomail.com>,
	Gabriele Paoloni <gpaoloni@redhat.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Ingo Molnar <mingo@redhat.com>, Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH v3 2/9] kernel/api: enable kerneldoc-based API specifications
Date: Tue,  5 May 2026 03:45:43 -0400	[thread overview]
Message-ID: <20260505074545.430334-4-sashal@kernel.org> (raw)
In-Reply-To: <afNrbm8URHlClZ-8@levanger>

On Thu, Apr 30, 2026 at 04:47:10PM +0200, Nicolas Schier wrote:
> On Fri, Apr 24, 2026 at 12:51:22PM -0400, Sasha Levin wrote:
> > +# Generate API spec headers from kernel-doc comments
> > +ifeq ($(CONFIG_KAPI_SPEC),y)
> > +# Function to check if a file has API specifications
> > +has-apispec = $(shell grep -qE '^\s*\*\s*context-flags:' $(src)/$(1) 2>/dev/null && echo $(1))
> > +
> > +# Get base names without directory prefix
> > +c-objs-base := $(notdir $(real-obj-y) $(real-obj-m))
> > +# Filter to only .o files with corresponding .c source files
> > +c-files := $(foreach o,$(c-objs-base),$(if $(wildcard $(src)/$(o:.o=.c)),$(o:.o=.c)))
>
> Looks to me as if the two lines above are redundant, since 'find'
> (below) will find all files gathered in $(c-files).

Right, those two lines are dropped in v4. The replacement uses the
kbuild-derived file list described below, so neither set survives.

> > +# Also check for any additional .c files that contain API specs but are included
> > +extra-c-files := $(shell find $(src) -maxdepth 1 -name "*.c" -exec grep -l '^\s*\*\s*\(long-desc\|context-flags\|state-trans\):' {} \; 2>/dev/null | xargs -r basename -a)
> > +# Combine both lists and remove duplicates
> > +all-c-files := $(sort $(c-files) $(extra-c-files))
> > +# Only include files that actually have API specifications
> > +apispec-files := $(foreach f,$(all-c-files),$(call has-apispec,$(f)))
> > +# Generate apispec targets with proper directory prefix
> > +apispec-y := $(addprefix $(obj)/,$(apispec-files:.c=.apispec.h))
>
> To goal is to find any relevant C file in $(src)/ (but not deeper below)
> that holds KAPI documentation, right?
>
> I do not like the find call, as it picks up anything.  Might it make
> sense to evaluate $(obj-) along with $(obj-y) and $(obj-m) to pick up
> all C files that are references in kbuild?
>
>
>
> # in top definition block -- before 'include $(kbuild-file)' et al.
> obj- :=
>
> # below the definitions of real-obj-{y,m}
> real-obj-any := $(call real-search, $(obj-y) $(obj-m) $(obj-), .o, -objs -y -m -)
>
> has-apispec = $(shell grep -lE '^\s*\*\s*context-flags:' $(1) 2>/dev/null)
> apispec-y := $(patsubst $(src)/%.c, $(obj)/%.apispec.h, $(call has-apispec,
> 		    $(patsubst $(obj)/%.o, $(src)/%.c, $(real-obj-any))))
>
> #...
>
> # Source files that include their own apispec.h need to depend on it
> $(apispec-y:.apispec.h=.o): $(obj)/%.o: $(obj)/%.apispec.h
>
> (untested)

Thanks, the kbuild-driven approach is much cleaner. v4 takes your sketch
with two adjustments:

1. obj-m is already addprefix'd with $(obj)/ by line 116 of
   Makefile.build at the point where this block runs, so calling
   real-search again on the mixed list double-prefixes the module
   entries (giving $(src)/$(obj)/foo.c). v4 uses the existing
   $(real-obj-y)/$(real-obj-m) and strips the prefix in patsubst
   instead:

       apispec-c-files := $(call has-apispec, \
           $(patsubst $(obj)/%.o,$(src)/%.c, \
               $(filter-out %/built-in.a,$(real-obj-y) $(real-obj-m))))
       apispec-y := $(patsubst $(src)/%.c,$(obj)/%.apispec.h,$(apispec-c-files))

2. The has-apispec grep needs to match the same set of keys that
   tools/lib/python/kdoc/kdoc_apispec.py actually parses, which is
   "contexts:", "context-flags:" and "context:" interchangeably (see
   _get_section calls around line 866 of kdoc_apispec.py). The original
   grep for "context-flags:" matched zero files in the tree (every
   instrumented file uses "contexts:"), which is the latent bug behind
   the build failure you saw. v4 widens the regex:

       has-apispec = $(shell grep -lE \
           '^[[:space:]]*\*[[:space:]]*(contexts|context-flags|context):' \
           $(1) 2>/dev/null)

> > diff --git a/scripts/Makefile.clean b/scripts/Makefile.clean
> > index 6ead00ec7313b..f78dbbe637f27 100644
> > --- a/scripts/Makefile.clean
> > +++ b/scripts/Makefile.clean
> > @@ -35,6 +35,9 @@ __clean-files   := $(filter-out $(no-clean-files), $(__clean-files))
> >
> >  __clean-files   := $(wildcard $(addprefix $(obj)/, $(__clean-files)))
> >
> > +# Also clean generated apispec headers (computed dynamically in Makefile.build)
> > +__clean-files   += $(wildcard $(obj)/*.apispec.h)
>
> We have a list of wildcard clean patterns in top-level Makefile
> (line 2114 ff.); please add '*.apispec.h' there instead.

Will fix.

> When I apply the series on top of v7.1, compilation fails with
>
> ../fs/open.c:2148:10: fatal error: open.apispec.h: No such file or directory
> ../fs/read_write.c:2519:10: fatal error: read_write.apispec.h: No such file or directory

This is the symptom of (2) above. fs/open.c and fs/read_write.c only
declare "contexts: process, sleepable" (no "context-flags:" anywhere in
the tree, confirmed via grep), so apispec-files was always empty and no
*.apispec.h ever got generated. With CONFIG_KAPI_SPEC=y the
"#if IS_ENABLED(CONFIG_KAPI_SPEC)" guarded include then fails. Also
reproducible on v7.0; thanks for catching it.

Thanks for the review and the kbuild sketch!

--
Thanks,
Sasha

  reply	other threads:[~2026-05-05  7:46 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-24 16:51 [PATCH v3 0/9] Kernel API Specification Framework Sasha Levin
2026-04-24 16:51 ` [PATCH v3 1/9] kernel/api: introduce kernel API specification framework Sasha Levin
2026-04-27  3:37   ` Nathan Chancellor
2026-04-29 16:32     ` Nicolas Schier
2026-05-05  7:45       ` Sasha Levin
2026-05-05  7:45     ` Sasha Levin
2026-04-29 16:43   ` Nicolas Schier
2026-05-05  7:45     ` Sasha Levin
2026-04-24 16:51 ` [PATCH v3 2/9] kernel/api: enable kerneldoc-based API specifications Sasha Levin
2026-04-30 14:47   ` Nicolas Schier
2026-05-05  7:45     ` Sasha Levin [this message]
2026-04-24 16:51 ` [PATCH v3 3/9] kernel/api: add debugfs interface for kernel " Sasha Levin
2026-04-24 16:51 ` [PATCH v3 4/9] tools/kapi: add kernel API specification extraction tool Sasha Levin
2026-04-24 16:51 ` [PATCH v3 5/9] kernel/api: add API specification for sys_open Sasha Levin
2026-04-24 16:51 ` [PATCH v3 6/9] kernel/api: add API specification for sys_close Sasha Levin
2026-04-24 16:51 ` [PATCH v3 7/9] kernel/api: add API specification for sys_read Sasha Levin
2026-04-24 16:51 ` [PATCH v3 8/9] kernel/api: add API specification for sys_write Sasha Levin
2026-04-24 16:51 ` [PATCH v3 9/9] kernel/api: add runtime verification selftest Sasha Levin

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=20260505074545.430334-4-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=brauner@kernel.org \
    --cc=chrubis@suse.cz \
    --cc=corbet@lwn.net \
    --cc=david.laight.linux@gmail.com \
    --cc=dvyukov@google.com \
    --cc=gpaoloni@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jake@lwn.net \
    --cc=kees@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mingo@redhat.com \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=safinaskar@zohomail.com \
    --cc=skhan@linuxfoundation.org \
    --cc=tglx@kernel.org \
    --cc=tools@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=workflows@vger.kernel.org \
    --cc=x86@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.