* [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset()
@ 2026-06-12 22:06 Ihor Solodrai
2026-06-12 22:37 ` bot+bpf-ci
2026-06-13 1:45 ` Alexei Starovoitov
0 siblings, 2 replies; 5+ messages in thread
From: Ihor Solodrai @ 2026-06-12 22:06 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu
Cc: bpf, kernel-team
This patch is a follow up to recent implementation of
stack_map_get_build_id_offset_sleepable() [1].
stack_map_get_build_id_offset() and its sleepable variant each cached
only the last successfully resolved VMA, with separate bookkeeping in
each function. A run of IPs in a VMA with no usable build ID will
repeat the lookup for every frame: find_vma() in the non-sleepable
path, a VMA lock and a blocking build_id_parse_file() in the sleepable.
Factor the per-call cache into a shared struct stack_map_build_id_cache
with two independent slots [2][3], used by both functions:
* resolved - last VMA that produced a build ID (file, build_id and
range), reused to skip the lookup and the parse;
* unresolved - last VMA with no usable build ID (range only), reused to
emit a raw IP without another lookup or parse.
Keeping the slots independent means a build-ID-less VMA no longer evicts
the last resolved build ID, so a trace alternating between a binary and a
region without one stops re-resolving the binary on every return.
The shared lookup tests [vm_start, vm_end), matching the sleepable path;
the non-sleepable path previously reused a build ID for ip == vm_end
(range_in_vma() is inclusive) and now re-resolves it correctly.
[1] https://lore.kernel.org/bpf/20260525223948.1920986-1-ihor.solodrai@linux.dev/
[2] https://lore.kernel.org/bpf/CAEf4Bza2fRDGhLQoPE-EzM7F34xaEJfi5Exmxb-iWVUN3F06=g@mail.gmail.com/
[3] https://lore.kernel.org/bpf/CAEf4BzZXJFr=1iiVx937ht=4PYQkQHg=eFk810zhMDzXQG3ihw@mail.gmail.com/
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
v1->v2:
* nits in attempt to de-claudify the diff (Alexei)
* factor out local variables in stack_map_build_id_set_from_cache()
to make the conditions more readable
* make comments more concise
* I am ignoring the issue flagged by sashiko, since it's unrelated
to the patch
v1: https://lore.kernel.org/bpf/20260609210749.3448062-1-ihor.solodrai@linux.dev/
---
kernel/bpf/stackmap.c | 175 +++++++++++++++++++++++++++++-------------
1 file changed, 122 insertions(+), 53 deletions(-)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 77ba03216c09..822b5cb188ea 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -175,6 +175,88 @@ static inline void stack_map_build_id_set_valid(struct bpf_stack_build_id *id,
memcpy(id->build_id, build_id, BUILD_ID_SIZE_MAX);
}
+/*
+ * Per stack_map_get_build_id_offset() call cache of the last VMA with a build ID
+ * resolved and the last VMA with no usable build ID.
+ * Adjacent stack frames tend to land in the same VMA or the same backing file,
+ * so caching the last result of each kind lets us skip unnecessary VMA lookups
+ * and build ID parse calls.
+ * A zero vm_end marks a slot as empty.
+ * resolved.build_id aliases the id_offs[] entry.
+ */
+struct stack_map_build_id_cache {
+ struct {
+ struct file *file; /* pinned in the sleepable path; NULL otherwise */
+ const unsigned char *build_id;
+ unsigned long vm_start;
+ unsigned long vm_end;
+ unsigned long vm_pgoff;
+ } resolved;
+ struct {
+ unsigned long vm_start;
+ unsigned long vm_end;
+ } unresolved;
+};
+
+/*
+ * Fill @id from a cached range covering @ip. On a hit this writes @id (resolved
+ * range -> build ID + offset, unresolved range -> raw ip) and returns 0; on a
+ * miss it leaves @id untouched and returns -ENOENT.
+ */
+static int stack_map_build_id_set_from_cache(struct stack_map_build_id_cache *cache,
+ struct bpf_stack_build_id *id, u64 ip)
+{
+ unsigned long vm_start, vm_end, vm_pgoff;
+ u64 offset;
+
+ vm_start = cache->resolved.vm_start;
+ vm_end = cache->resolved.vm_end;
+ if (vm_end && ip >= vm_start && ip < vm_end) {
+ vm_pgoff = cache->resolved.vm_pgoff;
+ offset = stack_map_build_id_offset(vm_pgoff, vm_start, ip);
+ stack_map_build_id_set_valid(id, offset, cache->resolved.build_id);
+ return 0;
+ }
+
+ vm_start = cache->unresolved.vm_start;
+ vm_end = cache->unresolved.vm_end;
+ if (vm_end && ip >= vm_start && ip < vm_end) {
+ stack_map_build_id_set_ip(id);
+ return 0;
+ }
+
+ return -ENOENT;
+}
+
+/*
+ * Record @vma's build ID as the last resolved one. @file is the pinned backing
+ * file in the sleepable path (released when evicted), or NULL otherwise.
+ */
+static void stack_map_build_id_cache_set_resolved(struct stack_map_build_id_cache *cache,
+ struct file *file,
+ const unsigned char *build_id,
+ unsigned long vm_start,
+ unsigned long vm_end,
+ unsigned long vm_pgoff)
+{
+ if (cache->resolved.file)
+ fput(cache->resolved.file);
+ cache->resolved.file = file;
+ cache->resolved.build_id = build_id;
+ cache->resolved.vm_start = vm_start;
+ cache->resolved.vm_end = vm_end;
+ cache->resolved.vm_pgoff = vm_pgoff;
+}
+
+/* Record [vm_start, vm_end) as a range with no usable build ID. */
+static void stack_map_build_id_cache_set_unresolved(struct stack_map_build_id_cache *cache,
+ unsigned long vm_start,
+ unsigned long vm_end)
+{
+ cache->unresolved.vm_start = vm_start;
+ cache->unresolved.vm_end = vm_end;
+}
+
struct stack_map_vma_lock {
struct vm_area_struct *vma;
struct mm_struct *mm;
@@ -244,15 +326,8 @@ static void stack_map_unlock_vma(struct stack_map_vma_lock *lock)
static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *id_offs,
u32 trace_nr)
{
- 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 = {};
+ struct stack_map_vma_lock lock = { .mm = current->mm };
+ struct stack_map_build_id_cache cache = {};
unsigned long vm_pgoff, vm_start, vm_end;
struct vm_area_struct *vma;
struct file *file;
@@ -262,44 +337,40 @@ 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);
+ if (!stack_map_build_id_set_from_cache(&cache, &id_offs[i], ip))
continue;
- }
vma = stack_map_lock_vma(&lock, ip);
if (!vma) {
stack_map_build_id_set_ip(&id_offs[i]);
continue;
}
+
+ vm_pgoff = vma->vm_pgoff;
+ vm_start = vma->vm_start;
+ vm_end = vma->vm_end;
+
if (vma_is_anonymous(vma) || !vma->vm_file) {
- stack_map_build_id_set_ip(&id_offs[i]);
stack_map_unlock_vma(&lock);
+ stack_map_build_id_set_ip(&id_offs[i]);
+ stack_map_build_id_cache_set_unresolved(&cache, vm_start, vm_end);
continue;
}
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.
+ * Same backing file as the last resolved VMA (another mapping
+ * of the same ELF binary): reuse its build_id without re-parsing.
*/
- if (file == cache.file) {
+ if (file == cache.resolved.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;
+ stack_map_build_id_set_valid(&id_offs[i], offset,
+ cache.resolved.build_id);
+ cache.resolved.vm_start = vm_start;
+ cache.resolved.vm_end = vm_end;
+ cache.resolved.vm_pgoff = vm_pgoff;
continue;
}
@@ -310,21 +381,17 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
if (build_id_parse_file(file, id_offs[i].build_id, NULL)) {
stack_map_build_id_set_ip(&id_offs[i]);
fput(file);
+ stack_map_build_id_cache_set_unresolved(&cache, vm_start, vm_end);
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;
+ stack_map_build_id_cache_set_resolved(&cache, file, id_offs[i].build_id,
+ vm_start, vm_end, vm_pgoff);
}
- if (cache.file)
- fput(cache.file);
+ if (cache.resolved.file)
+ fput(cache.resolved.file);
}
/*
@@ -343,8 +410,8 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
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 unsigned char *prev_build_id = NULL;
+ struct stack_map_build_id_cache cache = {};
+ struct vm_area_struct *vma;
int i;
if (may_fault && has_user_ctx) {
@@ -365,27 +432,29 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
for (i = 0; i < trace_nr; i++) {
u64 ip = READ_ONCE(id_offs[i].ip);
- u64 offset;
- if (prev_build_id && range_in_vma(prev_vma, ip, ip)) {
- vma = prev_vma;
- 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);
+ if (!stack_map_build_id_set_from_cache(&cache, &id_offs[i], ip))
continue;
- }
+
vma = find_vma(current->mm, ip);
if (!vma || vma_is_anonymous(vma) ||
fetch_build_id(vma, id_offs[i].build_id, may_fault)) {
- /* per entry fall back to ips */
+ /* per entry fall back to ips; cache build-ID-less range */
stack_map_build_id_set_ip(&id_offs[i]);
- prev_vma = vma;
- prev_build_id = NULL;
+ if (vma)
+ stack_map_build_id_cache_set_unresolved(&cache,
+ vma->vm_start, vma->vm_end);
continue;
}
- 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;
+ /* mmap_lock is held for the whole loop, so the cached VMA
+ * fields stay valid; no file pinning is needed here.
+ */
+ stack_map_build_id_set_valid(&id_offs[i],
+ stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip),
+ id_offs[i].build_id);
+ stack_map_build_id_cache_set_resolved(&cache, NULL, id_offs[i].build_id,
+ vma->vm_start, vma->vm_end,
+ vma->vm_pgoff);
}
bpf_mmap_unlock_mm(work, current->mm);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset()
2026-06-12 22:06 [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset() Ihor Solodrai
@ 2026-06-12 22:37 ` bot+bpf-ci
2026-06-12 23:59 ` Ihor Solodrai
2026-06-13 1:45 ` Alexei Starovoitov
1 sibling, 1 reply; 5+ messages in thread
From: bot+bpf-ci @ 2026-06-12 22:37 UTC (permalink / raw)
To: ihor.solodrai, ast, andrii, daniel, eddyz87, memxor, song
Cc: bpf, kernel-team, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2919 bytes --]
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 77ba03216c09..822b5cb188ea 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -343,8 +410,8 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
> 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 unsigned char *prev_build_id = NULL;
> + struct stack_map_build_id_cache cache = {};
> + struct vm_area_struct *vma;
> int i;
>
> if (may_fault && has_user_ctx) {
[ ... ]
> @@ -365,27 +432,29 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>
> for (i = 0; i < trace_nr; i++) {
> u64 ip = READ_ONCE(id_offs[i].ip);
> - u64 offset;
>
> - if (prev_build_id && range_in_vma(prev_vma, ip, ip)) {
> - vma = prev_vma;
> - 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);
> + if (!stack_map_build_id_set_from_cache(&cache, &id_offs[i], ip))
> continue;
> - }
> +
> vma = find_vma(current->mm, ip);
> if (!vma || vma_is_anonymous(vma) ||
> fetch_build_id(vma, id_offs[i].build_id, may_fault)) {
> - /* per entry fall back to ips */
> + /* per entry fall back to ips; cache build-ID-less range */
> stack_map_build_id_set_ip(&id_offs[i]);
> - prev_vma = vma;
> - prev_build_id = NULL;
> + if (vma)
> + stack_map_build_id_cache_set_unresolved(&cache,
> + vma->vm_start, vma->vm_end);
> continue;
> }
> - 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;
> + /* mmap_lock is held for the whole loop, so the cached VMA
> + * fields stay valid; no file pinning is needed here.
> + */
^^^^
This isn't a bug, but should the comment text start on the line after the
opening /* to match the BPF multi-line comment style?
The other multi-line comments added in this patch follow that convention,
such as in the new stack_map_build_id_cache structure and the helper
functions.
> + stack_map_build_id_set_valid(&id_offs[i],
> + stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip),
> + id_offs[i].build_id);
> + stack_map_build_id_cache_set_resolved(&cache, NULL, id_offs[i].build_id,
> + vma->vm_start, vma->vm_end,
> + vma->vm_pgoff);
> }
> bpf_mmap_unlock_mm(work, current->mm);
> }
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27446103592
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset()
2026-06-12 22:37 ` bot+bpf-ci
@ 2026-06-12 23:59 ` Ihor Solodrai
0 siblings, 0 replies; 5+ messages in thread
From: Ihor Solodrai @ 2026-06-12 23:59 UTC (permalink / raw)
To: bot+bpf-ci, ast, andrii, daniel, eddyz87, memxor, song
Cc: bpf, kernel-team, martin.lau, yonghong.song, clm
On 6/12/26 3:37 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
>> index 77ba03216c09..822b5cb188ea 100644
>> --- a/kernel/bpf/stackmap.c
>> +++ b/kernel/bpf/stackmap.c
>
> [ ... ]
>
>> @@ -343,8 +410,8 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>> 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 unsigned char *prev_build_id = NULL;
>> + struct stack_map_build_id_cache cache = {};
>> + struct vm_area_struct *vma;
>> int i;
>>
>> if (may_fault && has_user_ctx) {
>
> [ ... ]
>
>> @@ -365,27 +432,29 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>>
>> for (i = 0; i < trace_nr; i++) {
>> u64 ip = READ_ONCE(id_offs[i].ip);
>> - u64 offset;
>>
>> - if (prev_build_id && range_in_vma(prev_vma, ip, ip)) {
>> - vma = prev_vma;
>> - 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);
>> + if (!stack_map_build_id_set_from_cache(&cache, &id_offs[i], ip))
>> continue;
>> - }
>> +
>> vma = find_vma(current->mm, ip);
>> if (!vma || vma_is_anonymous(vma) ||
>> fetch_build_id(vma, id_offs[i].build_id, may_fault)) {
>> - /* per entry fall back to ips */
>> + /* per entry fall back to ips; cache build-ID-less range */
>> stack_map_build_id_set_ip(&id_offs[i]);
>> - prev_vma = vma;
>> - prev_build_id = NULL;
>> + if (vma)
>> + stack_map_build_id_cache_set_unresolved(&cache,
>> + vma->vm_start, vma->vm_end);
>> continue;
>> }
>> - 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;
>> + /* mmap_lock is held for the whole loop, so the cached VMA
>> + * fields stay valid; no file pinning is needed here.
>> + */
> ^^^^
>
> This isn't a bug, but should the comment text start on the line after the
> opening /* to match the BPF multi-line comment style?
sigh... Looks like this also was present in v1, and I missed it.
I'll wait in case there is more feedback before making a v3.
>
> The other multi-line comments added in this patch follow that convention,
> such as in the new stack_map_build_id_cache structure and the helper
> functions.
>
>> + stack_map_build_id_set_valid(&id_offs[i],
>> + stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip),
>> + id_offs[i].build_id);
>> + stack_map_build_id_cache_set_resolved(&cache, NULL, id_offs[i].build_id,
>> + vma->vm_start, vma->vm_end,
>> + vma->vm_pgoff);
>> }
>> bpf_mmap_unlock_mm(work, current->mm);
>> }
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27446103592
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset()
2026-06-12 22:06 [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset() Ihor Solodrai
2026-06-12 22:37 ` bot+bpf-ci
@ 2026-06-13 1:45 ` Alexei Starovoitov
2026-06-13 4:38 ` Ihor Solodrai
1 sibling, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2026-06-13 1:45 UTC (permalink / raw)
To: Ihor Solodrai, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Eduard Zingerman, Kumar Kartikeya Dwivedi,
Song Liu
Cc: bpf, kernel-team
On Fri Jun 12, 2026 at 3:06 PM PDT, Ihor Solodrai wrote:
> +static void stack_map_build_id_cache_set_resolved(struct stack_map_build_id_cache *cache,
> + struct file *file,
> + const unsigned char *build_id,
> + unsigned long vm_start,
> + unsigned long vm_end,
> + unsigned long vm_pgoff)
> +{
> + if (cache->resolved.file)
> + fput(cache->resolved.file);
> + cache->resolved.file = file;
> + cache->resolved.build_id = build_id;
> + cache->resolved.vm_start = vm_start;
> + cache->resolved.vm_end = vm_end;
> + cache->resolved.vm_pgoff = vm_pgoff;
> +}
sorry for broken record.
claude's copy paste doesn't bother you?
It could have been:
struct resolved *res = &cache->resolved;
if (res->file)
fput(res->file);
res->file = file;
res->build_id = build_id;
res->vm_start = vm_start;
res->vm_end = vm_end;
res->vm_pgoff = vm_pgoff;
or
cache->resolved = (struct resolved) { file, build_iud, vm_start, emnv_end, vm_pgoff };
and probably other options to make a code easier for humans to read.
You have to explictly tell claude to deduplicate otherwise it keeps
copy pasting left and right.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset()
2026-06-13 1:45 ` Alexei Starovoitov
@ 2026-06-13 4:38 ` Ihor Solodrai
0 siblings, 0 replies; 5+ messages in thread
From: Ihor Solodrai @ 2026-06-13 4:38 UTC (permalink / raw)
To: Alexei Starovoitov, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Eduard Zingerman, Kumar Kartikeya Dwivedi,
Song Liu
Cc: bpf, kernel-team
On 6/12/26 6:45 PM, Alexei Starovoitov wrote:
> On Fri Jun 12, 2026 at 3:06 PM PDT, Ihor Solodrai wrote:
>> +static void stack_map_build_id_cache_set_resolved(struct stack_map_build_id_cache *cache,
>> + struct file *file,
>> + const unsigned char *build_id,
>> + unsigned long vm_start,
>> + unsigned long vm_end,
>> + unsigned long vm_pgoff)
>> +{
>> + if (cache->resolved.file)
>> + fput(cache->resolved.file);
>> + cache->resolved.file = file;
>> + cache->resolved.build_id = build_id;
>> + cache->resolved.vm_start = vm_start;
>> + cache->resolved.vm_end = vm_end;
>> + cache->resolved.vm_pgoff = vm_pgoff;
>> +}
>
> sorry for broken record.
> claude's copy paste doesn't bother you?
tbh, this particular block is fine with me, it's certainly
not as bad as big expressions in the conditions that I rewrote
in the other helper. I guess I'm just used to lengthy qualified
expressions from my java days, so it doesn't bother me much
(maybe it should).
I briefly chatted with Eduard off-list about this, and I agree
with him that what makes this patch awkward is anonymous structs
as cache fields. I'll try something else in v3.
> It could have been:
>
> struct resolved *res = &cache->resolved;
> if (res->file)
> fput(res->file);
> res->file = file;
> res->build_id = build_id;
> res->vm_start = vm_start;
> res->vm_end = vm_end;
> res->vm_pgoff = vm_pgoff;
>
> or
>
> cache->resolved = (struct resolved) { file, build_iud, vm_start, emnv_end, vm_pgoff };
>
> and probably other options to make a code easier for humans to read.
>
> You have to explictly tell claude to deduplicate otherwise it keeps
> copy pasting left and right.
I am painfully aware of claude's quirks, yes :)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-13 4:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 22:06 [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset() Ihor Solodrai
2026-06-12 22:37 ` bot+bpf-ci
2026-06-12 23:59 ` Ihor Solodrai
2026-06-13 1:45 ` Alexei Starovoitov
2026-06-13 4:38 ` Ihor Solodrai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.