* [PATCH] sched/fair: Optimize dequeue_task_fair()
@ 2020-08-11 8:43 Jiang Biao
2020-08-11 16:55 ` Dietmar Eggemann
0 siblings, 1 reply; 6+ messages in thread
From: Jiang Biao @ 2020-08-11 8:43 UTC (permalink / raw)
To: mingo, peterz, juri.lelli, vincent.guittot
Cc: dietmar.eggemann, rostedt, bsegall, mgorman, linux-kernel, Jiang Biao
Similar optimization as what has been done in commit,
7d148be69e3a(sched/fair: Optimize enqueue_task_fair())
dequeue_task_fair jumps to dequeue_throttle label when cfs_rq_of(se) is
throttled which means that se can't be NULL. We can move the label after
the if (!se) statement and remove the if(!se) statment as se is always
NULL when reaching this point.
Besides, trying to keep the same pattern with enqueue_task_fair can make
it more readable.
Signed-off-by: Jiang Biao <benbjiang@tencent.com>
---
kernel/sched/fair.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 04fa8dbcfa4d..cbbeafdfa8b7 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5618,10 +5618,10 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
}
-dequeue_throttle:
- if (!se)
- sub_nr_running(rq, 1);
+ /* At this point se is NULL and we are at root level*/
+ sub_nr_running(rq, 1);
+dequeue_throttle:
/* balance early to pull high priority tasks */
if (unlikely(!was_sched_idle && sched_idle_rq(rq)))
rq->next_balance = jiffies;
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sched/fair: Optimize dequeue_task_fair()
2020-08-11 8:43 [PATCH] sched/fair: Optimize dequeue_task_fair() Jiang Biao
@ 2020-08-11 16:55 ` Dietmar Eggemann
2020-08-11 23:18 ` [PATCH] sched/fair: Optimize dequeue_task_fair()(Internet mail) benbjiang(蒋彪)
0 siblings, 1 reply; 6+ messages in thread
From: Dietmar Eggemann @ 2020-08-11 16:55 UTC (permalink / raw)
To: Jiang Biao, mingo, peterz, juri.lelli, vincent.guittot
Cc: rostedt, bsegall, mgorman, linux-kernel, Jiang Biao
On 11/08/2020 10:43, Jiang Biao wrote:
> Similar optimization as what has been done in commit,
> 7d148be69e3a(sched/fair: Optimize enqueue_task_fair())
>
> dequeue_task_fair jumps to dequeue_throttle label when cfs_rq_of(se) is
> throttled which means that se can't be NULL. We can move the label after
> the if (!se) statement and remove the if(!se) statment as se is always
> NULL when reaching this point.
>
> Besides, trying to keep the same pattern with enqueue_task_fair can make
> it more readable.
>
> Signed-off-by: Jiang Biao <benbjiang@tencent.com>
> ---
> kernel/sched/fair.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 04fa8dbcfa4d..cbbeafdfa8b7 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5618,10 +5618,10 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>
> }
>
> -dequeue_throttle:
> - if (!se)
> - sub_nr_running(rq, 1);
> + /* At this point se is NULL and we are at root level*/
> + sub_nr_running(rq, 1);
>
> +dequeue_throttle:
> /* balance early to pull high priority tasks */
> if (unlikely(!was_sched_idle && sched_idle_rq(rq)))
> rq->next_balance = jiffies;
There is already a similar patch in master.
423d02e1463b - sched/fair: Optimize dequeue_task_fair() (2020-06-25 Peng
Wang)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sched/fair: Optimize dequeue_task_fair()(Internet mail)
2020-08-11 16:55 ` Dietmar Eggemann
@ 2020-08-11 23:18 ` benbjiang(蒋彪)
0 siblings, 0 replies; 6+ messages in thread
From: benbjiang(蒋彪) @ 2020-08-11 23:18 UTC (permalink / raw)
To: Dietmar Eggemann
Cc: Jiang Biao, mingo, Peter Zijlstra, juri.lelli, vincent.guittot,
rostedt, bsegall, mgorman, LKML
Hi,
> On Aug 12, 2020, at 12:55 AM, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>
> On 11/08/2020 10:43, Jiang Biao wrote:
>> Similar optimization as what has been done in commit,
>> 7d148be69e3a(sched/fair: Optimize enqueue_task_fair())
>>
>> dequeue_task_fair jumps to dequeue_throttle label when cfs_rq_of(se) is
>> throttled which means that se can't be NULL. We can move the label after
>> the if (!se) statement and remove the if(!se) statment as se is always
>> NULL when reaching this point.
>>
>> Besides, trying to keep the same pattern with enqueue_task_fair can make
>> it more readable.
>>
>> Signed-off-by: Jiang Biao <benbjiang@tencent.com>
>> ---
>> kernel/sched/fair.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 04fa8dbcfa4d..cbbeafdfa8b7 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -5618,10 +5618,10 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>>
>> }
>>
>> -dequeue_throttle:
>> - if (!se)
>> - sub_nr_running(rq, 1);
>> + /* At this point se is NULL and we are at root level*/
>> + sub_nr_running(rq, 1);
>>
>> +dequeue_throttle:
>> /* balance early to pull high priority tasks */
>> if (unlikely(!was_sched_idle && sched_idle_rq(rq)))
>> rq->next_balance = jiffies;
>
> There is already a similar patch in master.
>
> 423d02e1463b - sched/fair: Optimize dequeue_task_fair() (2020-06-25 Peng
> Wang)
Indeed, my local repo has been outdated, sorry for the interruption. :)
Thx.
Regards,
Jiang
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] sched/fair: Optimize dequeue_task_fair()
@ 2020-06-15 14:16 Peng Wang
2020-06-15 15:09 ` Vincent Guittot
0 siblings, 1 reply; 6+ messages in thread
From: Peng Wang @ 2020-06-15 14:16 UTC (permalink / raw)
To: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann,
rostedt, bsegall, mgorman
Cc: linux-kernel
While looking at enqueue_task_fair and dequeue_task_fair, it occurred
to me that dequeue_task_fair can also be optimized as Vincent described
in commit 7d148be69e3a ("sched/fair: Optimize enqueue_task_fair()").
dequeue_throttle label can ensure se not to be NULL, and se is
always NULL when reaching root level.
Signed-off-by: Peng Wang <rocking@linux.alibaba.com>
---
kernel/sched/fair.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index cbcb2f7..e6be952 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5614,10 +5614,10 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
}
-dequeue_throttle:
- if (!se)
- sub_nr_running(rq, 1);
+ /* At this point se is NULL and we are at root level*/
+ sub_nr_running(rq, 1);
+dequeue_throttle:
/* balance early to pull high priority tasks */
if (unlikely(!was_sched_idle && sched_idle_rq(rq)))
rq->next_balance = jiffies;
--
2.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] sched/fair: Optimize dequeue_task_fair()
2020-06-15 14:16 [PATCH] sched/fair: Optimize dequeue_task_fair() Peng Wang
@ 2020-06-15 15:09 ` Vincent Guittot
2020-06-16 6:09 ` Peng Wang
0 siblings, 1 reply; 6+ messages in thread
From: Vincent Guittot @ 2020-06-15 15:09 UTC (permalink / raw)
To: Peng Wang
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, linux-kernel
On Mon, 15 Jun 2020 at 16:20, Peng Wang <rocking@linux.alibaba.com> wrote:
>
> While looking at enqueue_task_fair and dequeue_task_fair, it occurred
> to me that dequeue_task_fair can also be optimized as Vincent described
> in commit 7d148be69e3a ("sched/fair: Optimize enqueue_task_fair()").
good point
>
> dequeue_throttle label can ensure se not to be NULL, and se is
> always NULL when reaching root level.
>
> Signed-off-by: Peng Wang <rocking@linux.alibaba.com>
> ---
> kernel/sched/fair.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index cbcb2f7..e6be952 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5614,10 +5614,10 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>
> }
>
> -dequeue_throttle:
> - if (!se)
> - sub_nr_running(rq, 1);
> + /* At this point se is NULL and we are at root level*/
> + sub_nr_running(rq, 1);
>
> +dequeue_throttle:
> /* balance early to pull high priority tasks */
> if (unlikely(!was_sched_idle && sched_idle_rq(rq)))
> rq->next_balance = jiffies;
You can move the label here because sched_idle_rq() uses
rq->nr-running and rq->cfs.idle_h_nr_running so they will not change
if we jump to the label before reaching root level
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] sched/fair: Optimize dequeue_task_fair()
2020-06-15 15:09 ` Vincent Guittot
@ 2020-06-16 6:09 ` Peng Wang
0 siblings, 0 replies; 6+ messages in thread
From: Peng Wang @ 2020-06-16 6:09 UTC (permalink / raw)
To: Vincent Guittot
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, linux-kernel
On 6/15/20 11:09 PM, Vincent Guittot wrote:
> On Mon, 15 Jun 2020 at 16:20, Peng Wang <rocking@linux.alibaba.com> wrote:
>>
>> While looking at enqueue_task_fair and dequeue_task_fair, it occurred
>> to me that dequeue_task_fair can also be optimized as Vincent described
>> in commit 7d148be69e3a ("sched/fair: Optimize enqueue_task_fair()").
>
> good point
>
>>
>> dequeue_throttle label can ensure se not to be NULL, and se is
>> always NULL when reaching root level.
>>
>> Signed-off-by: Peng Wang <rocking@linux.alibaba.com>
>> ---
>> kernel/sched/fair.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index cbcb2f7..e6be952 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -5614,10 +5614,10 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>>
>> }
>>
>> -dequeue_throttle:
>> - if (!se)
>> - sub_nr_running(rq, 1);
>> + /* At this point se is NULL and we are at root level*/
>> + sub_nr_running(rq, 1);
>>
>> +dequeue_throttle:
>> /* balance early to pull high priority tasks */
>> if (unlikely(!was_sched_idle && sched_idle_rq(rq)))
>> rq->next_balance = jiffies;
>
> You can move the label here because sched_idle_rq() uses
> rq->nr-running and rq->cfs.idle_h_nr_running so they will not change
> if we jump to the label before reaching root level
Yes, then we can also skip the early balance check.
>
>> --
>> 2.9.5
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-11 23:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-11 8:43 [PATCH] sched/fair: Optimize dequeue_task_fair() Jiang Biao
2020-08-11 16:55 ` Dietmar Eggemann
2020-08-11 23:18 ` [PATCH] sched/fair: Optimize dequeue_task_fair()(Internet mail) benbjiang(蒋彪)
-- strict thread matches above, loose matches on Subject: below --
2020-06-15 14:16 [PATCH] sched/fair: Optimize dequeue_task_fair() Peng Wang
2020-06-15 15:09 ` Vincent Guittot
2020-06-16 6:09 ` Peng Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).