Re: [PATCHv2 bpf-next 05/11] bpf: Disable preemption in bpf_get_stackid

[Leon Hwang <leon.hwang@linux.dev>]

On 2026/7/30 19:38, Jiri Olsa wrote:
> On Wed, Jul 29, 2026 at 06:30:26PM +0800, Leon Hwang wrote:
>> On 29/7/26 16:38, Jiri Olsa wrote:
>>> The get_perf_callchain call needs disabled preemption plus we need
>>> it disabled as long as we access its returned trace entries buffer.
>>>
>>> Note the bpf_get_stackid_pe function is executed already with
>>> preemption disabled.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: d5a3b1f69186 ("bpf: introduce BPF_MAP_TYPE_STACK_TRACE")
>>> Reported-by: Tao Chen <chen.dylane@linux.dev>
>>> Closes: https://lore.kernel.org/bpf/20260206090653.1336687-2-chen.dylane@linux.dev/
>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>>> ---
>>>  kernel/bpf/stackmap.c | 19 +++++++++++++------
>>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
>>> index 43b1e5436047..e48ddb4edd47 100644
>>> --- a/kernel/bpf/stackmap.c
>>> +++ b/kernel/bpf/stackmap.c
>>> @@ -624,30 +624,37 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
>>>  	struct perf_callchain_entry *trace;
>>>  	struct stackid stackid;
>>>  	bool kernel = !user;
>>> +	int err = -EFAULT;
>>>  	u32 max_depth;
>>> -	int err;
>>>  
>>>  	if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
>>>  			       BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
>>>  		return -EINVAL;
>>>  
>>>  	max_depth = stack_map_calculate_max_depth(map->value_size, elem_size, flags);
>>> +
>>> +	preempt_disable();
>>
>> Better to use the guard-style guard(preempt)() in this series.
> 
> we prefer to call stackid_install with preemption enabled


Understand.

I wonder if we could avoid the "goto out".

scoped_guard(preempt) { ... } might work here.

But "goto out" is fine with me as well.

Thanks,
Leon

> 
> jirka
> 
>>
>> Thanks,
>> Leon
>>
>>>  	trace = get_perf_callchain(regs, kernel, user, max_depth,
>>>  				   false, false, 0);
>>>  
>>>  	if (unlikely(!trace))
>>>  		/* couldn't fetch the stack trace */
>>> -		return -EFAULT;
>>> +		goto out;
>>>  
>>>  	err = stackid_fastpath(&stackid, map, trace, flags);
>>>  	if (err != -ENOENT)
>>> -		return err;
>>> +		goto out;
>>>  
>>>  	new_bucket = stackid_new_bucket(&stackid, map);
>>> -	if (!new_bucket)
>>> -		return -ENOMEM;
>>> +	if (new_bucket) {
>>> +		preempt_enable();
>>> +		return stackid_install(&stackid, map, new_bucket, flags);
>>> +	}
>>> +	err = -ENOMEM;
>>>  
>>> -	return stackid_install(&stackid, map, new_bucket, flags);
>>> +out:
>>> +	preempt_enable();
>>> +	return err;
>>>  }
>>>  
>>>  const struct bpf_func_proto bpf_get_stackid_proto = {
>>