[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] msix: Drop tracking of used vectors

From: Jan Kiszka
Subject: Re: [Qemu-devel] [PATCH] msix: Drop tracking of used vectors
Date: Fri, 06 Jul 2012 00:56:53 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv: Gecko/20080226 SUSE/ Thunderbird/ Mnenhy/

On 2012-07-05 22:47, Michael S. Tsirkin wrote:
> On Thu, Jul 05, 2012 at 04:27:33PM +0200, Jan Kiszka wrote:
>> On 2012-07-05 15:17, Michael S. Tsirkin wrote:
>>> On Thu, Jul 05, 2012 at 11:42:14AM +0200, Jan Kiszka wrote:
>>>> This optimization was once used in qemu-kvm to keep KVM route usage low.
>>>> But now we solved that problem via lazy updates.
>>> What if we are using vhost which AFAIK can't use the lazy path?
>> It uses static vIRQs with irqfd, just as before.
> Exactly. And the API is helpful in that case.

Look at the code: this API serves no practical purpose anymore, not even
for the irqfd scenario. I'm not touching the irqfd code while removing

>>>> It also tried to handle
>>>> the case of vectors shared between different sources of the same device.
>>>> However, this never really worked
>>> Interesting. Guests actually have support and it seems to work for me.
>>> What's the issue?
>> The state of shared vectors is not tracked. Consider a device has two
>> sources for the same vector and one source which raised the "line"
>> before is deregistered, then we will leave the vector set.
> In virtio vectors are registered/deregistered during setup
> (before driver-ok), so this does not happen.

From that point of view, the current final msix_clr_pending in
msix_vector_unuse is also useless. Then what is the remaining purpose of
msix_entry_used? Also, we are not discussing a virtio API here but a
generic MSI-X feature that affects every MSI-X using device.

> Further, even in theory this might not be an issue since anyone sharing
> interrupts needs to deal with spurious events anyway.

This is not about spurious interrupts, it's about the proper state of
the PBA, something that should not contain any spurious states.

> So is or is there not a bug in virtio when it shares vectors?

There are deficits in virtio about how it handles potential vector
sharing internally. But the use/unuse API will not be involved in making
this more spec conforming. Nor are there other users behind the horizon.
That's my point.

>> What we rather need is to track the sharing at device level and report
>> the resulting state to the MSI-X layer. That's what the reduced API will
>> allow. I don't think that vector sharing belongs to the generic layer.
>>>> and will have to be addressed
>>>> differently anyway.
>>> what's the plan to address this?
>> As said above: Extend virtio to track their queue states, let it
>> calculate the vector state and report that to the generic layer.
>> If we really once identify another device model - besides virtio - that
>> does vector sharing, we can still try to model an optional extension to
>> the MSI-X layer for such devices. But the majority of device models
>> should not be bothered with such special cases.
>>>> So drop this obsolete interface.
>>>> We still need interfaces to clear pending vectors though. Provide
>>>> msix_clear_vector and msix_clear_all_vectors for this.
>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>> Looks like if guest tries to use too many vectors it will have to switch
>>> to userspace virtio where previously we would report failure to guest
>>> and it would configure less vectors, which seems better.
>> There is nothing in the current approach that tracks the availability of
>> vector. That's because it depends on the device if it considers a vector
>> used when a single event source grabbed it or if it is able to share it
>> internally.
> So I'd like to see some code that shows how we avoid the regression in the
> above scenario, before we drop the API.

Sorry, what regression? This patch does not change the behavior of
virtio. Please review it and point out the contrary.

>>> I'd like to understand whether this patch is a pre-requisite for
>>> anything or just a cleanup?
>> It avoids spreading of redundant msix_vector_use/unuse to more devices
>> (next ones will be VFIO and kvm device assignment).
>> Jan
> Another less intrusive way is to add a flag that say
> "device always uses all vectors".

The existing API is the intrusive one, that's why I'm trying to remove
it for a while now.

Can you sketch a scenario where it helps to track vector usage via a
central counter like it's done now? I simply don't see this. Therefore I
see no point in keeping this infrastructure around - or even adding a
flag to prolong its life.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]