linux-kernel - Re: [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3b1524b1-cb3b-4abc-8044-0a9ad30a26cd@suse.cz>
Date: Tue, 27 May 2025 12:11:17 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Zi Yan <ziy@...dia.com>, David Hildenbrand <david@...hat.com>,
 Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Oscar Salvador <osalvador@...e.de>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>,
 "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
 Mel Gorman <mgorman@...hsingularity.net>,
 Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
 Brendan Jackman <jackmanb@...gle.com>, Richard Chang
 <richardycc@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 2/6] mm/page_isolation: make page isolation a
 standalone bit.

On 5/23/25 21:12, Zi Yan wrote:
> During page isolation, the original migratetype is overwritten, since
> MIGRATE_* are enums and stored in pageblock bitmaps. Change
> MIGRATE_ISOLATE to be stored a standalone bit, PB_migrate_isolate, like
> PB_migrate_skip, so that migratetype is not lost during pageblock
> isolation.
> 
> Signed-off-by: Zi Yan <ziy@...dia.com>

<snip>

>  #define MEMORY_OFFLINE	0x1
> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
> index 3acbb271a29a..f2f8540b95ca 100644
> --- a/include/linux/pageblock-flags.h
> +++ b/include/linux/pageblock-flags.h
> @@ -20,7 +20,13 @@ enum pageblock_bits {
>  	PB_migrate_end = PB_migrate + PB_migratetype_bits - 1,
>  			/* 3 bits required for migrate types */
>  	PB_migrate_skip,/* If set the block is skipped by compaction */
> -
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	/*
> +	 * Pageblock isolation is represented with a separate bit, so that
> +	 * the migratetype of a block is not overwritten by isolation.
> +	 */
> +	PB_migrate_isolate, /* If set the block is isolated */
> +#endif
>  	/*
>  	 * Assume the bits will always align on a word. If this assumption
>  	 * changes then get/set pageblock needs updating.
> @@ -32,6 +38,11 @@ enum pageblock_bits {
>  
>  #define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +#define MIGRATETYPE_AND_ISO_MASK \
> +	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
> +#endif

I think if there was:

#else
#define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
#endif

you could avoid some #ifdef code later.

>  #if defined(CONFIG_HUGETLB_PAGE)
>  
>  #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0207164fcaf6..b2c623699461 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -360,8 +360,14 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
>  	unsigned long *bitmap;
>  	unsigned long word_bitidx;
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8);
> +	/* extra one for MIGRATE_ISOLATE */
> +	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits) + 1);

This implicitly assumes MIGRATE_ISOLATE is the last of migratetypes so we
can actually need less PB_migratetype_bits if we stop encoding it within
them anymore, but there's nothing enforcing that (not even as a comment)?

> +#else
>  	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
>  	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
> +#endif
>  	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
>  
>  	bitmap = get_pageblock_bitmap(page, pfn);
> @@ -435,7 +441,20 @@ bool get_pfnblock_bit(const struct page *page, unsigned long pfn,
>  __always_inline enum migratetype
>  get_pfnblock_migratetype(const struct page *page, unsigned long pfn)
>  {
> -	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
> +	unsigned long mask = MIGRATETYPE_MASK;

E.g. with my suggestion above you could use MIGRATETYPE_AND_ISO_MASK here.

> +	unsigned long flags;
> +
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	mask = MIGRATETYPE_AND_ISO_MASK;
> +#endif

And drop this.

> +	flags = __get_pfnblock_flags_mask(page, pfn, mask);
> +
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	if (flags & BIT(PB_migrate_isolate))
> +		return MIGRATE_ISOLATE;
> +#endif
> +	return flags & MIGRATETYPE_MASK;
>  }
>  
>  /**
> @@ -513,12 +532,22 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
>  __always_inline void set_pageblock_migratetype(struct page *page,
>  					       enum migratetype migratetype)
>  {
> +	unsigned long mask = MIGRATETYPE_MASK;
> +
>  	if (unlikely(page_group_by_mobility_disabled &&
>  		     migratetype < MIGRATE_PCPTYPES))
>  		migratetype = MIGRATE_UNMOVABLE;
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	if (migratetype == MIGRATE_ISOLATE) {
> +		set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
> +		return;
> +	}
> +	/* change mask to clear PB_migrate_isolate if it is set */
> +	mask = MIGRATETYPE_AND_ISO_MASK;
> +#endif
>  	__set_pfnblock_flags_mask(page, page_to_pfn(page),
> -				  (unsigned long)migratetype, MIGRATETYPE_MASK);
> +				  (unsigned long)migratetype, mask);

This could just pass MIGRATETYPE_AND_ISO_MASK here.

>  }
>  
>  #ifdef CONFIG_DEBUG_VM