assign-dev: Purpose of interrupt_work

assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

Hi,

I was starring at the IRQ delivery path of assigned devices for a while,
wondering why we have a work queue there. Now, after looking at some
prehistoric versions, I think the reason is that there once was a mutex
involved while we now use RCU. Am I right that we could actually drop
this indirection today?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 09:03:18AM +0200, Jan Kiszka wrote:
> Hi,
> 
> I was starring at the IRQ delivery path of assigned devices for a while,
> wondering why we have a work queue there. Now, after looking at some
> prehistoric versions, I think the reason is that there once was a mutex
> involved while we now use RCU. Am I right that we could actually drop
> this indirection today?
> 
ioapic/pic path still has mutex. If MSIX is used (like it should) we can
drop work queue.

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

Gleb Natapov wrote:
> On Mon, Oct 12, 2009 at 09:03:18AM +0200, Jan Kiszka wrote:
>> Hi,
>>
>> I was starring at the IRQ delivery path of assigned devices for a while,
>> wondering why we have a work queue there. Now, after looking at some
>> prehistoric versions, I think the reason is that there once was a mutex
>> involved while we now use RCU. Am I right that we could actually drop
>> this indirection today?
>>
> ioapic/pic path still has mutex. If MSIX is used (like it should) we can
> drop work queue.

I see. Wouldn't it be feasible to convert the fast paths of [io]apic to
spinlocks?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 09:27:19AM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Mon, Oct 12, 2009 at 09:03:18AM +0200, Jan Kiszka wrote:
> >> Hi,
> >>
> >> I was starring at the IRQ delivery path of assigned devices for a while,
> >> wondering why we have a work queue there. Now, after looking at some
> >> prehistoric versions, I think the reason is that there once was a mutex
> >> involved while we now use RCU. Am I right that we could actually drop
> >> this indirection today?
> >>
> > ioapic/pic path still has mutex. If MSIX is used (like it should) we can
> > drop work queue.
> 
> I see. Wouldn't it be feasible to convert the fast paths of [io]apic to
> spinlocks?
> 
Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
prefers mutexes. Theoretically it is possible to make them lockless,
but code will be complex and eventually more slow, since more then two
atomic operation will be used on irq injection path.

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

Gleb Natapov wrote:
> On Mon, Oct 12, 2009 at 09:27:19AM +0200, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> On Mon, Oct 12, 2009 at 09:03:18AM +0200, Jan Kiszka wrote:
>>>> Hi,
>>>>
>>>> I was starring at the IRQ delivery path of assigned devices for a while,
>>>> wondering why we have a work queue there. Now, after looking at some
>>>> prehistoric versions, I think the reason is that there once was a mutex
>>>> involved while we now use RCU. Am I right that we could actually drop
>>>> this indirection today?
>>>>
>>> ioapic/pic path still has mutex. If MSIX is used (like it should) we can
>>> drop work queue.
>> I see. Wouldn't it be feasible to convert the fast paths of [io]apic to
>> spinlocks?
>>
> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
> prefers mutexes. Theoretically it is possible to make them lockless,
> but code will be complex and eventually more slow, since more then two
> atomic operation will be used on irq injection path.

Well, lockless is another thing.

But also converting to spinlocks would indeed add some overhead:
irqsave/restore. But I wonder if this isn't worth it, at least when
looking at the (supposed to be fast) device passthrough scenario which
would be simpler and faster.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 09:50:58AM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Mon, Oct 12, 2009 at 09:27:19AM +0200, Jan Kiszka wrote:
> >> Gleb Natapov wrote:
> >>> On Mon, Oct 12, 2009 at 09:03:18AM +0200, Jan Kiszka wrote:
> >>>> Hi,
> >>>>
> >>>> I was starring at the IRQ delivery path of assigned devices for a while,
> >>>> wondering why we have a work queue there. Now, after looking at some
> >>>> prehistoric versions, I think the reason is that there once was a mutex
> >>>> involved while we now use RCU. Am I right that we could actually drop
> >>>> this indirection today?
> >>>>
> >>> ioapic/pic path still has mutex. If MSIX is used (like it should) we can
> >>> drop work queue.
> >> I see. Wouldn't it be feasible to convert the fast paths of [io]apic to
> >> spinlocks?
> >>
> > Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
> > prefers mutexes. Theoretically it is possible to make them lockless,
> > but code will be complex and eventually more slow, since more then two
> > atomic operation will be used on irq injection path.
> 
> Well, lockless is another thing.
> 
> But also converting to spinlocks would indeed add some overhead:
> irqsave/restore. But I wonder if this isn't worth it, at least when
> looking at the (supposed to be fast) device passthrough scenario which
> would be simpler and faster.
> 
Avi's point in favor of mutex is: they are as fast as spinlocks when
congested and allows preemption when held.

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]

Gleb Natapov wrote:
> On Mon, Oct 12, 2009 at 09:50:58AM +0200, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> On Mon, Oct 12, 2009 at 09:27:19AM +0200, Jan Kiszka wrote:
>>>> Gleb Natapov wrote:
>>>>> On Mon, Oct 12, 2009 at 09:03:18AM +0200, Jan Kiszka wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I was starring at the IRQ delivery path of assigned devices for a while,
>>>>>> wondering why we have a work queue there. Now, after looking at some
>>>>>> prehistoric versions, I think the reason is that there once was a mutex
>>>>>> involved while we now use RCU. Am I right that we could actually drop
>>>>>> this indirection today?
>>>>>>
>>>>> ioapic/pic path still has mutex. If MSIX is used (like it should) we can
>>>>> drop work queue.
>>>> I see. Wouldn't it be feasible to convert the fast paths of [io]apic to
>>>> spinlocks?
>>>>
>>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
>>> prefers mutexes. Theoretically it is possible to make them lockless,
>>> but code will be complex and eventually more slow, since more then two
>>> atomic operation will be used on irq injection path.
>> Well, lockless is another thing.
>>
>> But also converting to spinlocks would indeed add some overhead:
>> irqsave/restore. But I wonder if this isn't worth it, at least when
>> looking at the (supposed to be fast) device passthrough scenario which
>> would be simpler and faster.
>>
> Avi's point in favor of mutex is: they are as fast as spinlocks when
> congested and allows preemption when held.

...but require scheduler activity in case of dev-passthrough, which is
surely playing in a different league.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 10:16:56AM +0200, Jan Kiszka wrote:
> >>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
> >>> prefers mutexes. Theoretically it is possible to make them lockless,
> >>> but code will be complex and eventually more slow, since more then two
> >>> atomic operation will be used on irq injection path.
> >> Well, lockless is another thing.
> >>
> >> But also converting to spinlocks would indeed add some overhead:
> >> irqsave/restore. But I wonder if this isn't worth it, at least when
> >> looking at the (supposed to be fast) device passthrough scenario which
> >> would be simpler and faster.
> >>
> > Avi's point in favor of mutex is: they are as fast as spinlocks when
> > congested and allows preemption when held.
> 
> ...but require scheduler activity in case of dev-passthrough, which is
> surely playing in a different league.
> 
I'd rather remove dev-passthrough completely than continue adding hack upon hack
upon hack to make is some times kinda sorta work :)

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

Gleb Natapov wrote:
> On Mon, Oct 12, 2009 at 10:16:56AM +0200, Jan Kiszka wrote:
>>>>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
>>>>> prefers mutexes. Theoretically it is possible to make them lockless,
>>>>> but code will be complex and eventually more slow, since more then two
>>>>> atomic operation will be used on irq injection path.
>>>> Well, lockless is another thing.
>>>>
>>>> But also converting to spinlocks would indeed add some overhead:
>>>> irqsave/restore. But I wonder if this isn't worth it, at least when
>>>> looking at the (supposed to be fast) device passthrough scenario which
>>>> would be simpler and faster.
>>>>
>>> Avi's point in favor of mutex is: they are as fast as spinlocks when
>>> congested and allows preemption when held.
>> ...but require scheduler activity in case of dev-passthrough, which is
>> surely playing in a different league.
>>
> I'd rather remove dev-passthrough completely than continue adding hack upon hack
> upon hack to make is some times kinda sorta work :)

Hmm, is this code not needed for the VT-d & Co. case? Or what is the
alternative?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: assign-dev: Purpose of interrupt_work

[Avi Kivity]

On 10/12/2009 11:04 AM, Jan Kiszka wrote:
>> I'd rather remove dev-passthrough completely than continue adding hack upon hack
>> upon hack to make is some times kinda sorta work :)
>>      
> Hmm, is this code not needed for the VT-d&  Co. case?

It is needed.

> Or what is the
> alternative?
>    

irqfd (which only supports edge-triggered interrupts now).  Note irqfd 
needs the same love, it uses a workqueue as well.

The theory is:
  fd1 = eventfd()
  give fd1 to kvm as irqfd, vhost-net as trigger fd
  fd2 = eventfd()
  give fd2 to kvm as irqfd, uio as trigger fd
  fd3 = evenfd()
  give fd3 to kvm as irqfd, another kvm as ioeventfd

One interface, multiple users (in the kernel, userspace, or other processes)

-- 
error compiling committee.c: too many arguments to function

Re: assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

Avi Kivity wrote:
> On 10/12/2009 11:04 AM, Jan Kiszka wrote:
>>> I'd rather remove dev-passthrough completely than continue adding
>>> hack upon hack
>>> upon hack to make is some times kinda sorta work :)
>>>      
>> Hmm, is this code not needed for the VT-d&  Co. case?
> 
> It is needed.
> 
>> Or what is the
>> alternative?
>>    
> 
> irqfd (which only supports edge-triggered interrupts now).  Note irqfd
> needs the same love, it uses a workqueue as well.
> 
> The theory is:
>  fd1 = eventfd()
>  give fd1 to kvm as irqfd, vhost-net as trigger fd
>  fd2 = eventfd()
>  give fd2 to kvm as irqfd, uio as trigger fd
>  fd3 = evenfd()
>  give fd3 to kvm as irqfd, another kvm as ioeventfd
> 
> One interface, multiple users (in the kernel, userspace, or other
> processes)

I see to overall gain, but likely only worsens my objective (low
latency), at least it doesn't improve it.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: assign-dev: Purpose of interrupt_work

[Avi Kivity]

On 10/12/2009 11:25 AM, Jan Kiszka wrote:
>>> Or what is the
>>> alternative?
>>>
>>>        
>> irqfd (which only supports edge-triggered interrupts now).  Note irqfd
>> needs the same love, it uses a workqueue as well.
>>
>> The theory is:
>>   fd1 = eventfd()
>>   give fd1 to kvm as irqfd, vhost-net as trigger fd
>>   fd2 = eventfd()
>>   give fd2 to kvm as irqfd, uio as trigger fd
>>   fd3 = evenfd()
>>   give fd3 to kvm as irqfd, another kvm as ioeventfd
>>
>> One interface, multiple users (in the kernel, userspace, or other
>> processes)
>>      
> I see to overall gain, but likely only worsens my objective (low
> latency), at least it doesn't improve it.
>    

irqfd can work from interrupt context, so if we only use the workqueue 
for the many-vcpu cases, we should be fine.

I think we can do it cleanly, too: in ioapic code, if we're talking to 
one vcpu, wake it immediately; otherwise schedule work to continue in 
process context.  This was the common code can always work in interrupt 
context and not worry about the work queue.

-- 
error compiling committee.c: too many arguments to function

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 11:30:10AM +0200, Avi Kivity wrote:
> On 10/12/2009 11:25 AM, Jan Kiszka wrote:
> >>>Or what is the
> >>>alternative?
> >>>
> >>irqfd (which only supports edge-triggered interrupts now).  Note irqfd
> >>needs the same love, it uses a workqueue as well.
> >>
> >>The theory is:
> >>  fd1 = eventfd()
> >>  give fd1 to kvm as irqfd, vhost-net as trigger fd
> >>  fd2 = eventfd()
> >>  give fd2 to kvm as irqfd, uio as trigger fd
> >>  fd3 = evenfd()
> >>  give fd3 to kvm as irqfd, another kvm as ioeventfd
> >>
> >>One interface, multiple users (in the kernel, userspace, or other
> >>processes)
> >I see to overall gain, but likely only worsens my objective (low
> >latency), at least it doesn't improve it.
> 
> irqfd can work from interrupt context, so if we only use the
> workqueue for the many-vcpu cases, we should be fine.
> 
> I think we can do it cleanly, too: in ioapic code, if we're talking
> to one vcpu, wake it immediately; otherwise schedule work to
> continue in process context.  This was the common code can always
> work in interrupt context and not worry about the work queue.
> 
Even if we are talking to one cpu locking is still needed, so unless we
make it spinlock we can't inject from irq context. If we change mutex to
spinlock what is the point of work queue?

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Avi Kivity]

On 10/12/2009 11:38 AM, Gleb Natapov wrote:
>
>> irqfd can work from interrupt context, so if we only use the
>> workqueue for the many-vcpu cases, we should be fine.
>>
>> I think we can do it cleanly, too: in ioapic code, if we're talking
>> to one vcpu, wake it immediately; otherwise schedule work to
>> continue in process context.  This was the common code can always
>> work in interrupt context and not worry about the work queue.
>>
>>      
> Even if we are talking to one cpu locking is still needed, so unless we
> make it spinlock we can't inject from irq context. If we change mutex to
> spinlock what is the point of work queue?
>    

Not to loop over 4096 vcpus with interrupts disabled, triggered by guest 
action.

-- 
error compiling committee.c: too many arguments to function

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 11:40:59AM +0200, Avi Kivity wrote:
> On 10/12/2009 11:38 AM, Gleb Natapov wrote:
> >
> >>irqfd can work from interrupt context, so if we only use the
> >>workqueue for the many-vcpu cases, we should be fine.
> >>
> >>I think we can do it cleanly, too: in ioapic code, if we're talking
> >>to one vcpu, wake it immediately; otherwise schedule work to
> >>continue in process context.  This was the common code can always
> >>work in interrupt context and not worry about the work queue.
> >>
> >Even if we are talking to one cpu locking is still needed, so unless we
> >make it spinlock we can't inject from irq context. If we change mutex to
> >spinlock what is the point of work queue?
> 
> Not to loop over 4096 vcpus with interrupts disabled, triggered by
> guest action.
> 
But in a work queue you'll do this anyway. You can't access ioapic
without the lock.

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 11:04:23AM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Mon, Oct 12, 2009 at 10:16:56AM +0200, Jan Kiszka wrote:
> >>>>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
> >>>>> prefers mutexes. Theoretically it is possible to make them lockless,
> >>>>> but code will be complex and eventually more slow, since more then two
> >>>>> atomic operation will be used on irq injection path.
> >>>> Well, lockless is another thing.
> >>>>
> >>>> But also converting to spinlocks would indeed add some overhead:
> >>>> irqsave/restore. But I wonder if this isn't worth it, at least when
> >>>> looking at the (supposed to be fast) device passthrough scenario which
> >>>> would be simpler and faster.
> >>>>
> >>> Avi's point in favor of mutex is: they are as fast as spinlocks when
> >>> congested and allows preemption when held.
> >> ...but require scheduler activity in case of dev-passthrough, which is
> >> surely playing in a different league.
> >>
> > I'd rather remove dev-passthrough completely than continue adding hack upon hack
> > upon hack to make is some times kinda sorta work :)
> 
> Hmm, is this code not needed for the VT-d & Co. case? Or what is the
> alternative?
> 
The smile there is for a reason. My dislike of dev-passthrough will not
make it go away. Alternatives? I don't know what HW makers plans are, but
with interrupt remapping + APIC vitalization in HW theoretically it is
possible to inject interrupts directly from device to virtual machine.

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Avi Kivity]

On 10/12/2009 09:50 AM, Jan Kiszka wrote:
>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
>> prefers mutexes. Theoretically it is possible to make them lockless,
>> but code will be complex and eventually more slow, since more then two
>> atomic operation will be used on irq injection path.
>>      
> Well, lockless is another thing.
>
> But also converting to spinlocks would indeed add some overhead:
> irqsave/restore. But I wonder if this isn't worth it, at least when
> looking at the (supposed to be fast) device passthrough scenario which
> would be simpler and faster.
>    

I'm worried about disabling irqs for non-device-assignment cases.  It 
would be more palatable if ioapic was completely O(1) (there are some 
per-vcpu loops in there, shouldn't be too bad for 16 vcpus, but we want 
to scale).

-- 
error compiling committee.c: too many arguments to function

Re: assign-dev: Purpose of interrupt_work

[Gleb Natapov]

On Mon, Oct 12, 2009 at 10:39:09AM +0200, Avi Kivity wrote:
> On 10/12/2009 09:50 AM, Jan Kiszka wrote:
> >>Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
> >>prefers mutexes. Theoretically it is possible to make them lockless,
> >>but code will be complex and eventually more slow, since more then two
> >>atomic operation will be used on irq injection path.
> >Well, lockless is another thing.
> >
> >But also converting to spinlocks would indeed add some overhead:
> >irqsave/restore. But I wonder if this isn't worth it, at least when
> >looking at the (supposed to be fast) device passthrough scenario which
> >would be simpler and faster.
> 
> I'm worried about disabling irqs for non-device-assignment cases.
> It would be more palatable if ioapic was completely O(1) (there are
> some per-vcpu loops in there, shouldn't be too bad for 16 vcpus, but
> we want to scale).
> 
We can change it to hash for directed irqs (still not O(1)), but for
broadcast irq the loop will be required.

--
			Gleb.

Re: assign-dev: Purpose of interrupt_work

[Avi Kivity]

On 10/12/2009 11:05 AM, Gleb Natapov wrote:
>
>> I'm worried about disabling irqs for non-device-assignment cases.
>> It would be more palatable if ioapic was completely O(1) (there are
>> some per-vcpu loops in there, shouldn't be too bad for 16 vcpus, but
>> we want to scale).
>>
>>      
> We can change it to hash for directed irqs (still not O(1)),

Or cache the vcpu pointer in the redirection table entry.

> but for
> broadcast irq the loop will be required.
>    

That's fine, those are rare.

-- 
error compiling committee.c: too many arguments to function

Re: assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

Avi Kivity wrote:
> On 10/12/2009 09:50 AM, Jan Kiszka wrote:
>>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
>>> prefers mutexes. Theoretically it is possible to make them lockless,
>>> but code will be complex and eventually more slow, since more then two
>>> atomic operation will be used on irq injection path.
>>>      
>> Well, lockless is another thing.
>>
>> But also converting to spinlocks would indeed add some overhead:
>> irqsave/restore. But I wonder if this isn't worth it, at least when
>> looking at the (supposed to be fast) device passthrough scenario which
>> would be simpler and faster.
>>    
> 
> I'm worried about disabling irqs for non-device-assignment cases.  It
> would be more palatable if ioapic was completely O(1) (there are some
> per-vcpu loops in there, shouldn't be too bad for 16 vcpus, but we want
> to scale).

Yeah, what a pity. That's likely not solvable in a generic way, given
that the guest finally decided how many VCPUs may listen to a line.

OK, but dropping interrupt_work from the MSI path is still worthwhile,
and probably more future-proof anyway.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: assign-dev: Purpose of interrupt_work

[Avi Kivity]

On 10/12/2009 11:07 AM, Jan Kiszka wrote:
>> I'm worried about disabling irqs for non-device-assignment cases.  It
>> would be more palatable if ioapic was completely O(1) (there are some
>> per-vcpu loops in there, shouldn't be too bad for 16 vcpus, but we want
>> to scale).
>>      
> Yeah, what a pity. That's likely not solvable in a generic way, given
> that the guest finally decided how many VCPUs may listen to a line.
>
> OK, but dropping interrupt_work from the MSI path is still worthwhile,
> and probably more future-proof anyway.
>    

Yes - we can extend the irq routing table to contain this information.  
It means we need to invalidate it when the guest reprograms the 
interrupt controllers, but I think it should work out fine.

-- 
error compiling committee.c: too many arguments to function

Re: assign-dev: Purpose of interrupt_work

[Marcelo Tosatti]

On Mon, Oct 12, 2009 at 11:07:28AM +0200, Jan Kiszka wrote:
> Avi Kivity wrote:
> > On 10/12/2009 09:50 AM, Jan Kiszka wrote:
> >>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
> >>> prefers mutexes. Theoretically it is possible to make them lockless,
> >>> but code will be complex and eventually more slow, since more then two
> >>> atomic operation will be used on irq injection path.
> >>>      
> >> Well, lockless is another thing.
> >>
> >> But also converting to spinlocks would indeed add some overhead:
> >> irqsave/restore. But I wonder if this isn't worth it, at least when
> >> looking at the (supposed to be fast) device passthrough scenario which
> >> would be simpler and faster.
> >>    
> > 
> > I'm worried about disabling irqs for non-device-assignment cases.  It
> > would be more palatable if ioapic was completely O(1) (there are some
> > per-vcpu loops in there, shouldn't be too bad for 16 vcpus, but we want
> > to scale).
> 
> Yeah, what a pity. That's likely not solvable in a generic way, given
> that the guest finally decided how many VCPUs may listen to a line.
> 
> OK, but dropping interrupt_work from the MSI path is still worthwhile,
> and probably more future-proof anyway.

Seems appropriate to convert the process context work to threaded
interrupt (instead of workqueue). That should help latency.

Re: assign-dev: Purpose of interrupt_work

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

Marcelo Tosatti wrote:
> On Mon, Oct 12, 2009 at 11:07:28AM +0200, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>> On 10/12/2009 09:50 AM, Jan Kiszka wrote:
>>>>> Apic is lockless. For ioapic/pic I used spinlocks initially, but Avi
>>>>> prefers mutexes. Theoretically it is possible to make them lockless,
>>>>> but code will be complex and eventually more slow, since more then two
>>>>> atomic operation will be used on irq injection path.
>>>>>      
>>>> Well, lockless is another thing.
>>>>
>>>> But also converting to spinlocks would indeed add some overhead:
>>>> irqsave/restore. But I wonder if this isn't worth it, at least when
>>>> looking at the (supposed to be fast) device passthrough scenario which
>>>> would be simpler and faster.
>>>>    
>>> I'm worried about disabling irqs for non-device-assignment cases.  It
>>> would be more palatable if ioapic was completely O(1) (there are some
>>> per-vcpu loops in there, shouldn't be too bad for 16 vcpus, but we want
>>> to scale).
>> Yeah, what a pity. That's likely not solvable in a generic way, given
>> that the guest finally decided how many VCPUs may listen to a line.
>>
>> OK, but dropping interrupt_work from the MSI path is still worthwhile,
>> and probably more future-proof anyway.
> 
> Seems appropriate to convert the process context work to threaded
> interrupt (instead of workqueue). That should help latency.
> 

Not for the trivial case (I want to avoid scheduling as far as possible).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]