[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 37/47] xen/sched: move per-cpu variable scheduler to struct sched_resource



On 24.09.19 14:16, Jan Beulich wrote:
On 14.09.2019 10:52, Juergen Gross wrote:
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -352,9 +352,10 @@ DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
  static inline void __runq_tickle(struct csched_unit *new)
  {
      unsigned int cpu = sched_unit_cpu(new->unit);
+    struct sched_resource *sd = get_sched_res(cpu);
      struct sched_unit *unit = new->unit;
      struct csched_unit * const cur = CSCHED_UNIT(curr_on_cpu(cpu));
-    struct csched_private *prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
+    struct csched_private *prv = CSCHED_PRIV(sd->scheduler);

Is the single use "sd" (and, as mentioned elsewhere, mis-named)
really worth it? (Applicable further down as well.)

@@ -931,7 +932,8 @@ csched_unit_acct(struct csched_private *prv, unsigned int 
cpu)
  {
      struct sched_unit *currunit = current->sched_unit;
      struct csched_unit * const svc = CSCHED_UNIT(currunit);
-    const struct scheduler *ops = per_cpu(scheduler, cpu);
+    struct sched_resource *sd = get_sched_res(cpu);
+    const struct scheduler *ops = sd->scheduler;
ASSERT( sched_unit_cpu(currunit) == cpu );
      ASSERT( svc->sdom != NULL );
@@ -987,8 +989,7 @@ csched_unit_acct(struct csched_private *prv, unsigned int 
cpu)
               * idlers. But, if we are here, it means there is someone running
               * on it, and hence the bit must be zero already.
               */
-            ASSERT(!cpumask_test_cpu(cpu,
-                                     CSCHED_PRIV(per_cpu(scheduler, 
cpu))->idlers));
+            ASSERT(!cpumask_test_cpu(cpu, CSCHED_PRIV(sd->scheduler)->idlers));

While at the first glance "sd" is used twice here, this 2nd use is
actually unnecessary - "cpu" hasn't changed from the beginning of
the function afaics, and hence "ops" could be used here.

"sd" is now "sr" everywhere.

And yes, using ops here seems okay.


Preferably with the suggested adjustments (but final judgement is
with the scheduler maintainers anyway)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks,


Juergen

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.