Re: [PATCH v2 2/3] mm/vmalloc: fix vmalloc_dump_obj cross-zone VA lookup

[Ye Liu <ye.liu@linux.dev>]

在 2026/9/22 20:44, Uladzislau Rezki 写道:
> On Mon, Sep 21, 2026 at 09:17:22PM +0800, Ye Liu wrote:
>> From: Ye Liu <liuye@kylinos.cn>
>>
>> vmalloc_dump_obj() searches only one vmap node (addr_to_node(addr)),
>> but a vmalloc allocation may span multiple vmap zones.  The VA is
>> stored in only one node's rb-tree (addr_to_node(va_start)), so an
>> object pointer in a different zone than va_start maps to a different
>> node and the search misses.  This affects any allocation larger than
>> vmap_zone_size (64 KiB) on multi-CPU systems.
>>
>> Iterate all vmap nodes using for_each_vmap_node, like find_vmap_area()
>> does, but with spin_trylock instead of spin_lock as this function can
>> be called from atomic dump contexts (OOM, KASAN, RCU).
>>
>> Signed-off-by: Ye Liu <liuye@kylinos.cn>
>> ---
>>  mm/vmalloc.c | 24 ++++++++++++++++++------
>>  1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index df42d8a6f058..30c610f678dc 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -5278,17 +5278,29 @@ bool vmalloc_dump_obj(void *object)
>>  	unsigned long nr_pages;
>>  
>>  	addr = PAGE_ALIGN_DOWN((unsigned long) object);
>> -	vn = addr_to_node(addr);
>>  
>> -	if (!spin_trylock(&vn->busy.lock))
>> -		return false;
>> +	/*
>> +	 * A vmalloc allocation may span multiple vmap zones, so the
>> +	 * node whose rb-tree holds the VA may differ from the node
>> +	 * the address maps to.  Search all nodes.  Use trylock as
>> +	 * this function can be called from atomic dump contexts.
>> +	 */
>> +	va = NULL;
>> +	for_each_vmap_node(vn) {
>> +		if (!spin_trylock(&vn->busy.lock))
>> +			continue;
>> +
>> +		va = __find_vmap_area(addr, &vn->busy.root);
>> +		if (va && va->vm)
>> +			break;
>>  
>> -	va = __find_vmap_area(addr, &vn->busy.root);
>> -	if (!va || !va->vm) {
>>  		spin_unlock(&vn->busy.lock);
>> -		return false;
>> +		va = NULL;
>>  	}
>>  
>> +	if (!va)
>> +		return false;
>> +
>>  	vm = va->vm;
>>  	addr = (unsigned long) vm->addr;
>>  	caller = vm->caller;
>>
>> -- 
>> 2.25.1
>>
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 89c327a6ce7d..3719dc02dcaf 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2511,7 +2511,22 @@ static void free_unmap_vmap_area(struct vmap_area *va)
>  	free_vmap_area_noflush(va);
>  }
>  
> -struct vmap_area *find_vmap_area(unsigned long addr)
> +static inline int next_vmap_node_id(int i)
> +{
> +	return (i + nr_vmap_nodes - 1) % nr_vmap_nodes;
> +}
> +
> +enum vmap_lock_mode {
> +	VMAP_LOCK,
> +	VMAP_TRYLOCK,
> +};
> +
> +/*
> + * Add a comment here.
> + */
> +static struct vmap_area *
> +find_vmap_area_lock(unsigned long addr, struct vmap_node **locked_vn,
> +		enum vmap_lock_mode mode)
>  {
>  	struct vmap_node *vn;
>  	struct vmap_area *va;
> @@ -2534,16 +2549,40 @@ struct vmap_area *find_vmap_area(unsigned long addr)
>  	 * addr is within 2 or 0 nodes we should do extra work.
>  	 */
>  	i = j = addr_to_node_id(addr);
> +
>  	do {
>  		vn = &vmap_nodes[i];
>  
> -		spin_lock(&vn->busy.lock);
> +		if (mode == VMAP_LOCK) {
> +			spin_lock(&vn->busy.lock);
> +		} else {
> +			if (!spin_trylock(&vn->busy.lock))
> +				continue;
> +		}
> +
>  		va = __find_vmap_area(addr, &vn->busy.root);
> +		if (va) {
> +			*locked_vn = vn;
> +			return va;
> +		}
> +
>  		spin_unlock(&vn->busy.lock);
> +	} while ((i = next_vmap_node_id(i)) != j);
>  
> -		if (va)
> -			return va;
> -	} while ((i = (i + nr_vmap_nodes - 1) % nr_vmap_nodes) != j);
> +	*locked_vn = NULL;
> +	return NULL;
> +}
> +
> +struct vmap_area *find_vmap_area(unsigned long addr)
> +{
> +	struct vmap_node *vn;
> +	struct vmap_area *va;
> +
> +	va = find_vmap_area_lock(addr, &vn, VMAP_LOCK);
> +	if (va) {
> +		spin_unlock(&vn->busy.lock);
> +		return va;
> +	}
>  
>  	return NULL;
>  }
> 
> 
> and we use the helper in the vmalloc_dump_obj()?

Hi Uladzislau,                                                       
                                                                     
Thanks for the suggestion.  I have adopted the find_vmap_area_lock() 
helper approach in v3.  The helper unifies the cross-node iteration  
logic with a mode parameter for spin_lock and spin_trylock, the      
latter used by vmalloc_dump_obj() for atomic dump contexts.          
                                                                     
find_unlink_vmap_area() is also simplified to use the same helper,   
removing a third copy of the iteration loop.  The helper returns with
the node's busy.lock held, so the caller can unlink_va() under the   
same lock before releasing it.   
                                                                     
Will send v3 shortly.                                                

> 
> --
> Uladzislau Rezki

-- 
Thanks,
Ye Liu