Re: [Qemu-devel] vhost-scsi port to v1.1.0 + MSI-X performance regression

[Stefan Hajnoczi]

On Tue, Jul 24, 2012 at 8:57 AM, Jan Kiszka <address@hidden> wrote:
> On 2012-07-24 09:42, Nicholas A. Bellinger wrote:
>> Hi Anthony, Stefan & QEMU folks,
>>
>> So during the process of separating out the patches from Zhi's
>> vhost-scsi tree this evening, I managed to squash everything down to
>> nine nicely reviewable patches that apply against the current
>> qemu.git/master:
>>
>> Nicholas Bellinger (1):
>>   virtio-scsi: Set max_target=0 during vhost-scsi operation
>>
>> Stefan Hajnoczi (8):
>>   notifier: add validity check and notify function
>>   virtio-pci: support host notifiers in TCG mode
>>   virtio-pci: check that event notification worked
>>   vhost: Pass device path to vhost_dev_init()
>>   virtio-scsi: Add wwpn and tgpt properties
>>   virtio-scsi: Open and initialize /dev/vhost-scsi
>>   virtio-scsi: Start/stop vhost
>>   vhost-scsi: add -vhost-scsi host device
>>
>>
>> However, thus far I've not been able to get virtio-scsi <-> tcm_vhost
>> I/O to actually work against the latest qemu.git/master..
>>
>> So while doing a (manual) bisection w/ this series to track down the
>> issue with qemu/master, I managed to run across something else..  With
>> the vhost-scsi series applied, everything is working as expected up
>> until the following commit:
>>
>> commit 1523ed9e1d46b0b54540049d491475ccac7e6421
>> Author: Jan Kiszka <address@hidden>
>> Date:   Thu May 17 10:32:39 2012 -0300
>>
>>     virtio/vhost: Add support for KVM in-kernel MSI injection
>>
>>
>> This commit ends up triggering the following assert immediately after
>> starting qemu with virtio-scsi <-> tcm_vhost:
>>
>> qemu-system-x86_64: /usr/src/qemu.git/hw/msix.c:515:
>>        msix_unset_vector_notifiers: Assertion `dev->msix_vector_use_notifier 
>> &&
>>                                     dev->msix_vector_release_notifier' 
>> failed.
>>
>> OK, so adding the following hack allows me to boot:
>>
>> diff --git a/hw/msix.c b/hw/msix.c
>> index 59c7a83..6036909 100644
>> --- a/hw/msix.c
>> +++ b/hw/msix.c
>> @@ -511,6 +511,11 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
>>  {
>>      int vector;
>>
>> +    if (!dev->msix_vector_use_notifier && 
>> !dev->msix_vector_release_notifier) {
>> +        printf("Hit NULL msix_unset_vector_notifiers for: %s\n", dev->name);
>> +        return;
>> +    }
>> +
>>      assert(dev->msix_vector_use_notifier &&
>>             dev->msix_vector_release_notifier);
>>
>> --
>
> Can you post a backtrace from gdb?
>
> Also, is there a git tree and a way to reproduce this without special
> hardware needs?
>
>>
>> and virtio-scsi is then able to load + detect tcm_vhost LUNs as
>> expected.
>>
>> However the random I/O performance with commit 1523ed9e1d46b is off by a
>> couple of orders of magnitude, ~6K IOPs compared to ~60K IOPs on raw
>> block flash using just the previous commit bdd00bdc64ba in Jan's series.
>>
>> So AFAICT there appears to be a serious performance regression that is
>> easily reproducible with that patch, which is about as far along as I've
>> been able to diagnose yet.
>>
>> Interestingly enough, virtio-scsi-raw performance does not seem to be
>> effected AFAICT by this regression, and is still able to go ~20K IOPs
>> with the same workload using commit 1523ed9e1d46b.  (Roughly the same as
>> before)
>>
>> Does anyone have any idea why commit 1523ed9e1d46b would be killing
>> vhost / tcm_vhost performance so terribly, or is there something else
>> that vhost / vhost-scsi should be doing with new code..?
>
> No good idea yet, will have to look closer.
>
> Maybe you are somehow deassigning (via set_guest_notifiers) before
> assigning. But that would not yet explain performance regressions. Your
> target is exposing MSI-X, isn't it?

Regarding performance, have the command-line options to enable irqchip
or anything like that changed after the qemu-kvm.git -> qemu.git
refactoring?

Stefan