On Fri, 4 Oct 2024 10:18:59 -0400 Mathieu Desnoyers <[email protected]> wrote:
> On 2024-10-04 15:26, Steven Rostedt wrote: > > On Thu, 3 Oct 2024 21:33:16 -0400 > > Mathieu Desnoyers <[email protected]> wrote: > > > >> On 2024-10-04 03:04, Steven Rostedt wrote: > >>> On Thu, 3 Oct 2024 20:26:29 -0400 > >>> Mathieu Desnoyers <[email protected]> wrote: > >>> > >>> > >>>> static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long > >>>> id) > >>>> { > >>>> struct trace_array *tr = data; > >>>> struct trace_event_file *trace_file; > >>>> struct syscall_trace_enter *entry; > >>>> struct syscall_metadata *sys_data; > >>>> struct trace_event_buffer fbuffer; > >>>> unsigned long args[6]; > >>>> int syscall_nr; > >>>> int size; > >>>> > >>>> syscall_nr = trace_get_syscall_nr(current, regs); > >>>> if (syscall_nr < 0 || syscall_nr >= NR_syscalls) > >>>> return; > >>>> > >>>> /* Here we're inside tp handler's rcu_read_lock_sched > >>>> (__DO_TRACE) */ > >>>> trace_file = > >>>> rcu_dereference_sched(tr->enter_syscall_files[syscall_nr]); > >>>> > >>>> ^^^^ this function explicitly states that preempt needs to be disabled by > >>>> tracepoints. > >>> > >>> Ah, I should have known it was the syscall portion. I don't care for this > >>> hidden dependency. I rather add a preempt disable here and not expect it > >>> to > >>> be disabled when called. > >> > >> Which is exactly what this patch is doing. > > > > I was thinking of putting the protection in the function and not the macro. > > > > I'm confused by your comment. The protection is added to the function here: Ah, sorry. I'm the one confused. I was talking about this part: > +#undef DECLARE_EVENT_SYSCALL_CLASS > +#define DECLARE_EVENT_SYSCALL_CLASS(call, proto, args, tstruct, assign, > print) \ > +__DECLARE_EVENT_CLASS(call, PARAMS(proto), PARAMS(args), PARAMS(tstruct), \ > + PARAMS(assign), PARAMS(print)) \ > +static notrace void \ > +trace_event_raw_event_##call(void *__data, proto) \ > +{ \ > + guard(preempt_notrace)(); \ > + do_trace_event_raw_event_##call(__data, args); \ > +} > + But that's for the non-syscall case. This is why I shouldn't review patches just before going to bed :-p -- Steve
