mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] sched: Queue task on wakelist in the same llc if the wakee cpu is idle
@ 2022-06-02  4:06 Tianchen Ding
  2022-06-02  4:06 ` [PATCH v3 1/2] sched: Fix the check of nr_running at queue wakelist Tianchen Ding
  2022-06-02  4:06 ` [PATCH v3 2/2] sched: Remove the limitation of WF_ON_CPU on wakelist if wakee cpu is idle Tianchen Ding
  0 siblings, 2 replies; 6+ messages in thread
From: Tianchen Ding @ 2022-06-02  4:06 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider
  Cc: linux-kernel

Wakelist can help avoid cache bouncing and offload the overhead of waker
cpu. So far, using wakelist within the same llc only happens on
WF_ON_CPU, and this limitation could be removed to further improve
wakeup performance. The result of Unixbench Pipe-based Context Switching
can be improved up to ~10%.

v3:
Add a patch to fix the check of nr_running.
After that, we can remove WF_ON_CPU to achieve the same purpose.
Thank Valentin and Mel.

v2: https://lore.kernel.org/all/20220527090544.527411-1-dtcccc@linux.alibaba.com/
Modify commit log to describe key point in detail.
Add more benchmark results on more archs.

v1: https://lore.kernel.org/all/20220513062427.2375743-1-dtcccc@linux.alibaba.com/

Tianchen Ding (2):
  sched: Fix the check of nr_running at queue wakelist
  sched: Remove the limitation of WF_ON_CPU on wakelist if wakee cpu is
    idle

 kernel/sched/core.c  | 27 ++++++++++++++++-----------
 kernel/sched/sched.h |  1 -
 2 files changed, 16 insertions(+), 12 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] sched: Fix the check of nr_running at queue wakelist
  2022-06-02  4:06 [PATCH v3 0/2] sched: Queue task on wakelist in the same llc if the wakee cpu is idle Tianchen Ding
@ 2022-06-02  4:06 ` Tianchen Ding
  2022-06-06 10:39   ` Valentin Schneider
  2022-06-02  4:06 ` [PATCH v3 2/2] sched: Remove the limitation of WF_ON_CPU on wakelist if wakee cpu is idle Tianchen Ding
  1 sibling, 1 reply; 6+ messages in thread
From: Tianchen Ding @ 2022-06-02  4:06 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider
  Cc: linux-kernel

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 <dtcccc@linux.alibaba.com>
---
 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] sched: Remove the limitation of WF_ON_CPU on wakelist if wakee cpu is idle
  2022-06-02  4:06 [PATCH v3 0/2] sched: Queue task on wakelist in the same llc if the wakee cpu is idle Tianchen Ding
  2022-06-02  4:06 ` [PATCH v3 1/2] sched: Fix the check of nr_running at queue wakelist Tianchen Ding
@ 2022-06-02  4:06 ` Tianchen Ding
  2022-06-06 10:39   ` Valentin Schneider
  1 sibling, 1 reply; 6+ messages in thread
From: Tianchen Ding @ 2022-06-02  4:06 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider
  Cc: linux-kernel

Wakelist can help avoid cache bouncing and offload the overhead of waker
cpu. So far, using wakelist within the same llc only happens on
WF_ON_CPU, and this limitation could be removed to further improve
wakeup performance.

The commit 518cd6234178 ("sched: Only queue remote wakeups when
crossing cache boundaries") disabled queuing tasks on wakelist when
the cpus share llc. This is because, at that time, the scheduler must
send IPIs to do ttwu_queue_wakelist. Nowadays, ttwu_queue_wakelist also
supports TIF_POLLING, so this is not a problem now when the wakee cpu is
in idle polling.

Benefits:
  Queuing the task on idle cpu can help improving performance on waker cpu
  and utilization on wakee cpu, and further improve locality because
  the wakee cpu can handle its own rq. This patch helps improving rt on
  our real java workloads where wakeup happens frequently.

  Consider the normal condition (CPU0 and CPU1 share same llc)
  Before this patch:

         CPU0                                       CPU1

    select_task_rq()                                idle
    rq_lock(CPU1->rq)
    enqueue_task(CPU1->rq)
    notify CPU1 (by sending IPI or CPU1 polling)

                                                    resched()

  After this patch:

         CPU0                                       CPU1

    select_task_rq()                                idle
    add to wakelist of CPU1
    notify CPU1 (by sending IPI or CPU1 polling)

                                                    rq_lock(CPU1->rq)
                                                    enqueue_task(CPU1->rq)
                                                    resched()

  We see CPU0 can finish its work earlier. It only needs to put task to
  wakelist and return.
  While CPU1 is idle, so let itself handle its own runqueue data.

This patch brings no difference about IPI.
  This patch only takes effect when the wakee cpu is:
  1) idle polling
  2) idle not polling

  For 1), there will be no IPI with or without this patch.

  For 2), there will always be an IPI before or after this patch.
  Before this patch: waker cpu will enqueue task and check preempt. Since
  "idle" will be sure to be preempted, waker cpu must send a resched IPI.
  After this patch: waker cpu will put the task to the wakelist of wakee
  cpu, and send an IPI.

Benchmark:
We've tested schbench, unixbench, and hachbench on both x86 and arm64.

On x86 (Intel Xeon Platinum 8269CY):
  schbench -m 2 -t 8

    Latency percentiles (usec)             before         after
        50.0000th:                            8             6
        75.0000th:                           10             7
        90.0000th:                           11             8
        95.0000th:                           12             8
        *99.0000th:                          15            10
        99.5000th:                           16            11
        99.9000th:                           20            14

  Unixbench with full threads (104)
                                            before        after
    Dhrystone 2 using register variables  3004614211    3004725417   0.00%
    Double-Precision Whetstone              616764.3      617355.9   0.10%
    Execl Throughput                         26449.2       26468.6   0.07%
    File Copy 1024 bufsize 2000 maxblocks   832763.3      824099.4  -1.04%
    File Copy 256 bufsize 500 maxblocks     210718.7      211775.1   0.50%
    File Copy 4096 bufsize 8000 maxblocks  2393528.2     2398755.4   0.22%
    Pipe Throughput                      144559102.7   144605068.8   0.03%
    Pipe-based Context Switching           3192192.9     3571238.1  11.87%
    Process Creation                         95270.5       98865.4   3.77%
    Shell Scripts (1 concurrent)            113780.6      113924.7   0.13%
    Shell Scripts (8 concurrent)             15557.2       15508.9  -0.31%
    System Call Overhead                   5359984.1     5356711.4  -0.06%

  hackbench -g 1 -l 100000
                                            before        after
    Time                                     3.246        2.251

On arm64 (Ampere Altra):
  schbench -m 2 -t 8

    Latency percentiles (usec)             before         after
        50.0000th:                           14            10
        75.0000th:                           19            14
        90.0000th:                           22            16
        95.0000th:                           23            16
        *99.0000th:                          24            17
        99.5000th:                           24            17
        99.9000th:                           31            25

  Unixbench with full threads (80)
                                            before        after
    Dhrystone 2 using register variables  3536787968    3536476016  -0.01%
    Double-Precision Whetstone              629370.6      629333.3  -0.01%
    Execl Throughput                         66615.9       66288.8  -0.49%
    File Copy 1024 bufsize 2000 maxblocks  1038402.1     1050181.2   1.13%
    File Copy 256 bufsize 500 maxblocks     311054.2      310317.2  -0.24%
    File Copy 4096 bufsize 8000 maxblocks  2276795.6       2297703   0.92%
    Pipe Throughput                      130409359.9   130390848.7  -0.01%
    Pipe-based Context Switching           3148440.7     3383705.1   7.47%
    Process Creation                        111574.3      119728.6   7.31%
    Shell Scripts (1 concurrent)            122980.7      122657.4  -0.26%
    Shell Scripts (8 concurrent)             17482.8       17476.8  -0.03%
    System Call Overhead                   4424103.4     4430062.6   0.13%

  hackbench -g 1 -l 100000
                                            before        after
    Time                                     4.217        2.916

Our patch has improvement on schbench, hackbench
and Pipe-based Context Switching of unixbench
when there exists idle cpus,
and no obvious regression on other tests of unixbench.
This can help improve rt in scenes where wakeup happens frequently.

Signed-off-by: Tianchen Ding <dtcccc@linux.alibaba.com>
---
 kernel/sched/core.c  | 27 ++++++++++++++++-----------
 kernel/sched/sched.h |  1 -
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a9efe134fbe5..ccb9e0fbf49b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3808,7 +3808,7 @@ bool cpus_share_cache(int this_cpu, int that_cpu)
 	return per_cpu(sd_llc_id, this_cpu) == per_cpu(sd_llc_id, that_cpu);
 }
 
-static inline bool ttwu_queue_cond(int cpu, int wake_flags)
+static inline bool ttwu_queue_cond(int cpu)
 {
 	/*
 	 * Do not complicate things with the async wake_list while the CPU is
@@ -3824,13 +3824,21 @@ static inline bool ttwu_queue_cond(int cpu, int wake_flags)
 	if (!cpus_share_cache(smp_processor_id(), cpu))
 		return true;
 
+	if (cpu == smp_processor_id())
+		return false;
+
 	/*
-	 * If the task is descheduling and the only running task on the
-	 * CPU then use the wakelist to offload the task activation to
-	 * the soon-to-be-idle CPU as the current CPU is likely busy.
-	 * nr_running is checked to avoid unnecessary task stacking.
+	 * If the wakee cpu is idle, or the task is descheduling and the
+	 * only running task on the CPU, then use the wakelist to offload
+	 * the task activation to the idle (or soon-to-be-idle) CPU as
+	 * the current CPU is likely busy. nr_running is checked to
+	 * avoid unnecessary task stacking.
+	 *
+	 * Note that we can only get here with (wakee) p->on_rq=0,
+	 * p->on_cpu can be whatever, we've done the dequeue, so
+	 * the wakee has been accounted out of ->nr_running.
 	 */
-	if ((wake_flags & WF_ON_CPU) && !cpu_rq(cpu)->nr_running)
+	if (!cpu_rq(cpu)->nr_running)
 		return true;
 
 	return false;
@@ -3838,10 +3846,7 @@ static inline bool ttwu_queue_cond(int cpu, int wake_flags)
 
 static bool ttwu_queue_wakelist(struct task_struct *p, int cpu, int wake_flags)
 {
-	if (sched_feat(TTWU_QUEUE) && ttwu_queue_cond(cpu, wake_flags)) {
-		if (WARN_ON_ONCE(cpu == smp_processor_id()))
-			return false;
-
+	if (sched_feat(TTWU_QUEUE) && ttwu_queue_cond(cpu)) {
 		sched_clock_cpu(cpu); /* Sync clocks across CPUs */
 		__ttwu_queue_wakelist(p, cpu, wake_flags);
 		return true;
@@ -4163,7 +4168,7 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 	 * scheduling.
 	 */
 	if (smp_load_acquire(&p->on_cpu) &&
-	    ttwu_queue_wakelist(p, task_cpu(p), wake_flags | WF_ON_CPU))
+	    ttwu_queue_wakelist(p, task_cpu(p), wake_flags))
 		goto unlock;
 
 	/*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 01259611beb9..1e34bb4527fd 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2039,7 +2039,6 @@ static inline int task_on_rq_migrating(struct task_struct *p)
 
 #define WF_SYNC     0x10 /* Waker goes to sleep after wakeup */
 #define WF_MIGRATED 0x20 /* Internal use, task got migrated */
-#define WF_ON_CPU   0x40 /* Wakee is on_cpu */
 
 #ifdef CONFIG_SMP
 static_assert(WF_EXEC == SD_BALANCE_EXEC);
-- 
2.27.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] sched: Fix the check of nr_running at queue wakelist
  2022-06-02  4:06 ` [PATCH v3 1/2] sched: Fix the check of nr_running at queue wakelist Tianchen Ding
@ 2022-06-06 10:39   ` Valentin Schneider
  2022-06-08 15:38     ` Tianchen Ding
  0 siblings, 1 reply; 6+ messages in thread
From: Valentin Schneider @ 2022-06-06 10:39 UTC (permalink / raw)
  To: Tianchen Ding, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Daniel Bristot de Oliveira
  Cc: linux-kernel

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 <vschneid@redhat.com>

> Signed-off-by: Tianchen Ding <dtcccc@linux.alibaba.com>
> ---
>  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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] sched: Remove the limitation of WF_ON_CPU on wakelist if wakee cpu is idle
  2022-06-02  4:06 ` [PATCH v3 2/2] sched: Remove the limitation of WF_ON_CPU on wakelist if wakee cpu is idle Tianchen Ding
@ 2022-06-06 10:39   ` Valentin Schneider
  0 siblings, 0 replies; 6+ messages in thread
From: Valentin Schneider @ 2022-06-06 10:39 UTC (permalink / raw)
  To: Tianchen Ding, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Daniel Bristot de Oliveira
  Cc: linux-kernel

On 02/06/22 12:06, Tianchen Ding wrote:

> Signed-off-by: Tianchen Ding <dtcccc@linux.alibaba.com>

Minor nits below, otherwise:

Reviewed-by: Valentin Schneider <vschneid@redhat.com>

> ---
>  kernel/sched/core.c  | 27 ++++++++++++++++-----------
>  kernel/sched/sched.h |  1 -
>  2 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a9efe134fbe5..ccb9e0fbf49b 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3808,7 +3808,7 @@ bool cpus_share_cache(int this_cpu, int that_cpu)
>  	return per_cpu(sd_llc_id, this_cpu) == per_cpu(sd_llc_id, that_cpu);
>  }
>  
> -static inline bool ttwu_queue_cond(int cpu, int wake_flags)
> +static inline bool ttwu_queue_cond(int cpu)
>  {
>  	/*
>  	 * Do not complicate things with the async wake_list while the CPU is
> @@ -3824,13 +3824,21 @@ static inline bool ttwu_queue_cond(int cpu, int wake_flags)
>  	if (!cpus_share_cache(smp_processor_id(), cpu))
>  		return true;
>  
> +	if (cpu == smp_processor_id())
> +		return false;
> +
>  	/*
> -	 * If the task is descheduling and the only running task on the
> -	 * CPU then use the wakelist to offload the task activation to
> -	 * the soon-to-be-idle CPU as the current CPU is likely busy.
> -	 * nr_running is checked to avoid unnecessary task stacking.
> +	 * If the wakee cpu is idle, or the task is descheduling and the
> +	 * only running task on the CPU, then use the wakelist to offload
> +	 * the task activation to the idle (or soon-to-be-idle) CPU as
> +	 * the current CPU is likely busy. nr_running is checked to
> +	 * avoid unnecessary task stacking.
> +	 *

> +	 * Note that we can only get here with (wakee) p->on_rq=0,
> +	 * p->on_cpu can be whatever, we've done the dequeue, so
> +	 * the wakee has been accounted out of ->nr_running.

If you want to include it, that comment should be added in patch 1 rather
than in patch 2.

>  	 */
> -	if ((wake_flags & WF_ON_CPU) && !cpu_rq(cpu)->nr_running)
> +	if (!cpu_rq(cpu)->nr_running)
>  		return true;
>  
>  	return false;
> @@ -3838,10 +3846,7 @@ static inline bool ttwu_queue_cond(int cpu, int wake_flags)
>  
>  static bool ttwu_queue_wakelist(struct task_struct *p, int cpu, int wake_flags)
>  {
> -	if (sched_feat(TTWU_QUEUE) && ttwu_queue_cond(cpu, wake_flags)) {
> -		if (WARN_ON_ONCE(cpu == smp_processor_id()))
> -			return false;
> -
> +	if (sched_feat(TTWU_QUEUE) && ttwu_queue_cond(cpu)) {
>  		sched_clock_cpu(cpu); /* Sync clocks across CPUs */
>  		__ttwu_queue_wakelist(p, cpu, wake_flags);
>  		return true;
> @@ -4163,7 +4168,7 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
>  	 * scheduling.
>  	 */
>  	if (smp_load_acquire(&p->on_cpu) &&
> -	    ttwu_queue_wakelist(p, task_cpu(p), wake_flags | WF_ON_CPU))
> +	    ttwu_queue_wakelist(p, task_cpu(p), wake_flags))
>  		goto unlock;
>  
>  	/*
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 01259611beb9..1e34bb4527fd 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2039,7 +2039,6 @@ static inline int task_on_rq_migrating(struct task_struct *p)
>  
>  #define WF_SYNC     0x10 /* Waker goes to sleep after wakeup */
>  #define WF_MIGRATED 0x20 /* Internal use, task got migrated */
> -#define WF_ON_CPU   0x40 /* Wakee is on_cpu */

There still is a reference to WF_ON_CPU in a comment in prepare_task(),
maybe change that one to "smp_load_acquire(&p->on_cpu)".

>  
>  #ifdef CONFIG_SMP
>  static_assert(WF_EXEC == SD_BALANCE_EXEC);
> -- 
> 2.27.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] sched: Fix the check of nr_running at queue wakelist
  2022-06-06 10:39   ` Valentin Schneider
@ 2022-06-08 15:38     ` Tianchen Ding
  0 siblings, 0 replies; 6+ messages in thread
From: Tianchen Ding @ 2022-06-08 15:38 UTC (permalink / raw)
  To: Valentin Schneider, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Daniel Bristot de Oliveira
  Cc: linux-kernel

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 <vschneid@redhat.com>
> 

Thanks. Will update my patch and send v4 soon.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-06-08 15:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-02  4:06 [PATCH v3 0/2] sched: Queue task on wakelist in the same llc if the wakee cpu is idle Tianchen Ding
2022-06-02  4:06 ` [PATCH v3 1/2] sched: Fix the check of nr_running at queue wakelist Tianchen Ding
2022-06-06 10:39   ` Valentin Schneider
2022-06-08 15:38     ` Tianchen Ding
2022-06-02  4:06 ` [PATCH v3 2/2] sched: Remove the limitation of WF_ON_CPU on wakelist if wakee cpu is idle Tianchen Ding
2022-06-06 10:39   ` Valentin Schneider

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®