sched/eevdf: Minor fixes for reweight_entity()
v2 cover sched/eevdf: Minor fixes for reweight_entity()
According to the discussion in [1], fix a calculation bug in reweight_entity().
[1] https://lore.kernel.org/all/[email protected]/
v2:
Add fixes tag to the 1st patch.
v1: https://lore.kernel.org/all/[email protected]/
Tianchen Ding (2):
sched/eevdf: Always update V if se->on_rq when reweighting sched/eevdf: Fix miscalculation in reweight_entity() when se is not
curr
kernel/sched/fair.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Signature · 1 line
2.39.3
PATCH v2 1/2 sched/eevdf: Always update V if se->on_rq when reweighting
reweight_eevdf() needs the latest V to do accurate calculation for new ve and vd. So update V unconditionally when se is runnable.
--- kernel/sched/fair.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
kernel/sched/fair.c +2 -3
@@ -3790,9 +3790,8 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,if (se->on_rq) {/* commit outstanding execution time */ -if (curr) -update_curr (cfs_rq); -else +update_curr (cfs_rq); +if (!curr)__dequeue_entity (cfs_rq, se);update_load_sub (&cfs_rq->load, se->load.weight); }
Signature · 1 line
2.39.3
Re: [PATCH v2 1/2] sched/eevdf: Always update V if se->on_rq when reweighting
On 3/6/24 10:21 AM, Tianchen Ding Wrote:
Quoted 16 lines
reweight_eevdf()needs the latest V to do accurate calculation for new ve and vd. So update V unconditionally when se is runnable.Fixes eab03c23c2a1 ("sched/eevdf: Fix vruntime adjustment on reweight")Suggested-by Abel Wu <[email protected]>Signed-off-by Tianchen Ding <[email protected]>--- kernel/sched/fair.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)kernel/sched/fair.c +0
-0@@ -3790,9 +3790,8 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
kernel/sched/fair.c static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, · line 3797
Quoted 10 lines
kernel/sched/fair.c +2
-3if (se->on_rq) {/* commit outstanding execution time */ -if (curr) -update_curr (cfs_rq); -else +update_curr (cfs_rq); +if (!curr)__dequeue_entity (cfs_rq, se);update_load_sub (&cfs_rq->load, se->load.weight); }
Thanks!
[tip: sched/urgent] sched/eevdf: Always update V if se->on_rq when reweighting
In reply to Tianchen DingThe following commit has been merged into the sched/urgent branch of tip:
The following commit has been merged into the sched/urgent branch of tip:
Commit-ID: 11b1b8bc2b98e21ddf47e08b56c21502c685b2c3 Gitweb: https://git.kernel.org/tip/11b1b8bc2b98e21ddf47e08b56c21502c685b2c3 Author: Tianchen Ding <[email protected]> AuthorDate: Wed, 06 Mar 2024 10:21:32 +08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Mon, 22 Apr 2024 13:01:26 +02:00
sched/eevdf: Always update V if se->on_rq when reweighting
reweight_eevdf() needs the latest V to do accurate calculation for new ve and vd. So update V unconditionally when se is runnable.
--- kernel/sched/fair.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
kernel/sched/fair.c +2 -3
@@ -3790,9 +3790,8 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,if (se->on_rq) {/* commit outstanding execution time */ -if (curr) -update_curr (cfs_rq); -else +update_curr (cfs_rq); +if (!curr)__dequeue_entity (cfs_rq, se);update_load_sub (&cfs_rq->load, se->load.weight); }
PATCH v2 2/2 sched/eevdf: Fix miscalculation in reweight_entity() when se is not curr
In reply to Tianchen Dingreweight_eevdf() only keeps V unchanged inside itself. When se != cfs_rq->curr, it would be dequeued from rb tree first. So that V is changed and the result is wrong. Pass the original V to reweight_eevdf() to fix this issue.
--- kernel/sched/fair.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
kernel/sched/fair.c +4 -3
@@ -3676,11 +3676,10 @@ static inline void dequeue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) { }#endif -static void reweight_eevdf (struct cfs_rq *cfs_rq,struct sched_entity *se, +static void reweight_eevdf (struct sched_entity *se,u64 avruntime,unsigned long weight) {unsigned long old_weight = se->load.weight; -u64 avruntime =avg_vruntime (cfs_rq);s64 vlag, vslice;/*
@@ -3787,10 +3786,12 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, unsigned long weight) {bool curr = cfs_rq->curr == se; +u64 avruntime;if (se->on_rq) {/* commit outstanding execution time */ update_curr (cfs_rq); + avruntime =avg_vruntime (cfs_rq);if (!curr)__dequeue_entity (cfs_rq, se);update_load_sub (&cfs_rq->load, se->load.weight);
@@ -3804,7 +3805,7 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,*/ se->vlag =div_s64 (se->vlag * se->load.weight, weight); }else { -reweight_eevdf (cfs_rq, se, weight); +reweight_eevdf (se, avruntime, weight); }update_load_set (&se->load, weight);
Signature · 1 line
2.39.3
[tip: sched/urgent] sched/eevdf: Fix miscalculation in reweight_entity() when se is not curr
The following commit has been merged into the sched/urgent branch of tip:
The following commit has been merged into the sched/urgent branch of tip:
Commit-ID: afae8002b4fd3560c8f5f1567f3c3202c30a70fa Gitweb: https://git.kernel.org/tip/afae8002b4fd3560c8f5f1567f3c3202c30a70fa Author: Tianchen Ding <[email protected]> AuthorDate: Wed, 06 Mar 2024 10:21:33 +08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Mon, 22 Apr 2024 13:01:26 +02:00
sched/eevdf: Fix miscalculation in reweight_entity() when se is not curr
reweight_eevdf() only keeps V unchanged inside itself. When se != cfs_rq->curr, it would be dequeued from rb tree first. So that V is changed and the result is wrong. Pass the original V to reweight_eevdf() to fix this issue.
--- kernel/sched/fair.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
kernel/sched/fair.c +6 -5
@@ -3676,11 +3676,10 @@ static inline void dequeue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) { }#endif -static void reweight_eevdf (struct cfs_rq *cfs_rq,struct sched_entity *se, +static void reweight_eevdf (struct sched_entity *se,u64 avruntime,unsigned long weight) {unsigned long old_weight = se->load.weight; -u64 avruntime =avg_vruntime (cfs_rq);s64 vlag, vslice;/*
@@ -3787,24 +3786,26 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, unsigned long weight) {bool curr = cfs_rq->curr == se; +u64 avruntime;if (se->on_rq) {/* commit outstanding execution time */ update_curr (cfs_rq); + avruntime =avg_vruntime (cfs_rq);if (!curr)__dequeue_entity (cfs_rq, se);update_load_sub (&cfs_rq->load, se->load.weight); }dequeue_load_avg (cfs_rq, se); -if (!se->on_rq) { +if (se->on_rq) { +reweight_eevdf (se, avruntime, weight); + }else {/* * Because we keep se->vlag = V - v_i, while: lag_i = w_i*(V - v_i), * we need to scale se->vlag when w_i changes. */ se->vlag =div_s64 (se->vlag * se->load.weight, weight); - }else { -reweight_eevdf (cfs_rq, se, weight); }update_load_set (&se->load, weight);
Hello Tianchen,
Quoted 8 lines · Tianchen Ding
According to the discussion in [1], fix a calculation bug in
reweight_entity().[1] https://lore.kernel.org/all/[email protected]/
v2:
Add fixes tag to the 1st patch.
Quoted 6 lines
v1: https://lore.kernel.org/all/[email protected]/
Tianchen Ding (2):
sched/eevdf: Always update V ifse->on_rqwhen reweighting sched/eevdf: Fix miscalculation inreweight_entity()when se is not
curr
Tested the changes on a dual socket 3rd Generation EPYC system and am happy to inform that I see no regressions. I'll leave the full results below:
o System Details
- 3rd Generation EPYC System
- 2 x 64C/128T
- NPS1 mode
o Kernels
tip: tip:sched/core at commit 8cec3dd9e593
("sched/core: Simplify code by removing
duplicate #ifdefs")
reweight-fix: tip + this series.
o Results
==================================================================
Test : hackbench
Units : Normalized time in seconds
Interpretation: Lower is better
Statistic : AMean
==================================================================
Case: tip[pct imp](CV) reweight-fix[pct imp](CV)
1-groups 1.00 [ -0.00]( 2.08) 1.01 [ -1.06]( 4.18) 2-groups 1.00 [ -0.00]( 0.89) 1.02 [ -1.58]( 1.18) 4-groups 1.00 [ -0.00]( 0.81) 1.01 [ -1.05]( 0.81) 8-groups 1.00 [ -0.00]( 0.78) 1.00 [ 0.15]( 0.72) 16-groups 1.00 [ -0.00]( 1.60) 0.99 [ 1.20]( 1.55)
==================================================================
Test : tbench
Units : Normalized throughput
Interpretation: Higher is better
Statistic : AMean
==================================================================
Clients: tip[pct imp](CV) reweight-fix[pct imp](CV)
1 1.00 [ 0.00]( 0.71) 1.00 [ 0.36]( 0.12)
2 1.00 [ 0.00]( 0.25) 0.99 [ -0.52]( 0.26)
4 1.00 [ 0.00]( 0.85) 1.00 [ -0.35]( 0.93)
8 1.00 [ 0.00]( 1.00) 1.00 [ -0.27]( 0.56)
16 1.00 [ 0.00]( 1.25) 1.02 [ 1.73]( 0.63)
32 1.00 [ 0.00]( 0.35) 0.99 [ -0.75]( 1.01)
64 1.00 [ 0.00]( 0.71) 0.99 [ -1.00]( 0.51)
128 1.00 [ 0.00]( 0.46) 1.00 [ 0.29]( 0.56)
256 1.00 [ 0.00]( 0.24) 1.00 [ 0.08]( 1.07)
512 1.00 [ 0.00]( 0.30) 1.01 [ 1.16]( 0.28)
1024 1.00 [ 0.00]( 0.40) 1.01 [ 0.81]( 0.43)==================================================================
Test : stream-10
Units : Normalized Bandwidth, MB/s
Interpretation: Higher is better
Statistic : HMean
==================================================================
Test: tip[pct imp](CV) reweight-fix[pct imp](CV)
Copy 1.00 [ 0.00]( 9.73) 1.05 [ 4.93]( 1.81) Scale 1.00 [ 0.00]( 5.57) 0.97 [ -2.57]( 6.28) Add 1.00 [ 0.00]( 5.43) 0.98 [ -2.45]( 5.87) Triad 1.00 [ 0.00]( 5.50) 1.02 [ 1.85]( 3.19)
==================================================================
Test : stream-100
Units : Normalized Bandwidth, MB/s
Interpretation: Higher is better
Statistic : HMean
==================================================================
Test: tip[pct imp](CV) reweight-fix[pct imp](CV)
Copy 1.00 [ 0.00]( 3.26) 1.01 [ 0.54]( 2.31) Scale 1.00 [ 0.00]( 1.26) 0.99 [ -0.68]( 3.86) Add 1.00 [ 0.00]( 1.47) 0.99 [ -1.13]( 4.32) Triad 1.00 [ 0.00]( 1.77) 1.01 [ 0.81]( 3.03)
==================================================================
Test : netperf
Units : Normalized Througput
Interpretation: Higher is better
Statistic : AMean
==================================================================
Clients: tip[pct imp](CV) reweight-fix[pct imp](CV)
1-clients 1.00 [ 0.00]( 0.22) 1.00 [ -0.35]( 0.46) 2-clients 1.00 [ 0.00]( 0.57) 1.00 [ -0.12]( 0.16) 4-clients 1.00 [ 0.00]( 0.43) 1.00 [ -0.38]( 0.40) 8-clients 1.00 [ 0.00]( 0.27) 1.00 [ -0.33]( 0.59) 16-clients 1.00 [ 0.00]( 0.46) 1.00 [ -0.15]( 0.39) 32-clients 1.00 [ 0.00]( 0.95) 1.00 [ -0.26]( 0.60) 64-clients 1.00 [ 0.00]( 1.79) 1.00 [ 0.19]( 1.76) 128-clients 1.00 [ 0.00]( 0.89) 1.00 [ 0.45]( 0.81) 256-clients 1.00 [ 0.00]( 3.88) 1.00 [ 0.42]( 5.53) 512-clients 1.00 [ 0.00](35.06) 1.03 [ 3.05](53.21)
==================================================================
Test : schbench
Units : Normalized 99th percentile latency in us
Interpretation: Lower is better
Statistic : Median
==================================================================
#workers: tip[pct imp](CV) reweight-fix[pct imp](CV)
1 1.00 [ -0.00](27.28) 0.81 [ 18.75](29.23) 2 1.00 [ -0.00]( 3.85) 1.05 [ -5.00]( 4.76) 4 1.00 [ -0.00](14.00) 0.95 [ 5.26](17.91) 8 1.00 [ -0.00]( 4.68) 0.85 [ 14.58]( 4.12) 16 1.00 [ -0.00]( 4.08) 0.95 [ 4.84]( 2.60) 32 1.00 [ -0.00]( 6.68) 0.94 [ 6.12]( 2.28) 64 1.00 [ -0.00]( 1.79) 0.99 [ 1.02]( 1.08) 128 1.00 [ -0.00]( 6.30) 0.99 [ 1.13]( 3.68) 256 1.00 [ -0.00](43.39) 1.11 [-10.71](10.31) 512 1.00 [ -0.00]( 2.26) 1.00 [ -0.47]( 1.57)
==================================================================
Test : Unixbench
Units : Normalized scores
Interpretation: Lower is better
Statistic : Various (Mentioned)
==================================================================
kernel tip reweight-fix
Hmean unixbench-dhry2reg-1 0.00% -0.60% Hmean unixbench-dhry2reg-512 0.00% -0.10% Amean unixbench-syscall-1 0.00% 1.61% Amean unixbench-syscall-512 0.00% 4.94% Hmean unixbench-pipe-1 0.00% 1.01% Hmean unixbench-pipe-512 0.00% 0.73% Hmean unixbench-spawn-1 0.00% -5.24% Hmean unixbench-spawn-512 0.00% 2.83% Hmean unixbench-execl-1 0.00% 0.20% Hmean unixbench-execl-512 0.00% 0.10%
--
Feel free to include
Quoted 4 lines
kernel/sched/fair.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
--
Thanks and Regards,
Prateek
On 2024/3/6 10:21, Tianchen Ding wrote:
Gentle ping.
Quoted 18 lines
According to the discussion in [1], fix a calculation bug in
reweight_entity().[1] https://lore.kernel.org/all/[email protected]/
v2:
Add fixes tag to the 1st patch.v1: https://lore.kernel.org/all/[email protected]/
Tianchen Ding (2):
sched/eevdf: Always update V ifse->on_rqwhen reweighting sched/eevdf: Fix miscalculation inreweight_entity()when se is not
currkernel/sched/fair.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Quoted 10 lines · Tianchen Ding
According to the discussion in [1], fix a calculation bug in
reweight_entity().[1] https://lore.kernel.org/all/[email protected]/
v2:
Add fixes tag to the 1st patch.
Quoted 11 lines
Tianchen Ding (2):
sched/eevdf: Always update V ifse->on_rqwhen reweighting sched/eevdf: Fix miscalculation inreweight_entity()when se is not
currkernel/sched/fair.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)Signature · 1 line
2.39.3
Applied this patch set on top of the reweight_entity fix at
https://lore.kernel.org/lkml/ZiEaKOQwiNEglYtS@chenyu5-mobl2/
it has passed 100 cycles of trinity test without any issue.
For the whole serie,
thanks,
Chenyu
Peter Zijlstra
maintainerQuoted 23 lines · Chen Yu
Quoted 10 lines · Tianchen Ding
According to the discussion in [1], fix a calculation bug in
reweight_entity().[1] https://lore.kernel.org/all/[email protected]/
v2:
Add fixes tag to the 1st patch.Quoted 11 lines
Tianchen Ding (2):
sched/eevdf: Always update V ifse->on_rqwhen reweighting sched/eevdf: Fix miscalculation inreweight_entity()when se is not
currkernel/sched/fair.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)Signature · 1 line
2.39.3
Quoted 7 lines
Applied this patch set on top of the
reweight_entityfix at
https://lore.kernel.org/lkml/ZiEaKOQwiNEglYtS@chenyu5-mobl2/
it has passed 100 cycles of trinity test without any issue.For the whole serie,
Tested-by Chen Yu <[email protected]>
OK, grabbed these patches (and made some minor edits), now let me go stare at that other thread again.