On Wed, Feb 28, 2024 at 03:40:08PM +0800, Baolin Wang wrote: > > > On 2024/2/27 23:17, Oscar Salvador wrote: > > On Tue, Feb 27, 2024 at 09:52:26PM +0800, Baolin Wang wrote: > > > > > --- a/mm/hugetlb.c > > > +++ b/mm/hugetlb.c > > > @@ -2567,13 +2567,38 @@ static struct folio *alloc_surplus_hugetlb_folio(struct hstate *h, > > > } > > > static struct folio *alloc_migrate_hugetlb_folio(struct hstate *h, gfp_t gfp_mask, > > > - int nid, nodemask_t *nmask) > > > + int nid, nodemask_t *nmask, int reason) > > > > I still dislike taking the reason argument this far, and I'd rather have > > this as a boolean specifing whether we allow fallback on other nodes. > > That would mean parsing the reason in alloc_migration_target(). > > If we don't add a new helper e.g: gfp_allow_fallback(), we can just do > > it right there an opencode it with a e.g: macro etc. > > > > Although doing it in an inline helper might help hiding these details. > > > > That's my take on this, but let's see what others have to say. > > Sure. I also expressed my preference for hiding these details within the > hugetlb core as much as possible. > > Muchun, what do you think? Thanks. JFTR: I'm talking about https://lore.kernel.org/linux-mm/ZdxXLTDZn8fD3pEn@localhost.localdomain/ or maybe something cleaner which doesn't need a new helper (we could if we want though): diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index c1ee640d87b1..ddd794e861e6 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -73,6 +73,16 @@ struct resv_map { #endif }; +#define MIGRATE_MEMORY_HOTPLUG 1UL << MR_MEMORY_HOTPLUG +#define MIGRATE_MEMORY_FAILURE 1UL << MR_MEMORY_FAILURE +#define MIGRATE_SYSCALL 1UL << MR_SYSCALL +#define MIGRATE_MBIND 1UL << MR_MEMPOLICY_MBIND +#define HTLB_ALLOW_FALLBACK (MIGRATE_MEMORY_HOTPLUG| \ + MIGRATE_MEMORY_FAILURE| \ + MIGRATE_SYSCALL| \ + MIGRATE_MBIND) + + /* * Region tracking -- allows tracking of reservations and instantiated pages * across the pages in a mapping. diff --git a/mm/hugetlb.c b/mm/hugetlb.c index ed1581b670d4..7e8d6b5885d6 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -2619,7 +2619,7 @@ struct folio *alloc_buddy_hugetlb_folio_with_mpol(struct hstate *h, /* folio migration callback function */ struct folio *alloc_hugetlb_folio_nodemask(struct hstate *h, int preferred_nid, - nodemask_t *nmask, gfp_t gfp_mask) + nodemask_t *nmask, gfp_t gfp_mask, bool allow_fallback) { spin_lock_irq(&hugetlb_lock); if (available_huge_pages(h)) { @@ -2634,6 +2634,12 @@ struct folio *alloc_hugetlb_folio_nodemask(struct hstate *h, int preferred_nid, } spin_unlock_irq(&hugetlb_lock); + /* + * We cannot fallback to other nodes, as we could break the per-node pool + */ + if (!allow_fallback) + gfp_mask |= GFP_THISNODE; + return alloc_migrate_hugetlb_folio(h, gfp_mask, preferred_nid, nmask); } diff --git a/mm/migrate.c b/mm/migrate.c index cc9f2bcd73b4..c1f1d011629d 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2016,10 +2016,15 @@ struct folio *alloc_migration_target(struct folio *src, unsigned long private) if (folio_test_hugetlb(src)) { struct hstate *h = folio_hstate(src); + bool allow_fallback = false; + + if ((1UL << reason) & HTLB_ALLOW_FALLBACK) + allow_fallback = true; gfp_mask = htlb_modify_alloc_mask(h, gfp_mask); return alloc_hugetlb_folio_nodemask(h, nid, - mtc->nmask, gfp_mask); + mtc->nmask, gfp_mask, + allow_fallback); } if (folio_test_large(src)) { -- Oscar Salvador SUSE Labs
