[PATCH] doc: Always check kernel-doc

[PATCH] doc: Always check kernel-doc

[Matthew Wilcox (Oracle)]

kernel-doc checks were initially enabled only for builds which had extra
warnings enabled.  We have now eliminated enough kernel-doc warnings that
we can enable kernel-doc checking by default.  This comes at a slight
cost; for an allmodconfig build, make -j8 fs/ timings on my laptop
increase by less than 5%:

before real     4m7.456s        4m4.416s        4m6.663s
after real      4m18.960s       4m21.566s       4m23.234s
before user     29m35.370s      29m11.036s      29m30.092s
after user      30m55.602s      31m10.918s      31m20.311s
before sys      2m8.230s        2m6.392s        2m9.727s
after sys       2m19.896        2m23.422s       2m25.762s

This feels like a reasonable price to pay to force people to keep
documentation up to date.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
---
 scripts/Makefile.build | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 82e3fb19fdaf..52f57c0c5227 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -100,11 +100,9 @@ else ifeq ($(KBUILD_CHECKSRC),2)
         cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
 endif
 
-ifneq ($(KBUILD_EXTRA_WARN),)
-  cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $(KDOCFLAGS) \
+cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $(KDOCFLAGS) \
         $(if $(findstring 2, $(KBUILD_EXTRA_WARN)), -Wall) \
         $<
-endif
 
 # Compile C sources (.c)
 # ---------------------------------------------------------------------------
-- 
2.40.1

Re: [PATCH] doc: Always check kernel-doc

[Jonathan Corbet]

"Matthew Wilcox (Oracle)" <willy@infradead.org> writes:

> kernel-doc checks were initially enabled only for builds which had extra
> warnings enabled.  We have now eliminated enough kernel-doc warnings that
> we can enable kernel-doc checking by default.  This comes at a slight
> cost; for an allmodconfig build, make -j8 fs/ timings on my laptop
> increase by less than 5%:
>
> before real     4m7.456s        4m4.416s        4m6.663s
> after real      4m18.960s       4m21.566s       4m23.234s
> before user     29m35.370s      29m11.036s      29m30.092s
> after user      30m55.602s      31m10.918s      31m20.311s
> before sys      2m8.230s        2m6.392s        2m9.727s
> after sys       2m19.896        2m23.422s       2m25.762s
>
> This feels like a reasonable price to pay to force people to keep
> documentation up to date.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
> ---
>  scripts/Makefile.build | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 82e3fb19fdaf..52f57c0c5227 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -100,11 +100,9 @@ else ifeq ($(KBUILD_CHECKSRC),2)
>          cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
>  endif
>  
> -ifneq ($(KBUILD_EXTRA_WARN),)
> -  cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $(KDOCFLAGS) \
> +cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $(KDOCFLAGS) \
>          $(if $(findstring 2, $(KBUILD_EXTRA_WARN)), -Wall) \
>          $<
> -endif
>  

So I'm not opposed to this and can carry it in docs-next (after the
merge window, though, for something like this).  But, it seems to me, we
should copy Masahiro (added) on a build patch of this type.

Thanks,

jon

Re: [PATCH] doc: Always check kernel-doc

[Matthew Wilcox]

On Fri, Aug 18, 2023 at 10:49:37AM -0600, Jonathan Corbet wrote:
> "Matthew Wilcox (Oracle)" <willy@infradead.org> writes:
> > kernel-doc checks were initially enabled only for builds which had extra
> > warnings enabled.  We have now eliminated enough kernel-doc warnings that
> > we can enable kernel-doc checking by default.  This comes at a slight
> > cost; for an allmodconfig build, make -j8 fs/ timings on my laptop
> > increase by less than 5%:
> 
> So I'm not opposed to this and can carry it in docs-next (after the
> merge window, though, for something like this).  But, it seems to me, we
> should copy Masahiro (added) on a build patch of this type.

Thanks!  I've got a small collection of doc fixup patches redy to go;
I'm going to spray them at maintainers and see what lands in this
merge window.  I'm focusing on mm/ and fs/ since I know those areas
better than others.  net/ is in good shape; only 25 lines of errors
(21 of them in ceph).

I would suggest that we still have quite a lot of kernel-doc which is
not incorporated into .rst files, which seems like a shame.  Does anyone
have time to write a script that finds every file with kernel-doc in
it, then finds which of those files do not have ".. kernel-doc::"
lines in Documentation/ ?

Re: [PATCH] doc: Always check kernel-doc

[Jonathan Corbet]

Matthew Wilcox <willy@infradead.org> writes:

> I would suggest that we still have quite a lot of kernel-doc which is
> not incorporated into .rst files, which seems like a shame.  Does anyone
> have time to write a script that finds every file with kernel-doc in
> it, then finds which of those files do not have ".. kernel-doc::"
> lines in Documentation/ ?

Something like scripts/find-unused-docs.sh, maybe? :)

jon

Re: [PATCH] doc: Always check kernel-doc

[Masahiro Yamada]

On Sat, Aug 19, 2023 at 2:14 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Aug 18, 2023 at 10:49:37AM -0600, Jonathan Corbet wrote:
> > "Matthew Wilcox (Oracle)" <willy@infradead.org> writes:
> > > kernel-doc checks were initially enabled only for builds which had extra
> > > warnings enabled.  We have now eliminated enough kernel-doc warnings that
> > > we can enable kernel-doc checking by default.  This comes at a slight
> > > cost; for an allmodconfig build, make -j8 fs/ timings on my laptop
> > > increase by less than 5%:


Adding CONFIG_KDOC_CHECK or something
will allow people to avoid 5% build-time cost.
You can set "default y" or "default COMPILE_TEST".



> >
> > So I'm not opposed to this and can carry it in docs-next (after the
> > merge window, though, for something like this).  But, it seems to me, we
> > should copy Masahiro (added) on a build patch of this type.
>
> Thanks!  I've got a small collection of doc fixup patches redy to go;
> I'm going to spray them at maintainers and see what lands in this
> merge window.  I'm focusing on mm/ and fs/ since I know those areas
> better than others.  net/ is in good shape; only 25 lines of errors
> (21 of them in ceph).

Any single instance of warning may result in a rejection by Linus.
Anyway, we will see.




> I would suggest that we still have quite a lot of kernel-doc which is
> not incorporated into .rst files, which seems like a shame.  Does anyone
> have time to write a script that finds every file with kernel-doc in
> it, then finds which of those files do not have ".. kernel-doc::"
> lines in Documentation/ ?



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] doc: Always check kernel-doc

[Matthew Wilcox]

On Sun, Aug 20, 2023 at 07:48:26AM +0900, Masahiro Yamada wrote:
> On Sat, Aug 19, 2023 at 2:14 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Fri, Aug 18, 2023 at 10:49:37AM -0600, Jonathan Corbet wrote:
> > > "Matthew Wilcox (Oracle)" <willy@infradead.org> writes:
> > > > kernel-doc checks were initially enabled only for builds which had extra
> > > > warnings enabled.  We have now eliminated enough kernel-doc warnings that
> > > > we can enable kernel-doc checking by default.  This comes at a slight
> > > > cost; for an allmodconfig build, make -j8 fs/ timings on my laptop
> > > > increase by less than 5%:
> 
> Adding CONFIG_KDOC_CHECK or something
> will allow people to avoid 5% build-time cost.
> You can set "default y" or "default COMPILE_TEST".

No, people won't set it.

> > > So I'm not opposed to this and can carry it in docs-next (after the
> > > merge window, though, for something like this).  But, it seems to me, we
> > > should copy Masahiro (added) on a build patch of this type.
> >
> > Thanks!  I've got a small collection of doc fixup patches redy to go;
> > I'm going to spray them at maintainers and see what lands in this
> > merge window.  I'm focusing on mm/ and fs/ since I know those areas
> > better than others.  net/ is in good shape; only 25 lines of errors
> > (21 of them in ceph).
> 
> Any single instance of warning may result in a rejection by Linus.
> Anyway, we will see.

Can we have a way to enable kernel-doc coverage on a per-directory basis?
It will stop us from regressing.  I got mm/ down to zero warnings, and
that promptly regressed.  If we force kernel-doc to run on files under
mm/, developers will notice breakage quickly.

Re: [PATCH] doc: Always check kernel-doc

[Jani Nikula]

On Sat, 07 Oct 2023, Matthew Wilcox <willy@infradead.org> wrote:
> Can we have a way to enable kernel-doc coverage on a per-directory basis?
> It will stop us from regressing.  I got mm/ down to zero warnings, and
> that promptly regressed.  If we force kernel-doc to run on files under
> mm/, developers will notice breakage quickly.

It's not exactly pretty, but we do this in i915. Unfortunately, it needs
to be done separately and in different ways for kernel-doc in .c and .h.

Check out drivers/gpu/drm/i915/Makefile for ideas.

BR,
Jani.

-- 
Jani Nikula, Intel

Re: [PATCH] doc: Always check kernel-doc

[Jonathan Corbet]

"Matthew Wilcox (Oracle)" <willy@infradead.org> writes:

> kernel-doc checks were initially enabled only for builds which had extra
> warnings enabled.  We have now eliminated enough kernel-doc warnings that
> we can enable kernel-doc checking by default.  This comes at a slight
> cost; for an allmodconfig build, make -j8 fs/ timings on my laptop
> increase by less than 5%:
>
> before real     4m7.456s        4m4.416s        4m6.663s
> after real      4m18.960s       4m21.566s       4m23.234s
> before user     29m35.370s      29m11.036s      29m30.092s
> after user      30m55.602s      31m10.918s      31m20.311s
> before sys      2m8.230s        2m6.392s        2m9.727s
> after sys       2m19.896        2m23.422s       2m25.762s
>
> This feels like a reasonable price to pay to force people to keep
> documentation up to date.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
> ---
>  scripts/Makefile.build | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

So I finally got around to actually giving this a try ...

It adds 1,095 warnings to an allmodconfig build here.  If I merge that,
I suspect that next thing that happens will be One Of Those Emails from
Linus... and perhaps from others as well.

If we had a series that drove the number to zero prior to this change,
it would be a different story.  I'm kind of thrashing and don't think I
can do that in the near future, as nice as it would be.  I suspect
there's not a lot of other folks just waiting for a chance to do this
either.

As nice as it would be to have this, I don't think it would survive to a
mainline release if I tried to push it now.  But maybe others disagree?

Thanks,

jon

Re: [PATCH] doc: Always check kernel-doc

[Jani Nikula]

On Tue, 03 Oct 2023, Jonathan Corbet <corbet@lwn.net> wrote:
> "Matthew Wilcox (Oracle)" <willy@infradead.org> writes:
>
>> kernel-doc checks were initially enabled only for builds which had extra
>> warnings enabled.  We have now eliminated enough kernel-doc warnings that
>> we can enable kernel-doc checking by default.  This comes at a slight
>> cost; for an allmodconfig build, make -j8 fs/ timings on my laptop
>> increase by less than 5%:
>>
>> before real     4m7.456s        4m4.416s        4m6.663s
>> after real      4m18.960s       4m21.566s       4m23.234s
>> before user     29m35.370s      29m11.036s      29m30.092s
>> after user      30m55.602s      31m10.918s      31m20.311s
>> before sys      2m8.230s        2m6.392s        2m9.727s
>> after sys       2m19.896        2m23.422s       2m25.762s
>>
>> This feels like a reasonable price to pay to force people to keep
>> documentation up to date.
>>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>> Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
>> ---
>>  scripts/Makefile.build | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> So I finally got around to actually giving this a try ...
>
> It adds 1,095 warnings to an allmodconfig build here.  If I merge that,
> I suspect that next thing that happens will be One Of Those Emails from
> Linus... and perhaps from others as well.
>
> If we had a series that drove the number to zero prior to this change,
> it would be a different story.  I'm kind of thrashing and don't think I
> can do that in the near future, as nice as it would be.  I suspect
> there's not a lot of other folks just waiting for a chance to do this
> either.
>
> As nice as it would be to have this, I don't think it would survive to a
> mainline release if I tried to push it now.  But maybe others disagree?

I'll leave that decision up to the people who have to face the music.

But an alternative would be to add more warning levels to kernel-doc,
put the most verbose messages from the allmodconfig run behind some
-Wall knob, always run kernel-doc without that, and make W=1 run with
-Wall. It would be an iterative path forward.

Now Someone(tm) just needs to implement that. ;)


BR,
Jani.


-- 
Jani Nikula, Intel