On 1/12/21 3:24 PM, Xu, Yanfei wrote:
> Hi Vlastimil,
Hi,
> When I inspect the the codes about isolating pages, there are some lines
> from you make me confused. As blow:
>
> Locate in isolate_migratepages_block()
> mm/compaction.c
>
> 908 /*
> 909 * Skip if free. We read page order here without zone lock
> 910 * which is generally unsafe, but the race window is small and
> 911 * the worst thing that can happen is that we skip some
> 912 * potential isolation targets.
> 913 */
> 914 if (PageBuddy(page)) {
> 915 unsigned long freepage_order =
> buddy_order_unsafe(page);
> 916
> 917 /*
> 918 * Without lock, we cannot be sure that what we got is
> 919 * a valid page order. Consider only values in the
> 920 * valid order range to prevent low_pfn overflow.
> 921 */
> 922 if (freepage_order > 0 && freepage_order < MAX_ORDER))
> 923 low_pfn += (1UL << freepage_order) - 1;
> 924 continue;
> 925 }
>
> These lines was isntroduced by the commit 99c0fd5e51c("mm, compaction:
> skip buddy pages by their order in the migrate scanner")
>
> What I don't understand is that "the samll race window" mentioned in
> comments is which situation. I think before the
> isolate_migratepages_block() function is involved, those pageblocks have
> been marked MIGRATE_ISOLATE by set_migratetype_isolate() in
> start_isolate_page_range(). So the pages of those pageblocks in buddy
AFAIK that's only true when we start in alloc_contig_range(). In compact_zone()
-> isolate_migratepages() we don't use MIGRATE_ISOLATE as that would increase
the compaction cost a lot.
> will not be allocated, then the buddy_order_unsafe() here will get a
> certainly correct order value.
>
> Could you please tell me what situation it will race with? :)
>
>
>
> Thanks,
> Yanfei
>
