[PATCH V1] sched: Cancel the slice protection of the idle entity

zihan zhou posted 1 patch 11 months ago
There is a newer version of this series
kernel/sched/fair.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[PATCH V1] sched: Cancel the slice protection of the idle entity
Posted by zihan zhou 11 months ago
A wakeup non-idle entity should preempt idle entity at any time,
but because of the slice protection of the idle entity, the non-idle
entity has to wait, so just cancel it.

Signed-off-by: zihan zhou <15645113830zzh@gmail.com>
---
 kernel/sched/fair.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 3e9ca38512de..7777d110d053 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8851,8 +8851,19 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
 	 * Preempt an idle entity in favor of a non-idle entity (and don't preempt
 	 * in the inverse case).
 	 */
-	if (cse_is_idle && !pse_is_idle)
+	if (cse_is_idle && !pse_is_idle) {
+
+		/*
+		 * When non-idle entity preempt an idle entity,
+		 * don't give idle entity slice protection.
+		 */
+		if (se->vlag == se->deadline)
+			se->vlag = se->deadline + 1;
+
 		goto preempt;
+
+	}
+
 	if (cse_is_idle != pse_is_idle)
 		return;
 
-- 
2.33.0
Re: [PATCH V1] sched: Cancel the slice protection of the idle entity
Posted by Vincent Guittot 10 months, 2 weeks ago
On Tue, 21 Jan 2025 at 04:07, zihan zhou <15645113830zzh@gmail.com> wrote:
>
> A wakeup non-idle entity should preempt idle entity at any time,
> but because of the slice protection of the idle entity, the non-idle
> entity has to wait, so just cancel it.

Probably worth to add:
Fixes: 63304558ba5d ("sched/eevdf: Curb wakeup-preemption")

>
> Signed-off-by: zihan zhou <15645113830zzh@gmail.com>

I have been able to reproduce the problem with rt-app.

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Tested-by: Vincent Guittot <vincent.guittot@linaro.org>

> ---
>  kernel/sched/fair.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 3e9ca38512de..7777d110d053 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8851,8 +8851,19 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>          * Preempt an idle entity in favor of a non-idle entity (and don't preempt
>          * in the inverse case).
>          */
> -       if (cse_is_idle && !pse_is_idle)
> +       if (cse_is_idle && !pse_is_idle) {
> +

remove blank line above

> +               /*
> +                * When non-idle entity preempt an idle entity,
> +                * don't give idle entity slice protection.
> +                */
> +               if (se->vlag == se->deadline)
> +                       se->vlag = se->deadline + 1;
> +
>                 goto preempt;
> +

remove blank line above

> +       }
> +
>         if (cse_is_idle != pse_is_idle)
>                 return;
>
> --
> 2.33.0
>
Re: [PATCH V1] sched: Cancel the slice protection of the idle entity
Posted by zihan zhou 10 months, 2 weeks ago
Thanks for your reply and guidance!

I have submitted a patch v2 based on your feedback:
https://lore.kernel.org/all/20250208080850.16300-1-15645113830zzh@gmail.com/T/#u

Looking forward to your review!
Re: [PATCH V1] sched: Cancel the slice protection of the idle entity
Posted by Peter Zijlstra 10 months, 2 weeks ago
On Tue, Jan 21, 2025 at 11:06:29AM +0800, zihan zhou wrote:
> A wakeup non-idle entity should preempt idle entity at any time,
> but because of the slice protection of the idle entity, the non-idle
> entity has to wait, so just cancel it.
> 
> Signed-off-by: zihan zhou <15645113830zzh@gmail.com>
> ---
>  kernel/sched/fair.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 3e9ca38512de..7777d110d053 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8851,8 +8851,19 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>  	 * Preempt an idle entity in favor of a non-idle entity (and don't preempt
>  	 * in the inverse case).
>  	 */
> -	if (cse_is_idle && !pse_is_idle)
> +	if (cse_is_idle && !pse_is_idle) {
> +
> +		/*
> +		 * When non-idle entity preempt an idle entity,
> +		 * don't give idle entity slice protection.
> +		 */
> +		if (se->vlag == se->deadline)
> +			se->vlag = se->deadline + 1;
> +
>  		goto preempt;
> +
> +	}

Yes, I suppose we can do this.
Re: [PATCH V1] sched: Cancel the slice protection of the idle entity
Posted by zihan zhou 10 months, 2 weeks ago
> > A wakeup non-idle entity should preempt idle entity at any time,
> > but because of the slice protection of the idle entity, the non-idle
> > entity has to wait, so just cancel it.
> > 
> > Signed-off-by: zihan zhou <15645113830zzh@gmail.com>
> > ---
> >  kernel/sched/fair.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 3e9ca38512de..7777d110d053 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8851,8 +8851,19 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
> >  	 * Preempt an idle entity in favor of a non-idle entity (and don't preempt
> >  	 * in the inverse case).
> >  	 */
> > -	if (cse_is_idle && !pse_is_idle)
> > +	if (cse_is_idle && !pse_is_idle) {
> > +
> > +		/*
> > +		 * When non-idle entity preempt an idle entity,
> > +		 * don't give idle entity slice protection.
> > +		 */
> > +		if (se->vlag == se->deadline)
> > +			se->vlag = se->deadline + 1;
> > +
> >  		goto preempt;
> > +
> > +	}
> 
> Yes, I suppose we can do this.

Thank you very much for your affirmation!
Re: [PATCH V1] sched: Cancel the slice protection of the idle entity
Posted by zihan zhou 10 months, 2 weeks ago
Hello, the submission of this patch is aimed at minimizing the impact of
SCHED_IDLE on SCHED_NORMAL.

For example, a task with SCHED_IDLE policy that sleeps for 1s and then
runs for 3 ms, running cyclictest on the same cpu, has a maximum latency
of 3 ms, which is caused by the slice protection of the idle entity.
It is unreasonable. With this patch, the cyclictest latency under the
same conditions is basically the same on the cpu with idle processes and
on empty cpu.

Looking forward to your reply!