[Xen-devel] [PATCH v4] schedule: move credit scheduler specific member to its privates

Andrii Anisov posted 1 patch 13 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/1560332150-27712-1-git-send-email-andrii.anisov@gmail.com
xen/common/sched_credit.c | 11 ++++++++---
xen/common/schedule.c     |  1 -
xen/include/xen/sched.h   |  3 ---
3 files changed, 8 insertions(+), 7 deletions(-)

[Xen-devel] [PATCH v4] schedule: move credit scheduler specific member to its privates

Posted by Andrii Anisov 13 weeks ago
From: Andrii Anisov <andrii_anisov@epam.com>

The vcpu structure member last_run_time is used by credit scheduler only.
In order to get better encapsulation, it is moved from a generic
structure to the credit scheduler private vcpu definition. Also, rename
the member to last_sched_time in order to reflect that it is the time
when the vcpu went through the scheduling path.

With this move we have slight changes in functionality:
 - last_sched_time is not updated for an idle vcpu. But the idle vcpu is,
   in fact, a per-pcpu stub and never migrates so last_sched_time is
   meaningless for it.
 - The value of last_sched_time is updated on every schedule, even if the
   vcpu is not being changed. It is still ok, because last_sched_time is
   only used for runnable vcpu migration decision, and we have it correct
   at that moment. Scheduling parameters and statistics are tracked by
   other entities.

Reducing code and data usage when not running credit scheduler is another
nice side effect.

While here, also:
  - turn last_sched_time into s_time_t, which is more appropriate.
  - properly const-ify related argument of __csched_vcpu_is_cache_hot().

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
Changes in
 v4:
    - The member renamed to last_sched_time
    - removed a misleading comment
    - commit title and message updated describing member rename, 
      change motivation and a side effect
 v3:
    - commit message updated accordingly to [1]
 v2:
    - last_run_time type changed to s_time_t
    - scurr changed to svc
    - dropped stray blanks
    - pointers to const are used appropriately

[1] https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01201.html

---
 xen/common/sched_credit.c | 11 ++++++++---
 xen/common/schedule.c     |  1 -
 xen/include/xen/sched.h   |  3 ---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 7b7facb..07e442c 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -175,6 +175,8 @@ struct csched_vcpu {
     atomic_t credit;
     unsigned int residual;
 
+    s_time_t last_sched_time;
+
 #ifdef CSCHED_STATS
     struct {
         int credit_last;
@@ -701,10 +703,11 @@ static unsigned int vcpu_migration_delay_us;
 integer_param("vcpu_migration_delay", vcpu_migration_delay_us);
 
 static inline bool
-__csched_vcpu_is_cache_hot(const struct csched_private *prv, struct vcpu *v)
+__csched_vcpu_is_cache_hot(const struct csched_private *prv,
+                           const struct csched_vcpu *svc)
 {
     bool hot = prv->vcpu_migr_delay &&
-               (NOW() - v->last_run_time) < prv->vcpu_migr_delay;
+               (NOW() - svc->last_sched_time) < prv->vcpu_migr_delay;
 
     if ( hot )
         SCHED_STAT_CRANK(vcpu_hot);
@@ -716,6 +719,7 @@ static inline int
 __csched_vcpu_is_migrateable(const struct csched_private *prv, struct vcpu *vc,
                              int dest_cpu, cpumask_t *mask)
 {
+    const struct csched_vcpu *svc = CSCHED_VCPU(vc);
     /*
      * Don't pick up work that's hot on peer PCPU, or that can't (or
      * would prefer not to) run on cpu.
@@ -725,7 +729,7 @@ __csched_vcpu_is_migrateable(const struct csched_private *prv, struct vcpu *vc,
      */
     ASSERT(!vc->is_running);
 
-    return !__csched_vcpu_is_cache_hot(prv, vc) &&
+    return !__csched_vcpu_is_cache_hot(prv, svc) &&
            cpumask_test_cpu(dest_cpu, mask);
 }
 
@@ -1870,6 +1874,7 @@ csched_schedule(
         /* Update credits of a non-idle VCPU. */
         burn_credits(scurr, now);
         scurr->start_time -= now;
+        scurr->last_sched_time = now;
     }
     else
     {
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index ba942a7..047f767 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1486,7 +1486,6 @@ static void schedule(void)
         ((prev->pause_flags & VPF_blocked) ? RUNSTATE_blocked :
          (vcpu_runnable(prev) ? RUNSTATE_runnable : RUNSTATE_offline)),
         now);
-    prev->last_run_time = now;
 
     ASSERT(next->runstate.state != RUNSTATE_running);
     vcpu_runstate_change(next, RUNSTATE_running, now);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index ccd5347..97a3ab5 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -174,9 +174,6 @@ struct vcpu
     } runstate_guest; /* guest address */
 #endif
 
-    /* last time when vCPU is scheduled out */
-    uint64_t last_run_time;
-
     /* Has the FPU been initialised? */
     bool             fpu_initialised;
     /* Has the FPU been used since it was last saved? */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4] schedule: move credit scheduler specific member to its privates

Posted by Dario Faggioli 13 weeks ago
On Wed, 2019-06-12 at 12:35 +0300, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> The vcpu structure member last_run_time is used by credit scheduler
> only.
> In order to get better encapsulation, it is moved from a generic
> structure to the credit scheduler private vcpu definition. Also,
> rename
> the member to last_sched_time in order to reflect that it is the time
> when the vcpu went through the scheduling path.
> 
> With this move we have slight changes in functionality:
>  - last_sched_time is not updated for an idle vcpu. But the idle vcpu
> is,
>    in fact, a per-pcpu stub and never migrates so last_sched_time is
>    meaningless for it.
>  - The value of last_sched_time is updated on every schedule, even if
> the
>    vcpu is not being changed. It is still ok, because last_sched_time
> is
>    only used for runnable vcpu migration decision, and we have it
> correct
>    at that moment. Scheduling parameters and statistics are tracked
> by
>    other entities.
> 
> Reducing code and data usage when not running credit scheduler is
> another
> nice side effect.
> 
> While here, also:
>   - turn last_sched_time into s_time_t, which is more appropriate.
>   - properly const-ify related argument of
> __csched_vcpu_is_cache_hot().
> 
> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>

Thanks and Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel