linux-kernel - Re: [PATCH 1/5] mm: compaction: push watermark into compaction

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: <eecb9994-1fdb-4636-9430-9911fd3e28c4@suse.cz>
Date: Fri, 11 Apr 2025 09:32:35 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Johannes Weiner <hannes@...xchg.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Mel Gorman <mgorman@...hsingularity.net>, Zi Yan <ziy@...dia.com>,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] mm: compaction: push watermark into
 compaction_suitable() callers

On 4/10/25 22:17, Johannes Weiner wrote:
> On Thu, Apr 10, 2025 at 05:19:06PM +0200, Vlastimil Babka wrote:
>> On 3/13/25 22:05, Johannes Weiner wrote:
> 
> Ah yes, it would have made sense to point out.
> 
> I was wondering about this check. It was introduced to bail on
> compaction if there are not enough free non-CMA pages. But if there
> are, we still fall through and check the superset of regular + CMA
> pages against the watermarks as well. We know this will succeed, so
> this seems moot.

I guess we didn't want to avoid the fragindex part of compaction_suitable(),
which in theory may not succeed?

> It's also a little odd that compaction_suitable() hardcodes ALLOC_CMA
> with the explanation that "CMA are migration targets", but then this
> check says "actually, it doesn't help us if blocks are formed in CMA".

Hm yes.

> Does it make more sense to plumb alloc_flags to compaction_suitable()?

Possibly.

> There is more head-scratching, though. The check is meant to test
> whether compaction has a chance of forming non-CMA blocks. But free
> pages are targets. You could have plenty of non-contiguous, free
> non-CMA memory - compaction will then form blocks in CMA by moving CMA
> pages into those non-CMA targets.
> 
> The longer I look at this, the more I feel like this just hard-coded
> the very specific scenario the patch author had a problem with: CMA is
> massive. The page allocator fills up regular memory first. Once
> regular memory is full, non-CMA requests stall on compaction making
> CMA blocks. So just bail on compaction then.

Right.

> It's a valid problem, but I don't see how this code makes any general
> sense outside of this exact sequence of events. Especially once
> compaction has moved stuff around between regular and CMA memory, the
> issue will be back, and the check does nothing to prevent it.

Yeah, it seemed to fix a real problem and we both acked it :) but it's not
ideal.

Maybe the true solution (or a step towards it) would be for compaction for
!ALLOC_CMA only use non-CMA pageblocks as migration sources.
IMHO it's just another symptom of the general problem that CMA pageblocks
exist as part of a zone that's not otherwise ZONE_MOVABLE and suddenly the
watermarks have to depend on the allocation type. And of course for the
high-order allocations it doesn't just matter the amount of memory in cma vs
non-cma parts of the zone, but also its contiguity.