Re: [PATCH v5 6/8] tracing/bpf-trace: Add support for faultable tracepoints

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2024-07-02 02:02, Masami Hiramatsu (Google) wrote:
> On Wed, 26 Jun 2024 14:59:39 -0400
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> @@ -2443,9 +2443,15 @@ static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *
>>   	if (prog->aux->max_tp_access > btp->writable_size)
>>   		return -EINVAL;
>>   
>> -	return tracepoint_probe_register_prio_flags(tp, (void *)btp->bpf_func,
>> -						    prog, TRACEPOINT_DEFAULT_PRIO,
>> -						    TRACEPOINT_MAY_EXIST);
>> +	if (tp->flags & TRACEPOINT_MAY_FAULT) {
>> +		return tracepoint_probe_register_prio_flags(tp, (void *)btp->bpf_func,
>> +							    prog, TRACEPOINT_DEFAULT_PRIO,
>> +							    TRACEPOINT_MAY_EXIST | TRACEPOINT_MAY_FAULT);
>> +	} else {
>> +		return tracepoint_probe_register_prio_flags(tp, (void *)btp->bpf_func,
>> +							    prog, TRACEPOINT_DEFAULT_PRIO,
>> +							    TRACEPOINT_MAY_EXIST);
>> +	}
> 
> nit: you can also just pass the flag directly,
> 
> 		return tracepoint_probe_register_prio_flags(tp, (void *)btp->bpf_func,
> 							    prog, TRACEPOINT_DEFAULT_PRIO,
> 							    TRACEPOINT_MAY_EXIST | (tp->flags & TRACEPOINT_MAY_FAULT));
> 

I'll do that for both the ftrace and the bpf patches and eliminate this
odd duplication. That's a good idea.

I'll add your Reviewed-by to all the "tracing/*" patches.

Thanks,

Mathieu

> But others looks good to me.
> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Thank you,
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com