Re: [PATCH bpf v3 3/3] bpf: Cache build IDs in sleepable stackmap path

[Mykyta Yatsenko]

On 5/12/26 4:29 AM, Ihor Solodrai wrote:
> Stack traces often contain adjacent IPs from the same VMA or from
> different VMAs backed by the same ELF file. Cache the last successfully
> parsed build ID together with the resolved VMA range and backing file
> so the sleepable build-ID path can avoid repeated VMA locking and file
> parsing in common cases.
> 
> Suggested-by: Mykyta Yatsenko <mykyta.yatsenko5@xxxxxxxxx>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxxxxxx>
> ---
>  kernel/bpf/stackmap.c | 51 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index c1e96df360c3..318ce9ed0dd5 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -226,13 +226,34 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>  		.vma = NULL,
>  		.mm = mm,
>  	};
> -	unsigned long vm_pgoff, vm_start;
> +	struct {
> +		struct file *file;
> +		const char *build_id;
> +		unsigned long vm_start;
> +		unsigned long vm_end;
> +		unsigned long vm_pgoff;
> +	} cache = {};
> +	unsigned long vm_pgoff, vm_start, vm_end;
>  	struct vm_area_struct *vma;
>  	struct file *file;
>  	u64 ip;
>  
>  	for (u32 i = 0; i < trace_nr; i++) {
>  		ip = READ_ONCE(id_offs[i].ip);
> +
> +		/*
> +		 * Range cache fast path: if ip falls within the previously
> +		 * resolved VMA range, reuse the cache build_id without
> +		 * re-acquiring the VMA lock.
> +		 */
> +		if (cache.build_id && ip >= cache.vm_start && ip < cache.vm_end) {
> +			memcpy(id_offs[i].build_id, cache.build_id, BUILD_ID_SIZE_MAX);
> +			vm_start = cache.vm_start;
> +			vm_end = cache.vm_end;
> +			vm_pgoff = cache.vm_pgoff;
> +			goto build_id_valid;
> +		}
> +
>  		vma = stack_map_lock_vma(&lock, ip);
>  		if (!vma || !vma->vm_file) {
>  			stack_map_build_id_set_ip(&id_offs[i]);
> @@ -240,9 +261,22 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>  			continue;
>  		}
>  
> -		file = get_file(vma->vm_file);
> +		file = vma->vm_file;
>  		vm_pgoff = vma->vm_pgoff;
>  		vm_start = vma->vm_start;
> +		vm_end = vma->vm_end;
> +
> +		if (file == cache.file) {
> +			/*
> +			 * Same backing file as previous (e.g. different VMAs
> +			 * of the same ELF binary). Reuse the cache build_id.
> +			 */
> +			memcpy(id_offs[i].build_id, cache.build_id, BUILD_ID_SIZE_MAX);
> +			stack_map_unlock_vma(&lock);
> +			goto build_id_valid;
> +		}
> +
> +		file = get_file(file);
>  		stack_map_unlock_vma(&lock);
>  
>  		/* build_id_parse_file() may block on filesystem reads */
> @@ -251,11 +285,22 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>  			fput(file);
>  			continue;
>  		}
> -		fput(file);
>  
> +		if (cache.file)
> +			fput(cache.file);
> +		cache.file = file;
> +		cache.build_id = id_offs[i].build_id;
> +
> +build_id_valid:
> +		cache.vm_start = vm_start;
> +		cache.vm_end = vm_end;
> +		cache.vm_pgoff = vm_pgoff;

nit: Maybe we can also put 
if (cache.build_id != id_offs[i])
	memcpy(id_offs[i].build_id, cache.build_id, BUILD_ID_SIZE_MAX); 
here? So all writes to the id_offs[i] are at the same place for clearer structure?

Overall it looks correct. Did you try to run it to see if we hit this cache at all?
Acked-by: Mykyta Yatsenko <yatsenko@xxxxxxxx>

>  		id_offs[i].offset = (vm_pgoff << PAGE_SHIFT) + ip - vm_start;
>  		id_offs[i].status = BPF_STACK_BUILD_ID_VALID;
>  	}
> +
> +	if (cache.file)
> +		fput(cache.file);
>  }
>  
>  /*