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: <20250217113844.5ad7893c@gandalf.local.home>
Date: Mon, 17 Feb 2025 11:38:44 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>, Peter
 Zijlstra <peterz@...radead.org>, 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 Thu, 13 Feb 2025 10:08:01 +0100
Gabriele Monaco <gmonaco@...hat.com> wrote:

> diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
> index 9ea4c404bd4ef..cc3be04fe9986 100644
> --- a/include/trace/events/sched.h
> +++ b/include/trace/events/sched.h
> @@ -824,6 +824,19 @@ DECLARE_TRACE(sched_compute_energy_tp,
>  		 unsigned long max_util, unsigned long busy_time),
>  	TP_ARGS(p, dst_cpu, energy, max_util, busy_time));
>  
> +DECLARE_TRACE(sched_entry_tp,
> +	TP_PROTO(bool preempt, unsigned long ip),
> +	TP_ARGS(preempt, ip));
> +
> +DECLARE_TRACE(sched_exit_tp,
> +	TP_PROTO(bool is_switch, unsigned long ip),
> +	TP_ARGS(is_switch, ip));
> +
> +DECLARE_TRACE_CONDITION(sched_set_state_tp,
> +	TP_PROTO(struct task_struct *tsk, int curr_state, int state),
> +	TP_ARGS(tsk, curr_state, state),
> +	TP_CONDITION(!!curr_state != !!state));

I don't think you need to pass in state. Just have it be:

	TP_CONDITION(!!(tsk->__state) != !!state));


> +
>  #endif /* _TRACE_SCHED_H */
>  
>  /* This part must be outside protection */
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 165c90ba64ea9..4aa6af026e05c 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -491,6 +491,19 @@ sched_core_dequeue(struct rq *rq, struct task_struct *p, int flags) { }
>  
>  #endif /* CONFIG_SCHED_CORE */
>  
> +/* need a wrapper since we may need to trace from modules */
> +EXPORT_TRACEPOINT_SYMBOL(sched_set_state_tp);
> +
> +/*
> + * Do not call this function directly since it won't check if the tp is enabled.
> + * Call the helper macro trace_set_current_state instead.
> + */
> +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).


> +}
> +EXPORT_SYMBOL(__do_trace_set_current_state);
> +

-- Steve