Re: vhost-scsi port to v1.1.0 + MSI-X performance regression

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-07-24 14:05, Stefan Hajnoczi wrote:
> On Tue, Jul 24, 2012 at 8:57 AM, Jan Kiszka <jan.kiszka@siemens.com> 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 <jan.kiszka@siemens.com>
>>> 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?

Current qemu.git has kernel_irqchip=on by default now, so the command
line should not matter (unless you explicitly turn it off).

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux