linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/gfp: Add kernel-doc for gfp_t
@ 2021-02-15 20:49 Matthew Wilcox (Oracle)
  2021-02-15 21:07 ` Mike Rapoport
  2021-02-19 19:55 ` Nathan Chancellor
  0 siblings, 2 replies; 11+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-02-15 20:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Matthew Wilcox (Oracle), linux-mm, Mike Rapoport

The generated html will link to the definition of the gfp_t automatically
once we define it.  Move the one-paragraph overview of GFP flags from the
documentation directory into gfp.h and pull gfp.h into the documentation.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 Documentation/core-api/mm-api.rst |  7 ++-----
 include/linux/gfp.h               | 11 +++++++++++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/core-api/mm-api.rst b/Documentation/core-api/mm-api.rst
index 2adffb3f7914..201b5423303b 100644
--- a/Documentation/core-api/mm-api.rst
+++ b/Documentation/core-api/mm-api.rst
@@ -19,11 +19,8 @@ User Space Memory Access
 Memory Allocation Controls
 ==========================
 
-Functions which need to allocate memory often use GFP flags to express
-how that memory should be allocated. The GFP acronym stands for "get
-free pages", the underlying memory allocation function. Not every GFP
-flag is allowed to every function which may allocate memory. Most
-users will want to use a plain ``GFP_KERNEL``.
+.. kernel-doc:: include/linux/gfp.h
+   :internal:
 
 .. kernel-doc:: include/linux/gfp.h
    :doc: Page mobility and placement hints
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 52cd415b436c..ecd1b5d27936 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -8,6 +8,17 @@
 #include <linux/linkage.h>
 #include <linux/topology.h>
 
+/**
+ * typedef gfp_t - Memory allocation flags.
+ *
+ * GFP flags are commonly used throughout Linux to indicate how memory
+ * should be allocated.  The GFP acronym stands for "get free pages",
+ * the underlying memory allocation function.  Not every GFP flag is
+ * supported by every function which may allocate memory.  Most users
+ * will want to use a plain ``GFP_KERNEL``.
+ */
+typedef unsigned int __bitwise gfp_t;	// repeated here for kernel-doc
+
 struct vm_area_struct;
 
 /*
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-15 20:49 [PATCH] mm/gfp: Add kernel-doc for gfp_t Matthew Wilcox (Oracle)
@ 2021-02-15 21:07 ` Mike Rapoport
  2021-02-19 19:55 ` Nathan Chancellor
  1 sibling, 0 replies; 11+ messages in thread
From: Mike Rapoport @ 2021-02-15 21:07 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle); +Cc: Andrew Morton, linux-mm

On Mon, Feb 15, 2021 at 08:49:09PM +0000, Matthew Wilcox (Oracle) wrote:
> The generated html will link to the definition of the gfp_t automatically
> once we define it.  Move the one-paragraph overview of GFP flags from the
> documentation directory into gfp.h and pull gfp.h into the documentation.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Acked-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
>  Documentation/core-api/mm-api.rst |  7 ++-----
>  include/linux/gfp.h               | 11 +++++++++++
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/core-api/mm-api.rst b/Documentation/core-api/mm-api.rst
> index 2adffb3f7914..201b5423303b 100644
> --- a/Documentation/core-api/mm-api.rst
> +++ b/Documentation/core-api/mm-api.rst
> @@ -19,11 +19,8 @@ User Space Memory Access
>  Memory Allocation Controls
>  ==========================
>  
> -Functions which need to allocate memory often use GFP flags to express
> -how that memory should be allocated. The GFP acronym stands for "get
> -free pages", the underlying memory allocation function. Not every GFP
> -flag is allowed to every function which may allocate memory. Most
> -users will want to use a plain ``GFP_KERNEL``.
> +.. kernel-doc:: include/linux/gfp.h
> +   :internal:
>  
>  .. kernel-doc:: include/linux/gfp.h
>     :doc: Page mobility and placement hints
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 52cd415b436c..ecd1b5d27936 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -8,6 +8,17 @@
>  #include <linux/linkage.h>
>  #include <linux/topology.h>
>  
> +/**
> + * typedef gfp_t - Memory allocation flags.
> + *
> + * GFP flags are commonly used throughout Linux to indicate how memory
> + * should be allocated.  The GFP acronym stands for "get free pages",
> + * the underlying memory allocation function.  Not every GFP flag is
> + * supported by every function which may allocate memory.  Most users
> + * will want to use a plain ``GFP_KERNEL``.
> + */
> +typedef unsigned int __bitwise gfp_t;	// repeated here for kernel-doc
> +
>  struct vm_area_struct;
>  
>  /*
> -- 
> 2.29.2
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-15 20:49 [PATCH] mm/gfp: Add kernel-doc for gfp_t Matthew Wilcox (Oracle)
  2021-02-15 21:07 ` Mike Rapoport
@ 2021-02-19 19:55 ` Nathan Chancellor
  2021-02-19 20:54   ` Matthew Wilcox
  1 sibling, 1 reply; 11+ messages in thread
From: Nathan Chancellor @ 2021-02-19 19:55 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle)
  Cc: Andrew Morton, linux-mm, Mike Rapoport, clang-built-linux

On Mon, Feb 15, 2021 at 08:49:09PM +0000, Matthew Wilcox (Oracle) wrote:
> The generated html will link to the definition of the gfp_t automatically
> once we define it.  Move the one-paragraph overview of GFP flags from the
> documentation directory into gfp.h and pull gfp.h into the documentation.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

This patch causes a clang warning in basically every file on linux-next
now:

include/linux/gfp.h:20:32: warning: redefinition of typedef 'gfp_t' is a C11 feature [-Wtypedef-redefinition]
typedef unsigned int __bitwise gfp_t;   // repeated here for kernel-doc
                               ^
include/linux/types.h:148:32: note: previous definition is here
typedef unsigned int __bitwise gfp_t;
                               ^
1 warning generated.

Cheers,
Nathan


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-19 19:55 ` Nathan Chancellor
@ 2021-02-19 20:54   ` Matthew Wilcox
  2021-02-19 21:45     ` Nick Desaulniers
  2021-02-22 17:04     ` Nick Desaulniers
  0 siblings, 2 replies; 11+ messages in thread
From: Matthew Wilcox @ 2021-02-19 20:54 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Andrew Morton, linux-mm, Mike Rapoport, clang-built-linux

On Fri, Feb 19, 2021 at 12:55:09PM -0700, Nathan Chancellor wrote:
> On Mon, Feb 15, 2021 at 08:49:09PM +0000, Matthew Wilcox (Oracle) wrote:
> > The generated html will link to the definition of the gfp_t automatically
> > once we define it.  Move the one-paragraph overview of GFP flags from the
> > documentation directory into gfp.h and pull gfp.h into the documentation.
> > 
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> This patch causes a clang warning in basically every file on linux-next
> now:
> 
> include/linux/gfp.h:20:32: warning: redefinition of typedef 'gfp_t' is a C11 feature [-Wtypedef-redefinition]

Seems like it's also a gnu89 feature.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-19 20:54   ` Matthew Wilcox
@ 2021-02-19 21:45     ` Nick Desaulniers
  2021-02-19 22:15       ` Miguel Ojeda
  2021-02-22 17:04     ` Nick Desaulniers
  1 sibling, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2021-02-19 21:45 UTC (permalink / raw)
  To: Matthew Wilcox, Nathan Chancellor
  Cc: Andrew Morton, Linux Memory Management List, Mike Rapoport,
	clang-built-linux

On Fri, Feb 19, 2021 at 12:55 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Feb 19, 2021 at 12:55:09PM -0700, Nathan Chancellor wrote:
> > On Mon, Feb 15, 2021 at 08:49:09PM +0000, Matthew Wilcox (Oracle) wrote:
> > > The generated html will link to the definition of the gfp_t automatically
> > > once we define it.  Move the one-paragraph overview of GFP flags from the
> > > documentation directory into gfp.h and pull gfp.h into the documentation.
> > >
> > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> >
> > This patch causes a clang warning in basically every file on linux-next
> > now:
> >
> > include/linux/gfp.h:20:32: warning: redefinition of typedef 'gfp_t' is a C11 feature [-Wtypedef-redefinition]
>
> Seems like it's also a gnu89 feature.

I'm not sure a lack of a warning is an intentional feature, and would
bet that behavior is not documented.

That said, I'm fine disabling this warning; there's a separate error
for redefining a typedef to a different underlying type.  That's
what's useful IMO, this one really is not.

This warning doesn't really provide any value to us in the kernel; I
would guess the intent was to be helpful to code expected to be
portable across different -std=*, but that's not the case for the
Linux kernel.  (It's also trivial to change this in Clang, but we'd
have to disable this warning for older supported of clang anyways).

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index cee107096947..63529a43e797 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2381,7 +2381,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
   }

   // Modules always permit redefinition of typedefs, as does C11.
-  if (getLangOpts().Modules || getLangOpts().C11)
+  if (getLangOpts().Modules || getLangOpts().C11 || getLangOpts().GNUMode)
     return;

   // If we have a redefinition of a typedef in C, emit a warning.  This warning
-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-19 21:45     ` Nick Desaulniers
@ 2021-02-19 22:15       ` Miguel Ojeda
  2021-02-19 22:49         ` Nick Desaulniers
  0 siblings, 1 reply; 11+ messages in thread
From: Miguel Ojeda @ 2021-02-19 22:15 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Matthew Wilcox, Nathan Chancellor, Andrew Morton,
	Linux Memory Management List, Mike Rapoport, clang-built-linux

On Fri, Feb 19, 2021 at 10:45 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> That said, I'm fine disabling this warning; there's a separate error
> for redefining a typedef to a different underlying type.  That's
> what's useful IMO, this one really is not.
>
> This warning doesn't really provide any value to us in the kernel; I
> would guess the intent was to be helpful to code expected to be
> portable across different -std=*

It seems it would also be useful to sport unintended cases, e.g.:

  - Collisions on short identifiers (that by chance typedef to the same type).
  - Copy-pasting and forgetting to remove the original definition
(i.e. it should have be cut-pasting instead).
  - Double inclusion of headers (with missing or broken #ifdef guards).

Those may be providing value in the kernel. In particular, if we don't
see any warning at the moment, it means those cases are not happening
now anywhere, so we would be weakening things.

Having said that, I don't see the original patch, so perhaps I am
missing something.

Cheers,
Miguel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-19 22:15       ` Miguel Ojeda
@ 2021-02-19 22:49         ` Nick Desaulniers
  2021-02-20  9:43           ` Miguel Ojeda
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2021-02-19 22:49 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Matthew Wilcox, Nathan Chancellor, Andrew Morton,
	Linux Memory Management List, Mike Rapoport, clang-built-linux

On Fri, Feb 19, 2021 at 2:15 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Feb 19, 2021 at 10:45 PM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > That said, I'm fine disabling this warning; there's a separate error
> > for redefining a typedef to a different underlying type.  That's
> > what's useful IMO, this one really is not.
> >
> > This warning doesn't really provide any value to us in the kernel; I
> > would guess the intent was to be helpful to code expected to be
> > portable across different -std=*
>
> It seems it would also be useful to sport unintended cases, e.g.:
>
>   - Collisions on short identifiers (that by chance typedef to the same type).
>   - Copy-pasting and forgetting to remove the original definition
> (i.e. it should have be cut-pasting instead).
>   - Double inclusion of headers (with missing or broken #ifdef guards).

(There is a separate warning flag for broken header guards,
-Wheader-guard:
https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+label%3A-Wheader-guard+is%3Aclosed)

What happens should the kernel move to gnu11, as discussed once GCC
5.1+ becomes the minimum supported version for all arches?
https://lore.kernel.org/lkml/CAHk-=whnKkj5CSbj-uG_MVVUsPZ6ppd_MFhZf_kpXDkh2MAVRA@mail.gmail.com/

Then the warning will not fire, since it's only really meant to help C
code be portable between -std=c11.

(Another change to clang could be to move this flag into the
-Wpedantic group, which is only really meant for from guarding against
the use of non-ISO C functionality, but that still would require
disabling the warning for older but supported versions of clang).

>
> Those may be providing value in the kernel. In particular, if we don't
> see any warning at the moment, it means those cases are not happening
> now anywhere, so we would be weakening things.
>
> Having said that, I don't see the original patch, so perhaps I am
> missing something.

https://lore.kernel.org/linux-mm/20210215204909.3824509-1-willy@infradead.org/

-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-19 22:49         ` Nick Desaulniers
@ 2021-02-20  9:43           ` Miguel Ojeda
  0 siblings, 0 replies; 11+ messages in thread
From: Miguel Ojeda @ 2021-02-20  9:43 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Matthew Wilcox, Nathan Chancellor, Andrew Morton,
	Linux Memory Management List, Mike Rapoport, clang-built-linux

On Fri, Feb 19, 2021 at 11:50 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> (There is a separate warning flag for broken header guards,
> -Wheader-guard:
> https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+label%3A-Wheader-guard+is%3Aclosed)

Yeah, this would still help cases with headers without guards (not
really common, but...).

> What happens should the kernel move to gnu11, as discussed once GCC
> 5.1+ becomes the minimum supported version for all arches?
> https://lore.kernel.org/lkml/CAHk-=whnKkj5CSbj-uG_MVVUsPZ6ppd_MFhZf_kpXDkh2MAVRA@mail.gmail.com/
>
> Then the warning will not fire, since it's only really meant to help C
> code be portable between -std=c11.

I think it is a fine warning to keep in the compiler nevertheless
since you already have it, even if disabled by default or only in
-Wextra or something like that -- e.g. I would use it in my own
projects since I never intend to redefine a typedef.

> https://lore.kernel.org/linux-mm/20210215204909.3824509-1-willy@infradead.org/

Thanks for the link!

Cheers,
Miguel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-19 20:54   ` Matthew Wilcox
  2021-02-19 21:45     ` Nick Desaulniers
@ 2021-02-22 17:04     ` Nick Desaulniers
  2021-02-22 17:16       ` Matthew Wilcox
  1 sibling, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2021-02-22 17:04 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Nathan Chancellor, Andrew Morton, Linux Memory Management List,
	Mike Rapoport, clang-built-linux, Miguel Ojeda, Jonathan Corbet

On Fri, Feb 19, 2021 at 12:55 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Feb 19, 2021 at 12:55:09PM -0700, Nathan Chancellor wrote:
> > On Mon, Feb 15, 2021 at 08:49:09PM +0000, Matthew Wilcox (Oracle) wrote:
> > > The generated html will link to the definition of the gfp_t automatically
> > > once we define it.  Move the one-paragraph overview of GFP flags from the
> > > documentation directory into gfp.h and pull gfp.h into the documentation.
> > >
> > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> >
> > This patch causes a clang warning in basically every file on linux-next
> > now:
> >
> > include/linux/gfp.h:20:32: warning: redefinition of typedef 'gfp_t' is a C11 feature [-Wtypedef-redefinition]
>
> Seems like it's also a gnu89 feature.

Is there a preprocessor define when using kernel doc that you could
guard the redefinition with?

See include/linux/phylink.h which uses:

#if 0 /* For kernel-doc purposes only. */

to guard definitions for kernel-doc.
-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-22 17:04     ` Nick Desaulniers
@ 2021-02-22 17:16       ` Matthew Wilcox
  2021-02-22 17:34         ` Nick Desaulniers
  0 siblings, 1 reply; 11+ messages in thread
From: Matthew Wilcox @ 2021-02-22 17:16 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Andrew Morton, Linux Memory Management List,
	Mike Rapoport, clang-built-linux, Miguel Ojeda, Jonathan Corbet

On Mon, Feb 22, 2021 at 09:04:40AM -0800, Nick Desaulniers wrote:
> #if 0 /* For kernel-doc purposes only. */

Yeah, I did that already over the weekend.  Not sure if akpm has published
a new mmotm that includes it yet.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm/gfp: Add kernel-doc for gfp_t
  2021-02-22 17:16       ` Matthew Wilcox
@ 2021-02-22 17:34         ` Nick Desaulniers
  0 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2021-02-22 17:34 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Nathan Chancellor, Andrew Morton, Linux Memory Management List,
	Mike Rapoport, clang-built-linux, Miguel Ojeda, Jonathan Corbet

On Mon, Feb 22, 2021 at 9:16 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Feb 22, 2021 at 09:04:40AM -0800, Nick Desaulniers wrote:
> > #if 0 /* For kernel-doc purposes only. */
>
> Yeah, I did that already over the weekend.  Not sure if akpm has published
> a new mmotm that includes it yet.

ah, sorry I missed it then. Appreciated!

-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-02-22 17:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 20:49 [PATCH] mm/gfp: Add kernel-doc for gfp_t Matthew Wilcox (Oracle)
2021-02-15 21:07 ` Mike Rapoport
2021-02-19 19:55 ` Nathan Chancellor
2021-02-19 20:54   ` Matthew Wilcox
2021-02-19 21:45     ` Nick Desaulniers
2021-02-19 22:15       ` Miguel Ojeda
2021-02-19 22:49         ` Nick Desaulniers
2021-02-20  9:43           ` Miguel Ojeda
2021-02-22 17:04     ` Nick Desaulniers
2021-02-22 17:16       ` Matthew Wilcox
2021-02-22 17:34         ` Nick Desaulniers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).