Re: [PATCH bpf-next v6 1/3] bpf: Factor out stack_map build ID helpers

[Andrii Nakryiko]

On Thu, May 21, 2026 at 3:51 PM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> wrote:
>
> Factor out helpers from stack_map_get_build_id_offset() in
> preparation for adding a sleepable build ID resolution path.
>
> No functional changes.
>
> Acked-by: Mykyta Yatsenko <yatsenko@xxxxxxxx>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxxxxxx>
> ---
>  kernel/bpf/stackmap.c | 51 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 15 deletions(-)
>

[...]

>  /*
>   * Expects all id_offs[i].ip values to be set to correct initial IPs.
>   * They will be subsequently:
> @@ -165,44 +187,43 @@ static int fetch_build_id(struct vm_area_struct *vma, unsigned char *build_id, b
>  static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>                                           u32 trace_nr, bool user, bool may_fault)
>  {
> -       int i;
>         struct mmap_unlock_irq_work *work = NULL;
>         bool irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
> +       bool has_user_ctx = user && current && current->mm;
>         struct vm_area_struct *vma, *prev_vma = NULL;
> -       const char *prev_build_id;
> +       const unsigned char *prev_build_id;

= NULL ?

> +       int i;
>
>         /* If the irq_work is in use, fall back to report ips. Same
>          * fallback is used for kernel stack (!user) on a stackmap with
>          * build_id.
>          */
> -       if (!user || !current || !current->mm || irq_work_busy ||
> -           !mmap_read_trylock(current->mm)) {
> +       if (!has_user_ctx || irq_work_busy || !mmap_read_trylock(current->mm)) {
>                 /* cannot access current->mm, fall back to ips */
> -               for (i = 0; i < trace_nr; i++) {
> -                       id_offs[i].status = BPF_STACK_BUILD_ID_IP;
> -                       memset(id_offs[i].build_id, 0, BUILD_ID_SIZE_MAX);
> -               }
> +               for (i = 0; i < trace_nr; i++)
> +                       stack_map_build_id_set_ip(&id_offs[i]);
>                 return;
>         }
>
>         for (i = 0; i < trace_nr; i++) {
>                 u64 ip = READ_ONCE(id_offs[i].ip);
> +               u64 offset;
>
>                 if (range_in_vma(prev_vma, ip, ip)) {
>                         vma = prev_vma;
> -                       memcpy(id_offs[i].build_id, prev_build_id, BUILD_ID_SIZE_MAX);
> -                       goto build_id_valid;
> +                       offset = stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip);
> +                       stack_map_build_id_set_valid(&id_offs[i], offset, prev_build_id);
> +                       prev_build_id = id_offs[i].build_id;

you are not updating prev_vma, so why updating prev_build_id... it's
confusing because this serves no purpose, yet the code is there, so
whoever is reading this should be asking the question of "what am I
missing". Am I missing something?

> +                       continue;
>                 }
>                 vma = find_vma(current->mm, ip);
>                 if (!vma || fetch_build_id(vma, id_offs[i].build_id, may_fault)) {
>                         /* per entry fall back to ips */
> -                       id_offs[i].status = BPF_STACK_BUILD_ID_IP;
> -                       memset(id_offs[i].build_id, 0, BUILD_ID_SIZE_MAX);
> +                       stack_map_build_id_set_ip(&id_offs[i]);
>                         continue;
>                 }
> -build_id_valid:
> -               id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + ip - vma->vm_start;
> -               id_offs[i].status = BPF_STACK_BUILD_ID_VALID;
> +               offset = stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip);
> +               stack_map_build_id_set_valid(&id_offs[i], offset, id_offs[i].build_id);
>                 prev_vma = vma;
>                 prev_build_id = id_offs[i].build_id;
>         }
> --
> 2.54.0
>