On 1/15/25 03:17, Alexei Starovoitov wrote: > From: Alexei Starovoitov <ast@xxxxxxxxxx> > > Tracing BPF programs execute from tracepoints and kprobes where > running context is unknown, but they need to request additional > memory. The prior workarounds were using pre-allocated memory and > BPF specific freelists to satisfy such allocation requests. > Instead, introduce gfpflags_allow_spinning() condition that signals > to the allocator that running context is unknown. > Then rely on percpu free list of pages to allocate a page. > try_alloc_pages() -> get_page_from_freelist() -> rmqueue() -> > rmqueue_pcplist() will spin_trylock to grab the page from percpu > free list. If it fails (due to re-entrancy or list being empty) > then rmqueue_bulk()/rmqueue_buddy() will attempt to > spin_trylock zone->lock and grab the page from there. > spin_trylock() is not safe in RT when in NMI or in hard IRQ. > Bailout early in such case. > > The support for gfpflags_allow_spinning() mode for free_page and memcg > comes in the next patches. > > This is a first step towards supporting BPF requirements in SLUB > and getting rid of bpf_mem_alloc. > That goal was discussed at LSFMM: https://lwn.net/Articles/974138/ > > Acked-by: Michal Hocko <mhocko@xxxxxxxx> > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx> Acked-by: Vlastimil Babka <vbabka@xxxxxxx> Some nits below: > --- > include/linux/gfp.h | 22 ++++++++++ > mm/internal.h | 1 + > mm/page_alloc.c | 98 +++++++++++++++++++++++++++++++++++++++++++-- > 3 files changed, 118 insertions(+), 3 deletions(-) > > diff --git a/include/linux/gfp.h b/include/linux/gfp.h > index b0fe9f62d15b..b41bb6e01781 100644 > --- a/include/linux/gfp.h > +++ b/include/linux/gfp.h > @@ -39,6 +39,25 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags) > return !!(gfp_flags & __GFP_DIRECT_RECLAIM); > } > > +static inline bool gfpflags_allow_spinning(const gfp_t gfp_flags) > +{ > + /* > + * !__GFP_DIRECT_RECLAIM -> direct claim is not allowed. > + * !__GFP_KSWAPD_RECLAIM -> it's not safe to wake up kswapd. > + * All GFP_* flags including GFP_NOWAIT use one or both flags. > + * try_alloc_pages() is the only API that doesn't specify either flag. > + * > + * This is stronger than GFP_NOWAIT or GFP_ATOMIC because > + * those are guaranteed to never block on a sleeping lock. > + * Here we are enforcing that the allaaction doesn't ever spin allocation > + * on any locks (i.e. only trylocks). There is no highlevel > + * GFP_$FOO flag for this use in try_alloc_pages() as the > + * regular page allocator doesn't fully support this > + * allocation mode. > + */ > + return !(gfp_flags & __GFP_RECLAIM); > +} This function seems unused, guess the following patches will use. > @@ -4509,7 +4517,8 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order, > > might_alloc(gfp_mask); > > - if (should_fail_alloc_page(gfp_mask, order)) > + if (!(*alloc_flags & ALLOC_TRYLOCK) && > + should_fail_alloc_page(gfp_mask, order)) Is it because should_fail_alloc_page() might take some lock or whatnot? Maybe comment? > return false; > > *alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, *alloc_flags); > @@ -7023,3 +7032,86 @@ static bool __free_unaccepted(struct page *page) > } > > #endif /* CONFIG_UNACCEPTED_MEMORY */ > + > +/** > + * try_alloc_pages_noprof - opportunistic reentrant allocation from any context > + * @nid - node to allocate from > + * @order - allocation order size > + * > + * Allocates pages of a given order from the given node. This is safe to > + * call from any context (from atomic, NMI, and also reentrant > + * allocator -> tracepoint -> try_alloc_pages_noprof). > + * Allocation is best effort and to be expected to fail easily so nobody should > + * rely on the success. Failures are not reported via warn_alloc(). > + * > + * Return: allocated page or NULL on failure. > + */ > +struct page *try_alloc_pages_noprof(int nid, unsigned int order) > +{ > + /* > + * Do not specify __GFP_DIRECT_RECLAIM, since direct claim is not allowed. > + * Do not specify __GFP_KSWAPD_RECLAIM either, since wake up of kswapd > + * is not safe in arbitrary context. > + * > + * These two are the conditions for gfpflags_allow_spinning() being true. > + * > + * Specify __GFP_NOWARN since failing try_alloc_pages() is not a reason > + * to warn. Also warn would trigger printk() which is unsafe from > + * various contexts. We cannot use printk_deferred_enter() to mitigate, > + * since the running context is unknown. > + * > + * Specify __GFP_ZERO to make sure that call to kmsan_alloc_page() below > + * is safe in any context. Also zeroing the page is mandatory for > + * BPF use cases. > + * > + * Though __GFP_NOMEMALLOC is not checked in the code path below, > + * specify it here to highlight that try_alloc_pages() > + * doesn't want to deplete reserves. > + */ > + gfp_t alloc_gfp = __GFP_NOWARN | __GFP_ZERO | __GFP_NOMEMALLOC; > + unsigned int alloc_flags = ALLOC_TRYLOCK; > + struct alloc_context ac = { }; > + struct page *page; > + > + /* > + * In RT spin_trylock() may call raw_spin_lock() which is unsafe in NMI. > + * If spin_trylock() is called from hard IRQ the current task may be > + * waiting for one rt_spin_lock, but rt_spin_trylock() will mark the > + * task as the owner of another rt_spin_lock which will confuse PI > + * logic, so return immediately if called form hard IRQ or NMI. > + * > + * Note, irqs_disabled() case is ok. This function can be called > + * from raw_spin_lock_irqsave region. > + */ > + if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq())) > + return NULL; > + if (!pcp_allowed_order(order)) > + return NULL; > + > +#ifdef CONFIG_UNACCEPTED_MEMORY > + /* Bailout, since try_to_accept_memory_one() needs to take a lock */ > + if (has_unaccepted_memory()) > + return NULL; > +#endif > + /* Bailout, since _deferred_grow_zone() needs to take a lock */ > + if (deferred_pages_enabled()) > + return NULL; Is it fine for BPF that things will fail to allocate until all memory is deferred-initialized and accepted? I guess it's easy to teach those places later to evaluate if they can take the lock. > + > + if (nid == NUMA_NO_NODE) > + nid = numa_node_id(); > + > + prepare_alloc_pages(alloc_gfp, order, nid, NULL, &ac, > + &alloc_gfp, &alloc_flags); > + > + /* > + * Best effort allocation from percpu free list. > + * If it's empty attempt to spin_trylock zone->lock. > + */ > + page = get_page_from_freelist(alloc_gfp, order, alloc_flags, &ac); What about set_page_owner() from post_alloc_hook() and it's stackdepot saving. I guess not an issue until try_alloc_pages() gets used later, so just a mental note that it has to be resolved before. Or is it actually safe? > + > + /* Unlike regular alloc_pages() there is no __alloc_pages_slowpath(). */ > + > + trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype); > + kmsan_alloc_page(page, order, alloc_gfp); > + return page; > +}
