ops.cpu_release() function, if defined, must be invoked when preempted by
a higher priority scheduler class task. This scenario was skipped in
commit f422316d7466 ("sched_ext: Remove switch_class_scx()"). Let's fix
it.

Fixes: f422316d7466 ("sched_ext: Remove switch_class_scx()")
Signed-off-by: Honglei Wang <jameshongleiwang@126.com>
---
 kernel/sched/ext.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 19d2699cf638..ef0e3f2c1e6f 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -3034,7 +3034,7 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
 		 */
 		if (p->scx.slice && !scx_rq_bypassing(rq)) {
 			dispatch_enqueue(&rq->scx.local_dsq, p, SCX_ENQ_HEAD);
-			return;
+			goto switch_class;
 		}
 
 		/*
@@ -3051,6 +3051,7 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
 		}
 	}
 
+switch_class:
 	if (next && next->sched_class != &ext_sched_class)
 		switch_class(rq, next);
 }
-- 
2.45.2

On Wed, Jan 08, 2025 at 10:33:28AM +0800, Honglei Wang wrote:
> ops.cpu_release() function, if defined, must be invoked when preempted by
> a higher priority scheduler class task. This scenario was skipped in
> commit f422316d7466 ("sched_ext: Remove switch_class_scx()"). Let's fix
> it.
> 
> Fixes: f422316d7466 ("sched_ext: Remove switch_class_scx()")
> Signed-off-by: Honglei Wang <jameshongleiwang@126.com>

Applied to sched_ext/for-6.13-fixes.

Thanks.

-- 
tejun

Hi Honglei,

On Wed, Jan 08, 2025 at 10:33:28AM +0800, Honglei Wang wrote:
> ops.cpu_release() function, if defined, must be invoked when preempted by
> a higher priority scheduler class task. This scenario was skipped in
> commit f422316d7466 ("sched_ext: Remove switch_class_scx()"). Let's fix
> it.
> 
> Fixes: f422316d7466 ("sched_ext: Remove switch_class_scx()")
> Signed-off-by: Honglei Wang <jameshongleiwang@126.com>

Thanks a lot for catching this, I've also reproduced a stall (due to the
missing ops.cpu_release() invocation) and verified that your fixes resolve
the problem, so:

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

> ---
>  kernel/sched/ext.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 19d2699cf638..ef0e3f2c1e6f 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -3034,7 +3034,7 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
>  		 */
>  		if (p->scx.slice && !scx_rq_bypassing(rq)) {
>  			dispatch_enqueue(&rq->scx.local_dsq, p, SCX_ENQ_HEAD);
> -			return;
> +			goto switch_class;
>  		}
>  
>  		/*
> @@ -3051,6 +3051,7 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
>  		}
>  	}
>  
> +switch_class:
>  	if (next && next->sched_class != &ext_sched_class)
>  		switch_class(rq, next);
>  }
> -- 
> 2.45.2
> 

-Andrea

On 2025/1/8 17:05, Andrea Righi wrote:
> Hi Honglei,
> 
> On Wed, Jan 08, 2025 at 10:33:28AM +0800, Honglei Wang wrote:
>> ops.cpu_release() function, if defined, must be invoked when preempted by
>> a higher priority scheduler class task. This scenario was skipped in
>> commit f422316d7466 ("sched_ext: Remove switch_class_scx()"). Let's fix
>> it.
>>
>> Fixes: f422316d7466 ("sched_ext: Remove switch_class_scx()")
>> Signed-off-by: Honglei Wang <jameshongleiwang@126.com>
> 
> Thanks a lot for catching this, I've also reproduced a stall (due to the
> missing ops.cpu_release() invocation) and verified that your fixes resolve
> the problem, so:
> 
> Acked-by: Andrea Righi <arighi@nvidia.com>
> 

Thanks a lot for looking at the code and the test, Andrea.

>> ---
>>  kernel/sched/ext.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
>> index 19d2699cf638..ef0e3f2c1e6f 100644
>> --- a/kernel/sched/ext.c
>> +++ b/kernel/sched/ext.c
>> @@ -3034,7 +3034,7 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
>>  		 */
>>  		if (p->scx.slice && !scx_rq_bypassing(rq)) {
>>  			dispatch_enqueue(&rq->scx.local_dsq, p, SCX_ENQ_HEAD);
>> -			return;
>> +			goto switch_class;
>>  		}
>>  
>>  		/*
>> @@ -3051,6 +3051,7 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
>>  		}
>>  	}
>>  
>> +switch_class:
>>  	if (next && next->sched_class != &ext_sched_class)
>>  		switch_class(rq, next);
>>  }
>> -- 
>> 2.45.2
>>
> 
> -Andrea