From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yx1-f43.google.com (mail-yx1-f43.google.com [74.125.224.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A633D186284 for ; Mon, 22 Dec 2025 07:40:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.224.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766389263; cv=none; b=CwKWCzMuIGDsA4zp+tobquThxmqARrIiXeS32fJp2fi8dbce3dX69wLZS1QD/jgjt54TvyuTP7bJES0GA1g/2fDL0xSN/5Y3EG7vUPQbA8v7Gr6MZ7gjqRsF9wvRY/J1HzE6Gb6KgR8FZl/Mq0YbEx4vlVvzkPc5VaGaFKMdH7w= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766389263; c=relaxed/simple; bh=au5ZELYPJeuqO9AWGQnL/NaAwBPRPuc5+MK7Bjf48w4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Oql19s9vrnm6+bb5v/d6/9HrkLm/BIK+ajWTLctJFNFoYKs6H30h6xRBWBrlBRvBqSn/Ja0IT6q0W93DIVoMKIDMCR3LvFbxw3wtSdTHpmk935fwvrBxO4Y7WGfBEWXXvSY/w4ecI8UZieSmwdt1CSRoCvX6egjtFdJ9cL3zNM4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=aI3t626H; arc=none smtp.client-ip=74.125.224.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="aI3t626H" Received: by mail-yx1-f43.google.com with SMTP id 956f58d0204a3-6420c08f886so4754204d50.3 for ; Sun, 21 Dec 2025 23:40:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1766389258; x=1766994058; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=8s9VTdOnPQzZ6XjtjKZ846Ransc2bFfZ11jc5LmUyBY=; b=aI3t626Hio5Ynsfm+uA0uG1CRlmh1z8ooV/oFZ39ZnfnyH6IE+o8RF2qRPjt3/GQC+ VKN3OPga/aHdtydzUMs6rtRKO2Sg+DnD46jNnuqGjmQTMG1Pc9wbfYvfANjuSxO1QClF 3IoD+cBvgUvX/ZgjdC4aDoUDKf9hwfM1gp7QkK8aHGMCyZUUXX7HUtzecKq3kv6eveEA A6N1rsZpARXCKmRbEfCiLjfdWhErYanXs96AF0Do7qqvVZkn2Ta1HB4aJb+r/AfnzQec 2DB/AESvEVbGc7qoykS+/5YK65D6KSEIjZTFJwFDeCP1Of5T8ZrbJWEhJmI9vQnF71Da u3+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1766389258; x=1766994058; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=8s9VTdOnPQzZ6XjtjKZ846Ransc2bFfZ11jc5LmUyBY=; b=ASWc2KqXmbXQBkckBRQjNfXUzWTVE5QK1mpTteclMXnHgZ2AwDDGnVBFj3VrvGYNjP uihM/7ivrWY7oK/KqEOoUJfuQlt2fjauMsE5BpTMhPMyJS+11IHtlHHr8Zh38+OVztuF 2JZ3JKSzXvrrgIPjSY6cNYHQtR2vfOa52JeKSwdKtuWrfjm/gXzCYy8KvjPsFziqPQED 4oOs2kLqEF9slP4+46SDlI2UKcNuc3OOxEKMJkeKqac7kwdI0r96UuSq8EgkDZccltk/ MA93QPxiYivibcko28owwfmUHhKc60Xkdmn2VAbaL1BikfJNKYxL6jWYC7SaDeS7AGQB 3HBQ== X-Forwarded-Encrypted: i=1; AJvYcCVr7XPFOMGUaHQggBpmeG7Rs0YKMlxSWkruRbEHxUe1hzzN8UEUt0ZzJetL/jkw120dkx2BmpSwQ1ACwt8=@vger.kernel.org X-Gm-Message-State: AOJu0YwsTanSfp2DUoapAOW74HWJLvBJKX49ehR3MYNRpNnZtFGLjIxx R9STnZbPSVfd13k3UD0u0JdyDADCvYg3dLK13LS1vv78N2g8hG1CNiPm X-Gm-Gg: AY/fxX6mdt7wVdRFIuEfnkKQD7Pi6PSJR3jj5wEzmHzl940SwY9rMHL5CGKrP2Nvwbr V2ofcni2GDh+Xn449kOFf0SGpI6gvj05xk1deTcKC1gTJ9gK6CWQqijOsLzgl2WtJouA3AjAuvm wnL5OPgK2sQdH4jZMYVjZfsDoZRdX0w7j/TcJxCtl+ohhRxhaK/5rux1ClijmwB2rkLMGsPJ4i/ m13loyZi9ytWSBDruZorBYxiLvUSjpYtVOrRs6TFAFWNJFeC7v0kokrBVxCicXhHUOxUjr8Yomg yq2MLuU0pf66PQUblYE2X+bL+IR3sCMOp+ajVeEChSo0TKkL63Pr9Jgb7hMqEYi3NtEFoWv4m2e +Svf3h0yV4uCzB7DlTyk0aHiQ2bLk0VuMw6sRWj1KLTx0gXV50jc3V7ONSw37F6NrK4/XxWJ3w/ GhDuXww5d+b5hqX+vSdjxfiyfuJa6cv2G1 X-Google-Smtp-Source: AGHT+IF0kVZd0e5+x+GIT1QyifG02SxHGcMriy7BlBzMK5qLzfkrgVOl+g7Sr3qloTHpKRpjotxdZA== X-Received: by 2002:a05:690e:1519:b0:63f:b18a:7819 with SMTP id 956f58d0204a3-6466a85a952mr7381558d50.23.1766389258596; Sun, 21 Dec 2025 23:40:58 -0800 (PST) Received: from localhost ([2a03:2880:25ff:42::]) by smtp.gmail.com with ESMTPSA id 956f58d0204a3-64678d8d40asm3064976d50.1.2025.12.21.23.40.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 21 Dec 2025 23:40:57 -0800 (PST) From: Joshua Hahn To: Vlastimil Babka Cc: Andrew Morton , Suren Baghdasaryan , Michal Hocko , Brendan Jackman , Johannes Weiner , Zi Yan , David Rientjes , David Hildenbrand , Lorenzo Stoakes , "Liam R. Howlett" , Mike Rapoport , Pedro Falcato , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC v2 2/3] mm/page_alloc: refactor the initial compaction handling Date: Sun, 21 Dec 2025 23:40:55 -0800 Message-ID: <20251222074055.74545-1-joshua.hahnjy@gmail.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251219-thp-thisnode-tweak-v2-2-0c01f231fd1c@suse.cz> References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit On Fri, 19 Dec 2025 18:38:52 +0100 Vlastimil Babka wrote: Hi Vlastimil, I hope you are doing well, sorry for the late reply. The patch overall looks good to me, but I have a few very small nits. > The initial direct compaction done in some cases in > __alloc_pages_slowpath() stands out from the main retry loop of > reclaim + compaction. > > We can simplify this by instead skipping the initial reclaim attempt via > a new local variable compact_first, and handle the compact_prority to > match the original behavior. > > Suggested-by: Johannes Weiner > Signed-off-by: Vlastimil Babka > --- > mm/page_alloc.c | 106 +++++++++++++++++++++++++++++--------------------------- > 1 file changed, 54 insertions(+), 52 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 9e7b0967f1b5..cb8965fd5e20 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4512,6 +4512,11 @@ static bool oom_reserves_allowed(struct task_struct *tsk) > return true; > } > > +static inline bool gfp_thisnode_noretry(gfp_t gfp_mask) > +{ > + return (gfp_mask & __GFP_NORETRY) && (gfp_mask & __GFP_THISNODE); > +} NIT: Is there a reason why this was turned into its own function? The checks seem short enough to open-code and it seems like there's only one caller as far as I can tell. Actually I think there are some better candidates of turning boolean checks into functions, like the one below: [...snip...] > + /* > + * For costly allocations, try direct compaction first, as it's likely > + * that we have enough base pages and don't need to reclaim. For non- > + * movable high-order allocations, do that as well, as compaction will > + * try prevent permanent fragmentation by migrating from blocks of the > + * same migratetype. > + */ > + if (can_compact && (costly_order || (order > 0 && > + ac->migratetype != MIGRATE_MOVABLE))) { > + compact_first = true; > + compact_priority = INIT_COMPACT_PRIORITY; > + } > + It has indeed become shorter thanks to this patch, but I think if we want to make the code more "readable" we can stay consistent and move these into thier own boolean checks, or just leave them open-coded. No strong preference here, just wanted to offer my 2c and hear what you think. [...snip...] > + /* > + * For the initial compaction attempt we have lowered its > + * priority. Restore it for further retries. With __GFP_NORETRY > + * there will be a single round of reclaim+compaction with the > + * lowered priority. > + */ > + if (!(gfp_mask & __GFP_NORETRY)) { > + compact_priority = DEF_COMPACT_PRIORITY; > + } NIT: I think these braces are unecessary : -) Otherwise, LGTM! Thank you for your work, it looks a lot cleaner. Reviewed-by: Joshua Hahn