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(-)
--
2.39.3
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(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 03be0d1330a6..5551ce2af73e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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);
}
--
2.39.3
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.
Fixes: eab03c23c2a1 ("sched/eevdf: Fix vruntime adjustment on reweight")
Signed-off-by: Tianchen Ding <[email protected]>
Reviewed-by: Abel Wu <[email protected]>
---
kernel/sched/fair.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5551ce2af73e..091a1a750638 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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);
--
2.39.3
On 3/6/24 10:21 AM, Tianchen Ding Wrote:
> 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(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 03be0d1330a6..5551ce2af73e 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -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);
> }
Reviewed-by: Abel Wu <[email protected]>
Thanks!
Hello Tianchen,
On 3/6/2024 7:51 AM, Tianchen Ding wrote:
> 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
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
Tested-by: K Prateek Nayak <[email protected]>
>
> 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.
> 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(-)
>
On 2024-03-06 at 10:21:31 +0800, Tianchen Ding wrote:
> 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(-)
>
> --
> 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,
Tested-by: Chen Yu <[email protected]>
thanks,
Chenyu
On Fri, Apr 19, 2024 at 03:27:30PM +0800, Chen Yu wrote:
> On 2024-03-06 at 10:21:31 +0800, Tianchen Ding wrote:
> > 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(-)
> >
> > --
> > 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,
>
> 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.
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.
Fixes: eab03c23c2a1 ("sched/eevdf: Fix vruntime adjustment on reweight")
Signed-off-by: Tianchen Ding <[email protected]>
[peterz: flip if() condition for clarity]
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Abel Wu <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5551ce2..6d26691 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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);
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.
Fixes: eab03c23c2a1 ("sched/eevdf: Fix vruntime adjustment on reweight")
Suggested-by: Abel Wu <[email protected]>
Signed-off-by: Tianchen Ding <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Abel Wu <[email protected]>
Tested-by: K Prateek Nayak <[email protected]>
Tested-by: Chen Yu <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 03be0d1..5551ce2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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);
}