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 6ECCCC83F1B for ; Fri, 11 Jul 2025 12:59:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A11886B00A4; Fri, 11 Jul 2025 08:59:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9E2846B00A5; Fri, 11 Jul 2025 08:59:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8D15D6B00A6; Fri, 11 Jul 2025 08:59:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 7D9476B00A4 for ; Fri, 11 Jul 2025 08:59:46 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 06121110ED8 for ; Fri, 11 Jul 2025 12:59:46 +0000 (UTC) X-FDA: 83651990772.16.4A1A4ED Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf27.hostedemail.com (Postfix) with ESMTP id 92D414000D for ; Fri, 11 Jul 2025 12:59:43 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=VQZgw5oA; spf=pass (imf27.hostedemail.com: domain of luizcap@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=luizcap@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752238783; a=rsa-sha256; cv=none; b=i8xfKBzIFBiNWdOVabFE9ketNUjJtoSPgQ4WUQc7QReImLfwkQ11TIcV4kXwD6BgBPCFj6 oIu3Ghqao33p3QgUL+elTKkUKGTA7P4otEmlcAR3OgfNREOFYJ8LBk5Pnmqci5sDGy64pd Z+WaEherIv2d7VEoxBg1Sv9qXgp2SFE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752238783; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=s+KGefdoj7lli46Ixaj0pts4ezHX60fxoxyKo1M6BYo=; b=SDfUQ7EcEgCb/671nEBhgOQYCik4s/O2/JRXZrUK8nhIZyqkqq+XdVeSx3p9oY2gmTATUF 9c+EApyIjWYscaMh9da87cNlV1LDQ1oSv4IS9/3TH1xmBH8qjK2+eQ1d52D9e7fxDk0rlR 8G6t5tpwWDybgxk7F996UJpccrjtzvc= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=VQZgw5oA; spf=pass (imf27.hostedemail.com: domain of luizcap@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=luizcap@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1752238783; h=from:from: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s+KGefdoj7lli46Ixaj0pts4ezHX60fxoxyKo1M6BYo=; b=VQZgw5oARB8BpuLvFL1DgNiqKUhdaToRMjF6QSpJW2LkS3PkhWGLyOW4OqIfgOA1TltawB QXWqYfNFu8tL4F/dU8HbfEzUf/wW3WEMz7KF8BYQVLh1DlTmN7OojPEhIdn9qnVx7aNI0+ X5rkQKFkwp8h0KobWqR/si0rfM+vL78= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-83-U6dC9jPUOlyI4yKIRw0L6g-1; Fri, 11 Jul 2025 08:59:42 -0400 X-MC-Unique: U6dC9jPUOlyI4yKIRw0L6g-1 X-Mimecast-MFC-AGG-ID: U6dC9jPUOlyI4yKIRw0L6g_1752238780 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-7d099c1779dso290193585a.0 for ; Fri, 11 Jul 2025 05:59:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752238780; x=1752843580; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=s+KGefdoj7lli46Ixaj0pts4ezHX60fxoxyKo1M6BYo=; b=P+SS4jfEDIQ9dlz1xSUlsGp2mTOprxOeBDZB9mYvyUKNRoYwZkXE6PJAXHQifQ5qdr sV4kkzQu59KvZeFidp9Iktklg2ENJy+Fecgc9mZCrF7VGn23/WG1JIA0alAB3dDAv4nO jv1i/8LndXUnw+IiB5zNmrFlGEr0zd15QiCcuchjyx7Efx6m9XMjuY2IEeDTiB0NlmVt fJAt2AZkx+jJjlARUemjbNSxTyHuj0/7pG1x7fXLix0eQKKUX3RDrf0DmGNUqv6R1HfH 5+eU0lVamPB0ixtS35Fj2XYPAIo9Q69/I+Gbb/Ww6jyDuSq5v2tGOE+zZ6WESEwTykQf 2e1g== X-Forwarded-Encrypted: i=1; AJvYcCXH/O777HwoBWuj66mpNU9tCWMaRjpW3F2HvacMSi0ZQ2yX7d4YqhKuDWh85wR2uak3tB+vj3TWQQ==@kvack.org X-Gm-Message-State: AOJu0YwsGlgc67r8rwogvzfQ6aScZgjxKV/ZFu8Csmk25pO/vSb9z9uK m6esXIuw9q1e0XkBCzGOHh9GOOnksCwEnxpqFVqM4UEAY/E3BMshHJqSrlok+W2jOJgR7IoWkNF igHoZA7a3agJ5qGxBcEjl/nuKLmgJducTUwzg4WWwQUxgVGfOJxRL X-Gm-Gg: ASbGncsX+9zJfm+PEnvJMZVz/b3MFygjF/MSS/1kkwrq47cb2xbifp/VZLx7USWBJ77 uJy6E9tj/wtkDxuGhurnzIMwG8Uo/v0ZFhhqLQKIOxjBe/DUsa5RMsKGyfsMQ2WezteBDT1COz2 8vdQ4Y3eUy2jY5ICi5EDKC/Cm6KNkqNQOn49MhEnSmDbdu+NJFlQ06JHS5atUPnCOZHW/W00FGA +xg0k/qrV0QvjsEplVB4CMA2gzJSqHxV9jxZ3/GNUXF/DVAhFjEPA4shPa2hZy/grtvkfkDbB7c 9UPLpTi7LbGy0sea+rn0/IdYUtlX12jNpxziYk4fqaJUHCSEDPUo9ND3xiE3TEYn8+eTD1L7EmA ieHDVWeDL/771zxsl96nDas1r5Go= X-Received: by 2002:a05:620a:3714:b0:7d5:dbdc:ceef with SMTP id af79cd13be357-7ddea60e61dmr529029485a.18.1752238780279; Fri, 11 Jul 2025 05:59:40 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEI6zgWHWGx3tx10r/0vqCenxYD+GmCdSVxwP9RcNCwzDoMhzl5/VmC5ggoF7ay0iR7evJ9Sg== X-Received: by 2002:a05:620a:3714:b0:7d5:dbdc:ceef with SMTP id af79cd13be357-7ddea60e61dmr529024585a.18.1752238779653; Fri, 11 Jul 2025 05:59:39 -0700 (PDT) Received: from [192.168.2.110] (bras-base-aylmpq0104w-grc-59-70-29-229-84.dsl.bell.ca. [70.29.229.84]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-70497d39650sm19481236d6.71.2025.07.11.05.59.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 11 Jul 2025 05:59:39 -0700 (PDT) Message-ID: Date: Fri, 11 Jul 2025 08:59:38 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 2/4] mm/util: introduce snapshot_page() To: David Hildenbrand , willy@infradead.org, akpm@linux-foundation.org Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, shivankg@amd.com, sj@kernel.org References: <88d956248f7528b7998ef00ca8742061839d1036.1751914235.git.luizcap@redhat.com> <40d3d56a-e434-4d3d-ba0e-3f1204edbda5@redhat.com> From: Luiz Capitulino In-Reply-To: <40d3d56a-e434-4d3d-ba0e-3f1204edbda5@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 63cDpWdwNbprnl0EQrdIZ02OfwGw1XOGwxU1OYEr0d0_1752238780 X-Mimecast-Originator: redhat.com Content-Language: en-US, en-CA Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 92D414000D X-Stat-Signature: ohsyzhr6degaceae1ee8p5qe74of6198 X-HE-Tag: 1752238783-599987 X-HE-Meta: U2FsdGVkX1/hAAooZMNPHdFban2srjxE9F8zptxeWC8y562hVkyP631+45sBWC3szKhfm2Dnnpcm17cJ12zDX6i21hEiWoUg4vDETQcTYbOTLURXx7zhLNR9Y5yYEZHLM1NNQPsUf1bH19hLzadbrvghRVc8vhMRrrQfJk1RBU7PlhkiFbDunRPjFW+nJtY1w2/fN7+xFfn8UlMcxTrhn/pVPiw4fRbuL4oDm6imTb4ZVtFHPSA7ZSJk98DhEBIrZ5ZjSUulkO9D0rTBqSIktirloPsVF148MIBPVi8cYyDgP70KFPPtuFIuwNfpcHezaHQuD4o7vlqJN7Xfgdc2+XUiqh4HA0W4pzkbpu1xLt7vcoZFhWQwP04OObfuOmM0Sjv76JG2K6jNIIo4XUx/Va5z5QnR97OdeqW+TpOh5tUerFeUySL+8KuNttjXW1j971J5Y4UZS1muTXFx4SoQb4Wpq1tmR1RMkpKaUXuVHCbrCKu0BV74nUz0wxJ54+wfzmyGFTaaW3xlYppUDnrOUL74tR/yCXUoRyH6xKBFNYkvJSACie0ISRUtAewCB2Da6oIkqnLZoZmiFwkuylI0ytSRdOI4YlIZgtIZ3n4srTP8HzjXfitPqS6N/FB+MzljuqwIC8EgddiXvaPmttDm4GU4j8kFDFcDobX2cMce8qf4SduX5ZgAC+3QvK4RKa3aLKsSCj/TTQ8HFgunA8rL9QFt1nnZx9GSr3vmSth7q1bSGxlCQqPYFqSOrXegPdFlMclGQCMm8w9ybAjAHaaegI8/zSc5tjpL0sp6kaY0iwnXPPMbCx36yuglKa85+m8BA1lBpXplaSBRh7BgZq/ZC2BW0U+2sQZ5RTgCJTleGSjCfhbBCcJ2WvHJv37N9KyA2W/J7KvRnuoVcIcOgNff3sSXKlHxABzFlQsMWRQTY3zJrI45h/8KhVPUX9ZHsVG1Rw3s9ZzmqD6GErhPT9I QsFBtXmX bdJyGQRIL0mo12dOM03F0EufQ2G2rmEzbBp2QlEgpMjsdMzhUrBeinQh4HDH24A9vHTzv6+5QGQ+KhVo7/eWgRMSzczGAMFQqmwxzQ43LbLmUIDD3WE2/pYLeOf1dYt80spLI60CgTejGT2H97JmjiMTY6j5RFoOEY86PNNGz5FaiHqbwOau0EnvkG1SsPd0l0jcJBgglBJiL8CrxNf2jgmpKDEjlpxv/xw+RIy8jhD8+jaTXzXbmjJKrRSDlV7b1oz0PYZK2as72XYapTcXRV7uMKun8s6U2prmutGhoeWE1XHMEz5LEyvoRmgKgm7Nuh0xFgpx7Zh4KrJrru9aXlpkFLSoU0Ry2l9G2gLhiFYw/Lvj4ZfyoEFg7OIvPvsCwWe6Ha12sDZhZtEvKXnVUN2bIL60MIZTHY0S6nexBHW1J6FtEiriChj3PMMHkBImhbSw2LKamKKSCJBnuGVpuThFBx28DqIZeDeQENEzxpzVsY+NRSGrXhLuAkXoUAdfg2TTAElmb7SvPqVBhcSX5zS0B/Nsw5m3ASUJM9ky+iI3ir8LsfEOe7DWLXyWAC5eG5kchBNASImY0PI5wvz0os7z+zyE7QRPE+GE2 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 2025-07-11 07:56, David Hildenbrand wrote: > On 07.07.25 20:50, Luiz Capitulino wrote: >> This commit refactors __dump_page() into snapshot_page(). >> >> snapshot_page() tries to take a faithful snapshot of a page and its >> folio representation. The snapshot is returned in the struct >> page_snapshot parameter along with additional flags that are best >> retrieved at snapshot creation time to reduce race windows. >> >> This function is intended to be used by callers that need a stable >> representation of a struct page and struct folio so that pointers >> or page information doesn't change while working on a page. >> >> The idea and original implementation of snapshot_page() comes from >> Matthew Wilcox with suggestions for improvements from David Hildenbrand. >> All bugs and misconceptions are mine. >> >> Signed-off-by: Luiz Capitulino >> --- >>   include/linux/mm.h | 19 ++++++++++++ >>   mm/debug.c         | 42 +++---------------------- >>   mm/util.c          | 77 ++++++++++++++++++++++++++++++++++++++++++++++ >>   3 files changed, 100 insertions(+), 38 deletions(-) >> >> diff --git a/include/linux/mm.h b/include/linux/mm.h >> index 0ef2ba0c667a..090968c6eebb 100644 >> --- a/include/linux/mm.h >> +++ b/include/linux/mm.h >> @@ -4184,4 +4184,23 @@ static inline bool page_pool_page_is_pp(struct page *page) >>   } >>   #endif >> +#define PAGE_SNAPSHOT_FAITHFUL     (1 << 0) >> +#define PAGE_SNAPSHOT_PG_FREE      (1 << 1) > > Can we call this "BUDDY" instead of FREE > > There are other types of free pages -- in particular free hugetlb folios -- which won't be covered by this check. We really only care about "buddy" pages, which correspond to "free" pages. > > That's what we check and alter expose :) OK, I'll change. >> +#define PAGE_SNAPSHOT_PG_IDLE      (1 << 2) > > +> +struct page_snapshot { >> +    struct folio folio_snapshot; >> +    struct page page_snapshot; >> +    unsigned long pfn; >> +    unsigned long idx; >> +    unsigned long flags; >> +}; >> + >> +static inline bool snapshot_page_is_faithful(const struct page_snapshot *ps) >> +{ >> +    return ps->flags & PAGE_SNAPSHOT_FAITHFUL; >> +} >> + >> +void snapshot_page(struct page_snapshot *ps, const struct page *page); >> + >>   #endif /* _LINUX_MM_H */ >> diff --git a/mm/debug.c b/mm/debug.c >> index 907382257062..7349330ea506 100644 >> --- a/mm/debug.c >> +++ b/mm/debug.c >> @@ -129,47 +129,13 @@ static void __dump_folio(struct folio *folio, struct page *page, >>   static void __dump_page(const struct page *page) >>   { >> -    struct folio *foliop, folio; >> -    struct page precise; >> -    unsigned long head; >> -    unsigned long pfn = page_to_pfn(page); >> -    unsigned long idx, nr_pages = 1; >> -    int loops = 5; >> - >> -again: >> -    memcpy(&precise, page, sizeof(*page)); >> -    head = precise.compound_head; >> -    if ((head & 1) == 0) { >> -        foliop = (struct folio *)&precise; >> -        idx = 0; >> -        if (!folio_test_large(foliop)) >> -            goto dump; >> -        foliop = (struct folio *)page; >> -    } else { >> -        foliop = (struct folio *)(head - 1); >> -        idx = folio_page_idx(foliop, page); >> -    } >> +    struct page_snapshot ps; >> -    if (idx < MAX_FOLIO_NR_PAGES) { >> -        memcpy(&folio, foliop, 2 * sizeof(struct page)); >> -        nr_pages = folio_nr_pages(&folio); >> -        if (nr_pages > 1) >> -            memcpy(&folio.__page_2, &foliop->__page_2, >> -                   sizeof(struct page)); >> -        foliop = &folio; >> -    } >> - >> -    if (idx > nr_pages) { >> -        if (loops-- > 0) >> -            goto again; >> +    snapshot_page(&ps, page); >> +    if (!snapshot_page_is_faithful(&ps)) >>           pr_warn("page does not match folio\n"); >> -        precise.compound_head &= ~1UL; >> -        foliop = (struct folio *)&precise; >> -        idx = 0; >> -    } >> -dump: >> -    __dump_folio(foliop, &precise, pfn, idx); >> +    __dump_folio(&ps.folio_snapshot, &ps.page_snapshot, ps.pfn, ps.idx); >>   } >>   void dump_page(const struct page *page, const char *reason) >> diff --git a/mm/util.c b/mm/util.c >> index 0b270c43d7d1..c38d213be83f 100644 >> --- a/mm/util.c >> +++ b/mm/util.c >> @@ -25,6 +25,7 @@ >>   #include >>   #include >>   #include >> +#include >>   #include >> @@ -1171,3 +1172,79 @@ int compat_vma_mmap_prepare(struct file *file, struct vm_area_struct *vma) >>       return 0; >>   } >>   EXPORT_SYMBOL(compat_vma_mmap_prepare); >> + >> +static void set_flags(struct page_snapshot *ps, const struct folio *folio, >> +              const struct page *page) >> +{ >> +    /* >> +     * Caveats on high order pages: PG_buddy and PG_slab will only be set >> +     * on the head page. >> +     */ > > Talking about slab here is a bit misleading, IIRC they are proper compound pages today. For PG_buddy (which was renamed to PGTY_buddy), we > are dealing with non-compound higher allocations. > > So talking about "head" is also a bit misleading. > > /* >  * Only the first page of a high-order buddy page has PageBuddy() set. >  * So we have to check manually whether this page is part of a high- >  * order buddy page. >  */ I'll use this comment. >> +    if (PageBuddy(page)) >> +        ps->flags |= PAGE_SNAPSHOT_PG_FREE; >> +    else if (page_count(page) == 0 && is_free_buddy_page(page)) >> +        ps->flags |= PAGE_SNAPSHOT_PG_FREE; >> + >> +    if (folio_test_idle(folio)) >> +        ps->flags |= PAGE_SNAPSHOT_PG_IDLE; >> +} >> + >> +/* > > I would suggest writing proper kernel doc (you mostly have that already) > > Something like > > /** >  * snapshot_page() - Create a snapshot of a "struct page" >  * @ps: struct page_snapshot to store the page snapshot >  * @page: the page we want to snapshot >  * >  * Create a snapshot of a page and store its struct page and struct >  * folio ... I'll fix this too. > >> + * Create a snapshot of a page and store its struct page and struct folio >> + * representations in a struct page_snapshot. >> + * >> + * @ps: struct page_snapshot to store the page snapshot >> + * @page: the page we want to snapshot >> + * >> + * Note that creating a faithful snapshot of a page may fail if the page >> + * compound keeps changing (eg. due to folio split). In this case we set >> + * ps->faithful to false and the snapshot will assume that @page refers >> + * to a single page. >> + */ >