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

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Feb 11, 2025 at 08:46:10AM +0100, Gabriele Monaco wrote:

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 9632e3318e0d6..9ff0658095240 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -46,6 +46,7 @@
>  #include <linux/rv.h>
>  #include <linux/livepatch_sched.h>
>  #include <linux/uidgid_types.h>
> +#include <linux/tracepoint-defs.h>
>  #include <asm/kmap_size.h>
>  
>  /* task_struct member predeclarations (sorted alphabetically): */
> @@ -186,6 +187,12 @@ struct user_event_mm;
>  # define debug_rtlock_wait_restore_state()	do { } while (0)
>  #endif
>  
> +#define trace_set_current_state(state_value)                     \
> +	do {                                                     \
> +		if (tracepoint_enabled(sched_set_state_tp))      \
> +			do_trace_set_current_state(state_value); \
> +	} while (0)

Yeah, this is much nicer, thanks!

>  /*
>   * set_current_state() includes a barrier so that the write of current->__state
>   * is correctly serialised wrt the caller's subsequent test of whether to
> @@ -226,12 +233,14 @@ struct user_event_mm;
>  #define __set_current_state(state_value)				\
>  	do {								\
>  		debug_normal_state_change((state_value));		\
> +		trace_set_current_state(state_value);			\
>  		WRITE_ONCE(current->__state, (state_value));		\
>  	} while (0)
>  
>  #define set_current_state(state_value)					\
>  	do {								\
>  		debug_normal_state_change((state_value));		\
> +		trace_set_current_state(state_value);			\
>  		smp_store_mb(current->__state, (state_value));		\
>  	} while (0)
>  
> @@ -247,6 +256,7 @@ struct user_event_mm;
>  									\
>  		raw_spin_lock_irqsave(&current->pi_lock, flags);	\
>  		debug_special_state_change((state_value));		\
> +		trace_set_current_state(state_value);			\
>  		WRITE_ONCE(current->__state, (state_value));		\
>  		raw_spin_unlock_irqrestore(&current->pi_lock, flags);	\
>  	} while (0)
> @@ -282,6 +292,7 @@ struct user_event_mm;
>  		raw_spin_lock(&current->pi_lock);			\
>  		current->saved_state = current->__state;		\
>  		debug_rtlock_wait_set_state();				\
> +		trace_set_current_state(TASK_RTLOCK_WAIT);		\
>  		WRITE_ONCE(current->__state, TASK_RTLOCK_WAIT);		\
>  		raw_spin_unlock(&current->pi_lock);			\
>  	} while (0);
> @@ -291,6 +302,7 @@ struct user_event_mm;
>  		lockdep_assert_irqs_disabled();				\
>  		raw_spin_lock(&current->pi_lock);			\
>  		debug_rtlock_wait_restore_state();			\
> +		trace_set_current_state(TASK_RUNNING);			\
>  		WRITE_ONCE(current->__state, current->saved_state);	\
>  		current->saved_state = TASK_RUNNING;			\
>  		raw_spin_unlock(&current->pi_lock);			\


> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 165c90ba64ea9..9e33728bb57e8 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -491,6 +491,15 @@ 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);

_GPL

> +
> +void do_trace_set_current_state(int state_value)
> +{
> +	trace_sched_set_state_tp(current, current->__state, state_value);

Should this be:

	__do_trace_sched_set_state_tp() ?

> +}
> +EXPORT_SYMBOL(do_trace_set_current_state);

_GPL