| Message ID | 20220202003550.698768-1-longman@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mm/sparsemem: Fix 'mem_section' will never be NULL gcc 12 warning | expand |
On Tue, 1 Feb 2022 19:35:50 -0500 Waiman Long <longman@redhat.com> wrote: > The gcc 12 compiler reports a "'mem_section' will never be NULL" > warning on the following code: > > static inline struct mem_section *__nr_to_section(unsigned long nr) > { > #ifdef CONFIG_SPARSEMEM_EXTREME > if (!mem_section) > return NULL; > #endif > if (!mem_section[SECTION_NR_TO_ROOT(nr)]) > return NULL; > : > > It happens with both CONFIG_SPARSEMEM_EXTREME on and off. The mem_section > definition is > > #ifdef CONFIG_SPARSEMEM_EXTREME > extern struct mem_section **mem_section; > #else > extern struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT]; > #endif > > In the CONFIG_SPARSEMEM_EXTREME case, mem_section obviously cannot > be NULL, but *mem_section can be if memory hasn't been allocated for > the dynamic mem_section[] array yet. In the !CONFIG_SPARSEMEM_EXTREME > case, mem_section is a static 2-dimensional array and so the check > "!mem_section[SECTION_NR_TO_ROOT(nr)]" doesn't make sense. > > Fix this warning by checking for "!*mem_section" instead of > "!mem_section" and moving the "!mem_section[SECTION_NR_TO_ROOT(nr)]" > check up inside the CONFIG_SPARSEMEM_EXTREME block. > > ... > > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -1390,11 +1390,9 @@ static inline unsigned long *section_to_usemap(struct mem_section *ms) > static inline struct mem_section *__nr_to_section(unsigned long nr) > { > #ifdef CONFIG_SPARSEMEM_EXTREME > - if (!mem_section) > + if (!*mem_section || !mem_section[SECTION_NR_TO_ROOT(nr)]) > return NULL; > #endif > - if (!mem_section[SECTION_NR_TO_ROOT(nr)]) > - return NULL; > return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK]; > } > extern size_t mem_section_usage_size(void); What does the v1->v2 change do? --- a/include/linux/mmzone.h~mm-sparsemem-fix-mem_section-will-never-be-null-gcc-12-warning-v2 +++ a/include/linux/mmzone.h @@ -1390,11 +1390,9 @@ static inline unsigned long *section_to_ static inline struct mem_section *__nr_to_section(unsigned long nr) { #ifdef CONFIG_SPARSEMEM_EXTREME - if (!*mem_section) + if (!*mem_section || !mem_section[SECTION_NR_TO_ROOT(nr)]) return NULL; #endif - if (!mem_section[SECTION_NR_TO_ROOT(nr)]) - return NULL; return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK]; } extern size_t mem_section_usage_size(void);
On 2/3/22 18:11, Andrew Morton wrote: > On Tue, 1 Feb 2022 19:35:50 -0500 Waiman Long <longman@redhat.com> wrote: > >> The gcc 12 compiler reports a "'mem_section' will never be NULL" >> warning on the following code: >> >> static inline struct mem_section *__nr_to_section(unsigned long nr) >> { >> #ifdef CONFIG_SPARSEMEM_EXTREME >> if (!mem_section) >> return NULL; >> #endif >> if (!mem_section[SECTION_NR_TO_ROOT(nr)]) >> return NULL; >> : >> >> It happens with both CONFIG_SPARSEMEM_EXTREME on and off. The mem_section >> definition is >> >> #ifdef CONFIG_SPARSEMEM_EXTREME >> extern struct mem_section **mem_section; >> #else >> extern struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT]; >> #endif >> >> In the CONFIG_SPARSEMEM_EXTREME case, mem_section obviously cannot >> be NULL, but *mem_section can be if memory hasn't been allocated for >> the dynamic mem_section[] array yet. In the !CONFIG_SPARSEMEM_EXTREME >> case, mem_section is a static 2-dimensional array and so the check >> "!mem_section[SECTION_NR_TO_ROOT(nr)]" doesn't make sense. >> >> Fix this warning by checking for "!*mem_section" instead of >> "!mem_section" and moving the "!mem_section[SECTION_NR_TO_ROOT(nr)]" >> check up inside the CONFIG_SPARSEMEM_EXTREME block. >> >> ... >> >> --- a/include/linux/mmzone.h >> +++ b/include/linux/mmzone.h >> @@ -1390,11 +1390,9 @@ static inline unsigned long *section_to_usemap(struct mem_section *ms) >> static inline struct mem_section *__nr_to_section(unsigned long nr) >> { >> #ifdef CONFIG_SPARSEMEM_EXTREME >> - if (!mem_section) >> + if (!*mem_section || !mem_section[SECTION_NR_TO_ROOT(nr)]) >> return NULL; >> #endif >> - if (!mem_section[SECTION_NR_TO_ROOT(nr)]) >> - return NULL; >> return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK]; >> } >> extern size_t mem_section_usage_size(void); > What does the v1->v2 change do? > > --- a/include/linux/mmzone.h~mm-sparsemem-fix-mem_section-will-never-be-null-gcc-12-warning-v2 > +++ a/include/linux/mmzone.h > @@ -1390,11 +1390,9 @@ static inline unsigned long *section_to_ > static inline struct mem_section *__nr_to_section(unsigned long nr) > { > #ifdef CONFIG_SPARSEMEM_EXTREME > - if (!*mem_section) > + if (!*mem_section || !mem_section[SECTION_NR_TO_ROOT(nr)]) > return NULL; > #endif > - if (!mem_section[SECTION_NR_TO_ROOT(nr)]) > - return NULL; > return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK]; > } > extern size_t mem_section_usage_size(void); > _ When !CONFIG_SPARSEMEM_EXTREME, mem_section is really a static 2-D array. Since it is not a table of pointers, mem_section[SECTION_NR_TO_ROOT(nr)] has no real meaning. That is why the compiler is complaining. This check isn't applicable in the !CONFIG_SPARSEMEM_EXTREME case, but it is meaningful for CONFIG_SPARSEMEM_EXTREME. That is why it is pulled into the CONFIG_SPARSEMEM_EXTREME block. Thanks, Longman
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index aed44e9b5d89..08517376c765 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -1390,11 +1390,9 @@ static inline unsigned long *section_to_usemap(struct mem_section *ms) static inline struct mem_section *__nr_to_section(unsigned long nr) { #ifdef CONFIG_SPARSEMEM_EXTREME - if (!mem_section) + if (!*mem_section || !mem_section[SECTION_NR_TO_ROOT(nr)]) return NULL; #endif - if (!mem_section[SECTION_NR_TO_ROOT(nr)]) - return NULL; return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK]; } extern size_t mem_section_usage_size(void);
The gcc 12 compiler reports a "'mem_section' will never be NULL" warning on the following code: static inline struct mem_section *__nr_to_section(unsigned long nr) { #ifdef CONFIG_SPARSEMEM_EXTREME if (!mem_section) return NULL; #endif if (!mem_section[SECTION_NR_TO_ROOT(nr)]) return NULL; : It happens with both CONFIG_SPARSEMEM_EXTREME on and off. The mem_section definition is #ifdef CONFIG_SPARSEMEM_EXTREME extern struct mem_section **mem_section; #else extern struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT]; #endif In the CONFIG_SPARSEMEM_EXTREME case, mem_section obviously cannot be NULL, but *mem_section can be if memory hasn't been allocated for the dynamic mem_section[] array yet. In the !CONFIG_SPARSEMEM_EXTREME case, mem_section is a static 2-dimensional array and so the check "!mem_section[SECTION_NR_TO_ROOT(nr)]" doesn't make sense. Fix this warning by checking for "!*mem_section" instead of "!mem_section" and moving the "!mem_section[SECTION_NR_TO_ROOT(nr)]" check up inside the CONFIG_SPARSEMEM_EXTREME block. Fixes: 83e3c48729d9 ("mm/sparsemem: Allocate mem_section at runtime for CONFIG_SPARSEMEM_EXTREME=y") Fixes: 3e347261a80b ("sparsemem extreme implementation") Reported-by: Justin Forbes <jforbes@redhat.com> Signed-off-by: Waiman Long <longman@redhat.com> --- include/linux/mmzone.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)