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 2D096E7717F for ; Tue, 10 Dec 2024 21:16:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AD9516B0285; Tue, 10 Dec 2024 16:16:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A62CB6B0286; Tue, 10 Dec 2024 16:16:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8DC586B0287; Tue, 10 Dec 2024 16:16:21 -0500 (EST) 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 6B42F6B0285 for ; Tue, 10 Dec 2024 16:16:21 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 16A3F1203B4 for ; Tue, 10 Dec 2024 21:16:21 +0000 (UTC) X-FDA: 82880306880.25.89D3C91 Received: from mail-qt1-f169.google.com (mail-qt1-f169.google.com [209.85.160.169]) by imf18.hostedemail.com (Postfix) with ESMTP id B41AE1C0020 for ; Tue, 10 Dec 2024 21:16:08 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=W3Xe1fhS; spf=pass (imf18.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.169 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733865369; 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=gyk9ndnvoGvMP7pC2cu6SDb/gT8fltkKrw46Jm17ZPc=; b=QDYEcuYqpMTefuhk50/F5/QjaVQOxL0RoG7DoFdLbSOCm7cKPO2IvXd9tdK3uAzQqiv4jZ Vxr6NxayKNpKSJoi2FKnYhfx+6IZrPkXqhzXDNEOeFbHJ5IcygK7dG5cBD+rkp3E4Iu0bn tMfNHA6BIRIAOs57sJ47dMApse8yFNs= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=W3Xe1fhS; spf=pass (imf18.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.169 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733865369; a=rsa-sha256; cv=none; b=4QAPWVYgueMphoFXzAhsmtv5/tqIfybYnhyEvp54qQAyxEQ8cyyvF6Nzzycnm4yPcelC4n M++sTyHcTR42sufUY0HfsttIGwzexFUQAl//nz354CUSmZVj139K5o9/P+s6riwPeLzFYQ fJfx9YjwvYDi/7WOyahlh0eVyb0yjGM= Received: by mail-qt1-f169.google.com with SMTP id d75a77b69052e-46779ae3139so10248001cf.2 for ; Tue, 10 Dec 2024 13:16:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1733865378; x=1734470178; 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=gyk9ndnvoGvMP7pC2cu6SDb/gT8fltkKrw46Jm17ZPc=; b=W3Xe1fhSVbd1kSe9505feGg8m5puIkVyrOulC3luU8gNbrRw2IRmg/keRm2ai7SBmZ ahUU51kY29+PQ4xvmoxPR+Hg8LBeQYzdF9WozYbFwoWVAKvmoK8pt6NFT1lwSIfKOtei tSR6Gpv3ejSyt3Iret/ECvWKod89reEeir9VgNFg859rVWeeP4IqG4Lb7EtSsCfeLIza pHTHcYmCioKpy21dUcu9jrFilxBgxmomGKA5qScIS1Lm4RcpkI32lZM/6FfrXBc3xMR+ J5eVnTo1kJPutd7pBg4BbZU+EARL75CmaIml2xbApYs26CZfNrxFQqyq5eN4JsYf4tqF krfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733865378; x=1734470178; 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=gyk9ndnvoGvMP7pC2cu6SDb/gT8fltkKrw46Jm17ZPc=; b=WhD9aa6tJ3kjtbquQ2KUnIh6ALFYv5kwbgMw1dDZHBcZaaY+9txU3eF/4T3QkMbV4j lrAeVVDEki5ldtjZmzAHQO+yZ94pN/Ede3pcNg6M+CBA5NN+4vZwkwjf2f8PcXZ2syH/ U8ItRXbh1Uxfo5zBgAxItQWqO4XrHCjuLLgeSaXNc1KjcPd0T+8zuFaRwXd9Wz474Kd3 BgRLTXA26AU9j63HtANk8kGdtz6tyCZzD3U2bD2zTUtnR0fadHszLXxE8r8ceMsRxb5k tY/vFbajDMpwBIPTamwiCT009jpa1A0p4oWZ10M8yiHHdnZot1t6F7q4wIw851dEdoUO Iv9A== X-Forwarded-Encrypted: i=1; AJvYcCXglqhQO9TgbxjQGq/P40cnjPhjuRd2iYPf++zUIgiJzyeR2g3RT1OxYTqCr9QCGKbUOYCgnesUfQ==@kvack.org X-Gm-Message-State: AOJu0YylbI1bidWCbN9/5R+SFG2sZn6bnfMmmHYSZTtFpWUpLd8Q0x5R NFgw4h0UzcKUPzLW13uATwMku5uHl+mapexzt1RRWRO2Hdp5xEAU9A6kLQbuQ5s= X-Gm-Gg: ASbGncvxtVA45G6saRXq/Y84OOLOrjgWxmNpMUx3et98QouZZVasrtwak3v+jVyNJC+ 8Hp5JbH+uPJLw/3aNxENFrvV1hnmgCwA87ifmfTywViy2Ckx1GUeMX/SIf+XWaW7jj/FIW5Fl42 YYFE8lQkztBIHDRDGER34EutmzqOqPjM+b+zREKhqfW7qnN/NDtSvnU2IYpM6Y6LbM3V4zoRCP6 IKX5SXIuTuFt9c6gThr5txxHoTIWXbxlPnZanY60lgCn04OsTQr X-Google-Smtp-Source: AGHT+IGbonnfLU/NBu46yYYQXI1v4DRq430cXoE7IJ5mqfU9VxFIjoQbjkh3PuJphJoRzWCExYATJQ== X-Received: by 2002:a05:6214:21a5:b0:6d8:8ca0:a7 with SMTP id 6a1803df08f44-6d934b90095mr6115126d6.30.1733865377709; Tue, 10 Dec 2024 13:16:17 -0800 (PST) Received: from localhost ([2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6d8eb1ce58bsm51692076d6.49.2024.12.10.13.16.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Dec 2024 13:16:17 -0800 (PST) Date: Tue, 10 Dec 2024 16:16:13 -0500 From: Johannes Weiner To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Zi Yan , Vlastimil Babka , Yu Zhao Subject: Re: [PATCH v2 1/2] mm/page_alloc: conditionally split > pageblock_order pages in free_one_page() and move_freepages_block_isolate() Message-ID: <20241210211613.GC2508492@cmpxchg.org> References: <20241210102953.218122-1-david@redhat.com> <20241210102953.218122-2-david@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241210102953.218122-2-david@redhat.com> X-Rspamd-Queue-Id: B41AE1C0020 X-Stat-Signature: qb8y5z8umge94xyjsh784joid57g5szm X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1733865368-254536 X-HE-Meta: U2FsdGVkX1+PrBHZfswjkOcklZHaVyPY2fcxBafQeQl3ybpVXgeOmiqXN98VeSzaqH5k2jfCjoSkrKiM7IjnrvCBPQNrWpzT5EPwV2fVoIetKBfTmCWzc2HNqCwM3l10ovKNpuO8kfdCiwQVet6zqKLrmVUGcgXdgE6x9bAGQtRYJhyhWrl8gN0lsmIvu/VXJgX6kO7iOB6AUHb6ax1oDuL/0tJVilS+4rUJnTh1HqLr38jYVyDz5sIeRjANmU8JRfWjcIb9c3zevdZS5RojgFsCygX+Qh0c1r17rbm1TWHTx4d7mBAPKtAHWtQre6/QjMuu839lI01JNXjVFuvAjMMJQc9mOJFCJzsnoioMMs1q1MQxy0u9DgJdtaYE0moaisPkG/hWuEYdRYO9y+uJ649NQ8YZAxBuqmJX9jpkqMADBggDVhJi7rrBLc+e0zkTRD6dAbHqRdZGOkQkniZwFBQ+frG8I6lKt7HO26pg+DOyC2HdyB9cB9fL2CREwIK/yNeMI4VG/P+iD1gVYWdV8e8YS3TXRG2wz7nmJlivmJRyhpeYaegisqAIBD1USrvWJ0y1/kJeyYIV7F1s+8AJctiYPlibyp0jcPeSgoqNKKY3WJv+qxJeXmj9sRFn8wIjairayPjywp1VNoJQ+dHfN8qjZqKViz+pL8e2JyF+67clQ86xjmhe2OQ3mGAkJDWKREB0QkcqfBiFcrmpu/3DXrhjM6sBr+3OV1g8wcvOFSlN/M95mM+FZGsfQqxVv/tbDEYRY66TJ/Dj2K9zoSRhPNVxcAD/QTGppGU6re6NkBX6xKTpa7VeC9hUUb0CVa9gVIr1+1rli6KdkZv2Tkwbooyza0pFFZ3wOh76oKiNio+HZCvzjGNYJ6ypZHUK+B4UvTeymEihzJkT5GVukVzIN7ubDp4FfwdheDUOmr3UDso00poU4tYVv7W+sGwiSQrv4Cl5JPANfLIe7q1PQUa /8MdEhwU b1SLMtTIf/UVkMCwOl2sXipZ728j311uhcoRGOYcWXY16RpbGmZgfJAMydrtGtyxAoGNMnbcvw8d4OwUNt2kJCv+tb+sZBEABmxBNcJzLogb5pXnos0kvAwaMc7vP3j3fO9Xk0ApjEQr0w3FcTUYz2xrE/u6VNTBUZIgPYGCGcHihEbXgBfYH4/zW2jtuZksryXaaqirxZUN4tURu5pvxiWmYNYk1wKYT7yxU0HKMqrSL8ncSj/I8MCGAnD/KPvJarICskyQ3vz/luneRqFhyjRaxtv/H3UWYMBfvZ9pwOOHguq/Khdj4MVyby7+ZCdVkP13Mh6PhTMPs+eOfuSsNX4pazvqatzIi2ZhYrjL7v0ECoAhjdeY1wyn/JFWeYcTcQyXVF3KsvMxNZGboKRdGBC08XNxqgXf0IJHgL+ZNaWbr9f4PY0jv7kw6Hc6U9z6vGbYszS9SivuXIZRg4hrP6zb3SUrIkDzWmBqfILk6L0If25DVasgeIjKqV6L4Kjewsv9s X-Bogosity: Ham, tests=bogofilter, spamicity=0.000003, 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 Tue, Dec 10, 2024 at 11:29:52AM +0100, David Hildenbrand wrote: > @@ -1225,27 +1225,53 @@ static void free_pcppages_bulk(struct zone *zone, int count, > spin_unlock_irqrestore(&zone->lock, flags); > } > > -/* Split a multi-block free page into its individual pageblocks. */ > -static void split_large_buddy(struct zone *zone, struct page *page, > - unsigned long pfn, int order, fpi_t fpi) > +static bool pfnblock_migratetype_equal(unsigned long pfn, > + unsigned long end_pfn, int mt) > { > - unsigned long end = pfn + (1 << order); > + VM_WARN_ON_ONCE(!IS_ALIGNED(pfn | end_pfn, pageblock_nr_pages)); > > + while (pfn != end_pfn) { > + struct page *page = pfn_to_page(pfn); > + > + if (unlikely(mt != get_pfnblock_migratetype(page, pfn))) > + return false; > + pfn += pageblock_nr_pages; > + } > + return true; > +} > + > +static void __free_one_page_maybe_split(struct zone *zone, struct page *page, > + unsigned long pfn, int order, fpi_t fpi_flags) > +{ > + const unsigned long end_pfn = pfn + (1 << order); > + int mt = get_pfnblock_migratetype(page, pfn); > + > + VM_WARN_ON_ONCE(order > MAX_PAGE_ORDER); > VM_WARN_ON_ONCE(!IS_ALIGNED(pfn, 1 << order)); > /* Caller removed page from freelist, buddy info cleared! */ > VM_WARN_ON_ONCE(PageBuddy(page)); > > - if (order > pageblock_order) > - order = pageblock_order; > + /* > + * With CONFIG_MEMORY_ISOLATION, we might be freeing MAX_ORDER_NR_PAGES > + * pages that cover pageblocks with different migratetypes; for example > + * only some migratetypes might be MIGRATE_ISOLATE. In that (unlikely) > + * case, fallback to freeing individual pageblocks so they get put > + * onto the right lists. > + */ > + if (!IS_ENABLED(CONFIG_MEMORY_ISOLATION) || > + likely(order <= pageblock_order) || > + pfnblock_migratetype_equal(pfn + pageblock_nr_pages, end_pfn, mt)) { > + __free_one_page(page, pfn, zone, order, mt, fpi_flags); > + return; > + } Ok, if memory isolation is disabled, we know the migratetypes are all matching up, and we can skip the check. However, if memory isolation is enabled, but this isn't move_freepages_block_isolate() calling, we still do the check unnecessarily and slow down the boot, no? Having a function guess the caller is a bit of an anti-pattern. The resulting code is hard to follow, and it's very easy to unintentionally burden some cases with unnecessary stuff. It's better to unshare paths until you don't need conditionals like this. In addition to the fastpath, I think you're also punishing the move_freepages_block_isolate() case. We *know* we just changed the type of one of the buddy's blocks, and yet you're still checking the the range again to decide whether to split. All of this to accomodate hugetlb, which might not even be compiled in? Grrrr. Like you, I was quite surprised to see that GFP_COMP patch in the buddy hotpath splitting *everything* into blocks - on the offchance that somebody might free a hugetlb page. Even if !CONFIG_HUGETLB. Just - what the hell. We shouldn't merge "I only care about my niche usecase at the expense of literally everybody else" patches like this. My vote is NAK on this patch, and a retro-NAK on the GFP_COMP patch. The buddy allocator operates on the assumption of MAX_PAGE_ORDER. If we support folios of a larger size sourced from other allocators, then it should be the folio layer discriminating. So if folio_put() detects this is a massive alloc_contig chunk, then it should take a different freeing path. Do the splitting in there, then pass valid chunks back to the buddy. That would keep the layering cleaner and the cornercase overhead out of the allocator fastpath. It would also avoid the pointless and fragile attempt at freeing a big, non-buddy chunk through the PCP.