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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <98ff750749a78c50d882f24e89fdc10ab145fa5d.camel@redhat.com>
Date: Tue, 18 Feb 2025 08:06:43 +0100
From: Gabriele Monaco <gmonaco@...hat.com>
To: Steven Rostedt <rostedt@...dmis.org>, Peter Zijlstra
 <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>, Masami
 Hiramatsu <mhiramat@...nel.org>, linux-trace-kernel@...r.kernel.org, Juri
 Lelli <juri.lelli@...hat.com>
Subject: Re: [PATCH v2 03/11] sched: Add sched tracepoints for RV task model



On Mon, 2025-02-17 at 12:16 -0500, Steven Rostedt wrote:
> On Mon, 17 Feb 2025 17:49:17 +0100
> Peter Zijlstra <peterz@...radead.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