On 1/12/21 10:31 PM, Vlastimil Babka wrote:
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.
Ah， got it. Compaction doesn't need these buddy pages during isolating，
so it doesn't need to mark them MIGRATE_ISOLATE. THANK YOU.
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
