Re: [Xen-devel] [PATCH 1/4] xen/arm: gic: Ensure we have an ISB between ack and do_IRQ()

[Julien Grall <julien.grall@xxxxxxx>]

On 11/27/18 1:30 PM, Andrii Anisov wrote:
> Hello Julien,
Hi Andrii,

> On 23.11.18 15:27, Julien Grall wrote:
> > > But we can't use it, because our system is overcommitted.
> > Can you describe how overcommitted your system is?
> I did mean that we have a requirement to not limit VCPU number with PCPU 
> number. Also we should not use cpu pinning. I guess I used a wrong word, 
> it must be "oversubscribed".
Oversubscribing is usually a pretty bad idea if you want to have good 
latency. There are no promise when the vCPU will get scheduled.
Furthermore, if you want to have more vCPU than pCPU, then you still 
need to make sure that the total amount of vCPU usage is always lower 
(or equal) than the total capacity of your pCPUs.
So can you describe how oversubscribed your platform is when doing the 
benchmark?
> > I don't understand what you mean. Can you details it?
> I hope you do not mind I draw a picture for this. I failed to find a 
> simple wording for it :(
> At some rate of IRQs from different sources, slight reducing of IRQ 
> processing time in the hypervisor might result in an overly slower IRQs 
> processing.
> So, on the picture attaches, in case2 IRQ processing path in XEN made 
> shorter. But when we have IRQ1 and IRQ2 asserted at some rate, we result 
> in two context switches and going through the IRQ processing path twice. 
> What is longer than catching the IRQ2 right away with IRQ1 in XEN itself.
> We come to this idea after observing a strange effect: dropping a bit of 
> code from IRQ processing path (i.e. several if's) led to the benchmark 
> shown us a bit smaller numbers.
I think I now understand your problem. The problem is not because of 
re-enabling the interrupt. Instead, it is because the GIC CPU priority 
is been dropped using EOI early (via desc->handler->end()). As soon as 
you drop the priority another interrupt with the same (or lower) 
priority can fire.
Looking at do_IRQ, we don't handle the same way guest IRQ and Xen IRQ.

The steps for Xen IRQ is roughly:
        -> read_irq
        -> local_irq_enable
        -> do_IRQ
           -> local_irq_enable (via spin_unlock_irq)
           -> call handlers
           -> local_irq_disable (via spin_lock_irq)
           -> EOI
           -> DIR
        -> local_irq_disable

The steps of for Guest IRQ is roughly:
        -> read_irq
        -> local_irq_enable
        -> do_IRQ
                -> EOI
                -> vgic_inject_irq
                        -> local_irq_disable  (via spin_lock_irqsave)
                        -> local_irq_enable   (via spin_lock_irqrestore)
        -> local_irq_disable

All vgic_inject_irq is pretty much running with interrupts disabled. The 
Xen IRQ path seem to continue pointless enable/disable.
So I think the following steps should suit you.

Xen IRQ:
        -> read_irq
        -> do_IRQ
                -> local_irq_enable (via spin_unlock_irq)
                -> call handlers
                -> local_irq_disable (via spin_lock_irq)
                -> EOI
                -> DIR
Guest IRQ:
        -> read_irq
        -> local_irq_enable
        -> do_IRQ
                -> EOI
                -> vgic_inject_irq

SGIs seems to be handled with IRQ disabled, so no change there. For 
LPIs, we might want to do the same (needs some investigation).
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel