| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <06e260cf-9b63-4d7c-809c-a9f2cda58939@lucifer.local>
Date: Tue, 1 Jul 2025 09:27:25 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
linux-doc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
virtualization@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
Jonathan Corbet <corbet@....net>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Jerrin Shaji George <jerrin.shaji-george@...adcom.com>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Eugenio Pérez <eperezma@...hat.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
Zi Yan <ziy@...dia.com>, Matthew Brost <matthew.brost@...el.com>,
Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
Ying Huang <ying.huang@...ux.alibaba.com>,
Alistair Popple <apopple@...dia.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Minchan Kim <minchan@...nel.org>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
Brendan Jackman <jackmanb@...gle.com>,
Johannes Weiner <hannes@...xchg.org>, Jason Gunthorpe <jgg@...pe.ca>,
John Hubbard <jhubbard@...dia.com>, Peter Xu <peterx@...hat.com>,
Xu Xin <xu.xin16@....com.cn>,
Chengming Zhou <chengming.zhou@...ux.dev>,
Miaohe Lin <linmiaohe@...wei.com>,
Naoya Horiguchi <nao.horiguchi@...il.com>,
Oscar Salvador <osalvador@...e.de>, Rik van Riel <riel@...riel.com>,
Harry Yoo <harry.yoo@...cle.com>,
Qi Zheng <zhengqi.arch@...edance.com>,
Shakeel Butt <shakeel.butt@...ux.dev>
Subject: Re: [PATCH v1 04/29] mm/page_alloc: let page freeing clear any set
page type
On Tue, Jul 01, 2025 at 10:17:13AM +0200, David Hildenbrand wrote:
> On 30.06.25 17:27, Lorenzo Stoakes wrote:
> > On Mon, Jun 30, 2025 at 02:59:45PM +0200, David Hildenbrand wrote:
> > > Currently, any user of page types must clear that type before freeing
> > > a page back to the buddy, otherwise we'll run into mapcount related
> > > sanity checks (because the page type currently overlays the page
> > > mapcount).
> > >
> > > Let's allow for not clearing the page type by page type users by letting
> > > the buddy handle it instead.
> > >
> > > We'll focus on having a page type set on the first page of a larger
> > > allocation only.
> > >
> > > With this change, we can reliably identify typed folios even though
> > > they might be in the process of getting freed, which will come in handy
> > > in migration code (at least in the transition phase).
> > >
> > > In the future we might want to warn on some page types. Instead of
> > > having an "allow list", let's rather wait until we know about once that
> > > should go on such a "disallow list".
> >
> > Is the idea here to get this to show up on folio dumps or?
>
> As part of the netmem_desc series, there was a discussion about removing the
> mystical PP checks -- page_pool_page_is_pp() in page_alloc.c and replacing
> them by a proper page type check.
>
> In that case, we would probably want to warn in case we get such a netmem
> page unexpectedly freed.
>
> But, that page type does not exist yet in code, so the sanity check must be
> added once introduced.
OK, and I realise that the UINT_MAX thing is a convention for how a reset
page_type looks anyway now.
>
> >
> > >
> > > Reviewed-by: Zi Yan <ziy@...dia.com>
> > > Acked-by: Harry Yoo <harry.yoo@...cle.com>
> > > Signed-off-by: David Hildenbrand <david@...hat.com>
> > > ---
> > > mm/page_alloc.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 858bc17653af9..44e56d31cfeb1 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -1380,6 +1380,9 @@ __always_inline bool free_pages_prepare(struct page *page,
> > > mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1);
> > > page->mapping = NULL;
> > > }
> > > + if (unlikely(page_has_type(page)))
> > > + page->page_type = UINT_MAX;
> >
> > Feels like this could do with a comment!
>
> /* Reset the page_type -> _mapcount to -1 */
Hm this feels like saying 'the reason we set it to -1 is to set it to -1' :P
I'd be fine with something simple like
/* Set page_type to reset value */
But... Can't we just put a #define somewhere here to make life easy? Like:
include/linux/page-flags.h | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 4fe5ee67535b..c2abf66ebbce 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -197,6 +197,8 @@ enum pageflags {
#ifdef CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP
DECLARE_STATIC_KEY_FALSE(hugetlb_optimize_vmemmap_key);
+#define PAGE_TYPE_RESET (UINT_MAX)
+
/*
* Return the real head page struct iff the @page is a fake head page, otherwise
* return the @page itself. See Documentation/mm/vmemmap_dedup.rst.
@@ -986,16 +988,16 @@ static __always_inline void __folio_set_##fname(struct folio *folio) \
{ \
if (folio_test_##fname(folio)) \
return; \
- VM_BUG_ON_FOLIO(data_race(folio->page.page_type) != UINT_MAX, \
- folio); \
+ VM_WARN_ON_FOLIO(data_race(folio->page.page_type) != \
+ PAGE_TYPE_RESET, folio); \
folio->page.page_type = (unsigned int)PGTY_##lname << 24; \
} \
static __always_inline void __folio_clear_##fname(struct folio *folio) \
{ \
- if (folio->page.page_type == UINT_MAX) \
+ if (folio->page.page_type == PAGE_TYPE_RESET) \
return; \
VM_BUG_ON_FOLIO(!folio_test_##fname(folio), folio); \
- folio->page.page_type = UINT_MAX; \
+ folio->page.page_type = PAGE_TYPE_RESET; \
}
#define PAGE_TYPE_OPS(uname, lname, fname) \
@@ -1008,15 +1010,16 @@ static __always_inline void __SetPage##uname(struct page *page) \
{ \
if (Page##uname(page)) \
return; \
- VM_BUG_ON_PAGE(data_race(page->page_type) != UINT_MAX, page); \
+ VM_BUG_ON_PAGE(data_race(page->page_type) != \
+ PAGE_TYPE_RESET, page); \
page->page_type = (unsigned int)PGTY_##lname << 24; \
} \
static __always_inline void __ClearPage##uname(struct page *page) \
{ \
- if (page->page_type == UINT_MAX) \
+ if (page->page_type == PAGE_TYPE_RESET) \
return; \
VM_BUG_ON_PAGE(!Page##uname(page), page); \
- page->page_type = UINT_MAX; \
+ page->page_type = PAGE_TYPE_RESET; \
}
/*
--
2.50.0
>
> --
> Cheers,
>
> David / dhildenb
>
Powered by blists - more mailing lists