2022-04-07 07:48:25

by Chengming Zhou

Subject: [PATCH 2/2] sched/fair: Delete useless condition in tg_unthrottle_up()

Fully decayed cfs_rq is impossible to have queued entities,
the first condition "!cfs_rq_is_decayed(cfs_rq)" is enough
to cover.

Signed-off-by: Chengming Zhou <[email protected]>
---
kernel/sched/fair.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e6fa5d1141b4..17c13c38b1c2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4850,7 +4850,7 @@ static int tg_unthrottle_up(struct task_group *tg, void *data)
cfs_rq->throttled_clock_task;

/* Add cfs_rq with load or one or more already running entities to the list */
- if (!cfs_rq_is_decayed(cfs_rq) || cfs_rq->nr_running)
+ if (!cfs_rq_is_decayed(cfs_rq))
list_add_leaf_cfs_rq(cfs_rq);
}

--
2.35.1

2022-04-07 21:44:25

by Benjamin Segall

Subject: Re: [PATCH 2/2] sched/fair: Delete useless condition in tg_unthrottle_up()

Chengming Zhou <[email protected]> writes:

> Fully decayed cfs_rq is impossible to have queued entities,
> the first condition "!cfs_rq_is_decayed(cfs_rq)" is enough
> to cover.

In particular, cfs_rq->load.weight is part of cfs_rq_is_decayed.

Reviewed-by: Ben Segall <[email protected]>

>
> Signed-off-by: Chengming Zhou <[email protected]>
> ---
> kernel/sched/fair.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e6fa5d1141b4..17c13c38b1c2 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4850,7 +4850,7 @@ static int tg_unthrottle_up(struct task_group *tg, void *data)
> cfs_rq->throttled_clock_task;
>
> /* Add cfs_rq with load or one or more already running entities to the list */
> - if (!cfs_rq_is_decayed(cfs_rq) || cfs_rq->nr_running)
> + if (!cfs_rq_is_decayed(cfs_rq))
> list_add_leaf_cfs_rq(cfs_rq);
> }

2022-04-08 07:43:40

by Vincent Guittot

Subject: Re: [PATCH 2/2] sched/fair: Delete useless condition in tg_unthrottle_up()

On Thu, 7 Apr 2022 at 23:00, Benjamin Segall <[email protected]> wrote:
>
> Chengming Zhou <[email protected]> writes:
>
> > Fully decayed cfs_rq is impossible to have queued entities,
> > the first condition "!cfs_rq_is_decayed(cfs_rq)" is enough
> > to cover.
>
> In particular, cfs_rq->load.weight is part of cfs_rq_is_decayed.

Testing cfs_rq->load.weight is the key point because nothing prevent
to add a task with null load

Reviewed-by: Vincent Guittot <[email protected]>

>
> Reviewed-by: Ben Segall <[email protected]>
>
> >
> > Signed-off-by: Chengming Zhou <[email protected]>
> > ---
> > kernel/sched/fair.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index e6fa5d1141b4..17c13c38b1c2 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -4850,7 +4850,7 @@ static int tg_unthrottle_up(struct task_group *tg, void *data)
> > cfs_rq->throttled_clock_task;
> >
> > /* Add cfs_rq with load or one or more already running entities to the list */
> > - if (!cfs_rq_is_decayed(cfs_rq) || cfs_rq->nr_running)
> > + if (!cfs_rq_is_decayed(cfs_rq))
> > list_add_leaf_cfs_rq(cfs_rq);
> > }