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

[Andrii Nakryiko]

On Mon, May 25, 2026 at 3:40 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>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxxxxxx>
> ---
>  kernel/bpf/stackmap.c | 61 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index c53cfd9a67cf..77ba03216c09 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -246,6 +246,14 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
>  {
>         struct mm_struct *mm = current->mm;
>         struct stack_map_vma_lock lock = { .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;
> @@ -254,6 +262,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;
> +               }
> +

Same issue as with patch #1, this is a lost opportunity to avoid
unnecessary work if VMA has no matching build ID. We should do
negative caching (i.e., have build_id set to NULL, but use
vm_start/vm_end for subsequent ip matches) and avoid unnecessary vma
lookups. Let's treat it as optimization and address this in a follow
up patch.

>                 vma = stack_map_lock_vma(&lock, ip);
>                 if (!vma) {
>                         stack_map_build_id_set_ip(&id_offs[i]);
> @@ -265,17 +284,47 @@ 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);
> +
> +               /*
> +                * Same backing file as previous (e.g. different VMAs
> +                * of the same ELF binary). Reuse the cache build_id.
> +                */
> +               if (file == cache.file) {
> +                       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 */
> -               if (build_id_parse_file(file, id_offs[i].build_id, NULL))
> +               if (build_id_parse_file(file, id_offs[i].build_id, NULL)) {
>                         stack_map_build_id_set_ip(&id_offs[i]);
> -               else
> -                       stack_map_build_id_set_valid(&id_offs[i], offset, id_offs[i].build_id);
> -               fput(file);
> +                       fput(file);

we should probably negative-cache this lack of build ID and update
vm_start/vm_end here, in a follow up


> +                       continue;
> +               }
> +
> +               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
>