Re: [PATCH v1 2/6] mm: allow reuse of the lower 16 bit of the page type with an actual type

From: David Hildenbrand
Date: Mon May 27 2024 - 14:49:42 EST


Am 27.05.24 um 17:26 schrieb Matthew Wilcox:
On Mon, May 27, 2024 at 04:14:50PM +0200, David Hildenbrand wrote:
As long as the owner sets a page type first, we can allow reuse of the
lower 18 bit: sufficient to store an offset into a 64 KiB page, which

You say 18 here and 16 below.

Thanks, missed to fixup one instance after going back and forth.


is the maximum base page size in *common* configurations (ignoring the
256 KiB variant). Restrict it to the head page.

We'll use that for zsmalloc next, to set a proper type while still
reusing that field to store information (offset into a base page) that
cannot go elsewhere for now.

Fear of running out of bits for storing the actual type? Actually, we
don't need one bit per type, we could store a single value instead.
Further, we could likely limit PAGE_TYPE_BASE to a single (highest) bit.

We could, but it's more instructions to check.

Maybe, and maybe not sufficient more that we care.

I was thinking of something like the following (probably broken but you should get the idea):

/*
* If the _mapcount is negative, we might store a page type. The
* page_type field corresponds to the most significant byte of the
* _mapcount field. As the mapcount is initialized to -1, we have no
* type as defaults. We have plenty of room to underflow the mapcount
* before we would end up indicating a valid page_type.
*/
#define PAGE_TYPE_BASE 0x80
enum page_type {
PT_buddy = PAGE_TYPE_BASE,
PT_offline,
PT_table,
PT_guard,
PT_hugetlb,
PT_slab,
/* we must forbid page_type == -1 */
PT_unusable = 0xff
};

In struct page:

union {
atomic_t _mapcount;

#if __BYTE_ORDER == __BIG_ENDIAN
struct {
uint16_t page_type_data;
uint8_t page_type_reserved;
uint8_t page_type;
};
#else
struct {
uint8_t page_type;
uint8_t page_type_reserved;
uint16_t page_type_data;
};
#end
};

#define PageType(page, type) (page->page_type == type)

Once could maybe also change page_has_type to simply work on the
fact that the highest bit must be set and any other bit of the type must be clear:

static inline int page_has_type(const struct page *page)
{
return (page->page_type & PAGE_TYPE_BASE) &&
page->page_type != 0xffff;
}

But just some thought.


+++ b/include/linux/page-flags.h
@@ -945,14 +945,18 @@ PAGEFLAG_FALSE(HasHWPoisoned, has_hwpoisoned)
*/
#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
+/*
+ * Reserve 0x0000ffff to catch underflows of _mapcount and
+ * allow owners that set a type to reuse the lower 16 bit for their own
+ * purposes.
+ */
+#define PAGE_MAPCOUNT_RESERVE -65536

I think my original comment was misleading. This should be:

* Reserve 0xffff0000 - 0xfffffffe to catch _mapcount underflow.

Makes sense.


How about we start at the top end and let people extend down? ie:

#define PAGE_TYPE_BASE 0x80000000
#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 PAGE_MAPCOUNT_RESERVE (~0x0000ffff)

Now we can see that we have 9 flags remaining, which should last until
we can have proper memdesc typing.

Also works for me.

Thanks!

--
Thanks,

David / dhildenb