The commit 2ebb17717550 ("sched/core: Offload wakee task activation if it
the wakee is descheduling") checked nr_running <= 1 to avoid task
stacking when WF_ON_CPU. Consider the order of p->on_rq and p->on_cpu,
if we have WF_ON_CPU here then we must have !p->on_rq, so the deactivate
has happened, thus the task being alone on the rq implies nr_running==0.
Change the check to !cpu_rq(cpu)->nr_running to fix it.
Signed-off-by: Tianchen Ding <[email protected]>
---
kernel/sched/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index bfa7452ca92e..a9efe134fbe5 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3830,7 +3830,7 @@ static inline bool ttwu_queue_cond(int cpu, int wake_flags)
* the soon-to-be-idle CPU as the current CPU is likely busy.
* nr_running is checked to avoid unnecessary task stacking.
*/
- if ((wake_flags & WF_ON_CPU) && cpu_rq(cpu)->nr_running <= 1)
+ if ((wake_flags & WF_ON_CPU) && !cpu_rq(cpu)->nr_running)
return true;
return false;
--
2.27.0
On 02/06/22 12:06, Tianchen Ding wrote:
> The commit 2ebb17717550 ("sched/core: Offload wakee task activation if it
> the wakee is descheduling") checked nr_running <= 1 to avoid task
> stacking when WF_ON_CPU. Consider the order of p->on_rq and p->on_cpu,
> if we have WF_ON_CPU here then we must have !p->on_rq, so the deactivate
> has happened, thus the task being alone on the rq implies nr_running==0.
> Change the check to !cpu_rq(cpu)->nr_running to fix it.
>
I'd flesh this out a bit as in the below:
"""
The commit 2ebb17717550 ("sched/core: Offload wakee task activation if it
the wakee is descheduling") checked rq->nr_running <= 1 to avoid task
stacking when WF_ON_CPU.
Per the ordering of writes to p->on_rq and p->on_cpu, observing p->on_cpu
(WF_ON_CPU) in ttwu_queue_cond() implies !p->on_rq, IOW p has gone through
the deactivate_task() in __schedule(), thus p has been accounted out of
rq->nr_running. As such, the task being the only runnable task on the rq
implies reading rq->nr_running == 0 at that point.
Change the check to !cpu_rq(cpu)->nr_running.
"""
Also, this is lacking some mention of tests that have been run to verify
this isn't causing a regression. This does however make sense to me, so as
long as nothing gets hurt by the change:
Reviewed-by: Valentin Schneider <[email protected]>
> Signed-off-by: Tianchen Ding <[email protected]>
> ---
> kernel/sched/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index bfa7452ca92e..a9efe134fbe5 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3830,7 +3830,7 @@ static inline bool ttwu_queue_cond(int cpu, int wake_flags)
> * the soon-to-be-idle CPU as the current CPU is likely busy.
> * nr_running is checked to avoid unnecessary task stacking.
> */
> - if ((wake_flags & WF_ON_CPU) && cpu_rq(cpu)->nr_running <= 1)
> + if ((wake_flags & WF_ON_CPU) && !cpu_rq(cpu)->nr_running)
> return true;
>
> return false;
> --
> 2.27.0
On 2022/6/6 18:39, Valentin Schneider wrote:
> On 02/06/22 12:06, Tianchen Ding wrote:
>> The commit 2ebb17717550 ("sched/core: Offload wakee task activation if it
>> the wakee is descheduling") checked nr_running <= 1 to avoid task
>> stacking when WF_ON_CPU. Consider the order of p->on_rq and p->on_cpu,
>> if we have WF_ON_CPU here then we must have !p->on_rq, so the deactivate
>> has happened, thus the task being alone on the rq implies nr_running==0.
>> Change the check to !cpu_rq(cpu)->nr_running to fix it.
>>
>
> I'd flesh this out a bit as in the below:
>
> """
> The commit 2ebb17717550 ("sched/core: Offload wakee task activation if it
> the wakee is descheduling") checked rq->nr_running <= 1 to avoid task
> stacking when WF_ON_CPU.
>
> Per the ordering of writes to p->on_rq and p->on_cpu, observing p->on_cpu
> (WF_ON_CPU) in ttwu_queue_cond() implies !p->on_rq, IOW p has gone through
> the deactivate_task() in __schedule(), thus p has been accounted out of
> rq->nr_running. As such, the task being the only runnable task on the rq
> implies reading rq->nr_running == 0 at that point.
>
> Change the check to !cpu_rq(cpu)->nr_running.
> """
>
> Also, this is lacking some mention of tests that have been run to verify
> this isn't causing a regression. This does however make sense to me, so as
> long as nothing gets hurt by the change:
>
I've run the complete test cases of unixbench and it seems no regression (which
is expected). The result of Pipe-based Context Switching seems to be stable when
I simply type "./Run" to run all cases. :-/
On x86 (Intel Xeon Platinum 8269CY):
schbench -m 2 -t 8
Latency percentiles (usec) before after
50.0000th: 8 8
75.0000th: 10 10
90.0000th: 11 11
95.0000th: 12 12
*99.0000th: 15 13
99.5000th: 16 15
99.9000th: 20 18
Unixbench with full threads (104)
before after
Dhrystone 2 using register variables 3004715731 3011862938 0.24%
Double-Precision Whetstone 616685.8 617119.3 0.07%
Execl Throughput 27162.1 27667.3 1.86%
File Copy 1024 bufsize 2000 maxblocks 786221.4 785871.4 -0.04%
File Copy 256 bufsize 500 maxblocks 209420.6 210113.6 0.33%
File Copy 4096 bufsize 8000 maxblocks 2340458.8 2328862.2 -0.50%
Pipe Throughput 145249195.6 145535622.8 0.20%
Pipe-based Context Switching 3195567.7 3221686.4 0.82%
Process Creation 100597.6 101347.1 0.75%
Shell Scripts (1 concurrent) 120943.6 120193.5 -0.62%
Shell Scripts (8 concurrent) 17289.7 17233.4 -0.33%
System Call Overhead 5286847.6 5300604.8 0.26%
On arm64 (Ampere Altra):
schbench -m 2 -t 8
Latency percentiles (usec) before after
50.0000th: 14 14
75.0000th: 19 19
90.0000th: 22 22
95.0000th: 23 23
*99.0000th: 23 24
99.5000th: 23 24
99.9000th: 28 28
Unixbench with full threads (80)
before after
Dhrystone 2 using register variables 3536273441 3536194249 0.00%
Double-Precision Whetstone 629406.9 629383.6 0.00%
Execl Throughput 66419.3 65920.5 -0.75%
File Copy 1024 bufsize 2000 maxblocks 1060185.2 1063722.8 0.33%
File Copy 256 bufsize 500 maxblocks 317495.4 322684.5 1.63%
File Copy 4096 bufsize 8000 maxblocks 2350706.8 2348285.3 -0.10%
Pipe Throughput 133516462.4 133542875.3 0.02%
Pipe-based Context Switching 3227430.6 3215356.1 -0.37%
Process Creation 108958.3 108520.5 -0.40%
Shell Scripts (1 concurrent) 122821.4 122636.3 -0.15%
Shell Scripts (8 concurrent) 17456.5 17462.1 0.03%
System Call Overhead 4430303.2 4429998.9 -0.01%
> Reviewed-by: Valentin Schneider <[email protected]>
>
Thanks. Will update my patch and send v4 soon.