* [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).