| Message ID | 20250214154215.717537-2-ziy@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Make MIGRATE_ISOLATE a standalone bit | expand |
On Fri, Feb 14, 2025 at 10:42:12AM -0500, Zi Yan wrote: > During page isolation, the original migratetype is overwritten, since > MIGRATE_* are enums. Change MIGRATE_ISOLATE to be a standalone bit like > PB_migrate_skip. pageblock bits needs to be word aligned, so expand > the number of pageblock bits from 4 to 8 and make migrate isolate bit 7. > > Signed-off-by: Zi Yan <ziy@nvidia.com> > --- > include/linux/mmzone.h | 18 +++++++++++++----- > include/linux/page-isolation.h | 2 +- > include/linux/pageblock-flags.h | 33 ++++++++++++++++++++++++++++++++- > mm/page_alloc.c | 21 +++++++++++++++++++-- > 4 files changed, 65 insertions(+), 9 deletions(-) > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index 8aecbbb0b685..3c7d3f22ccb2 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -106,14 +106,22 @@ static inline bool migratetype_is_mergeable(int mt) > > extern int page_group_by_mobility_disabled; > > -#define MIGRATETYPE_MASK ((1UL << PB_migratetype_bits) - 1) > +#ifdef CONFIG_MEMORY_ISOLATION > +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1) > +#define MIGRATETYPE_MASK (MIGRATETYPE_NO_ISO_MASK | PB_migrate_isolate_bit) > +#else > +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1) > +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1) > +#endif There is no user of the NO_ISO_MASK until the last patch. Can you please defer introduction until then? > -#define get_pageblock_migratetype(page) \ > - get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK) > +#define get_pageblock_migratetype(page) \ > + get_pfnblock_flags_mask(page, page_to_pfn(page), \ > + MIGRATETYPE_MASK) > > -#define folio_migratetype(folio) \ > - get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \ > +#define folio_migratetype(folio) \ > + get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \ > MIGRATETYPE_MASK) That's a spurious change currently. I assume you tweaked the MIGRATETYPE_MASK parameter during development, but I can't see a functional difference now. > @@ -373,7 +374,13 @@ unsigned long get_pfnblock_flags_mask(const struct page *page, > * racy, are not corrupted. > */ > word = READ_ONCE(bitmap[word_bitidx]); > - return (word >> bitidx) & mask; > + flags = (word >> bitidx) & mask; > + > +#ifdef CONFIG_MEMORY_ISOLATION > + if (flags & PB_migrate_isolate_bit) > + return MIGRATE_ISOLATE; > +#endif > + return flags; > } > > static __always_inline int get_pfnblock_migratetype(const struct page *page, > @@ -397,8 +404,18 @@ void set_pfnblock_flags_mask(struct page *page, unsigned long flags, > unsigned long bitidx, word_bitidx; > unsigned long word; > > +#ifdef CONFIG_MEMORY_ISOLATION > + BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8); > + /* keep other migratetype bits if MIGRATE_ISOLATE is set */ > + if (flags == MIGRATE_ISOLATE) { > + mask &= ~((1UL << PB_migratetype_bits) - 1); > + flags = PB_migrate_isolate_bit; > + } Please change the callers in both cases to pass the appropriate masks of interest instead. That's likely a bit of churn in the allocator code, but adding caller specifics to this function violates abstraction layering rules.
On 14 Feb 2025, at 11:45, Johannes Weiner wrote: > On Fri, Feb 14, 2025 at 10:42:12AM -0500, Zi Yan wrote: >> During page isolation, the original migratetype is overwritten, since >> MIGRATE_* are enums. Change MIGRATE_ISOLATE to be a standalone bit like >> PB_migrate_skip. pageblock bits needs to be word aligned, so expand >> the number of pageblock bits from 4 to 8 and make migrate isolate bit 7. >> >> Signed-off-by: Zi Yan <ziy@nvidia.com> >> --- >> include/linux/mmzone.h | 18 +++++++++++++----- >> include/linux/page-isolation.h | 2 +- >> include/linux/pageblock-flags.h | 33 ++++++++++++++++++++++++++++++++- >> mm/page_alloc.c | 21 +++++++++++++++++++-- >> 4 files changed, 65 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h >> index 8aecbbb0b685..3c7d3f22ccb2 100644 >> --- a/include/linux/mmzone.h >> +++ b/include/linux/mmzone.h >> @@ -106,14 +106,22 @@ static inline bool migratetype_is_mergeable(int mt) >> >> extern int page_group_by_mobility_disabled; >> >> -#define MIGRATETYPE_MASK ((1UL << PB_migratetype_bits) - 1) >> +#ifdef CONFIG_MEMORY_ISOLATION >> +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1) >> +#define MIGRATETYPE_MASK (MIGRATETYPE_NO_ISO_MASK | PB_migrate_isolate_bit) >> +#else >> +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1) >> +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1) >> +#endif > > There is no user of the NO_ISO_MASK until the last patch. Can you > please defer introduction until then? Sure. > >> -#define get_pageblock_migratetype(page) \ >> - get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK) >> +#define get_pageblock_migratetype(page) \ >> + get_pfnblock_flags_mask(page, page_to_pfn(page), \ >> + MIGRATETYPE_MASK) >> >> -#define folio_migratetype(folio) \ >> - get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \ >> +#define folio_migratetype(folio) \ >> + get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \ >> MIGRATETYPE_MASK) > > That's a spurious change currently. I assume you tweaked the > MIGRATETYPE_MASK parameter during development, but I can't see a > functional difference now. Right, will remove it. > >> @@ -373,7 +374,13 @@ unsigned long get_pfnblock_flags_mask(const struct page *page, >> * racy, are not corrupted. >> */ >> word = READ_ONCE(bitmap[word_bitidx]); >> - return (word >> bitidx) & mask; >> + flags = (word >> bitidx) & mask; >> + >> +#ifdef CONFIG_MEMORY_ISOLATION >> + if (flags & PB_migrate_isolate_bit) >> + return MIGRATE_ISOLATE; >> +#endif >> + return flags; >> } >> >> static __always_inline int get_pfnblock_migratetype(const struct page *page, >> @@ -397,8 +404,18 @@ void set_pfnblock_flags_mask(struct page *page, unsigned long flags, >> unsigned long bitidx, word_bitidx; >> unsigned long word; >> >> +#ifdef CONFIG_MEMORY_ISOLATION >> + BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8); >> + /* keep other migratetype bits if MIGRATE_ISOLATE is set */ >> + if (flags == MIGRATE_ISOLATE) { >> + mask &= ~((1UL << PB_migratetype_bits) - 1); >> + flags = PB_migrate_isolate_bit; >> + } > > Please change the callers in both cases to pass the appropriate masks > of interest instead. > > That's likely a bit of churn in the allocator code, but adding caller > specifics to this function violates abstraction layering rules. Basically, expose this to the caller. Namely, if it is setting MIGRATE_ISOLATE, it should use the MIGRATETYPE_ISO_ONLY_MASK. Otherwise, use MIGRATETYPE_MASK. Maybe adds two helper functions for setting pageblock isolation and clearing pageblock isolation, like {set,clear}_pageblock_skip()? I thought about it and did not do that due to code churn, but now you ask for it explicitly, I am going to do that. Thank you for the review. Best Regards, Yan, Zi
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 8aecbbb0b685..3c7d3f22ccb2 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -106,14 +106,22 @@ static inline bool migratetype_is_mergeable(int mt) extern int page_group_by_mobility_disabled; -#define MIGRATETYPE_MASK ((1UL << PB_migratetype_bits) - 1) +#ifdef CONFIG_MEMORY_ISOLATION +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1) +#define MIGRATETYPE_MASK (MIGRATETYPE_NO_ISO_MASK | PB_migrate_isolate_bit) +#else +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1) +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1) +#endif -#define get_pageblock_migratetype(page) \ - get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK) +#define get_pageblock_migratetype(page) \ + get_pfnblock_flags_mask(page, page_to_pfn(page), \ + MIGRATETYPE_MASK) -#define folio_migratetype(folio) \ - get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \ +#define folio_migratetype(folio) \ + get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \ MIGRATETYPE_MASK) + struct free_area { struct list_head free_list[MIGRATE_TYPES]; unsigned long nr_free; diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h index 898bb788243b..51797dc39cbc 100644 --- a/include/linux/page-isolation.h +++ b/include/linux/page-isolation.h @@ -5,7 +5,7 @@ #ifdef CONFIG_MEMORY_ISOLATION static inline bool is_migrate_isolate_page(struct page *page) { - return get_pageblock_migratetype(page) == MIGRATE_ISOLATE; + return get_pageblock_isolate(page); } static inline bool is_migrate_isolate(int migratetype) { diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h index fc6b9c87cb0a..d6fe17bf0c9b 100644 --- a/include/linux/pageblock-flags.h +++ b/include/linux/pageblock-flags.h @@ -20,7 +20,10 @@ 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 + PB_migrate_isolate = 7, /* If set the block is isolated */ + /* set it to 7 to make pageblock bit word aligned */ +#endif /* * Assume the bits will always align on a word. If this assumption * changes then get/set pageblock needs updating. @@ -28,6 +31,10 @@ enum pageblock_bits { NR_PAGEBLOCK_BITS }; +#ifdef CONFIG_MEMORY_ISOLATION +#define PB_migrate_isolate_bit BIT(PB_migrate_isolate) +#endif + #if defined(CONFIG_HUGETLB_PAGE) #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE @@ -99,4 +106,28 @@ static inline void set_pageblock_skip(struct page *page) } #endif /* CONFIG_COMPACTION */ +#ifdef CONFIG_MEMORY_ISOLATION +#define get_pageblock_isolate(page) \ + get_pfnblock_flags_mask(page, page_to_pfn(page), \ + (1 << (PB_migrate_isolate))) +#define clear_pageblock_isolate(page) \ + set_pfnblock_flags_mask(page, 0, page_to_pfn(page), \ + (1 << PB_migrate_isolate)) +#define set_pageblock_isolate(page) \ + set_pfnblock_flags_mask(page, (1 << PB_migrate_isolate), \ + page_to_pfn(page), \ + (1 << PB_migrate_isolate)) +#else +static inline bool get_pageblock_isolate(struct page *page) +{ + return false; +} +static inline void clear_pageblock_isolate(struct page *page) +{ +} +static inline void set_pageblock_isolate(struct page *page) +{ +} +#endif /* CONFIG_MEMORY_ISOLATION */ + #endif /* PAGEBLOCK_FLAGS_H */ diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 16dfcf7ade74..f17f4acc38c6 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -362,6 +362,7 @@ unsigned long get_pfnblock_flags_mask(const struct page *page, unsigned long *bitmap; unsigned long bitidx, word_bitidx; unsigned long word; + unsigned long flags; bitmap = get_pageblock_bitmap(page, pfn); bitidx = pfn_to_bitidx(page, pfn); @@ -373,7 +374,13 @@ unsigned long get_pfnblock_flags_mask(const struct page *page, * racy, are not corrupted. */ word = READ_ONCE(bitmap[word_bitidx]); - return (word >> bitidx) & mask; + flags = (word >> bitidx) & mask; + +#ifdef CONFIG_MEMORY_ISOLATION + if (flags & PB_migrate_isolate_bit) + return MIGRATE_ISOLATE; +#endif + return flags; } static __always_inline int get_pfnblock_migratetype(const struct page *page, @@ -397,8 +404,18 @@ void set_pfnblock_flags_mask(struct page *page, unsigned long flags, unsigned long bitidx, word_bitidx; unsigned long word; +#ifdef CONFIG_MEMORY_ISOLATION + BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8); + /* keep other migratetype bits if MIGRATE_ISOLATE is set */ + if (flags == MIGRATE_ISOLATE) { + mask &= ~((1UL << PB_migratetype_bits) - 1); + flags = PB_migrate_isolate_bit; + } +#else BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4); - BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits)); +#endif + /* extra one for MIGRATE_ISOLATE */ + BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits) + 1); bitmap = get_pageblock_bitmap(page, pfn); bitidx = pfn_to_bitidx(page, pfn);
During page isolation, the original migratetype is overwritten, since MIGRATE_* are enums. Change MIGRATE_ISOLATE to be a standalone bit like PB_migrate_skip. pageblock bits needs to be word aligned, so expand the number of pageblock bits from 4 to 8 and make migrate isolate bit 7. Signed-off-by: Zi Yan <ziy@nvidia.com> --- include/linux/mmzone.h | 18 +++++++++++++----- include/linux/page-isolation.h | 2 +- include/linux/pageblock-flags.h | 33 ++++++++++++++++++++++++++++++++- mm/page_alloc.c | 21 +++++++++++++++++++-- 4 files changed, 65 insertions(+), 9 deletions(-)