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 93510CDB465 for ; Mon, 16 Oct 2023 20:39:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 237C88D00BA; Mon, 16 Oct 2023 16:39:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1E7B88D00B8; Mon, 16 Oct 2023 16:39:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 089368D00BA; Mon, 16 Oct 2023 16:39:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id EDF478D00B8 for ; Mon, 16 Oct 2023 16:39:39 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id C166A1A0BDA for ; Mon, 16 Oct 2023 20:39:39 +0000 (UTC) X-FDA: 81352490478.06.205037D Received: from mail-qk1-f178.google.com (mail-qk1-f178.google.com [209.85.222.178]) by imf14.hostedemail.com (Postfix) with ESMTP id 77EBB100033 for ; Mon, 16 Oct 2023 20:39:37 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=Uq24o3Ad; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf14.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.178 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=1697488777; 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=5gvhAK4y1mW18aa/4bI9UW2L7Ts1L6F0sYJiOtaB/J8=; b=Kxuq9l9TJETTO7OCEZbFuqrU4/XzKkR2mWq7auw3m2ui4c4wypGzQb+jOOIh8ZRI7oEq3T AgOIM3oYO7gLHroH8pWlZ0M2Hek75abK4nsUdoz7/5YRRGUUmhvqazleiUHHbPN0FHupZb oxmi7IARUZqtmfjaaCM21rtxvi6EtvY= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=Uq24o3Ad; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf14.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.178 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697488777; a=rsa-sha256; cv=none; b=u+p+1ZbKC/dlJec/8ozLOpnXppzDiPm5EsIADxJ3JCC9eGCV4w5UQH3U4A1mGC/fnoFG9N tDSzHzrWsXR+jRmUGPFv9PepdDl7uSBc4GBn1nFL+8eJf8Om/Szb9QA9sNYhrOASmoRKLh gsYLxbIXfktSB8cP2oznHniNquxrRMw= Received: by mail-qk1-f178.google.com with SMTP id af79cd13be357-7741c2e76a3so326068985a.1 for ; Mon, 16 Oct 2023 13:39:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1697488776; x=1698093576; 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=5gvhAK4y1mW18aa/4bI9UW2L7Ts1L6F0sYJiOtaB/J8=; b=Uq24o3AdGWPCZPKSMqFpArkhliJf+o2s69+DFyToSzhE/dZFIryTkVcRFqyxbMEDb6 yXL6jvFnLwzcmBidnUNWpjKV6caC1bkIZM0Kw1AZDNlkS/Od8WkWuwgbGC3hikkmcTQN +5xmGjek74WWGTpQD9WUooT9gXO4wyttfvawcqXusOjdKsMiLdHccbTTddGKf4HAh/7K M/tLDRdC0cCC0O3+XJbC0by1cBEte6Sm/JHTaA96EX0AN73FL4ulg83xg1GY/MtU7JIR oT5d1FEa5CpmTSvDfe68dNHwMulEmJ/bZfaX6KxvpW5UKEycTsIHeBUGLZkI0c755DBV 1ilg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697488776; x=1698093576; 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=5gvhAK4y1mW18aa/4bI9UW2L7Ts1L6F0sYJiOtaB/J8=; b=WsrC9xNNF6ti21dn8zHcbTNAwJg2t/eVo3PWdjscM+lgVpKTQCAaN0PmjzqvidJF52 C2MnmhGeHJen7LGHcq0jL0SnYGAXB/aq88yb165bslzoVLoABJYNt7qJ5u5N7dAhSwdx 8nDgsHXGtEbLSlfQBAPL6YfBfOIEC8Yk+5OPJX1kG2yNk3iitQKIcEUS9m6q1thTlc12 ozMQgizKGLOCvbI6wZGpw2dS8Y74En2s88Rt8tc3bqH+Ovdmk1wxR7CqkyC8xsLgjNif T1goVFZNI7wug/4X50uLkkEGTKUWfPbNsTaVzUrwCzvx7bo5+XNF4joY8L3dfrcw8f3Y uoVQ== X-Gm-Message-State: AOJu0YxN068PEJzUC0AAgmO42OD/UoBxlIEAmXyMF7rS5F5yE5IYO8O1 zf7fbTU2YNVjhR1Li1WcJiCQGA== X-Google-Smtp-Source: AGHT+IFa+rOelSG40PYiJRIPR5GpI96EyYgFl6gHHNFjZnQauJ24jCGfD0dX9bRvSRZHSRCbCDJLtw== X-Received: by 2002:ac8:5bd2:0:b0:412:2d6d:932a with SMTP id b18-20020ac85bd2000000b004122d6d932amr505562qtb.56.1697488776376; Mon, 16 Oct 2023 13:39:36 -0700 (PDT) Received: from localhost (2603-7000-0c01-2716-3012-16a2-6bc2-2937.res6.spectrum.com. [2603:7000:c01:2716:3012:16a2:6bc2:2937]) by smtp.gmail.com with ESMTPSA id j13-20020ac8664d000000b004182037f655sm43221qtp.14.2023.10.16.13.39.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Oct 2023 13:39:35 -0700 (PDT) Date: Mon, 16 Oct 2023 16:39:35 -0400 From: Johannes Weiner To: Zi Yan Cc: David Hildenbrand , Vlastimil Babka , Mike Kravetz , Andrew Morton , Mel Gorman , Miaohe Lin , Kefeng Wang , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH V2 0/6] mm: page_alloc: freelist migratetype hygiene Message-ID: <20231016203935.GB1042487@cmpxchg.org> References: <329AB331-DDC1-4074-A85E-AB5CF866CE84@nvidia.com> <20231010211200.GA129823@cmpxchg.org> <43350F6E-E56D-41C8-8441-A96D83B7D7C1@nvidia.com> <20231016143717.GH470544@cmpxchg.org> <5B61B70F-E85D-4E6F-8856-17A90B899F98@nvidia.com> <20231016185113.GI470544@cmpxchg.org> <6F2C52B6-2031-4694-A124-0DFDE9F88E88@nvidia.com> <20231016202629.GA1042487@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231016202629.GA1042487@cmpxchg.org> X-Rspam-User: X-Stat-Signature: mfbxq6wdiz11qau4bxqth53p5rcpc4jk X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 77EBB100033 X-HE-Tag: 1697488777-427276 X-HE-Meta: U2FsdGVkX1/kxw8Ax2Kp47VSwUHz/ym/PTZU1GatN+PsmdyIz/yBWbHOB/3ZcEI03EHoBFCnsYqZ1h2+omjkSNLpB5jdIsxo/88pouDUrrJjKter2o9gwSLoppD5spdmocgLlDwWDSttTxhy5VdYGTXq/tMXTT4WMNBwyJQlqZLp4mq7buO3RijPkQMn3N8qiwVqk9MWhYhxG3xgS4MiHlVzqppb0+3UpdymqKTJH5ZTytdTEEvum6dFCZ8DgC8kxjjWtBAjX4+0uSuOiL4W65zEK+y7it1ysISfTpZyoO8lj3fOO2HZ9TpURZmhfpzUVDwccMTdjcmZi4YDHjG4xdcN/knC7BJjkjn/kMxVhav3rGkC3Ds2eANhDRLi2VR4TZVies1NUdWZbnnXBx1WN9CnnFwTwBmZSYVdZSaa41nlUOHIl3ue1e+0qkOQ/aXlUrlBossCtB3uFuRKyZx5QyEMZsbfVhL9gw27Qrw+SDrmdyU7Z4VQpX9233jLCJWZMqHb5lvj6kXaig4D1y5Lh/nO9Dym/yVZC7vbhy5Zmb0Bgg2pAlxnkOdUDpu0o6ZXjhEsrOllLpnzkX3Q6JUZBvzseUcSPMlaxHCDdN1QrTXMybkuWcmbORX1dnA5oU2bgOnU8Ltr2OOd/HN+FlBoJjUk+sr+u/l69lNAaSFOQ4Xg3WvbugPPO7lem4zXjBH17IGDJaxmRp0mihuyewcM+HmEtg8cYYsLYVDL4HN49EIoDQOiGw3W8TiBsTjCjRRzYDCRSngJRh0MJwi2wRCeZ9JDxwwbAKJjp6HM4tqxIRZw463Rbp7zvrPXPn8k1q61mcbhvJcLHaP+05jRh+a1f86S05c02OdZwqJchCIaj9/4Q3imYiiVCnYqgpobPwAIK/Ou+Henlm2vgxSFMJgrHbWtLINrj5aDid202xI2i54+tOvXJgDyWjBzBvSkGt9qhA/LNX2s34lzHcSxAVg ImqAyywn sOSv04o0EF+ljc3H+mtjU7hg71WXmA+wwxaqW+HD9yR7wQfcl0OZ73eVGwlaJO8LNTJW3octpMa0gWvOIO8Slr5NUOWo7jdeMAcL25bTjSgoWG5epgTCeznX2NFFUqA7K1Lv94k2pbIJ7TYfi/LwvfuGHB93SKJXL14O3Kz85Pw1vFLKHWxt4rBSv8kl5L+BWlbLGg8cNeZf4AyY4BxDqRO8aWC93ZPXCKWIYGScGU8vbzHe8ACsj4WHDEt+poqYuRTgEZDFfcSKiraEyJukPZJe7kGNi+5j5AEg3BDE9pyKlVA+eQjeGYW+SvvF0khdS7rWhDkV0Yx2Hsrlbndw6ro738F8MJVAlOnJE2Z3RMy1m5MKOvVYv3i5Igv1efHFo37hxZtHz3UJO1o3RyNn1xbgXZzHjn8wyiq5qYq/MYiLJBIyJqx1o50hozBG4bYydo8FW 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: On Mon, Oct 16, 2023 at 04:26:30PM -0400, Johannes Weiner wrote: > On Mon, Oct 16, 2023 at 03:49:49PM -0400, Zi Yan wrote: > > On 16 Oct 2023, at 14:51, Johannes Weiner wrote: > > > > > On Mon, Oct 16, 2023 at 11:00:33AM -0400, Zi Yan wrote: > > >> On 16 Oct 2023, at 10:37, Johannes Weiner wrote: > > >> > > >>> On Mon, Oct 16, 2023 at 09:35:34AM -0400, Zi Yan wrote: > > >>>>> The attached patch has all the suggested changes, let me know how it > > >>>>> looks to you. Thanks. > > >>>> > > >>>> The one I sent has free page accounting issues. The attached one fixes them. > > >>> > > >>> Do you still have the warnings? I wonder what went wrong. > > >> > > >> No warnings. But something with the code: > > >> > > >> 1. in your version, split_free_page() is called without changing any pageblock > > >> migratetypes, then split_free_page() is just a no-op, since the page is > > >> just deleted from the free list, then freed via different orders. Buddy allocator > > >> will merge them back. > > > > > > Hm not quite. > > > > > > If it's the tail block of a buddy, I update its type before > > > splitting. The splitting loop looks up the type of each block for > > > sorting it onto freelists. > > > > > > If it's the head block, yes I split it first according to its old > > > type. But then I let it fall through to scanning the block, which will > > > find that buddy, update its type and move it. > > > > That is the issue, since split_free_page() assumes the pageblocks of > > that free page have different types. It basically just free the page > > with different small orders summed up to the original free page order. > > If all pageblocks of the free page have the same migratetype, __free_one_page() > > will merge these small order pages back to the original order free page. > > duh, of course, you're right. Thanks for patiently explaining this. > > > >> 2. in my version, I set pageblock migratetype to new_mt before split_free_page(), > > >> but it causes free page accounting issues, since in the case of head, free pages > > >> are deleted from new_mt when they are in old_mt free list and the accounting > > >> decreases new_mt free page number instead of old_mt one. > > > > > > Right, that makes sense. > > > > > >> Basically, split_free_page() is awkward as it relies on preset migratetypes, > > >> which changes migratetypes without deleting the free pages from the list first. > > >> That is why I came up with the new split_free_page() below. > > > > > > Yeah, the in-between thing is bad. Either it fixes the migratetype > > > before deletion, or it doesn't do the deletion. I'm thinking it would > > > be simpler to move the deletion out instead. > > > > Yes and no. After deletion, a free page no longer has PageBuddy set and > > has buddy_order information cleared. Either we reset PageBuddy and order > > to the deleted free page, or split_free_page() needs to be changed to > > accept pages without the information (basically remove the PageBuddy > > and order check code). > > Good point, that requires extra care. > > It's correct in the code now, but it deserves a comment, especially > because of the "buddy" naming in the new split function. > > > >> Hmm, if CONFIG_ARCH_FORCE_MAX_ORDER can make a buddy have more than one > > >> pageblock and in turn makes an in-use page have more than one pageblock, > > >> we will have problems. Since in isolate_single_pageblock(), an in-use page > > >> can have part of its pageblock set to a different migratetype and be freed, > > >> causing the free page with unmatched migratetypes. We might need to > > >> free pages at pageblock_order if their orders are bigger than pageblock_order. > > > > > > Is this a practical issue? You mentioned that right now only gigantic > > > pages can be larger than a pageblock, and those are freed in order-0 > > > chunks. > > > > Only if the system allocates a page (non hugetlb pages) with >pageblock_order > > and frees it with the same order. I just do not know if such pages exist on > > other arch than x86. Maybe I just think too much. > > Hm, I removed LRU pages from the handling (and added the warning) but > I left in PageMovable(). The only users are z3fold, zsmalloc and > memory ballooning. AFAICS none of them can be bigger than a pageblock. > Let me remove that and add a warning for that case as well. > > This way, we only attempt to migrate hugetlb, where we know the free > path - and get warnings for anything else that's larger than expected. > > This seems like the safest option. On the off chance that there is a > regression, it won't jeopardize anybody's systems, while the warning > provides all the information we need to debug what's going on. This delta on top? diff --git a/mm/page_alloc.c b/mm/page_alloc.c index b5292ad9860c..0da7c61af37e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1628,7 +1628,7 @@ static int move_freepages_block(struct zone *zone, struct page *page, } #ifdef CONFIG_MEMORY_ISOLATION -/* Look for a multi-block buddy that straddles start_pfn */ +/* Look for a buddy that straddles start_pfn */ static unsigned long find_large_buddy(unsigned long start_pfn) { int order = 0; @@ -1652,7 +1652,7 @@ static unsigned long find_large_buddy(unsigned long start_pfn) return start_pfn; } -/* Split a multi-block buddy into its individual pageblocks */ +/* 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) { @@ -1661,6 +1661,9 @@ static void split_large_buddy(struct zone *zone, struct page *page, VM_WARN_ON_ONCE(order < pageblock_order); VM_WARN_ON_ONCE(pfn & (pageblock_nr_pages - 1)); + /* Caller removed page from freelist, buddy info cleared! */ + VM_WARN_ON_ONCE(PageBuddy(page)); + while (pfn != end_pfn) { int mt = get_pfnblock_migratetype(page, pfn); diff --git a/mm/page_isolation.c b/mm/page_isolation.c index b4d53545496d..c8b3c0699683 100644 --- a/mm/page_isolation.c +++ b/mm/page_isolation.c @@ -399,14 +399,8 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags, continue; } - VM_WARN_ON_ONCE_PAGE(PageLRU(page), page); - #if defined CONFIG_COMPACTION || defined CONFIG_CMA - /* - * hugetlb, and movable compound pages can be - * migrated. Otherwise, fail the isolation. - */ - if (PageHuge(page) || __PageMovable(page)) { + if (PageHuge(page)) { struct compact_control cc = { .nr_migratepages = 0, .order = -1, @@ -426,9 +420,19 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags, pfn = head_pfn + nr_pages; continue; - } else + } + + /* + * These pages are movable too, but they're + * not expected to exceed pageblock_order. + * + * Let us know when they do, so we can add + * proper free and split handling for them. + */ + VM_WARN_ON_ONCE_PAGE(PageLRU(page), page); + VM_WARN_ON_ONCE_PAGE(__PageMovable(page), page); #endif - goto failed; + goto failed; } pfn++;