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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
