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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250217121647.0b9ead66@gandalf.local.home>
Date: Mon, 17 Feb 2025 12:16:47 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Gabriele Monaco <gmonaco@...hat.com>, 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, 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