Re: [PATCH v1 5/8] tracing: Allow system call tracepoints to handle page faults

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

On 2024-10-04 00:29, Steven Rostedt wrote:
> On Thu,  3 Oct 2024 11:16:35 -0400
[...[
>> -#define __DO_TRACE(name, args, cond, rcuidle)				\
>> +#define __DO_TRACE(name, args, cond, rcuidle, syscall)			\
>>   	do {								\
>>   		int __maybe_unused __idx = 0;				\
>>   									\
>> @@ -222,8 +224,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>>   			      "Bad RCU usage for tracepoint"))		\
>>   			return;						\
>>   									\
>> -		/* keep srcu and sched-rcu usage consistent */		\
>> -		preempt_disable_notrace();				\
>> +		if (syscall) {						\
>> +			rcu_read_lock_trace();				\
>> +		} else {						\
>> +			/* keep srcu and sched-rcu usage consistent */	\
>> +			preempt_disable_notrace();			\
>> +		}							\
>>   									\
> 
> I'm thinking we just use rcu_read_lock_trace() and get rid of the
> preempt_disable and srcu locks for all tracepoints. Oh crap! I should get
> rid of srcu locking too, as it was only needed for the rcuidle code :-p

How about we do it one step at a time ? First introduce use of the
(lightly tested) rcu_read_lock_trace() (at least in comparison with
preempt disable RCU) only for syscalls, and if this works well,
then eventually consider moving the preempt off users to
rcu_read_lock_trace as well ?

Of course it should all work well, in theory. But considering the
vast number of tracepoints we have in the kernel, I am reluctant
to change too many things at once in that area. We may very well
be bitten by unforeseen corner-cases.

Thanks,

Mathieu

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