2022-04-18 09:47:08

by Fabio M. De Francesco

[permalink] [raw]
Subject: [PATCH] Documentation/vm: Include kernel-doc from highmem-internal.h

In Documentation/vm/highmem.rst the kernel-doc comments are missing,
therefore kunmap_local() and kunmap_atomic() are yet undocumented.

Add a kernel-doc directive to include the above-mentioned kernel-doc
comments in highmem.rst.

Cc: Jonathan Corbet <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ira Weiny <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---

This patch must be applied and built on top of
"[PATCH v3 2/2] Documentation/vm: Include kernel-doc to highmem.rst"
https://lore.kernel.org/lkml/[email protected]/

Documentation/vm/highmem.rst | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/highmem.rst
index 5dcee6233cd5..1a3e6a8f5d3e 100644
--- a/Documentation/vm/highmem.rst
+++ b/Documentation/vm/highmem.rst
@@ -161,3 +161,4 @@ Functions
=========

.. kernel-doc:: include/linux/highmem.h
+.. kernel-doc:: include/linux/highmem-internal.h
--
2.34.1


2022-04-19 03:02:18

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] Documentation/vm: Include kernel-doc from highmem-internal.h

On lunedì 18 aprile 2022 23:36:49 CEST Ira Weiny wrote:
> On Mon, Apr 18, 2022 at 10:17:40AM +0200, Fabio M. De Francesco wrote:
> > In Documentation/vm/highmem.rst the kernel-doc comments are missing,
> > therefore kunmap_local() and kunmap_atomic() are yet undocumented.
> >
> > Add a kernel-doc directive to include the above-mentioned kernel-doc
> > comments in highmem.rst.
> >
> > Cc: Jonathan Corbet <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Ira Weiny <[email protected]>
> > Cc: Matthew Wilcox <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Signed-off-by: Fabio M. De Francesco <[email protected]>
>
> I thought I saw a patch fly by which changed the comment of
kunmap_atomic() in
> include/linux/highmem-internal.h? Did I miss that somewhere?

No, you're right. I had noticed just some minutes before reading your
review that a made a typo.

I wanted to write: "In Documentation/vm/highmem.rst some kernel-doc
comments are missing" instead for some unknown reason I wrote "the" in
place of "some" :(

I'll fix it ASAP.

> This is a good inclusion for now. However, I think I would rather see
this as
> part of a new series which puts the bulk of the documentation with the
code as
> I proposed in patch 1/2 in your other series.

I have also read your review of the patch above. I agree with you that the
"how" should be placed in the kernel-doc comments in the code. And I also
agree that this patch should be part of a new version of a reorganized
series.

I'll start working on your suggestions by tomorrow morning (it's midnight
here in Italy).

> For this particular change:
>
> Reviewed-by: Ira Weiny <[email protected]>

Thank you so much for your review,

Fabio

> > ---
> >
> > This patch must be applied and built on top of
> > "[PATCH v3 2/2] Documentation/vm: Include kernel-doc to highmem.rst"
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Documentation/vm/highmem.rst | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/
highmem.rst
> > index 5dcee6233cd5..1a3e6a8f5d3e 100644
> > --- a/Documentation/vm/highmem.rst
> > +++ b/Documentation/vm/highmem.rst
> > @@ -161,3 +161,4 @@ Functions
> > =========
> >
> > .. kernel-doc:: include/linux/highmem.h
> > +.. kernel-doc:: include/linux/highmem-internal.h
> > --
> > 2.34.1
> >
>




2022-04-20 01:15:02

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH] Documentation/vm: Include kernel-doc from highmem-internal.h

On Mon, Apr 18, 2022 at 10:17:40AM +0200, Fabio M. De Francesco wrote:
> In Documentation/vm/highmem.rst the kernel-doc comments are missing,
> therefore kunmap_local() and kunmap_atomic() are yet undocumented.
>
> Add a kernel-doc directive to include the above-mentioned kernel-doc
> comments in highmem.rst.
>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ira Weiny <[email protected]>
> Cc: Matthew Wilcox <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>

I thought I saw a patch fly by which changed the comment of kunmap_atomic() in
include/linux/highmem-internal.h? Did I miss that somewhere?

This is a good inclusion for now. However, I think I would rather see this as
part of a new series which puts the bulk of the documentation with the code as
I proposed in patch 1/2 in your other series.

For this particular change:

Reviewed-by: Ira Weiny <[email protected]>

> ---
>
> This patch must be applied and built on top of
> "[PATCH v3 2/2] Documentation/vm: Include kernel-doc to highmem.rst"
> https://lore.kernel.org/lkml/[email protected]/
>
> Documentation/vm/highmem.rst | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/highmem.rst
> index 5dcee6233cd5..1a3e6a8f5d3e 100644
> --- a/Documentation/vm/highmem.rst
> +++ b/Documentation/vm/highmem.rst
> @@ -161,3 +161,4 @@ Functions
> =========
>
> .. kernel-doc:: include/linux/highmem.h
> +.. kernel-doc:: include/linux/highmem-internal.h
> --
> 2.34.1
>

2022-04-20 04:49:58

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] Documentation/vm: Include kernel-doc from highmem-internal.h

On lunedì 18 aprile 2022 23:36:49 CEST Ira Weiny wrote:
> On Mon, Apr 18, 2022 at 10:17:40AM +0200, Fabio M. De Francesco wrote:
> > In Documentation/vm/highmem.rst the kernel-doc comments are missing,
> > therefore kunmap_local() and kunmap_atomic() are yet undocumented.
> >
> > Add a kernel-doc directive to include the above-mentioned kernel-doc
> > comments in highmem.rst.
> >
> > Cc: Jonathan Corbet <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Ira Weiny <[email protected]>
> > Cc: Matthew Wilcox <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Signed-off-by: Fabio M. De Francesco <[email protected]>
>
> I thought I saw a patch fly by which changed the comment of
kunmap_atomic() in
> include/linux/highmem-internal.h? Did I miss that somewhere?

Sorry, in my other email I responded after misunderstanding your question
because I had just noticed a typo so that I had something else in my mind.

It looks that you are talking about the following patch and, if so, you saw
it correctly:

"[PATCH v2] mm/highmem: Fix kernel-doc warnings in highmem*.h" at
https://lore.kernel.org/lkml/[email protected]/

I forgot to send it to the Outreachy list. I'm about to resend it only to
this list in order to make Outreachy's organizers to know about it. For v1
already had an "Acked-by:" tag from Mike Rapoport.

Thanks,

Fabio M. De Francesco


2022-04-22 21:01:07

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] Documentation/vm: Include kernel-doc from highmem-internal.h

On lunedì 18 aprile 2022 10:17:40 CEST Fabio M. De Francesco wrote:
> In Documentation/vm/highmem.rst the kernel-doc comments are missing,
> therefore kunmap_local() and kunmap_atomic() are yet undocumented.
>
> Add a kernel-doc directive to include the above-mentioned kernel-doc
> comments in highmem.rst.
>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ira Weiny <[email protected]>
> Cc: Matthew Wilcox <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
>
> This patch must be applied and built on top of
> "[PATCH v3 2/2] Documentation/vm: Include kernel-doc to highmem.rst"
> https://lore.kernel.org/lkml/[email protected]/
>
> Documentation/vm/highmem.rst | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/highmem.rst
> index 5dcee6233cd5..1a3e6a8f5d3e 100644
> --- a/Documentation/vm/highmem.rst
> +++ b/Documentation/vm/highmem.rst
> @@ -161,3 +161,4 @@ Functions
> =========
>
> .. kernel-doc:: include/linux/highmem.h
> +.. kernel-doc:: include/linux/highmem-internal.h
> --
> 2.34.1
>
Dear Maintainers,

Please drop this patch because it has been reworked and inserted as part of
a new series whose subject is "Extend and reorganize Highmem's
documentation" and which is about to be submitted.

Thanks,

Fabio M. De Francesco