* mm: Question about the race condition of pages in buddy during isolating pages
@ 2021-01-12 14:24 Xu, Yanfei
2021-01-12 14:31 ` Vlastimil Babka
0 siblings, 1 reply; 3+ messages in thread
From: Xu, Yanfei @ 2021-01-12 14:24 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: Linux Memory Management List
Hi Vlastimil,
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
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: mm: Question about the race condition of pages in buddy during isolating pages
2021-01-12 14:24 mm: Question about the race condition of pages in buddy during isolating pages Xu, Yanfei
@ 2021-01-12 14:31 ` Vlastimil Babka
2021-01-12 15:21 ` Xu, Yanfei
0 siblings, 1 reply; 3+ messages in thread
From: Vlastimil Babka @ 2021-01-12 14:31 UTC (permalink / raw)
To: Xu, Yanfei; +Cc: Linux Memory Management List
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: mm: Question about the race condition of pages in buddy during isolating pages
2021-01-12 14:31 ` Vlastimil Babka
@ 2021-01-12 15:21 ` Xu, Yanfei
0 siblings, 0 replies; 3+ messages in thread
From: Xu, Yanfei @ 2021-01-12 15:21 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: Linux Memory Management List
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
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-12 15:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12 14:24 mm: Question about the race condition of pages in buddy during isolating pages Xu, Yanfei
2021-01-12 14:31 ` Vlastimil Babka
2021-01-12 15:21 ` Xu, Yanfei
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.