On Fri, 4 Oct 2024 10:18:59 -0400
Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
> On 2024-10-04 15:26, Steven Rostedt wrote:
> > On Thu, 3 Oct 2024 21:33:16 -0400
> > Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
> >
> >> On 2024-10-04 03:04, Steven Rostedt wrote:
> >>> On Thu, 3 Oct 2024 20:26:29 -0400
> >>> Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> 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
