Re: [PATCH v2 03/11] sched: Add sched tracepoints for RV task model

[Gabriele Monaco]

On Mon, 2025-02-17 at 12:16 -0500, Steven Rostedt wrote:
> On Mon, 17 Feb 2025 17:49:17 +0100
> Peter Zijlstra <pet...@infradead.org> wrote:
> 
> > On Mon, Feb 17, 2025 at 11:38:44AM -0500, Steven Rostedt wrote:
> > 
> > > > +void __do_trace_set_current_state(int state_value)
> > > > +{
> > > > +       __do_trace_sched_set_state_tp(current, current-
> > > > >__state, state_value);  
> > > 
> > > And this should not be using the internal macros of a trace
> > > point. It should be:
> > > 
> > >   trace_sched_set_state_tp(current, state_value);
> > > 
> > > (I removed the current->__state as mentioned above).  
> > 
> > But the static branch is already in the caller, no point
> > duplicating
> > that.
> 
> Regardless, you should not be using internals of the tracepoints.
> That can
> change at any time and is not reliable (as the kernel test robot
> pointed out).
> 
> It's a static branch, who cares if it's added twice? One is used to
> jump to
> the function, the other is for the tracepoint logic itself.
> 
> There's several places that do this.
> 
> Perhaps, in the future we could create a normal API that will always
> call
> the tracepoint, but until then, let's not use code that wasn't meant
> for
> that purpose.
> 
> -- Steve
> 

Mmh I get your point. We definitely don't want this piece of code to
break whenever something changes in the tracepoint API..

I will revert the change and prepare a V3

Thanks,
Gabriele