Re: [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2882 bytes --]

On Wed, 2015-09-16 at 19:18 +0200, Dario Faggioli wrote:
> On Fri, 2015-09-11 at 16:29 +0800, Feng Wu wrote:
> > One remaining item:
> > Jan has concern about calling vcpu_unblock() in vmx_pre_ctx_switch_pi(),
> > need Dario or George's input about this.
> > 
> Hi,
> 
> Sorry for the delay in replying, I was on PTO for a few time.
> 
> Coming to the issue, well, it's a though call.
> 
> First of all, Feng, have you tested this with a debug build of Xen? I'm
> asking because it looks to me that you're ending up calling vcpu_wake()
> with IRQ disabled which, if my brain is not too rusty after a few weeks
> of vacation, should result in check_lock() (in xen/common/spinlock.c)
> complaining, doesn't it?
> 
Mmm.. My bad (so, yes, I'm a bit rusty, I guess). check_lock(), in case
of spin_lock_irqsave(), is called _after_ local_irq_disable(), inside
_spin_lock_irqsave(), so this above should not be an issue. Sorry for
the noise. :-(

The rest of what's said below, and the fact that I agree with George's
and Jan's concerns, still stand. :-)

So, in particular...

> In fact, in principle this is not too much different from what happens
> in other places. More specifically, what we have is a vcpu being
> re-inserted in  a runqueue, and the need for re-running the scheduler on
> a(some) PCPU(s) is evaluated. That is similar to what happens in Credit2
> (and in RTDS) in csched2_context_saved(), which is called from within
> context_saved(), still from the context switch code (if
> __CSFLAG_delayed_runq_add is true).
> 
> So it's not the thing per se that is that terrible, IMO. The differences
> between that and your case are:
>  - in the Credit2 case, it happens later down in the context switch
>    path (which would look already better to me) and, more important,
>    with IRQs already re-enabled;
>  - in the Credit2 case, the effect that something like that can have on 
>    the scheduler is much more evident, as it happens inside a scheduler
>    hook, rather than buried down in arch specific code, which makes me a
>    lot less concerned about the possibility of latent issues Jan was
>    hinting at, with which I concur.
> 
> So, I guess, first of all, can you confirm whether or not it's exploding
> in debug builds? And in either case (just tossing out ideas) would it be
> possible to deal with the "interrupt already raised when blocking" case:
>  - later in the context switching function ?
>  - with another hook, perhaps in vcpu_block() ?
> 
... let me/us know what you think about these alternatives.

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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