| Message ID | alpine.DEB.2.21.1810051355490.212229@chino.kir.corp.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm, page_alloc: set num_movable in move_freepages() | expand |
On Fri, 5 Oct 2018 13:56:39 -0700 (PDT) David Rientjes <rientjes@google.com> wrote: > If move_freepages() returns 0 because zone_spans_pfn(), *num_movable can move_free_pages_block()? !zone_spans_pfn()? > hold the value from the stack because it does not get initialized in > move_freepages(). > > Move the initialization to move_freepages_block() to guarantee the value > actually makes sense. > > This currently doesn't affect its only caller where num_movable != NULL, > so no bug fix, but just more robust. > > ...
On Fri, 5 Oct 2018, Andrew Morton wrote: > On Fri, 5 Oct 2018 13:56:39 -0700 (PDT) David Rientjes <rientjes@google.com> wrote: > > > If move_freepages() returns 0 because zone_spans_pfn(), *num_movable can > > move_free_pages_block()? !zone_spans_pfn()? > move_freepages_block() more accurately, yes. And yes, it depends on the return value of zone_spans_pfn().
On 10/5/18 11:21 PM, Andrew Morton wrote: > On Fri, 5 Oct 2018 13:56:39 -0700 (PDT) David Rientjes <rientjes@google.com> wrote: > >> If move_freepages() returns 0 because zone_spans_pfn(), *num_movable can > > move_free_pages_block()? !zone_spans_pfn()? Also the subject would be more accurate if it said "initialize num_movable in move_freepages_block()" ? Otherwise, Acked-by: Vlastimil Babka <vbabka@suse.cz> >> hold the value from the stack because it does not get initialized in >> move_freepages(). >> >> Move the initialization to move_freepages_block() to guarantee the value >> actually makes sense. >> >> This currently doesn't affect its only caller where num_movable != NULL, >> so no bug fix, but just more robust. >> >> ... >
diff --git a/mm/page_alloc.c b/mm/page_alloc.c --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2015,10 +2015,6 @@ static int move_freepages(struct zone *zone, pfn_valid(page_to_pfn(end_page)) && page_zone(start_page) != page_zone(end_page)); #endif - - if (num_movable) - *num_movable = 0; - for (page = start_page; page <= end_page;) { if (!pfn_valid_within(page_to_pfn(page))) { page++; @@ -2058,6 +2054,9 @@ int move_freepages_block(struct zone *zone, struct page *page, unsigned long start_pfn, end_pfn; struct page *start_page, *end_page; + if (num_movable) + *num_movable = 0; + start_pfn = page_to_pfn(page); start_pfn = start_pfn & ~(pageblock_nr_pages-1); start_page = pfn_to_page(start_pfn);
If move_freepages() returns 0 because zone_spans_pfn(), *num_movable can hold the value from the stack because it does not get initialized in move_freepages(). Move the initialization to move_freepages_block() to guarantee the value actually makes sense. This currently doesn't affect its only caller where num_movable != NULL, so no bug fix, but just more robust. Signed-off-by: David Rientjes <rientjes@google.com> --- mm/page_alloc.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)