Skip to content

Commit ee2a098

Browse files
namhyungAlexei Starovoitov
authored andcommitted
bpf: Adjust BPF stack helper functions to accommodate skip > 0
Let's say that the caller has storage for num_elem stack frames. Then, the BPF stack helper functions walk the stack for only num_elem frames. This means that if skip > 0, one keeps only 'num_elem - skip' frames. This is because it sets init_nr in the perf_callchain_entry to the end of the buffer to save num_elem entries only. I believe it was because the perf callchain code unwound the stack frames until it reached the global max size (sysctl_perf_event_max_stack). However it now has perf_callchain_entry_ctx.max_stack to limit the iteration locally. This simplifies the code to handle init_nr in the BPF callstack entries and removes the confusion with the perf_event's __PERF_SAMPLE_CALLCHAIN_EARLY which sets init_nr to 0. Also change the comment on bpf_get_stack() in the header file to be more explicit what the return value means. Fixes: c195651 ("bpf: add bpf_get_stack helper") Signed-off-by: Namhyung Kim <namhyung@kernel.org> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Acked-by: Yonghong Song <yhs@fb.com> Link: https://lore.kernel.org/bpf/30a7b5d5-6726-1cc2-eaee-8da2828a9a9c@oracle.com Link: https://lore.kernel.org/bpf/20220314182042.71025-1-namhyung@kernel.org Based-on-patch-by: Eugene Loh <eugene.loh@oracle.com>
1 parent ef07860 commit ee2a098

File tree

2 files changed

+28
-36
lines changed

2 files changed

+28
-36
lines changed

include/uapi/linux/bpf.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3009,8 +3009,8 @@ union bpf_attr {
30093009
*
30103010
* # sysctl kernel.perf_event_max_stack=<new value>
30113011
* Return
3012-
* A non-negative value equal to or less than *size* on success,
3013-
* or a negative error in case of failure.
3012+
* The non-negative copied *buf* length equal to or less than
3013+
* *size* on success, or a negative error in case of failure.
30143014
*
30153015
* long bpf_skb_load_bytes_relative(const void *skb, u32 offset, void *to, u32 len, u32 start_header)
30163016
* Description
@@ -4316,8 +4316,8 @@ union bpf_attr {
43164316
*
43174317
* # sysctl kernel.perf_event_max_stack=<new value>
43184318
* Return
4319-
* A non-negative value equal to or less than *size* on success,
4320-
* or a negative error in case of failure.
4319+
* The non-negative copied *buf* length equal to or less than
4320+
* *size* on success, or a negative error in case of failure.
43214321
*
43224322
* long bpf_load_hdr_opt(struct bpf_sock_ops *skops, void *searchby_res, u32 len, u64 flags)
43234323
* Description

kernel/bpf/stackmap.c

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
176176
}
177177

178178
static struct perf_callchain_entry *
179-
get_callchain_entry_for_task(struct task_struct *task, u32 init_nr)
179+
get_callchain_entry_for_task(struct task_struct *task, u32 max_depth)
180180
{
181181
#ifdef CONFIG_STACKTRACE
182182
struct perf_callchain_entry *entry;
@@ -187,9 +187,8 @@ get_callchain_entry_for_task(struct task_struct *task, u32 init_nr)
187187
if (!entry)
188188
return NULL;
189189

190-
entry->nr = init_nr +
191-
stack_trace_save_tsk(task, (unsigned long *)(entry->ip + init_nr),
192-
sysctl_perf_event_max_stack - init_nr, 0);
190+
entry->nr = stack_trace_save_tsk(task, (unsigned long *)entry->ip,
191+
max_depth, 0);
193192

194193
/* stack_trace_save_tsk() works on unsigned long array, while
195194
* perf_callchain_entry uses u64 array. For 32-bit systems, it is
@@ -201,7 +200,7 @@ get_callchain_entry_for_task(struct task_struct *task, u32 init_nr)
201200
int i;
202201

203202
/* copy data from the end to avoid using extra buffer */
204-
for (i = entry->nr - 1; i >= (int)init_nr; i--)
203+
for (i = entry->nr - 1; i >= 0; i--)
205204
to[i] = (u64)(from[i]);
206205
}
207206

@@ -218,27 +217,19 @@ static long __bpf_get_stackid(struct bpf_map *map,
218217
{
219218
struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
220219
struct stack_map_bucket *bucket, *new_bucket, *old_bucket;
221-
u32 max_depth = map->value_size / stack_map_data_size(map);
222-
/* stack_map_alloc() checks that max_depth <= sysctl_perf_event_max_stack */
223-
u32 init_nr = sysctl_perf_event_max_stack - max_depth;
224220
u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
225221
u32 hash, id, trace_nr, trace_len;
226222
bool user = flags & BPF_F_USER_STACK;
227223
u64 *ips;
228224
bool hash_matches;
229225

230-
/* get_perf_callchain() guarantees that trace->nr >= init_nr
231-
* and trace-nr <= sysctl_perf_event_max_stack, so trace_nr <= max_depth
232-
*/
233-
trace_nr = trace->nr - init_nr;
234-
235-
if (trace_nr <= skip)
226+
if (trace->nr <= skip)
236227
/* skipping more than usable stack trace */
237228
return -EFAULT;
238229

239-
trace_nr -= skip;
230+
trace_nr = trace->nr - skip;
240231
trace_len = trace_nr * sizeof(u64);
241-
ips = trace->ip + skip + init_nr;
232+
ips = trace->ip + skip;
242233
hash = jhash2((u32 *)ips, trace_len / sizeof(u32), 0);
243234
id = hash & (smap->n_buckets - 1);
244235
bucket = READ_ONCE(smap->buckets[id]);
@@ -295,8 +286,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
295286
u64, flags)
296287
{
297288
u32 max_depth = map->value_size / stack_map_data_size(map);
298-
/* stack_map_alloc() checks that max_depth <= sysctl_perf_event_max_stack */
299-
u32 init_nr = sysctl_perf_event_max_stack - max_depth;
289+
u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
300290
bool user = flags & BPF_F_USER_STACK;
301291
struct perf_callchain_entry *trace;
302292
bool kernel = !user;
@@ -305,8 +295,12 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
305295
BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
306296
return -EINVAL;
307297

308-
trace = get_perf_callchain(regs, init_nr, kernel, user,
309-
sysctl_perf_event_max_stack, false, false);
298+
max_depth += skip;
299+
if (max_depth > sysctl_perf_event_max_stack)
300+
max_depth = sysctl_perf_event_max_stack;
301+
302+
trace = get_perf_callchain(regs, 0, kernel, user, max_depth,
303+
false, false);
310304

311305
if (unlikely(!trace))
312306
/* couldn't fetch the stack trace */
@@ -397,7 +391,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
397391
struct perf_callchain_entry *trace_in,
398392
void *buf, u32 size, u64 flags)
399393
{
400-
u32 init_nr, trace_nr, copy_len, elem_size, num_elem;
394+
u32 trace_nr, copy_len, elem_size, num_elem, max_depth;
401395
bool user_build_id = flags & BPF_F_USER_BUILD_ID;
402396
u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
403397
bool user = flags & BPF_F_USER_STACK;
@@ -422,30 +416,28 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
422416
goto err_fault;
423417

424418
num_elem = size / elem_size;
425-
if (sysctl_perf_event_max_stack < num_elem)
426-
init_nr = 0;
427-
else
428-
init_nr = sysctl_perf_event_max_stack - num_elem;
419+
max_depth = num_elem + skip;
420+
if (sysctl_perf_event_max_stack < max_depth)
421+
max_depth = sysctl_perf_event_max_stack;
429422

430423
if (trace_in)
431424
trace = trace_in;
432425
else if (kernel && task)
433-
trace = get_callchain_entry_for_task(task, init_nr);
426+
trace = get_callchain_entry_for_task(task, max_depth);
434427
else
435-
trace = get_perf_callchain(regs, init_nr, kernel, user,
436-
sysctl_perf_event_max_stack,
428+
trace = get_perf_callchain(regs, 0, kernel, user, max_depth,
437429
false, false);
438430
if (unlikely(!trace))
439431
goto err_fault;
440432

441-
trace_nr = trace->nr - init_nr;
442-
if (trace_nr < skip)
433+
if (trace->nr < skip)
443434
goto err_fault;
444435

445-
trace_nr -= skip;
436+
trace_nr = trace->nr - skip;
446437
trace_nr = (trace_nr <= num_elem) ? trace_nr : num_elem;
447438
copy_len = trace_nr * elem_size;
448-
ips = trace->ip + skip + init_nr;
439+
440+
ips = trace->ip + skip;
449441
if (user && user_build_id)
450442
stack_map_get_build_id_offset(buf, ips, trace_nr, user);
451443
else

0 commit comments

Comments
 (0)