From patchwork Fri Feb 14 18:14:01 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brendan Jackman X-Patchwork-Id: 13975489 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 ADF58C02198 for ; Fri, 14 Feb 2025 18:14:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 43D1F280009; Fri, 14 Feb 2025 13:14:35 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3C606280002; Fri, 14 Feb 2025 13:14:35 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 23FCB280009; Fri, 14 Feb 2025 13:14:35 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 02EA8280002 for ; Fri, 14 Feb 2025 13:14:34 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 487671A07F6 for ; Fri, 14 Feb 2025 18:14:15 +0000 (UTC) X-FDA: 83119349670.04.89FB56F Received: from mail-wm1-f73.google.com (mail-wm1-f73.google.com [209.85.128.73]) by imf06.hostedemail.com (Postfix) with ESMTP id 589D118000B for ; Fri, 14 Feb 2025 18:14:13 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=kvE9wpJf; spf=pass (imf06.hostedemail.com: domain of 384evZwgKCAYpgiqsgthmuumrk.iusrot03-ssq1giq.uxm@flex--jackmanb.bounces.google.com designates 209.85.128.73 as permitted sender) smtp.mailfrom=384evZwgKCAYpgiqsgthmuumrk.iusrot03-ssq1giq.uxm@flex--jackmanb.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1739556853; a=rsa-sha256; cv=none; b=wYDfSar1HYytJ+BC9Prqbj1IAjq+rnewfcfU/3li3qnp7BhBx0r0/ldTg1J34n8pL6WpRV HdzAcx4c1QeHxTPPTWvMR4UvoA6gm94U1DP/Z9/V19r6C/MkqDb5k0AXwyssniP7wofwmK q7PxtBDjokXbNRSmMGJQc+X2bXNSVVg= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=kvE9wpJf; spf=pass (imf06.hostedemail.com: domain of 384evZwgKCAYpgiqsgthmuumrk.iusrot03-ssq1giq.uxm@flex--jackmanb.bounces.google.com designates 209.85.128.73 as permitted sender) smtp.mailfrom=384evZwgKCAYpgiqsgthmuumrk.iusrot03-ssq1giq.uxm@flex--jackmanb.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1739556853; 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: references:dkim-signature; bh=kM2Ovaxxrgq7wE5ezhQqwh/r8fPzqX2R4YTVCdFxQPY=; b=S4rYhhefsgArW8YVuEYK9ouCDVkpsWcUR7OxImuAo80ZV2zfL4pVc8ecweELkBvEV7rDVc GZWAbQZDXRlsawiJ6MEWLkoKz7EQmEZd/dUK/4WR4lVlVyyb1JpyCl8fvU/3pWaYGCpOga zJVpRnttRDf8b/DMSkVFELWUKp6BEFU= Received: by mail-wm1-f73.google.com with SMTP id 5b1f17b1804b1-438e4e9a53fso18043805e9.1 for ; Fri, 14 Feb 2025 10:14:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1739556852; x=1740161652; darn=kvack.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=kM2Ovaxxrgq7wE5ezhQqwh/r8fPzqX2R4YTVCdFxQPY=; b=kvE9wpJf9YXoUc9fT2KUijFAOHIgTWdDuGDcoeV+PwpsHbpBgD9ZRyP1zPNVAW8Mn6 QfkCFY+a/ffeaNnay+mtllu4hDjuJ8VFh6vH5XCcwhokPjUL6hbMt9jeNe8BkFu+eY1n ThhmGOzpR6Akx2+tKY4VSLJIMJ0U0/TvxL6YWN9JKazc2H1c/BBnDLveheqSTHG0g+92 RM/zQwIM1txXCkirEFxSpmOXltWEKjA///iZ6yX4jmAL55d1LTs7h0bR4mKV5hflFnz5 p9+xbX7Koa02585wF1Ag/c8Ih4z8g28DIeLVln73X8CXnZFbJtCnmbcTlnxnGVYeDMHv 4SJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739556852; x=1740161652; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=kM2Ovaxxrgq7wE5ezhQqwh/r8fPzqX2R4YTVCdFxQPY=; b=CrKO6nE2lEqKlj/SRihMQ0FbI3RCrN7NOqvwiR0zZBW3WgUlMp4eUlDwWcRXayP8t9 7tDG3/orGpU3W1sH+SRtPgEA2U2yEa1vuDdP0OnQ/o29zVcV+KjH063X0zI+260ct2jL Yezcz499/Yxi9K1qpFReEGwWx7DA1wHbIZs5ZkNNRY/PDsM8kgALXbjvcxGe583SKc0Y fmQnF8nyw2acXNjNP5fMVDxyvvLFMeuGd8mbFCy69+/bTC5DS7T5O4gB/WxrKBKDHxxO KffPrgMoQ40TT2GVFaNEyy0VtuMmeb2ihO3OU9rjaDQtSN9XWwcyXW4bxUKkNpa4mjI8 nVog== X-Forwarded-Encrypted: i=1; AJvYcCWl8qzfV4Q5i0/iEHcDdcZgHwhpwbx8jrLRKLMi9mfWKgBTwPOoiFsihn5/C+rOkuk83zagXluRZg==@kvack.org X-Gm-Message-State: AOJu0YzSRKmawRJ1wP6QTdz21tC6qUW2394NBQiiGJNZuLaxesd/YZ8w 4ksebKmN7UAHr88LAF1UZ2KpSTSEs6ir7tp8d/bR72UqfPvSEXFohh7geS/fudxk8KW6Zc1v3WS i8daiT5n07g== X-Google-Smtp-Source: AGHT+IG48U3jLPPy8jsbIodS96OA4IQYGYtRES+t7y5quifE8e/WTqGxWQRdp+LiC0BV3+kDmbunyWp60rKipA== X-Received: from wmgg17.prod.google.com ([2002:a05:600d:11:b0:439:5fab:db0f]) (user=jackmanb job=prod-delivery.src-stubby-dispatcher) by 2002:a05:600c:3546:b0:439:419d:ae01 with SMTP id 5b1f17b1804b1-4396e6eda19mr3229325e9.10.1739556851656; Fri, 14 Feb 2025 10:14:11 -0800 (PST) Date: Fri, 14 Feb 2025 18:14:01 +0000 Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIAOiHr2cC/x3MQQqAIBBA0avIrBNUJpCuEi3ExhoQC40oxLsnL d/i/wqFMlOBSVTIdHPhI3XoQYDfXdpI8toNRplRGY3SR5c5vLJc5KIMBtFahai9ht6cmQI//29 eWvsAgAjIYl8AAAA= X-Change-Id: 20250214-clarify-steal-f244880441c1 X-Mailer: b4 0.15-dev Message-ID: <20250214-clarify-steal-v1-1-79dc5adf1b79@google.com> Subject: [PATCH] mm/page_alloc: Clarify some migratetype fallback code From: Brendan Jackman To: Andrew Morton Cc: Vlastimil Babka , Mel Gorman , Michal Hocko , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Yosry Ahmed , Brendan Jackman X-Rspamd-Queue-Id: 589D118000B X-Stat-Signature: czu6onziq7nur8bb4hz73zfzx91k8zfo X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1739556853-633157 X-HE-Meta: U2FsdGVkX1966yM7qUq7OaRNuI705f2ah7bXzmkNbmC+pIlxS1mQbkNd4uDEutqIMfM2TzIhZ4V+cb4yxHFLuTjUnZJscS0SnvsfXOxZ6ZHVwLFnXfd9t47MDNoYjj8p67wI4erRyVqf2OuNO91Hs8k1dOQ5+y34ffZ92/3X3ZE56wQvHPb/9XlgN+Fh9LveudRRFUzQl+t+qX5jq89qLg2+F0pgAOUGQ7t6zoBLb8fgLrpQ/wVFNutsglTnf8dZW8+fWqId52ldOdVEnvwQpJgwCFEo0NTzQZgJM1QHnZncjTK0MxmDikeoaQhSxtaksFzQiwTLV01bw/6vmWcyl8mYiQzU6YaQN1nqGihM/iKKbKrWBDyi5eZ9KF0NTwM48d1DvB/vIGWB31NU5hYvtFiYSzDxr8Sw4SFF//xsLsbB4gEk/IaOsoXq7ABpmo3p2b9OEOF7b8m+H3TmdzuMzB7BXdQWYS44Nj64F+3sG/SorA9E7aR+2czlhTSnKbOyEWru3wYZj3UszBcB3Jb9z8BFBquR2ic6FygqSEjnmQRokk1MOfQxb6J7hW2ca51lsvI1K8em0bfKEDgboRRJkoCpdW0e1MwUGP8Lk7uSlQHOo9AR6OchNJ3HDL1QVCrM7NHpLo+2Tsj6ayiMrVdxy1892/Emp1Nrr0FEprNulCNroehEp/R7PSZE0dDJO0YuyryAsisXu5pShrJ0MwJTQ7CcSMYBDenqRQPGAp0ARRIFC8qoMMdf0V4piU6JXNLkMFS7XdBkZrcGe/HbmYLC9Wc4otecpMEjf12gUkkQ7FP/tOIObREFTGB6qtxvMsB6e0cxogSgaiAZgjqlIPfWeyBOkSqB04jY2cb6NED4byDXzi4Ns/nVE076GmcV58mg7n79OE2UYjk8zQUWKag7gCqnnPlnPol0/ohkbmSPRLG40oSkkya3nkZ48xWCNZpfbTDsNySBw3gGkyHQIXH WfKNoc0g FzkAYOPbaA8cZln0XRqlyvmFuLQhnXTh+be1Rngz0d2GO76Z0+3CVdtN5ngwexSkkEAG3jOOWMvds+eeB6ucDIngrVFEWXmskTr2CPT8/ZGXOpirVYjn+xfh7c5E2kx1+sMDpPRbPH/rRZdAewg+R6bl7rxOuM2ogKpZvU2cz+Yod7ve4p66lUHHJB4cFINMdks3jzMFCmhO1BfywFc7cN7pDvqPcOlcOMrF1vtrc2ETbKm52z6yv2xNrubSVNmfKKubuOoyH1Pkb5fi4bSSaoHu22Ec30VD6jnqXD4Q4utlo77su47euGWkIy0ZYQONZjFf5KXsWIlNYRSRPiRY3eXsuY2B3F7N/pmmIOcnfxuImoFcI8xMW4/uHbc0zkWcGn8xLRuI/PyCHUAaCdyWZoosBcAf2BSHcXKRYKJaOlnDIy0LVhiK1rbl4aIHD/xiZP9PC+yErFAcR6eKttU08h3AfGRpl3D85AOgEd+YtLNj14fJNSzSbjOM2lnT0L3mr61KwLJEY1N8+UludJQrWdvwR0FrXZ7Q2zRmheeKpyC0j+01oOZmPfTxIljpiS0tgHsWVolzFwW11qWEc3MJ7YkpusqBBorwdlhbo 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: This code is rather confusing because: 1. "Steal" is sometimes used to refer to the general concept of allocating from a from a block of a fallback migratetype (steal_suitable_fallback()) but sometimes it refers specifically to converting a whole block's migratetype (can_steal_fallback()). 2. can_steal_fallback() sounds as though it's answering the question "am I functionally permitted to allocate from that other type" but in fact it is encoding a heuristic preference. 3. The same piece of data has different names in different places: can_steal vs whole_block. This reinforces point 2 because it looks like the different names reflect a shift in intent from "am I allowed to steal" to "do I want to steal", but no such shift exists. Fix 1. by avoiding the term "steal" in ambiguous contexts. This fixes 3. automatically since the natural name for can_steal is whole_block. Fix 2. by using "should" instead of "can", and also rename its parameters and add some commentary to make it more explicit what they mean. Signed-off-by: Brendan Jackman --- mm/compaction.c | 4 ++-- mm/internal.h | 2 +- mm/page_alloc.c | 42 ++++++++++++++++++++++-------------------- 3 files changed, 25 insertions(+), 23 deletions(-) --- base-commit: 128c8f96eb8638c060cd3532dc394d046ce64fe1 change-id: 20250214-clarify-steal-f244880441c1 Best regards, diff --git a/mm/compaction.c b/mm/compaction.c index 12ed8425fa175c5dec50bac3dddb13499abaaa11..8dccb2e388f128dd134ec6f24c924c118c9c93bb 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -2332,7 +2332,7 @@ static enum compact_result __compact_finished(struct compact_control *cc) ret = COMPACT_NO_SUITABLE_PAGE; for (order = cc->order; order < NR_PAGE_ORDERS; order++) { struct free_area *area = &cc->zone->free_area[order]; - bool can_steal; + bool whole_block; /* Job done if page is free of the right migratetype */ if (!free_area_empty(area, migratetype)) @@ -2349,7 +2349,7 @@ static enum compact_result __compact_finished(struct compact_control *cc) * other migratetype buddy lists. */ if (find_suitable_fallback(area, order, migratetype, - true, &can_steal) != -1) + true, &whole_block) != -1) /* * Movable pages are OK in any pageblock. If we are * stealing for a non-movable allocation, make sure diff --git a/mm/internal.h b/mm/internal.h index 109ef30fee11f8b399f6bac42eab078cd51e01a5..c22d2826fd8d8681c89bb783ed269cc9346b5d92 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -847,7 +847,7 @@ void init_cma_reserved_pageblock(struct page *page); #endif /* CONFIG_COMPACTION || CONFIG_CMA */ int find_suitable_fallback(struct free_area *area, unsigned int order, - int migratetype, bool only_stealable, bool *can_steal); + int migratetype, bool need_whole_block, bool *whole_block); static inline bool free_area_empty(struct free_area *area, int migratetype) { diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 579789600a3c7bfb7b0d847d51af702a9d4b139a..75900f9b538eb0a241401af888643df850840436 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1832,12 +1832,12 @@ static void change_pageblock_range(struct page *pageblock_page, * * If we are stealing a relatively large buddy page, it is likely there will * be more free pages in the pageblock, so try to steal them all. For - * reclaimable and unmovable allocations, we steal regardless of page size, - * as fragmentation caused by those allocations polluting movable pageblocks - * is worse than movable allocations stealing from unmovable and reclaimable - * pageblocks. + * reclaimable and unmovable allocations, we steal the whole block regardless of + * page size, as fragmentation caused by those allocations polluting movable + * pageblocks is worse than movable allocations stealing from unmovable and + * reclaimable pageblocks. */ -static bool can_steal_fallback(unsigned int order, int start_mt) +static bool should_steal_whole_block(unsigned int order, int start_mt) { /* * Leaving this order check is intended, although there is @@ -1855,7 +1855,7 @@ static bool can_steal_fallback(unsigned int order, int start_mt) * reclaimable pages that are closest to the request size. After a * while, memory compaction may occur to form large contiguous pages, * and the next movable allocation may not need to steal. Unmovable and - * reclaimable allocations need to actually steal pages. + * reclaimable allocations need to actually steal the whole block. */ if (order >= pageblock_order / 2 || start_mt == MIGRATE_RECLAIMABLE || @@ -1948,7 +1948,7 @@ steal_suitable_fallback(struct zone *zone, struct page *page, if (boost_watermark(zone) && (alloc_flags & ALLOC_KSWAPD)) set_bit(ZONE_BOOSTED_WATERMARK, &zone->flags); - /* We are not allowed to try stealing from the whole block */ + /* No point in stealing from the whole block */ if (!whole_block) goto single_page; @@ -1995,12 +1995,14 @@ steal_suitable_fallback(struct zone *zone, struct page *page, /* * Check whether there is a suitable fallback freepage with requested order. - * If only_stealable is true, this function returns fallback_mt only if - * we can steal other freepages all together. This would help to reduce + * Sets *whole_block to instruct the caller whether it should convert a whole + * pageblock to the returned migratetype. + * If need_whole_block is true, this function returns fallback_mt only if + * we would do this whole-block stealing. This would help to reduce * fragmentation due to mixed migratetype pages in one pageblock. */ int find_suitable_fallback(struct free_area *area, unsigned int order, - int migratetype, bool only_stealable, bool *can_steal) + int migratetype, bool need_whole_block, bool *whole_block) { int i; int fallback_mt; @@ -2008,19 +2010,19 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, if (area->nr_free == 0) return -1; - *can_steal = false; + *whole_block = false; for (i = 0; i < MIGRATE_PCPTYPES - 1 ; i++) { fallback_mt = fallbacks[migratetype][i]; if (free_area_empty(area, fallback_mt)) continue; - if (can_steal_fallback(order, migratetype)) - *can_steal = true; + if (should_steal_whole_block(order, migratetype)) + *whole_block = true; - if (!only_stealable) + if (!need_whole_block) return fallback_mt; - if (*can_steal) + if (*whole_block) return fallback_mt; } @@ -2190,7 +2192,7 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype, int min_order = order; struct page *page; int fallback_mt; - bool can_steal; + bool whole_block; /* * Do not steal pages from freelists belonging to other pageblocks @@ -2209,7 +2211,7 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype, --current_order) { area = &(zone->free_area[current_order]); fallback_mt = find_suitable_fallback(area, current_order, - start_migratetype, false, &can_steal); + start_migratetype, false, &whole_block); if (fallback_mt == -1) continue; @@ -2221,7 +2223,7 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype, * allocation falls back into a different pageblock than this * one, it won't cause permanent fragmentation. */ - if (!can_steal && start_migratetype == MIGRATE_MOVABLE + if (!whole_block && start_migratetype == MIGRATE_MOVABLE && current_order > order) goto find_smallest; @@ -2234,7 +2236,7 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype, for (current_order = order; current_order < NR_PAGE_ORDERS; current_order++) { area = &(zone->free_area[current_order]); fallback_mt = find_suitable_fallback(area, current_order, - start_migratetype, false, &can_steal); + start_migratetype, false, &whole_block); if (fallback_mt != -1) break; } @@ -2250,7 +2252,7 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype, /* take off list, maybe claim block, expand remainder */ page = steal_suitable_fallback(zone, page, current_order, order, - start_migratetype, alloc_flags, can_steal); + start_migratetype, alloc_flags, whole_block); trace_mm_page_alloc_extfrag(page, order, current_order, start_migratetype, fallback_mt);