Messages in this thread Patch in this message |  | | From | Sasha Levin <> | | Subject | Re: [PATCH v3 2/9] kernel/api: enable kerneldoc-based API specifications | | Date | Tue, 5 May 2026 03:45:43 -0400 |
| |
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
|  |