linux-kernel - Re: [PATCH V3] mm: compaction: skip memory compaction when there are not enough migratable pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1ada3f6d-72bb-46a4-9fef-0bf6a7b32e62@suse.cz>
Date: Tue, 14 Jan 2025 13:51:33 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Ge Yang <yangge1116@....com>, akpm@...ux-foundation.org
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org, 21cnbao@...il.com,
 david@...hat.com, baolin.wang@...ux.alibaba.com, hannes@...xchg.org,
 liuzixing@...on.cn
Subject: Re: [PATCH V3] mm: compaction: skip memory compaction when there are
 not enough migratable pages

On 1/14/25 13:24, Ge Yang wrote:
>> Hopefully also when done from the pin_user_pages_remote(..., FOLL_LONGTERM,
>> ...) context the allocation gfp_mask correctly lacks __GFP_MOVABLE? 
> yes.
> I guess
>> it has to, otherwise it would allocate from the CMA pageblocks.
>> 
>> Then I wonder if we could use the real allocation context to determine
>> watermarks, as __compaction_suitable() is passing ALLOC_CMA instead because
>> it's checking only for migration targets, which have to be CMA compatible by
>> definition. But we could use the real unmovable allocation context to have
>> __zone_watermark_unusable_free() subtract CMA pages, and thus we won't pass
>> the order-0 check anymore once the non-CMA part is exhausted.
>> 
>> There's some risk that in some different scenario the compaction could in
>> fact migrate pages from the exhausted non-CMA part of the zone to the CMA
>> part and succeed, and we'll skip it instead. But that should be rare?
>> 
> Below is the previous discussion：
> https://lore.kernel.org/lkml/1734436004-1212-1-git-send-email-yangge1116@126.com/

Right so Johannes had the same concern.

>> Anyway given that concern I'm not sure about changing
>> __compaction_suitable() for every caller like this. We could (at least
>> initially) target this heuristic only for COMPACT_PRIO_ASYNC which is being
>> used for this THP opportunistic attempt.
>> 
>> So for example:
>> - add a new bool flag to compact_control that is true for COMPACT_PRIO_ASYNC
>> - pass cc pointer to compaction_suit_allocation_order()
>> - in that function, add another check if the the new cc flag is true,
>> between the current zone_watermark_ok() and compaction_suitable() checks,
>> which works like __compaction_suitable() but uses alloc_flags (which should
>> not be ALLOC_CMA in our pinned allocation case) instead of ALLOC_CMA, return
>> COMPACT_SKIPPED if it fails.
>> 
> I will send a new version of the patch based on the suggestions here. 
> Thank you.

Yeah that way should hopefully limit the concern sufficiently. Maybe we
could also add costly_order condition in addition to COMPACT_PRIO_ASYNC
condition to set the new compact_control flag. But only __GFP_NORETRY
allocations should be affected in the immediate "goto nopage" when
compaction is skipped, others will attempt with DEF_COMPACT_PRIORITY anyway
and won't fail without trying to compact-migrate the non-CMA pageblocks into
CMA pageblocks first, so it should be fine.