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

[Vlastimil Babka <vbabka@suse.cz>]

On 6/13/20 6:16 AM, Jaewon Kim wrote:
> 
> 
> 2020년 6월 12일 (금) 오후 11:34, Vlastimil Babka <vbabka@suse.cz
> <mailto:vbabka@suse.cz>>님이 작성:
>>
>> 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.