On 5/22/26 10:16 AM, Andrii Nakryiko wrote:

On Thu, May 21, 2026 at 3:51 PM Ihor Solodrai [off-list ref] wrote:

quoted

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 <redacted>
Signed-off-by: Ihor Solodrai <redacted>
---
 kernel/bpf/stackmap.c | 51 ++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 15 deletions(-)

[...]

quoted

 /*
  * 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 ?

Sure, why not.

quoted

+       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?

No, you're right, it doesn't look necessary.

I think this is AI slop: it took too literally the "no functional
changes" part and essentially copy-pasted a piece of code under
build_id_valid label before continue. My oversight, will remove.

quoted

+                       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