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

[David Hildenbrand <david@redhat.com>]

On 17.06.24 23:29, Andrew Morton wrote:
> On Mon, 17 Jun 2024 13:34:21 -0700 Stephen Brennan <stephen.s.brennan@oracle.com> wrote:
> 
>>>>> 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
>>
>> Hi Andrew,
>>
>> I saw that you've merged this into mm-unstable, thank you!
>>
>> Since 46df8e73a4a3 ("mm: free up PG_slab") is part of the current 6.10
>> RC, it would be great if this patch could be part of the 6.10 release so
>> we don't release a kernel missing the PG_slab info.
>>
>> Can you confirm if mm-unstable will get merged in this release cycle? Or
>> else, would it be possible to include it in a branch that will?
> 
> Turns out the patch as sent was based on David's "mm: allow reuse of
> the lower 16 bit of the page type with an actual type", which changed
> the page flags a lot,  I redid this patch thusly:
> 
> --- a/include/linux/page-flags.h~mm-convert-page-type-macros-to-enum
> +++ a/include/linux/page-flags.h
> @@ -944,15 +944,22 @@ PAGEFLAG_FALSE(HasHWPoisoned, has_hwpois
>    * mistaken for a page type value.
>    */
>   
> -#define PAGE_TYPE_BASE	0xf0000000
> -/* Reserve		0x0000007f to catch underflows of _mapcount */
> -#define PAGE_MAPCOUNT_RESERVE	-128
> -#define PG_buddy	0x00000080
> -#define PG_offline	0x00000100
> -#define PG_table	0x00000200
> -#define PG_guard	0x00000400
> -#define PG_hugetlb	0x00000800
> -#define PG_slab		0x00001000
> +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.
> +	 */

As noted, we better maintain the original comment here.

> +	PG_buddy	= 0x00000080,
> +	PG_offline	= 0x00000100,
> +	PG_table	= 0x00000200,
> +	PG_guard	= 0x00000400,
> +	PG_hugetlb	= 0x00000800,
> +	PG_slab		= 0x00001000,
> +
> +	PAGE_TYPE_BASE	= 0xf0000000,
> +	PAGE_MAPCOUNT_RESERVE	= -128,
> +};
>   
>   #define PageType(page, flag)						\
>   	((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE)
> _
>

Apart from that LGTM.


> (please check carefully)
> 
> and David's later "mm: allow reuse of the lower 16 bit of the page type
> with an actual type" becomes
> 
>   include/linux/mm_types.h   |    5 +++++
>   include/linux/page-flags.h |   16 ++++++++--------
>   2 files changed, 13 insertions(+), 8 deletions(-)
> 
> --- a/include/linux/mm_types.h~mm-allow-reuse-of-the-lower-16-bit-of-the-page-type-with-an-actual-type
> +++ a/include/linux/mm_types.h
> @@ -157,6 +157,11 @@ struct page {
>   		 *
>   		 * See page-flags.h for a list of page types which are currently
>   		 * stored here.
> +		 *
> +		 * Owners of typed folios may reuse the lower 16 bit of the
> +		 * head page page_type field after setting the page type,
> +		 * but must reset these 16 bit to -1 before clearing the
> +		 * page type.
>   		 */
>   		unsigned int page_type;
>   
> --- a/include/linux/page-flags.h~mm-allow-reuse-of-the-lower-16-bit-of-the-page-type-with-an-actual-type
> +++ a/include/linux/page-flags.h
> @@ -951,15 +951,15 @@ enum pagetype {
>   	 * allow owners that set a type to reuse the lower 16 bit for their own
>   	 * purposes.
>   	 */
> -	PG_buddy	= 0x00000080,
> -	PG_offline	= 0x00000100,
> -	PG_table	= 0x00000200,
> -	PG_guard	= 0x00000400,
> -	PG_hugetlb	= 0x00000800,
> -	PG_slab		= 0x00001000,
> +	PG_buddy	= 0x40000000,
> +	PG_offline	= 0x20000000,
> +	PG_table	= 0x10000000,
> +	PG_guard	= 0x08000000,
> +	PG_hugetlb	= 0x04008000,
> +	PG_slab		= 0x02000000,
>   
> -	PAGE_TYPE_BASE	= 0xf0000000,
> -	PAGE_MAPCOUNT_RESERVE	= -128,
> +	PAGE_TYPE_BASE	= 0x80000000,
> +	PAGE_MAPCOUNT_RESERVE	=  (~0x0000ffff),
>   };
>   
>   #define PageType(page, flag)						\
> _
> 
> and that patch's fixup becomes
> 
> --- a/include/linux/page-flags.h~mm-allow-reuse-of-the-lower-16-bit-of-the-page-type-with-an-actual-type-fix
> +++ a/include/linux/page-flags.h
> @@ -955,7 +955,7 @@ enum pagetype {
>   	PG_offline	= 0x20000000,
>   	PG_table	= 0x10000000,
>   	PG_guard	= 0x08000000,
> -	PG_hugetlb	= 0x04008000,
> +	PG_hugetlb	= 0x04000000,
>   	PG_slab		= 0x02000000,
>   
>   	PAGE_TYPE_BASE	= 0x80000000,
> _
> 
> and "mm/zsmalloc: use a proper page type" becomes, in part,
> 
> --- a/include/linux/page-flags.h~mm-zsmalloc-use-a-proper-page-type
> +++ a/include/linux/page-flags.h
> @@ -957,6 +957,7 @@ enum pagetype {
>   	PG_guard	= 0x08000000,
>   	PG_hugetlb	= 0x04000000,
>   	PG_slab		= 0x02000000,
> +	PG_zsmalloc	= 0x01000000,
>   
>   	PAGE_TYPE_BASE	= 0x80000000,
>   	PAGE_MAPCOUNT_RESERVE	=  (~0x0000ffff),
> 
> 
> 
> and the end result is identical to yesterday's mm-everything so that's
> all good.

Looks good.

> 
> However I wouldn't want to send the altered version of "mm: convert
> page type macros to enum" into 6.10-rcX because it gets so altered by
> David's mm-unstable changes for the next merge window.  The new version
> of the hotfixes patch won't have had any valid testing on its own.
> 
> So I'll temporarily drop David's "mm: page_type, zsmalloc and
> page_mapcount_reset()" series from mm-unstable.  To permit the new "mm:
> convert page type macros to enum" to get some linux-next exposure.
> David, please remind me to restore that series in a week or so?

I'll be on vacation next week, but will try to think of it. If you want 
a resend based on the new mm-unstable state with that hotfix, let me know.

-- 
Cheers,

David / dhildenb