2018-10-05 20:58:47

by David Rientjes

Subject: [patch] mm, page_alloc: set num_movable in move_freepages()

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 <[email protected]>
---
mm/page_alloc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

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);

2018-10-05 21:22:32

by Andrew Morton

Subject: Re: [patch] mm, page_alloc: set num_movable in move_freepages()

On Fri, 5 Oct 2018 13:56:39 -0700 (PDT) David Rientjes <[email protected]> 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.
>
> ...

2018-10-07 22:43:19

by David Rientjes

Subject: Re: [patch] mm, page_alloc: set num_movable in move_freepages()

On Fri, 5 Oct 2018, Andrew Morton wrote:

> On Fri, 5 Oct 2018 13:56:39 -0700 (PDT) David Rientjes <[email protected]> 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().

2018-10-16 12:24:48

by Vlastimil Babka

Subject: Re: [patch] mm, page_alloc: set num_movable in move_freepages()

On 10/5/18 11:21 PM, Andrew Morton wrote:
> On Fri, 5 Oct 2018 13:56:39 -0700 (PDT) David Rientjes <[email protected]> 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 <[email protected]>

>> 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.
>>
>> ...
>