Re: [PATCH] mm:slub:avoid wake up kswapd in set_track_prepare

[Harry Yoo <harry.yoo@oracle.com>]

nit: the subject needs a whitespace between subsystems and the header.
"mm: slub: avoid waking up kswapd in set_track_prepare()"?

On Fri, Aug 01, 2025 at 02:51:21PM +0800, yangshiguang1011@163.com wrote:
> From: yangshiguang <yangshiguang1011@163.com>
> 
> set_track_prepare() can incur lock recursion.
> The issue is that it is called from hrtimer_start_range_ns
> holding the per_cpu(hrtimer_bases)[n].lock, but when enabled
> CONFIG_DEBUG_OBJECTS_TIMERS, may wake up kswapd in set_track_prepare,
> and try to hold the per_cpu(hrtimer_bases)[n].lock.
> 
> So avoid waking up kswapd.The oops looks something like:
> 
> BUG: spinlock recursion on CPU#3, swapper/3/0
>  lock: 0xffffff8a4bf29c80, .magic: dead4ead, .owner: swapper/3/0, .owner_cpu: 3
> Hardware name: Qualcomm Technologies, Inc. Popsicle based on SM8850 (DT)
> Call trace:
> spin_bug+0x0
> _raw_spin_lock_irqsave+0x80
> hrtimer_try_to_cancel+0x94
> task_contending+0x10c
> enqueue_dl_entity+0x2a4
> dl_server_start+0x74
> enqueue_task_fair+0x568
> enqueue_task+0xac
> do_activate_task+0x14c
> ttwu_do_activate+0xcc
> try_to_wake_up+0x6c8
> default_wake_function+0x20
> autoremove_wake_function+0x1c
> __wake_up+0xac
> wakeup_kswapd+0x19c
> wake_all_kswapds+0x78
> __alloc_pages_slowpath+0x1ac
> __alloc_pages_noprof+0x298
> stack_depot_save_flags+0x6b0
> stack_depot_save+0x14
> set_track_prepare+0x5c
> ___slab_alloc+0xccc
> __kmalloc_cache_noprof+0x470
> __set_page_owner+0x2bc
> post_alloc_hook[jt]+0x1b8
> prep_new_page+0x28
> get_page_from_freelist+0x1edc
> __alloc_pages_noprof+0x13c
> alloc_slab_page+0x244
> allocate_slab+0x7c
> ___slab_alloc+0x8e8
> kmem_cache_alloc_noprof+0x450
> debug_objects_fill_pool+0x22c
> debug_object_activate+0x40
> enqueue_hrtimer[jt]+0xdc
> hrtimer_start_range_ns+0x5f8
> ...

So some allocations can't even use __GFP_KSWAPD_RECLAIM (e.g., eb799279fb1
("debugobjects: Don't wake up kswapd from fill_pool()")) and
stack_depot_save() does not respect that.

> Signed-off-by: yangshiguang <yangshiguang1011@163.com>
> ---

In general,
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

Let's add Fixes: 5cf909c553e9 ("mm/slub: use stackdepot to save stack
trace in objects") and potentially Cc: stable@vger.kernel.org too?
(It's hard to imagine use both configs in production, though)

>  mm/slub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index cf7c6032d5fd..14e3bac0c6ad 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -969,7 +969,7 @@ static noinline depot_stack_handle_t set_track_prepare(void)
>  	unsigned int nr_entries;
>  
>  	nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 3);
> -	handle = stack_depot_save(entries, nr_entries, GFP_NOWAIT);
> +	handle = stack_depot_save(entries, nr_entries, __GFP_NOWARN);

In the future, perhaps it might be better to propagate gfp flags to
set_track_prepare() and pass it to stack_depot_save()? That's what KASAN
does.

-- 
Cheers,
Harry / Hyeonggon