>>> On 16.06.15 at 02:17, <feng...@intel.com> wrote: >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: Wednesday, June 10, 2015 2:44 PM >> >>> On 08.05.15 at 11:07, <feng...@intel.com> wrote: >> > + >> > + /* >> > + * Delete the vCPU from the related block list >> > + * if we are resuming from blocked state >> > + */ >> > + spin_lock_irqsave(&per_cpu(blocked_vcpu_lock, >> > + v->pre_pcpu), flags); >> > + list_del(&v->blocked_vcpu_list); >> > + spin_unlock_irqrestore(&per_cpu(blocked_vcpu_lock, >> > + v->pre_pcpu), flags); >> > + } >> > + break; >> > + >> > + case RUNSTATE_blocked: >> > + /* >> > + * The vCPU is blocked on the block list. >> > + * Add the blocked vCPU on the list of the >> > + * vcpu->pre_pcpu, which is the destination >> > + * of the wake-up notification event. >> > + */ >> > + v->pre_pcpu = v->processor; >> >> Is latching this upon runstate change really enough? I.e. what about >> the v->processor changes that sched_move_domain() or individual >> schedulers do? Or if it really just matters on which CPU's blocked list >> the vCPU is (while its ->processor changing subsequently doesn't >> matter) I'd like to see the field named more after its purpose (e.g. >> pi_block_cpu; list and lock should btw also have a connection to PI >> in their names). > > Yes, It doesn't matter if vCPU changes. The key point is that we put > the vCPU on a pCPU list and we change the NDST field to this pCPU, > then the wakeup notification event will get there. You are right, I > need to rename them to reflect the real purpose of it. > >> >> In the end, if the placement on a list followed v->processor, you >> would likely get away without the extra new field. Are there >> synchronization constraints speaking against such a model? > > I don't understand this quit well. Do you mean using 'v->processor' > as the pCPU list for the blocked vCPUs? Then what about 'v->processor' > changes, seems we cannot handle this case.
That was the question - does anything speak against such a model? >> > @@ -157,7 +158,11 @@ static inline void vcpu_runstate_change( >> > v->runstate.state_entry_time = new_entry_time; >> > } >> > >> > + old_state = v->runstate.state; >> > v->runstate.state = new_state; >> > + >> > + if ( is_hvm_vcpu(v) && hvm_funcs.pi_desc_update ) >> > + hvm_funcs.pi_desc_update(v, old_state); >> >> I don't see how this would build on ARM. > > So what about adding " #ifdef CONFIG_X86 ..." here? That would yield ugly code. If this needs to be here, you'll have to introduce a suitable arch_...() hook (doing nothing on ARM). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel