On 17/01/2017 20:00, Michael S. Tsirkin wrote: >>> vhost user is not supposed to use maskfd at all. >>> >>> We have this code: >>> if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) { >>> dev->use_guest_notifier_mask = false; >>> } >>> >>> isn't it effective? >> >> I'm observing this problem when using vhost-user-scsi, not -net. So the code >> above is not in effect. Anyway, I'd expect the race I described to also >> happen on vhost-scsi. > > maskfd is just racy for vhost-user ATM. I'm guessing vhost-scsi should > just set use_guest_notifier_mask, that will fix it. Alternatively, > rework masking to support sync with the backend - but I doubt it's > useful.
I agree with Michael that this is a QEMU bug, not a libvhost-user bug. Thanks, Paolo >>> >>> >>> >>>> Qemu sends the new callfd to the application >>>> >>>> It's not hard to repro. How could this situation be avoided? >>>> >>>> Cheers, >>>> Felipe >>>> >>>> >>>>> >>>>>> >>>>>>> >>>>>>>>> >>>>>>>>> Perhaps it's best for now to delay the callfd notification with a >>>>>>>>> flag until it is received? >>>>>>>> >>>>>>>> The other idea is to always kick when we receive the callfd. I >>>>>>>> remember discussing that alternative with you before libvhost-user >>>>>>>> went in. The protocol says both the driver and the backend must handle >>>>>>>> spurious kicks. This approach also fixes the bug. >>>>>>>> >>>>>>>> I'm happy with whatever alternative you want, as long it makes >>>>>>>> libvhost-user usable for storage devices. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Felipe >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> Signed-off-by: Felipe Franciosi <fel...@nutanix.com> >>>>>>>>>> --- >>>>>>>>>> contrib/libvhost-user/libvhost-user.c | 26 +++++++++++++------------- >>>>>>>>>> 1 file changed, 13 insertions(+), 13 deletions(-) >>>>>>>>>> >>>>>>>>>> diff --git a/contrib/libvhost-user/libvhost-user.c >>>>>>>>>> b/contrib/libvhost-user/libvhost-user.c >>>>>>>>>> index af4faad..a46ef90 100644 >>>>>>>>>> --- a/contrib/libvhost-user/libvhost-user.c >>>>>>>>>> +++ b/contrib/libvhost-user/libvhost-user.c >>>>>>>>>> @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg >>>>>>>>>> *vmsg) >>>>>>>>>> DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index); >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> - dev->vq[index].started = true; >>>>>>>>>> - if (dev->iface->queue_set_started) { >>>>>>>>>> - dev->iface->queue_set_started(dev, index, true); >>>>>>>>>> - } >>>>>>>>>> - >>>>>>>>>> - if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) { >>>>>>>>>> - dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN, >>>>>>>>>> - vu_kick_cb, (void *)(long)index); >>>>>>>>>> - >>>>>>>>>> - DPRINT("Waiting for kicks on fd: %d for vq: %d\n", >>>>>>>>>> - dev->vq[index].kick_fd, index); >>>>>>>>>> - } >>>>>>>>>> - >>>>>>>>>> return false; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg >>>>>>>>>> *vmsg) >>>>>>>>>> >>>>>>>>>> DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index); >>>>>>>>>> >>>>>>>>>> + dev->vq[index].started = true; >>>>>>>>>> + if (dev->iface->queue_set_started) { >>>>>>>>>> + dev->iface->queue_set_started(dev, index, true); >>>>>>>>>> + } >>>>>>>>>> + >>>>>>>>>> + if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) { >>>>>>>>>> + dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN, >>>>>>>>>> + vu_kick_cb, (void *)(long)index); >>>>>>>>>> + >>>>>>>>>> + DPRINT("Waiting for kicks on fd: %d for vq: %d\n", >>>>>>>>>> + dev->vq[index].kick_fd, index); >>>>>>>>>> + } >>>>>>>>>> + >>>>>>>>>> return false; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> 1.9.4 >>>>>>>>>> >>>>>>>>>> >>>>>>>> > >