Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH] mm, compaction: avoid uninitialized variable use | Date | Wed, 11 May 2016 16:52:41 +0200 |
| |
On Wednesday 11 May 2016 16:44:07 Michal Hocko wrote: > On Wed 11-05-16 15:24:44, Arnd Bergmann wrote: > > A recent rework of the compaction code introduced a warning about > > an uninitialized variable when CONFIG_COMPACTION is disabled and > > __alloc_pages_direct_compact() does not set its 'compact_result' > > output argument: > > > > mm/page_alloc.c: In function '__alloc_pages_nodemask': > > mm/page_alloc.c:3651:6: error: 'compact_result' may be used uninitialized in this function [-Werror=maybe-uninitialized] > > > > This adds another check for CONFIG_COMPACTION to ensure we never > > evaluate the uninitialized variable in this configuration, which > > is probably the simplest way to avoid the warning. > > I think that hiding this into __alloc_pages_direct_compact is a better > idea. See the diff below
Ok, sounds good.
> --- > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 4950d01ff935..14e3b4d93adc 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -3300,6 +3300,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, > unsigned int alloc_flags, const struct alloc_context *ac, > enum migrate_mode mode, enum compact_result *compact_result) > { > + *compact_result = COMPACT_DEFERRED; > return NULL; > } >
I thought about this but didn't know which COMPACT_* value was appropriate here.
The behavior then changes a bit with your approach compared to mine, because
if (compact_result == COMPACT_DEFERRED) goto nopage;
is true now. I assume this is what we want though.
Arnd
| |