Re: [Xen-devel] [v4 16/17] vmx: Add some scheduler hooks for VT-d posted interrupts

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.07.15 at 20:26, <dario.faggioli@xxxxxxxxxx> wrote:
> On Thu, 2015-07-30 at 02:04 +0000, Wu, Feng wrote:
>> > -----Original Message-----
>> > From: Dario Faggioli [mailto:dario.faggioli@xxxxxxxxxx]
>> > Since this is one of the differences
>> > between the two, was it the cause of the issues you were seeing? If yes,
>> > can you elaborate on how and why?
>> > 
>> > In the end, I'm not too opposed to the hook being at the beginning
>> > rather than at the end, but there has to be a reason, which may well end
>> > up better be stated in a comment...
>> 
>> Here is the reason I put arch_vcpu_wake() ahead of vcpu_wake():
>> arch_vcpu_wake() does some prerequisites for a vCPU which is about
>> to run, such as, setting SN again, changing NV filed back to
>> ' posted_intr_vector ', which should be finished before the vCPU is
>> actually scheduled to run. However, if we put arch_vcpu_wake() later
>> in vcpu_wake() right before ' vcpu_schedule_unlock_irqrestore', after
>> the 'wake' hook get finished, the vcpu can run at any time (maybe in
>> another pCPU since the current pCPU is protected by the lock), if
>> this can happen, it is incorrect. Does my understanding make sense?
>> 
> It's safe in any case. In fact, the spinlock will  prevent both the
> vcpu's processor to schedule, as well as any other processors to steal
> the waking vcpu from the runqueue to run it.
> 
> That's actually why I wanted to double check you changing the position
> of the hook (wrt the draft), as it felt weird that the issue were in
> there. :-)
> 
> So, now that we know that safety is not an issue, where should we put
> the hook?
> 
> Having it before SCHED_OP(wake) may make people think that arch specific
> code is (or can, at some point) somehow influencing the scheduler
> specific wakeup code, which is not (and should not become, if possible)
> the case.
> 
> However, I kind of like the fact that the spinlock is released as soon
> as possible, after the call to SCHED_OP(wake). That will make it more
> likely, for the processors we may have sent IPIs to, during the
> scheduler specific wakeup code, to find the spinlock free. So, looking
> at things from this angle, it would be better to avoid putting stuff in
> between SCHED_OP(wake) and vcpu_schedule_unlock().
> 
> So, all in all, I'd say leave it on top, where it is in this patch. Of
> course, if others have opinions, I'm all ears. :-)

If it is kept at the beginning, the hook should be renamed to
something like arch_vcpu_wake_prepare().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel