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

Re: [Xen-devel] [PATCH v2 31/48] xen/sched: add support for multiple vcpus per sched unit where missing



On 11.09.19 12:43, Jan Beulich wrote:
On 09.08.2019 16:58, Juergen Gross wrote:
V1:
- add special handling for idle unit in unit_runnable() and
   unit_runnable_state()

Why was this done? Isn't vcpu_runnable() going to always return
true for idle vCPU-s?

The problem is the for_each_sched_unit_vcpu() loop handling.

The loop is ending as soon as vcpu->sched_unit != unit. But this
might be true for idle vcpus in case they are temporarily active
for a normal domain's unit.


--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1255,7 +1255,10 @@ int vcpu_reset(struct vcpu *v)
      v->async_exception_mask = 0;
      memset(v->async_exception_state, 0, sizeof(v->async_exception_state));
  #endif
-    v->affinity_broken = 0;
+    if ( v->affinity_broken & VCPU_AFFINITY_OVERRIDE )
+        vcpu_temporary_affinity(v, NR_CPUS, VCPU_AFFINITY_OVERRIDE);
+    if ( v->affinity_broken & VCPU_AFFINITY_WAIT )
+        vcpu_temporary_affinity(v, NR_CPUS, VCPU_AFFINITY_WAIT);

Shouldn't this live in an earlier patch? I guess the same question
is applicable to almost everything here, as also already mentioned
e.g. in the previous patch.

Hmm, this _is_ a missing part for multiple vcpus per unit.

I wouldn't know which other patch would be more suitable.


  static inline void sched_set_res(struct sched_unit *unit,
                                   struct sched_resource *res)
  {
-    unit->vcpu_list->processor = res->processor;
+    int cpu = cpumask_first(res->cpus);

unsigned int

Yes.


  static inline void sched_set_pause_flags_atomic(struct sched_unit *unit,
                                                  unsigned int bit)
  {
-    set_bit(bit, &unit->vcpu_list->pause_flags);
+    struct vcpu *v;
+
+    for_each_sched_unit_vcpu ( unit, v )
+        set_bit(bit, &v->pause_flags);
  }
static inline void sched_clear_pause_flags_atomic(struct sched_unit *unit,
                                                    unsigned int bit)
  {
-    clear_bit(bit, &unit->vcpu_list->pause_flags);
+    struct vcpu *v;
+
+    for_each_sched_unit_vcpu ( unit, v )
+        clear_bit(bit, &v->pause_flags);
  }

The atomicity is (necessarily) limited here - it's atomic for an
individual vCPU, but not atomic for a unit as a whole. If this
is indeed a useful hybrid, then I think it would be nice if there
was a comment next to these functions clarifying under what
conditions use of them is correct without it also being correct
to use their non-atomic counterparts (e.g. due to use of an
external lock).

Okay, I'll add that.


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®.