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

[Mathieu Desnoyers]

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