As discussed in the linked patch, the there is some inconsistency between "trylock" and "nolock" nomenclature, let's align it. Since "nolock" is used in the public API it seems to have more mindshare so do that. The linked patch did this for the ALLOC_ flag but forgot about FPI_. Link: https://lore.kernel.org/all/20260703-alloc-trylock-v5-1-c87b714e19d3@google.com/ Signed-off-by: Brendan Jackman --- mm/page_alloc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 9c97a86da2b9f..f3f08d0313cfc 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -90,7 +90,7 @@ typedef int __bitwise fpi_t; #define FPI_TO_TAIL ((__force fpi_t)BIT(1)) /* Free the page without taking locks. Rely on trylock only. */ -#define FPI_TRYLOCK ((__force fpi_t)BIT(2)) +#define FPI_NOLOCK ((__force fpi_t)BIT(2)) /* free_pages_prepare() has already been called for page(s) being freed. */ #define FPI_PREPARED ((__force fpi_t)BIT(3)) @@ -1419,7 +1419,7 @@ static __always_inline bool __free_pages_prepare(struct page *page, page_table_check_free(page, order); pgalloc_tag_sub(page, 1 << order); - if (!PageHighMem(page) && !(fpi_flags & FPI_TRYLOCK)) { + if (!PageHighMem(page) && !(fpi_flags & FPI_NOLOCK)) { debug_check_no_locks_freed(page_address(page), PAGE_SIZE << order); debug_check_no_obj_freed(page_address(page), @@ -1558,7 +1558,7 @@ static void free_one_page(struct zone *zone, struct page *page, struct llist_head *llhead; unsigned long flags; - if (unlikely(fpi_flags & FPI_TRYLOCK)) { + if (unlikely(fpi_flags & FPI_NOLOCK)) { if (!spin_trylock_irqsave(&zone->lock, flags)) { add_page_to_zone_llist(zone, page, order); return; @@ -1569,7 +1569,7 @@ static void free_one_page(struct zone *zone, struct page *page, /* The lock succeeded. Process deferred pages. */ llhead = &zone->trylock_free_pages; - if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_TRYLOCK))) { + if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_NOLOCK))) { struct llist_node *llnode; struct page *p, *tmp; @@ -2882,7 +2882,7 @@ static bool free_frozen_page_commit(struct zone *zone, if (pcp->free_count < (batch << CONFIG_PCP_BATCH_SCALE_MAX)) pcp->free_count += (1 << order); - if (unlikely(fpi_flags & FPI_TRYLOCK)) { + if (unlikely(fpi_flags & FPI_NOLOCK)) { /* * Do not attempt to take a zone lock. Let pcp->count get * over high mark temporarily. @@ -2979,7 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order, migratetype = MIGRATE_MOVABLE; } - if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT) + if (unlikely((fpi_flags & FPI_NOLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))) { add_page_to_zone_llist(zone, page, order); return; @@ -3002,7 +3002,7 @@ void free_frozen_pages(struct page *page, unsigned int order) void free_frozen_pages_nolock(struct page *page, unsigned int order) { - __free_frozen_pages(page, order, FPI_TRYLOCK); + __free_frozen_pages(page, order, FPI_NOLOCK); } /* @@ -5410,7 +5410,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order, if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT) && page && unlikely(__memcg_kmem_charge_page(page, gfp, order) != 0)) { __free_frozen_pages(page, order, - alloc_flags & ALLOC_NOLOCK ? FPI_TRYLOCK : 0); + alloc_flags & ALLOC_NOLOCK ? FPI_NOLOCK : 0); page = NULL; } @@ -5533,7 +5533,7 @@ EXPORT_SYMBOL(__free_pages); */ void free_pages_nolock(struct page *page, unsigned int order) { - ___free_pages(page, order, FPI_TRYLOCK); + ___free_pages(page, order, FPI_NOLOCK); } /** -- 2.54.0 These comments describing the page allocator are out of date: - __alloc_pages() is no longer a public API and has no business being described outside of mm/. - The `wait` variable is gone. It may be out of date for other reasons too but this patch is just fixing the issues that stood out. To fix it: - Instead of referring to a specific function, instead to "the page allocator" - Completely drop out-of-date details of that function's internal behaviour, since they were irrelevant anyway. Suggested-by: Zi Yan Link: https://lore.kernel.org/all/DJP11T5V7BDW.2FZZZ8R6LOY4I@nvidia.com/ Signed-off-by: Brendan Jackman --- kernel/cgroup/cpuset.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c index 24ea2d09cdbdb..dfd0f827e3b92 100644 --- a/kernel/cgroup/cpuset.c +++ b/kernel/cgroup/cpuset.c @@ -4193,7 +4193,7 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs) * nearest enclosing hardwalled ancestor cpuset. * * Scanning up parent cpusets requires callback_lock. The - * __alloc_pages() routine only calls here with __GFP_HARDWALL bit + * page allocator only calls here with __GFP_HARDWALL bit * _not_ set if it's a GFP_KERNEL allocation, and all nodes in the * current tasks mems_allowed came up empty on the first pass over * the zonelist. So only GFP_KERNEL allocations, if all nodes in the @@ -4206,11 +4206,8 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs) * come before the __GFP_HARDWALL check, otherwise a dying task * would be blocked on the fast path. * - * The second pass through get_page_from_freelist() doesn't even call - * here for GFP_ATOMIC calls. For those calls, the __alloc_pages() - * variable 'wait' is not set, and the bit ALLOC_CPUSET is not set - * in alloc_flags. That logic and the checks below have the combined - * affect that: + * The second pass through get_page_from_freelist() doesn't even call here for + * GFP_ATOMIC calls. That, and the checks below have the combined affect that: * in_interrupt - any node ok (current task context irrelevant) * GFP_ATOMIC - any node ok * tsk_is_oom_victim - any node ok @@ -4327,8 +4324,8 @@ void cpuset_nodes_allowed(struct cgroup *cgroup, nodemask_t *mask) * should not be possible for the following code to return an * offline node. But if it did, that would be ok, as this routine * is not returning the node where the allocation must be, only - * the node where the search should start. The zonelist passed to - * __alloc_pages() will include all nodes. If the slab allocator + * the node where the search should start. The zonelist used by + * the allocator will include all nodes. If the slab allocator * is passed an offline node, it will fall back to the local node. * See kmem_cache_alloc_node(). */ -- 2.54.0 Update the comment to reflect the recent change to allow flags in gfp_nolock. Reported-by: sashiko-bot@kernel.org Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com Signed-off-by: Brendan Jackman --- mm/page_alloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index f3f08d0313cfc..d53f858e518f7 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -7994,7 +7994,8 @@ struct page *alloc_frozen_pages_nolock_noprof(gfp_t gfp_flags, int nid, unsigned } /** * alloc_pages_nolock - opportunistic reentrant allocation from any context - * @gfp_flags: GFP flags. Only __GFP_ACCOUNT allowed. + * @gfp_flags: GFP flags. Only __GFP_ACCOUNT, plus some flags that get set + * internally regardless (see @gfp_nolock) are allowed. * @nid: node to allocate from * @order: allocation order size * -- 2.54.0 VM_BUG_ON() is out of favour and on the way to removal, since I recently touched this code I am removing this invocation. If this precondition is violated, the system will soon crash anyway. Suggested-by: Zi Yan Link: https://lore.kernel.org/all/7F866265-3F2E-4765-B9D4-9AB898A9C4AC@nvidia.com/ Signed-off-by: Brendan Jackman --- mm/page_alloc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index d53f858e518f7..0db1a7281fc33 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5438,7 +5438,6 @@ struct page *alloc_pages_node_noprof(int nid, gfp_t gfp_mask, unsigned int order if (nid == NUMA_NO_NODE) nid = numa_mem_id(); - VM_BUG_ON(nid < 0 || nid >= MAX_NUMNODES); warn_if_node_offline(nid, gfp_mask); return __alloc_pages_noprof(gfp_mask, order, nid, NULL, ALLOC_DEFAULT); -- 2.54.0