* [PATCH] mm/vmalloc: avoid bogus -Wmaybe-uninitialized warning
@ 2019-06-18 9:26 Arnd Bergmann
2019-06-18 13:40 ` Joel Fernandes
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2019-06-18 9:26 UTC (permalink / raw)
To: Roman Penyaev, Uladzislau Rezki, Andrew Morton
Cc: linux-mm, Arnd Bergmann, Roman Gushchin, Michal Hocko,
Matthew Wilcox, Thomas Garnier, Oleksiy Avramchenko,
Steven Rostedt, Joel Fernandes, Thomas Gleixner, Ingo Molnar,
Tejun Heo, Linus Torvalds, Rick Edgecombe, Andrey Ryabinin,
Mike Rapoport, linux-kernel
gcc gets confused in pcpu_get_vm_areas() because there are too many
branches that affect whether 'lva' was initialized before it gets
used:
mm/vmalloc.c: In function 'pcpu_get_vm_areas':
mm/vmalloc.c:991:4: error: 'lva' may be used uninitialized in this function [-Werror=maybe-uninitialized]
insert_vmap_area_augment(lva, &va->rb_node,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
&free_vmap_area_root, &free_vmap_area_list);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mm/vmalloc.c:916:20: note: 'lva' was declared here
struct vmap_area *lva;
^~~
Add an intialization to NULL, and check whether this has changed
before the first use.
Fixes: 68ad4a330433 ("mm/vmalloc.c: keep track of free blocks for vmap allocation")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
mm/vmalloc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index a9213fc3802d..42a6f795c3ee 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -913,7 +913,12 @@ adjust_va_to_fit_type(struct vmap_area *va,
unsigned long nva_start_addr, unsigned long size,
enum fit_type type)
{
- struct vmap_area *lva;
+ /*
+ * GCC cannot always keep track of whether this variable
+ * was initialized across many branches, therefore set
+ * it NULL here to avoid a warning.
+ */
+ struct vmap_area *lva = NULL;
if (type == FL_FIT_TYPE) {
/*
@@ -987,7 +992,7 @@ adjust_va_to_fit_type(struct vmap_area *va,
if (type != FL_FIT_TYPE) {
augment_tree_propagate_from(va);
- if (type == NE_FIT_TYPE)
+ if (lva)
insert_vmap_area_augment(lva, &va->rb_node,
&free_vmap_area_root, &free_vmap_area_list);
}
--
2.20.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmalloc: avoid bogus -Wmaybe-uninitialized warning
2019-06-18 9:26 [PATCH] mm/vmalloc: avoid bogus -Wmaybe-uninitialized warning Arnd Bergmann
@ 2019-06-18 13:40 ` Joel Fernandes
2019-06-18 14:06 ` Uladzislau Rezki
0 siblings, 1 reply; 5+ messages in thread
From: Joel Fernandes @ 2019-06-18 13:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Roman Penyaev, Uladzislau Rezki, Andrew Morton,
open list:MEMORY MANAGEMENT, Roman Gushchin, Michal Hocko,
Matthew Wilcox, Thomas Garnier, Oleksiy Avramchenko,
Steven Rostedt, Thomas Gleixner, Ingo Molnar, Tejun Heo,
Linus Torvalds, Rick Edgecombe, Andrey Ryabinin, Mike Rapoport,
LKML
On Tue, Jun 18, 2019 at 5:27 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> gcc gets confused in pcpu_get_vm_areas() because there are too many
> branches that affect whether 'lva' was initialized before it gets
> used:
>
> mm/vmalloc.c: In function 'pcpu_get_vm_areas':
> mm/vmalloc.c:991:4: error: 'lva' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> insert_vmap_area_augment(lva, &va->rb_node,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> &free_vmap_area_root, &free_vmap_area_list);
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mm/vmalloc.c:916:20: note: 'lva' was declared here
> struct vmap_area *lva;
> ^~~
>
> Add an intialization to NULL, and check whether this has changed
> before the first use.
>
> Fixes: 68ad4a330433 ("mm/vmalloc.c: keep track of free blocks for vmap allocation")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> mm/vmalloc.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index a9213fc3802d..42a6f795c3ee 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -913,7 +913,12 @@ adjust_va_to_fit_type(struct vmap_area *va,
> unsigned long nva_start_addr, unsigned long size,
> enum fit_type type)
> {
> - struct vmap_area *lva;
> + /*
> + * GCC cannot always keep track of whether this variable
> + * was initialized across many branches, therefore set
> + * it NULL here to avoid a warning.
> + */
> + struct vmap_area *lva = NULL;
Fair enough, but is this 5-line comment really needed here?
- Joel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmalloc: avoid bogus -Wmaybe-uninitialized warning
2019-06-18 13:40 ` Joel Fernandes
@ 2019-06-18 14:06 ` Uladzislau Rezki
2019-06-18 20:59 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Uladzislau Rezki @ 2019-06-18 14:06 UTC (permalink / raw)
To: Joel Fernandes
Cc: Arnd Bergmann, Roman Penyaev, Uladzislau Rezki, Andrew Morton,
open list:MEMORY MANAGEMENT, Roman Gushchin, Michal Hocko,
Matthew Wilcox, Thomas Garnier, Oleksiy Avramchenko,
Steven Rostedt, Thomas Gleixner, Ingo Molnar, Tejun Heo,
Linus Torvalds, Rick Edgecombe, Andrey Ryabinin, Mike Rapoport,
LKML
On Tue, Jun 18, 2019 at 09:40:28AM -0400, Joel Fernandes wrote:
> On Tue, Jun 18, 2019 at 5:27 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > gcc gets confused in pcpu_get_vm_areas() because there are too many
> > branches that affect whether 'lva' was initialized before it gets
> > used:
> >
> > mm/vmalloc.c: In function 'pcpu_get_vm_areas':
> > mm/vmalloc.c:991:4: error: 'lva' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > insert_vmap_area_augment(lva, &va->rb_node,
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > &free_vmap_area_root, &free_vmap_area_list);
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > mm/vmalloc.c:916:20: note: 'lva' was declared here
> > struct vmap_area *lva;
> > ^~~
> >
> > Add an intialization to NULL, and check whether this has changed
> > before the first use.
> >
> > Fixes: 68ad4a330433 ("mm/vmalloc.c: keep track of free blocks for vmap allocation")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > mm/vmalloc.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index a9213fc3802d..42a6f795c3ee 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -913,7 +913,12 @@ adjust_va_to_fit_type(struct vmap_area *va,
> > unsigned long nva_start_addr, unsigned long size,
> > enum fit_type type)
> > {
> > - struct vmap_area *lva;
> > + /*
> > + * GCC cannot always keep track of whether this variable
> > + * was initialized across many branches, therefore set
> > + * it NULL here to avoid a warning.
> > + */
> > + struct vmap_area *lva = NULL;
>
> Fair enough, but is this 5-line comment really needed here?
>
How it is rewritten now, probably not. I would just set it NULL and
leave the comment, but that is IMHO. Anyway
Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
Thanks!
--
Vlad Rezki
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmalloc: avoid bogus -Wmaybe-uninitialized warning
2019-06-18 14:06 ` Uladzislau Rezki
@ 2019-06-18 20:59 ` Andrew Morton
2019-06-19 10:36 ` Uladzislau Rezki
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2019-06-18 20:59 UTC (permalink / raw)
To: Uladzislau Rezki
Cc: Joel Fernandes, Arnd Bergmann, Roman Penyaev,
open list:MEMORY MANAGEMENT, Roman Gushchin, Michal Hocko,
Matthew Wilcox, Thomas Garnier, Oleksiy Avramchenko,
Steven Rostedt, Thomas Gleixner, Ingo Molnar, Tejun Heo,
Linus Torvalds, Rick Edgecombe, Andrey Ryabinin, Mike Rapoport,
LKML
On Tue, 18 Jun 2019 16:06:22 +0200 Uladzislau Rezki <urezki@gmail.com> wrote:
> On Tue, Jun 18, 2019 at 09:40:28AM -0400, Joel Fernandes wrote:
> > On Tue, Jun 18, 2019 at 5:27 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > gcc gets confused in pcpu_get_vm_areas() because there are too many
> > > branches that affect whether 'lva' was initialized before it gets
> > > used:
> > >
> > > mm/vmalloc.c: In function 'pcpu_get_vm_areas':
> > > mm/vmalloc.c:991:4: error: 'lva' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > insert_vmap_area_augment(lva, &va->rb_node,
> > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > &free_vmap_area_root, &free_vmap_area_list);
> > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > mm/vmalloc.c:916:20: note: 'lva' was declared here
> > > struct vmap_area *lva;
> > > ^~~
> > >
> > > Add an intialization to NULL, and check whether this has changed
> > > before the first use.
> > >
> > > Fixes: 68ad4a330433 ("mm/vmalloc.c: keep track of free blocks for vmap allocation")
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > mm/vmalloc.c | 9 +++++++--
> > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index a9213fc3802d..42a6f795c3ee 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -913,7 +913,12 @@ adjust_va_to_fit_type(struct vmap_area *va,
> > > unsigned long nva_start_addr, unsigned long size,
> > > enum fit_type type)
> > > {
> > > - struct vmap_area *lva;
> > > + /*
> > > + * GCC cannot always keep track of whether this variable
> > > + * was initialized across many branches, therefore set
> > > + * it NULL here to avoid a warning.
> > > + */
> > > + struct vmap_area *lva = NULL;
> >
> > Fair enough, but is this 5-line comment really needed here?
> >
> How it is rewritten now, probably not. I would just set it NULL and
> leave the comment, but that is IMHO. Anyway
>
I agree - given that the patch does this:
@@ -972,7 +977,7 @@ adjust_va_to_fit_type(struct vmap_area *
if (type != FL_FIT_TYPE) {
augment_tree_propagate_from(va);
- if (type == NE_FIT_TYPE)
+ if (lva)
insert_vmap_area_augment(lva, &va->rb_node,
&free_vmap_area_root, &free_vmap_area_list);
}
the comment simply isn't relevant any more. Although I guess this
might be a bit helpful:
@@ -977,7 +972,7 @@ adjust_va_to_fit_type(struct vmap_area *
if (type != FL_FIT_TYPE) {
augment_tree_propagate_from(va);
- if (lva)
+ if (lva) /* type == NE_FIT_TYPE */
insert_vmap_area_augment(lva, &va->rb_node,
&free_vmap_area_root, &free_vmap_area_list);
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmalloc: avoid bogus -Wmaybe-uninitialized warning
2019-06-18 20:59 ` Andrew Morton
@ 2019-06-19 10:36 ` Uladzislau Rezki
0 siblings, 0 replies; 5+ messages in thread
From: Uladzislau Rezki @ 2019-06-19 10:36 UTC (permalink / raw)
To: Andrew Morton
Cc: Uladzislau Rezki, Joel Fernandes, Arnd Bergmann, Roman Penyaev,
open list:MEMORY MANAGEMENT, Roman Gushchin, Michal Hocko,
Matthew Wilcox, Thomas Garnier, Oleksiy Avramchenko,
Steven Rostedt, Thomas Gleixner, Ingo Molnar, Tejun Heo,
Linus Torvalds, Rick Edgecombe, Andrey Ryabinin, Mike Rapoport,
LKML
On Tue, Jun 18, 2019 at 01:59:20PM -0700, Andrew Morton wrote:
> On Tue, 18 Jun 2019 16:06:22 +0200 Uladzislau Rezki <urezki@gmail.com> wrote:
>
> > On Tue, Jun 18, 2019 at 09:40:28AM -0400, Joel Fernandes wrote:
> > > On Tue, Jun 18, 2019 at 5:27 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > gcc gets confused in pcpu_get_vm_areas() because there are too many
> > > > branches that affect whether 'lva' was initialized before it gets
> > > > used:
> > > >
> > > > mm/vmalloc.c: In function 'pcpu_get_vm_areas':
> > > > mm/vmalloc.c:991:4: error: 'lva' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > insert_vmap_area_augment(lva, &va->rb_node,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > &free_vmap_area_root, &free_vmap_area_list);
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > mm/vmalloc.c:916:20: note: 'lva' was declared here
> > > > struct vmap_area *lva;
> > > > ^~~
> > > >
> > > > Add an intialization to NULL, and check whether this has changed
> > > > before the first use.
> > > >
> > > > Fixes: 68ad4a330433 ("mm/vmalloc.c: keep track of free blocks for vmap allocation")
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > > mm/vmalloc.c | 9 +++++++--
> > > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > > index a9213fc3802d..42a6f795c3ee 100644
> > > > --- a/mm/vmalloc.c
> > > > +++ b/mm/vmalloc.c
> > > > @@ -913,7 +913,12 @@ adjust_va_to_fit_type(struct vmap_area *va,
> > > > unsigned long nva_start_addr, unsigned long size,
> > > > enum fit_type type)
> > > > {
> > > > - struct vmap_area *lva;
> > > > + /*
> > > > + * GCC cannot always keep track of whether this variable
> > > > + * was initialized across many branches, therefore set
> > > > + * it NULL here to avoid a warning.
> > > > + */
> > > > + struct vmap_area *lva = NULL;
> > >
> > > Fair enough, but is this 5-line comment really needed here?
> > >
> > How it is rewritten now, probably not. I would just set it NULL and
> > leave the comment, but that is IMHO. Anyway
> >
>
> I agree - given that the patch does this:
>
> @@ -972,7 +977,7 @@ adjust_va_to_fit_type(struct vmap_area *
> if (type != FL_FIT_TYPE) {
> augment_tree_propagate_from(va);
>
> - if (type == NE_FIT_TYPE)
> + if (lva)
> insert_vmap_area_augment(lva, &va->rb_node,
> &free_vmap_area_root, &free_vmap_area_list);
> }
>
> the comment simply isn't relevant any more. Although I guess this
> might be a bit helpful:
>
> @@ -977,7 +972,7 @@ adjust_va_to_fit_type(struct vmap_area *
> if (type != FL_FIT_TYPE) {
> augment_tree_propagate_from(va);
>
> - if (lva)
> + if (lva) /* type == NE_FIT_TYPE */
> insert_vmap_area_augment(lva, &va->rb_node,
> &free_vmap_area_root, &free_vmap_area_list);
> }
>
That comment makes it much clear, thanks!
--
Vlad Rezki
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-19 10:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-18 9:26 [PATCH] mm/vmalloc: avoid bogus -Wmaybe-uninitialized warning Arnd Bergmann
2019-06-18 13:40 ` Joel Fernandes
2019-06-18 14:06 ` Uladzislau Rezki
2019-06-18 20:59 ` Andrew Morton
2019-06-19 10:36 ` Uladzislau Rezki
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).