From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1D7F4C36010 for ; Fri, 11 Apr 2025 13:45:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 83EE4680006; Fri, 11 Apr 2025 09:45:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7ED45680005; Fri, 11 Apr 2025 09:45:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 63F07680006; Fri, 11 Apr 2025 09:45:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 43925680005 for ; Fri, 11 Apr 2025 09:45:58 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id E70B71202A8 for ; Fri, 11 Apr 2025 13:45:58 +0000 (UTC) X-FDA: 83321886396.03.70D09AC Received: from mail-qt1-f177.google.com (mail-qt1-f177.google.com [209.85.160.177]) by imf04.hostedemail.com (Postfix) with ESMTP id 97A024000E for ; Fri, 11 Apr 2025 13:45:56 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=PJQeeOgY; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf04.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.177 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744379156; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Zxmmu2NCRuHQ/p0IZo4CKnrYoaf6gi4nt3+l5uXITBc=; b=lwJczUO1ilahJdye//hdRuvSsjjuaJ4555HDu2HAJ+ssBDQfuTR2ChLO/6s6PoqA+6OLA3 9760khDeVNKtUrUP+wpVPQtaG5jTWIwGz1SaPnhQbvF/e01TfHgaA2hPyW4k2uyZqjlplR aWJbTZGqj/Ac8w/8bS1HNF4UrtFj2wE= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=PJQeeOgY; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf04.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.177 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744379156; a=rsa-sha256; cv=none; b=58P2MAsZ9wop3lOlV7sxGEWrvm5+y3D/E4U3eYRT0wKzWY7lUV6pf5ha/+KNGvF/z5z5OL mV+q86kjnVZY3AG9VP0MO+1x9UfeGbJfhAvwgTkRFDG+0WhzojyRlZTSdQDQmKjnklVUwd 582+JsL1nexPFMk7VFNDMegwy0IACW8= Received: by mail-qt1-f177.google.com with SMTP id d75a77b69052e-4767e969b94so32960011cf.2 for ; Fri, 11 Apr 2025 06:45:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1744379155; x=1744983955; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Zxmmu2NCRuHQ/p0IZo4CKnrYoaf6gi4nt3+l5uXITBc=; b=PJQeeOgY5+yAM4Zt+IinULUsNaYQfs0lYb10xir50aTvDyqhOEFA97twGyx268olxV kPwsSAQOrGFYtDRqjImJ0aDnB3l4c8v8P1mv+/l5uydMiU3BNgnlbFGKaNEOyywjM0ey HI/fPPQzKbUXYOLFysyrsAkz+AaZUT2jEVCqS+857GGcfqdFOPNwJfEwpI26d5v3QptT H6EDh3lsoXxVlFvGpZmdoDZX5eMA8Q2uvh5YsSoRD7j6EI+DdQo1HnVn0CxQsBH8yYUx 9Yq+75vL0JAIHw4crw1SJCbUw/YYgmOS2OmxrvAqg0maFkPdltme5H/ewWnTQYPFadum CwHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744379155; x=1744983955; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Zxmmu2NCRuHQ/p0IZo4CKnrYoaf6gi4nt3+l5uXITBc=; b=To3t4R985rFk9YOIYUMEd+rQP46S/889QjY+fLXex8x91ZJqvfpPUcHpWICuiHcV8V k7iVATdm125F7enKlc494FU3qyk9AtzTiO72j5bir8LH2yvfvCglHZ5aGWaxpid+T7tJ ZwLkx1b83D0Gr/FraLauu/bK1g1Hoz9MfSioc0DBcJp40cu3h1iVkAp/8jgMXXEbaldG 9ly+T5yJDqw/viGNl2lg/JgN3SjhIWPc4CDdbITmVpYcuUD6FJ0Tl1GaC+FGpgw0S72v e8m4Biev/faSbRFSav2hQ4ayn+cWDt01W/ls4BLsiIptZoVpXqCyYGECjBMsMxiis8rZ V7lw== X-Forwarded-Encrypted: i=1; AJvYcCXYJmhbKB3vliKUG8qLtFhoAZRwTTM3KaeuGKaTVoIdFDgq3IGObCwKGyi5PPvUVTD2rf/uKZmlDA==@kvack.org X-Gm-Message-State: AOJu0YwqomB1tFQLZXbvpqYdXcJr223w11Szz40rqVtBRq7YDHnhmL4p 2G8prYkOeJoPw8Y9asGxiW4UHajYnz6LSBa9VECDF8vLJUWfXsGdFofIIt1cmPE= X-Gm-Gg: ASbGncvuuyFLj1FLX8BHqFJkb8zpuTHf9P4GbN9n99cRLilH+QpZzHJmWgUU5fjtD1u hIqeF9wKwovV7N0FC2sl90ieZIIeNmLta6FUZGUWzkkozrS5F/JPCWaxfe+XCgmk0kW1CSo4TlD iFz+SgXt2ifzSuQnLSvnlBUyNuCO0NGBDjrz/kBu9fTgfM1VQMN8ni4fNdXvYMI4WsDe3VDClBT Oh0c1GNu9cLsByX4rMKz30RuyzMSAAEkUNJ2d9ZUK/beUQDCk9aVT5NvlvZAmXvJP1uAPkZRya7 hpO3KnZffc7MtPD4SclWDHZqk5ZfIz9RyTJfCTk= X-Google-Smtp-Source: AGHT+IHocieVa/GLTrNp8bq2cpL9Zyfxy3zMjOWH8Uc94+yoMWWfQgNkIoyy9EBomrM62q71TWWrWQ== X-Received: by 2002:ac8:5992:0:b0:476:91f1:9de with SMTP id d75a77b69052e-479775e95afmr32144771cf.47.1744379155606; Fri, 11 Apr 2025 06:45:55 -0700 (PDT) Received: from localhost ([2603:7000:c01:2716:365a:60ff:fe62:ff29]) by smtp.gmail.com with UTF8SMTPSA id d75a77b69052e-4796edc1c1esm25559621cf.68.2025.04.11.06.45.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Apr 2025 06:45:54 -0700 (PDT) Date: Fri, 11 Apr 2025 09:45:50 -0400 From: Johannes Weiner To: Brendan Jackman Cc: Andrew Morton , Vlastimil Babka , Mel Gorman , Carlos Song , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] mm: page_alloc: tighten up find_suitable_fallback() Message-ID: <20250411134550.GB366747@cmpxchg.org> References: <20250407180154.63348-1-hannes@cmpxchg.org> <20250407180154.63348-2-hannes@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam01 X-Stat-Signature: kogjjg1xiewgrjf19daaxtwemeemb93k X-Rspam-User: X-Rspamd-Queue-Id: 97A024000E X-HE-Tag: 1744379156-680651 X-HE-Meta: U2FsdGVkX1/6GEOZy8H6XN5UBmfe+rshAWAs6UlU357lInqg3hg/43VBLb9bagxYuuy9NntRZsnWhvU2H0Qvcic3D8S+JCeIVE1SxyT65wQn0a4NeZ2cAJ243fqsrz/GUGowhrsEBHqRmjKG3BwkCDbXhhejhBjXQ2YZbxRxj1GP4ImD3cFuJ/0QozZJDXrxlqXolEbiDTIyIvmpO7RFm0MP2uLO9oUpz3A9YL+ik9I5mpxFRJW2guIeIvumkWm/saVZ0UTdK7LyUbFjXeRq/uSDb7EXTFxWQRF+x0zCae0/RW+zMFnOts8vLgpzPUnCmGAXgIpAReBBLafL/CYXAzps39clSvIO2nUQ0TN2f9VEkx5roeBhKjucZcE6LOI9IxsCNxK2WS9Ht2vl1qhJ3DzgJOSI53Upsa1T17mmKUnzKu9TXr2eae9SfOW/icIgw6KFEa8Em9XJZQpf4ZoxSN1MLezj2ahMWHmQFXRDGzRtwgfHE51NzCg3ZIBPDIkW7SWcNPgzKiyLRbwRWG2HHPxMDkq02v2vv4jXvmFXRcCbgp9ss+N8Gck6twxkrLQUDGfcnPDvmRBeuo5t2LLuMJXkDadKsY2K4qEFhgEI/fD7064m6m3cavbnBQtSsd46wbroBu406bWuTDKorL99EO4+1DzUfv9O24n+hLohqirXBILvZR33aGkJuyJZTKxZo4d4JRvbsaoN7qcw0WH92tULNpc7CXlN2oDKsDTtoLpQn1ypyxTwq97g2CQNqsezExMb4aLWsKrfdcCB7YCJky0el1ar3UnYvjI5xYwHIb3nkjHbl6ibkwD+mDxu8HXkW2PJsjRdbESrlEf8W+/DMPU2LQAsY+/J+lLbn3WZIrGGNwsdjFR2Uf2M6FuIGIg1vCAFSb0yMi3VAsAEe89/NjIiTLy83/yBpNnXTVyvEht8AUqpJ7FDHU9MiCOD7Ar7fCYjeVl2SYJH5Kzsi0k Etaxtqd5 kTm0wcqB5Yta0zEhhQeg1WhxRHtaoEVrBRb3MzJcQzf6ClYUDrcVXayvtnDDcfm7H5UNDPK8pYxOH7FoXpM/f2Xw2QJxLHIC/A5jygNAGLU74432Y+JMMax1KhEGBiyWBD+5PKVN3bYkmFdneh0H316Tb2WBnzMuqaUpbAUu17dsr2RarsyogQ1yrPDnxZdn12l6k/t/ECMCUIG7tYmRot4wZKDUZF7MQZ+zn4yb/TIu2H7A9uFOa4xaPulMskCuivxqyH+U37OcxsVuP7rdTyo+bnkSHK+/mb6nNxGGBdJpTMSaWes/qSSs8ETobbENZcpNkffPscWiyD2dp8ZKUVwL4JPbRvgPgKKi1/DwEb82C6EqTWDtfN7daGyRQvktER6xnCrxIxh4vPyPFk6wO5q04Mke2xSKAT2c0XYTCcsvYwSG/MurvvWqZCjSDbrI8We3rUtdbpuq6MXd+64vNbv2ADKTCOPP5iCMVRngtAGyMnE+Ip35LHsHSDkE54yNsskNBQRrZc1bC62lWJF3lyvlMPg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Apr 10, 2025 at 01:55:27PM +0000, Brendan Jackman wrote: > On Mon Apr 7, 2025 at 6:01 PM UTC, Johannes Weiner wrote: > > find_suitable_fallback() is not as efficient as it could be, and > > somewhat difficult to follow. > > > > 1. should_try_claim_block() is a loop invariant. There is no point in > > checking fallback areas if the caller is interested in claimable > > blocks but the order and the migratetype don't allow for that. > > > > 2. __rmqueue_steal() doesn't care about claimability, so it shouldn't > > have to run those tests. > > > > Different callers want different things from this helper: > > > > 1. __compact_finished() scans orders up until it finds a claimable block > > 2. __rmqueue_claim() scans orders down as long as blocks are claimable > > 3. __rmqueue_steal() doesn't care about claimability at all > > > > Move should_try_claim_block() out of the loop. Only test it for the > > two callers who care in the first place. Distinguish "no blocks" from > > "order + mt are not claimable" in the return value; __rmqueue_claim() > > can stop once order becomes unclaimable, __compact_finished() can keep > > advancing until order becomes claimable. > > Nice! > > My initial thought was: now we can drop the boolean arg and just have > the callers who care about claimability just call > should_try_claim_block() themselves. Then we can also get rid of the > magic -2 return value and find_suitable_fallback() becomes a pretty > obvious function. > > I think it's a win on balance but it does make more verbosity at the > callsites, and an extra interface between page_alloc.c and compaction.c > So maybe it's a wash, maybe you already considered it and decided you > don't prefer it. > > So, LGTM either way, I will attempt to attach the optional additional > patch for your perusal, hopefully without molesting the mail encoding > this time... > > Reviewed-by: Brendan Jackman Thanks! > From 25f77012674db95354fb2496bc89954b8f8b4e6c Mon Sep 17 00:00:00 2001 > From: Brendan Jackman > Date: Thu, 10 Apr 2025 13:22:58 +0000 > Subject: [PATCH] mm: page_alloc: Split up find_suitable_fallback() > > Now that it's been simplified, it's clear that the bool arg isn't > needed, callers can just use should_try_claim_block(). Once that logic > is stripped out, the function becomes very obvious and can get a more > straightforward name and comment. > > Signed-off-by: Brendan Jackman > --- > mm/compaction.c | 3 ++- > mm/internal.h | 5 +++-- > mm/page_alloc.c | 33 +++++++++++++-------------------- > 3 files changed, 18 insertions(+), 23 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 39a4d178dff3c..d735c22e71029 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -2363,7 +2363,8 @@ static enum compact_result __compact_finished(struct compact_control *cc) > * Job done if allocation would steal freepages from > * other migratetype buddy lists. > */ > - if (find_suitable_fallback(area, order, migratetype, true) >= 0) > + if (should_try_claim_block(order, migratetype) && > + find_fallback_migratetype(area, order, migratetype) >= 0) So I agree with pushing the test into the callers. However, I think the name "should_try_claim_block()" is not great for this. It makes sense in the alloc/fallback path, but compaction here doesn't claim anything. It just wants to know if this order + migratetype is eligible under block claiming rules. IMO this would be more readable with the old terminology: if (can_claim_block(order, migratetype) && find_fallback_migratetype(area, order, migratetype) >= 0)