On Mon 24-08-20 14:58:11, Li Xinhai wrote:
> In has_unmovable_pages(), the page parameter would not always be the
> first page within a pageblock (see how the page pointer is passed in from
> start_isolate_page_range() after call __first_valid_page()), so that
> would cause checking unmovable pages span two pageblocks.
This might lead to false negatives when an unrelated block would cause
an isolation failure.
> After this patch, the checking is enforced within one pageblock no matter
> the page is first one or not, and obey the semantics of this function.
>
> This issue is found by code inspection.
>
> Signed-off-by: Li Xinhai <lixinhai.lxh@xxxxxxxxx>
I have tried to find the commit which has introduced this but there was
so much churn around that I gave up. Not that we care all that much
because it seems that we simply never try to isolate pageblocks with
holes. Memory hotplug disallows that explicitly and the CMA allocator
doesn't trip over that either. Or maybe we were just lucky or a silent
failure didn't really trigger any attention.
Anyway, good to have it fixed.
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> ---
> mm/page_alloc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0e2bab486fea..c2c5b565f1f3 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -8213,6 +8213,7 @@ struct page *has_unmovable_pages(struct zone *zone, struct page *page,
> {
> unsigned long iter = 0;
> unsigned long pfn = page_to_pfn(page);
> + unsigned long offset = pfn % pageblock_nr_pages;
>
> /*
> * TODO we could make this much more efficient by not checking every
> @@ -8234,7 +8235,7 @@ struct page *has_unmovable_pages(struct zone *zone, struct page *page,
> return page;
> }
>
> - for (; iter < pageblock_nr_pages; iter++) {
> + for (; iter < pageblock_nr_pages - offset; iter++) {
> if (!pfn_valid_within(pfn + iter))
> continue;
>
> --
> 2.18.4
>
--
Michal Hocko
SUSE Labs
