Re: [PATCH bpf-next v6 3/3] bpf: Cache build IDs in sleepable stackmap path

[Andrii Nakryiko]

On Thu, May 21, 2026 at 3:51 PM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> 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 <yatsenko@xxxxxxxx>
> Acked-by: Mykyta Yatsenko <yatsenko@xxxxxxxx>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxxxxxx>
> ---
>  kernel/bpf/stackmap.c | 52 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 49 insertions(+), 3 deletions(-)
>

LGTM, one nit below

Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 95336c0e8b56..0d641ac39227 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -245,6 +245,14 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>                 .vma = NULL,
>                 .mm = mm,
>         };
> +       struct {
> +               struct file *file;
> +               const unsigned 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 offset;
> @@ -253,6 +261,17 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>         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) {
> +                       offset = stack_map_build_id_offset(cache.vm_pgoff, cache.vm_start, ip);
> +                       stack_map_build_id_set_valid(&id_offs[i], offset, cache.build_id);
> +                       continue;
> +               }
> +
>                 vma = stack_map_lock_vma(&lock, ip);
>                 if (!vma || !vma->vm_file) {
>                         stack_map_build_id_set_ip(&id_offs[i]);
> @@ -260,8 +279,26 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>                         continue;
>                 }
>
> -               file = get_file(vma->vm_file);
> -               offset = stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip);
> +               file = vma->vm_file;
> +               vm_pgoff = vma->vm_pgoff;
> +               vm_start = vma->vm_start;
> +               vm_end = vma->vm_end;
> +               offset = stack_map_build_id_offset(vm_pgoff, vm_start, ip);
> +
> +               if (file == cache.file) {
> +                       /*
> +                        * Same backing file as previous (e.g. different VMAs
> +                        * of the same ELF binary). Reuse the cache build_id.
> +                        */

nit, for fast path comment you put it before if() condition, while
here you put it inside, I'd move this one outside as well

> +                       stack_map_unlock_vma(&lock);
> +                       stack_map_build_id_set_valid(&id_offs[i], offset, cache.build_id);
> +                       cache.vm_start = vm_start;
> +                       cache.vm_end = vm_end;
> +                       cache.vm_pgoff = vm_pgoff;
> +                       continue;
> +               }
> +
> +               file = get_file(file);
>                 stack_map_unlock_vma(&lock);
>
>                 /* build_id_parse_file() may block on filesystem reads */
> @@ -270,10 +307,19 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>                         fput(file);
>                         continue;
>                 }
> -               fput(file);
>
>                 stack_map_build_id_set_valid(&id_offs[i], offset, id_offs[i].build_id);
> +               if (cache.file)
> +                       fput(cache.file);
> +               cache.file = file;
> +               cache.build_id = id_offs[i].build_id;
> +               cache.vm_start = vm_start;
> +               cache.vm_end = vm_end;
> +               cache.vm_pgoff = vm_pgoff;
>         }
> +
> +       if (cache.file)
> +               fput(cache.file);
>  }
>
>  /*
> --
> 2.54.0
>