Re: [PATCH v2] page_alloc: consider highatomic reserve in wmartermark fast

[Vlastimil Babka]

On 6/13/20 6:16 AM, Jaewon Kim wrote:
> 
> 
> 2020ë 6ì 12ì (ê) ìí 11:34, Vlastimil Babka <vbabka@xxxxxxx
> <mailto:vbabka@xxxxxxx>>ëì ìì:
>>
>> On 6/13/20 4:51 AM, Jaewon Kim wrote:
>> > zone_watermark_fast was introduced by commit 48ee5f3696f6 ("mm,
>> > page_alloc: shortcut watermark checks for order-0 pages"). The commit
>> > simply checks if free pages is bigger than watermark without additional
>> > calculation such like reducing watermark.
>> >
>> > It considered free cma pages but it did not consider highatomic
>> > reserved. This may incur exhaustion of free pages except high order
>> > atomic free pages.
>> >
>> > Assume that reserved_highatomic pageblock is bigger than watermark min,
>> > and there are only few free pages except high order atomic free. Because
>> > zone_watermark_fast passes the allocation without considering high order
>> > atomic free, normal reclaimable allocation like GFP_HIGHUSER will
>> > consume all the free pages. Then finally order-0 atomic allocation may
>> > fail on allocation.
>>
>> I don't understand why order-0 atomic allocation will fail. Is it because of
>> watermark check, or finding no suitable pages?
>> - watermark check should be OK as atomic allocations can use reserves
>> - suitable pages should be OK, even if all free pages are in the highatomic
>> reserves, because rmqueue() contains:
>>
>> if (alloc_flags & ALLOC_HARDER)
>> Â Â Â Â page = __rmqueue_smallest(zone, order, MIGRATE_HIGHATOMIC);
>>
>> So what am I missing?
>>
> Hello
> The order-0 atomic allocation can be failed because ofÂdepletion of suitable
> free page.
> Watermark check passes order-0 atomic allocation but it will be failed at
> finding a free page.
> The __rmqueue_smallest(zone, order, MIGRATE_HIGHATOMIC) can be used
> only for highorder.

Ah, that's what I missed, rmqueue() will divert all order-0 allocations to
rmqueue_pcplist() so those will not reach the hunk above. Thanks.

>> > @@ -3598,9 +3604,12 @@ static inline bool zone_watermark_fast(struct zone
> *z, unsigned int order,
>> Â Â Â Â /*
>> Â Â Â Â Â* Fast check for order-0 only. If this fails then the reserves
>> Â Â Â Â Â* need to be calculated. There is a corner case where the check
>> Â Â Â Â Â* passes but only the high-order atomic reserve are free. If
>> > Â Â Â Â* the caller is !atomic then it'll uselessly search the free
>> > Â Â Â Â* list. That corner case is then slower but it is harmless.
>> > Â Â Â Â*/
>>
>> The comment stops being true after this patch? It also suggests that Mel
>> anticipated this corner case, but that it should only cause a false positive
>> zone_watermark_fast() and then rmqueue() fails for !ALLOC_HARDER as it cannot
>> use MIGRATE_HIGHATOMIC blocks. It expects atomic order-0 still works. So what's
>> going on?
> 
> As Mel also agreed with me in v1 mail thread, this highatomicÂreserved should
> be considered even in watermark fast.
> 
> The comment, I think, may need to be changed. Prior to this patch, non highatomic
> allocation may do useless search, but it also can take ALL non highatomicÂfree.
> 
> With this patch, non highatomicÂallocation will NOT do useless search. Rather,

Yes, that's what I meant.