Re: [PATCH 1/2] sched_ext: Fix exit_cpu accuracy for lockup paths

[Andrea Righi <arighi@nvidia.com>]

Hi Cheng-Yang,

On Sun, May 31, 2026 at 11:25:26PM +0800, Cheng-Yang Chou wrote:
> handle_lockup() uses raw_smp_processor_id() for exit_cpu, which is wrong
> for two paths:
> 
> - scx_hardlockup_irq_workfn() has the hung CPU in a local variable but
>   irq_work may run elsewhere. Pass the local cpu explicitly.
> - scx_rcu_cpu_stall() records the detector CPU rather than the stalled
>   one. Pass -1 for now. The next patch fixes this properly.
> 
> Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>

Small nit below, apart than that looks good me.

Reviewed-by: Andrea Righi <arighi@nvidia.com>

> ---
>  kernel/sched/ext.c          | 12 +++++++-----
>  kernel/sched/ext_internal.h |  2 --
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index ffad1a90196f..0c37b5fd58b0 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -5205,6 +5205,7 @@ bool scx_allow_ttwu_queue(const struct task_struct *p)
>  
>  /**
>   * handle_lockup - sched_ext common lockup handler
> + * @exit_cpu: CPU to record in exit_info. Pass the stalled/hung CPU, not current.
>   * @fmt: format string
>   *
>   * Called on system stall or lockup condition and initiates abort of sched_ext
> @@ -5214,7 +5215,7 @@ bool scx_allow_ttwu_queue(const struct task_struct *p)
>   * resolve the lockup. %false if sched_ext is not enabled or abort was already
>   * initiated by someone else.
>   */
> -static __printf(1, 2) bool handle_lockup(const char *fmt, ...)
> +static __printf(2, 3) bool handle_lockup(int exit_cpu, const char *fmt, ...)
>  {
>  	struct scx_sched *sch;
>  	va_list args;
> @@ -5230,7 +5231,7 @@ static __printf(1, 2) bool handle_lockup(const char *fmt, ...)
>  	case SCX_ENABLING:
>  	case SCX_ENABLED:
>  		va_start(args, fmt);
> -		ret = scx_verror(sch, fmt, args);
> +		ret = scx_vexit(sch, SCX_EXIT_ERROR, 0, exit_cpu, fmt, args);
>  		va_end(args);
>  		return ret;
>  	default:
> @@ -5252,7 +5253,7 @@ static __printf(1, 2) bool handle_lockup(const char *fmt, ...)
>   */
>  bool scx_rcu_cpu_stall(void)
>  {
> -	return handle_lockup("RCU CPU stall detected!");
> +	return handle_lockup(-1, "RCU CPU stall detected!");
>  }
>  
>  /**
> @@ -5267,7 +5268,8 @@ bool scx_rcu_cpu_stall(void)
>   */
>  void scx_softlockup(u32 dur_s)
>  {
> -	if (!handle_lockup("soft lockup - CPU %d stuck for %us", smp_processor_id(), dur_s))
> +	if (!handle_lockup(smp_processor_id(), "soft lockup - CPU %d stuck for %us",
> +			   smp_processor_id(), dur_s))

nit: maybe we can use smp_processor_id() once here, like:

 int cpu = smp_processor_id();

 if (!handle_lockup(cpu, ..., cpu, dur_s))

Thanks,
-Andrea