diff mbox series

[v2] sched/credit: avoid priority boost for capped domains when unpark

Message ID 20190503194349.42831-1-elnikety@amazon.com (mailing list archive)
State New, archived
Headers show
Series [v2] sched/credit: avoid priority boost for capped domains when unpark | expand

Commit Message

Eslam Elnikety May 3, 2019, 7:43 p.m. UTC 
When unpausing a capped domain, the scheduler currently clears the
CSCHED_FLAG_VCPU_PARKED flag before vcpu_wake(). This, in turn, causes the
vcpu_wake to set CSCHED_PRI_TS_BOOST, resulting in an unfair credit boost. The
comment around the changed lines already states that clearing the flag should
happen AFTER the unpause. This bug was introduced in commit be650750945
"credit1: Use atomic bit operations for the flags structure".

Original patch author credit: Xi Xiong while at Amazon.

Signed-off-by: Eslam Elnikety <elnikety@amazon.com>
Reviewed-by: Leonard Foerster <foersleo@amazon.de>
Reviewed-by: Petre Eftime <epetre@amazon.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>

---
    Changes in v2:
        - Adjusted commit message, adding Xi's previous affiliation
        - Dropped comment: /* Now clear the PARKED flag */
        - Added Dario's A-b
---
 xen/common/sched_credit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andrew Cooper May 6, 2019, 3:35 p.m. UTC | #1 
On 03/05/2019 20:43, Eslam Elnikety wrote:
> When unpausing a capped domain, the scheduler currently clears the
> CSCHED_FLAG_VCPU_PARKED flag before vcpu_wake(). This, in turn, causes the
> vcpu_wake to set CSCHED_PRI_TS_BOOST, resulting in an unfair credit boost. The
> comment around the changed lines already states that clearing the flag should
> happen AFTER the unpause. This bug was introduced in commit be650750945
> "credit1: Use atomic bit operations for the flags structure".
>
> Original patch author credit: Xi Xiong while at Amazon.
>
> Signed-off-by: Eslam Elnikety <elnikety@amazon.com>
> Reviewed-by: Leonard Foerster <foersleo@amazon.de>
> Reviewed-by: Petre Eftime <epetre@amazon.com>
> Acked-by: Dario Faggioli <dfaggioli@suse.com>

Pulled into x86-next.

~Andrew
diff mbox series

Patch

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 3abe20def8..7b7facbace 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1538,7 +1538,7 @@  csched_acct(void* dummy)
                 svc->pri = CSCHED_PRI_TS_UNDER;
 
                 /* Unpark any capped domains whose credits go positive */
-                if ( test_and_clear_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
+                if ( test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
                 {
                     /*
                      * It's important to unset the flag AFTER the unpause()
@@ -1547,6 +1547,7 @@  csched_acct(void* dummy)
                      */
                     SCHED_STAT_CRANK(vcpu_unpark);
                     vcpu_unpause(svc->vcpu);
+                    clear_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags);
                 }
 
                 /* Upper bound on credits means VCPU stops earning */