qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v1 2/2] virtio-scsi: dataplane: notify guest as


From: Ming Lei
Subject: Re: [Qemu-devel] [PATCH v1 2/2] virtio-scsi: dataplane: notify guest as batch
Date: Wed, 12 Nov 2014 10:33:08 +0800

On Wed, Nov 12, 2014 at 1:28 AM, Stefan Hajnoczi <address@hidden> wrote:
> On Tue, Nov 11, 2014 at 09:17:10AM +0800, Ming Lei wrote:
>> +static void notify_guest_bh(void *opaque)
>> +{
>> +    VirtIOSCSI *s = opaque;
>> +    unsigned int qid;
>> +    uint64_t pending = s->pending_guest_notify;
>> +
>> +    s->pending_guest_notify = 0;
>> +
>> +    while ((qid = ffsl(pending))) {
>> +        qid--;
>> +        event_notifier_set(&s->cmd_vrings[qid]->guest_notifier);
>> +        pending &= ~(1 << qid);
>> +    }
>> +}
>
> Looks like we're not honoring virtio's usual interrupt mitigation
> mechanism (e.g. EVENT_IDX) for virtio-scsi.  Why is
> vring_should_notify() not used?

Yes, that should be used, but in virtio-blk, BH still can suppress
about ~13K/sec write(), and for virtio-scsi, it should be more since
there may be more queues.

>
>> +
>>  static void virtio_scsi_bad_req(void)
>>  {
>>      error_report("wrong size for virtio-scsi headers");
>> @@ -824,7 +839,12 @@ void virtio_scsi_common_realize(DeviceState *dev, Error 
>> **errp,
>>      }
>>
>>      if (s->conf.iothread) {
>> -        virtio_scsi_set_iothread(VIRTIO_SCSI(s), s->conf.iothread);
>> +        VirtIOSCSI *vis = VIRTIO_SCSI(s);
>> +
>> +        QEMU_BUILD_BUG_ON(VIRTIO_PCI_QUEUE_MAX > 64);
>> +        virtio_scsi_set_iothread(vis, s->conf.iothread);
>> +        vis->pending_guest_notify = 0;
>> +        vis->guest_notify_bh = aio_bh_new(vis->ctx, notify_guest_bh, vis);
>
> Have you checked state transitions like PCI or virtio reset?  They need
> to cancel the BH and clear ->pending_guest_notify.
>
> There is also a transition from dataplane to non-dataplane mode for live
> migration.
>
> Please make sure no interrupts are dropped or stale data is kept across
> these transitions.

Good catch, and I will make sure that in next version, and virtio-blk
dataplane need to consider that too.

Thanks,



reply via email to

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