2024-06-07 20:32:24

by Stephen Brennan

[permalink] [raw]
Subject: [PATCH v4] mm: convert page type macros to enum

Changing PG_slab from a page flag to a page type in commit 46df8e73a4a3
("mm: free up PG_slab") in has the unintended consequence of removing
the PG_slab constant from kernel debuginfo. The commit does add the
value to the vmcoreinfo note, which allows debuggers to find the value
without hardcoding it. However it's most flexible to continue
representing the constant with an enum. To that end, convert the page
type fields into an enum. Debuggers will now be able to detect that
PG_slab's type has changed from enum pageflags to enum pagetype.

Fixes: 46df8e73a4a3 ("mm: free up PG_slab")

Signed-off-by: Stephen Brennan <[email protected]>
---
v3 -> v4: rename to enum pagetype, avoiding conflict in f2fs.h and matching
the name of enum pageflags
v2 -> v3: rebase on mm-unstable
v1 -> v2: include PAGE_TYPE_BASE and PAGE_MAPCOUNT_RESERVE

include/linux/page-flags.h | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index f04fea86324d9..32722c6e8397b 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -945,20 +945,23 @@ PAGEFLAG_FALSE(HasHWPoisoned, has_hwpoisoned)
* mistaken for a page type value.
*/

-#define PAGE_TYPE_BASE 0x80000000
-/*
- * Reserve 0xffff0000 - 0xfffffffe to catch _mapcount underflows and
- * allow owners that set a type to reuse the lower 16 bit for their own
- * purposes.
- */
-#define PG_buddy 0x40000000
-#define PG_offline 0x20000000
-#define PG_table 0x10000000
-#define PG_guard 0x08000000
-#define PG_hugetlb 0x04000000
-#define PG_slab 0x02000000
-#define PG_zsmalloc 0x01000000
-#define PAGE_MAPCOUNT_RESERVE (~0x0000ffff)
+enum pagetype {
+ /*
+ * Reserve 0xffff0000 - 0xfffffffe to catch _mapcount underflows and
+ * allow owners that set a type to reuse the lower 16 bit for their own
+ * purposes.
+ */
+ PG_buddy = 0x40000000,
+ PG_offline = 0x20000000,
+ PG_table = 0x10000000,
+ PG_guard = 0x08000000,
+ PG_hugetlb = 0x04000000,
+ PG_slab = 0x02000000,
+ PG_zsmalloc = 0x01000000,
+
+ PAGE_TYPE_BASE = 0x80000000,
+ PAGE_MAPCOUNT_RESERVE = ~0x0000ffff,
+};

#define PageType(page, flag) \
((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE)
--
2.43.0



2024-06-08 04:27:45

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v4] mm: convert page type macros to enum

On Fri, 7 Jun 2024 13:29:53 -0700 Stephen Brennan <[email protected]> wrote:

> Changing PG_slab from a page flag to a page type in commit 46df8e73a4a3
> ("mm: free up PG_slab") in has the unintended consequence of removing
> the PG_slab constant from kernel debuginfo. The commit does add the
> value to the vmcoreinfo note, which allows debuggers to find the value
> without hardcoding it. However it's most flexible to continue
> representing the constant with an enum. To that end, convert the page
> type fields into an enum. Debuggers will now be able to detect that
> PG_slab's type has changed from enum pageflags to enum pagetype.
>
> Fixes: 46df8e73a4a3 ("mm: free up PG_slab")

Should we backport this into 6.9.x?

2024-06-10 22:42:09

by Stephen Brennan

[permalink] [raw]
Subject: Re: [PATCH v4] mm: convert page type macros to enum

Andrew Morton <[email protected]> writes:

> On Fri, 7 Jun 2024 13:29:53 -0700 Stephen Brennan <[email protected]> wrote:
>
>> Changing PG_slab from a page flag to a page type in commit 46df8e73a4a3
>> ("mm: free up PG_slab") in has the unintended consequence of removing
>> the PG_slab constant from kernel debuginfo. The commit does add the
>> value to the vmcoreinfo note, which allows debuggers to find the value
>> without hardcoding it. However it's most flexible to continue
>> representing the constant with an enum. To that end, convert the page
>> type fields into an enum. Debuggers will now be able to detect that
>> PG_slab's type has changed from enum pageflags to enum pagetype.
>>
>> Fixes: 46df8e73a4a3 ("mm: free up PG_slab")
>
> Should we backport this into 6.9.x?

Hi Andrew,

Looks like commit 46df8e73a4a3 ("mm: free up PG_slab") is introduced in
the v6.10-rc's, and not backported to 6.9. So PG_slab is still part of
enum pageflags in 6.9. From the perspective of the issue which motivated
this patch, there's no reason to backport.

Backporting could make the other enum pagetype constants available in
6.9, but I'm not sure there are any users who would care for that. I'd
say there's no need.

Thanks,
Stephen

2024-06-11 13:08:33

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH v4] mm: convert page type macros to enum

On 6/7/24 10:29 PM, Stephen Brennan wrote:
> Changing PG_slab from a page flag to a page type in commit 46df8e73a4a3
> ("mm: free up PG_slab") in has the unintended consequence of removing
> the PG_slab constant from kernel debuginfo. The commit does add the
> value to the vmcoreinfo note, which allows debuggers to find the value
> without hardcoding it. However it's most flexible to continue
> representing the constant with an enum. To that end, convert the page
> type fields into an enum. Debuggers will now be able to detect that
> PG_slab's type has changed from enum pageflags to enum pagetype.
>
> Fixes: 46df8e73a4a3 ("mm: free up PG_slab")
>
> Signed-off-by: Stephen Brennan <[email protected]>

Acked-by: Vlastimil Babka <[email protected]>

> ---
> v3 -> v4: rename to enum pagetype, avoiding conflict in f2fs.h and matching
> the name of enum pageflags
> v2 -> v3: rebase on mm-unstable
> v1 -> v2: include PAGE_TYPE_BASE and PAGE_MAPCOUNT_RESERVE
>
> include/linux/page-flags.h | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index f04fea86324d9..32722c6e8397b 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -945,20 +945,23 @@ PAGEFLAG_FALSE(HasHWPoisoned, has_hwpoisoned)
> * mistaken for a page type value.
> */
>
> -#define PAGE_TYPE_BASE 0x80000000
> -/*
> - * Reserve 0xffff0000 - 0xfffffffe to catch _mapcount underflows and
> - * allow owners that set a type to reuse the lower 16 bit for their own
> - * purposes.
> - */
> -#define PG_buddy 0x40000000
> -#define PG_offline 0x20000000
> -#define PG_table 0x10000000
> -#define PG_guard 0x08000000
> -#define PG_hugetlb 0x04000000
> -#define PG_slab 0x02000000
> -#define PG_zsmalloc 0x01000000
> -#define PAGE_MAPCOUNT_RESERVE (~0x0000ffff)
> +enum pagetype {
> + /*
> + * Reserve 0xffff0000 - 0xfffffffe to catch _mapcount underflows and
> + * allow owners that set a type to reuse the lower 16 bit for their own
> + * purposes.
> + */
> + PG_buddy = 0x40000000,
> + PG_offline = 0x20000000,
> + PG_table = 0x10000000,
> + PG_guard = 0x08000000,
> + PG_hugetlb = 0x04000000,
> + PG_slab = 0x02000000,
> + PG_zsmalloc = 0x01000000,
> +
> + PAGE_TYPE_BASE = 0x80000000,
> + PAGE_MAPCOUNT_RESERVE = ~0x0000ffff,
> +};
>
> #define PageType(page, flag) \
> ((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE)