[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL |
Date: |
Tue, 17 Jan 2017 20:43:06 +0200 |
On Mon, Jan 16, 2017 at 06:27:12PM +0000, Felipe Franciosi wrote:
> Hi Michael,
>
> > On 16 Jan 2017, at 10:22, Michael S. Tsirkin <address@hidden> wrote:
> >
> > On Thu, Jan 12, 2017 at 05:14:07PM -0800, Felipe Franciosi wrote:
> >> Currently, VQs are started as soon as a SET_VRING_KICK is received. That
> >> is too early in the VQ setup process, as the backend might not yet have
> >> a callfd to notify in case it received a kick and fully processed the
> >> request/command. This patch only starts a VQ when a SET_VRING_CALL is
> >> received.
> >>
> >> Signed-off-by: Felipe Franciosi <address@hidden>
> >
> > Doesn't look right to me.
> > It should happen when the fd supplies becomes readable.
>
> Apologies, but I'm confused now. What should happen when which fd becomes
> readable? I thought we were discussing this further down in the thread. My
> last comments are here:
>
> http://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg02755.html
>
> I pointed out what appears to be a race and am looking forward to your
> comments. There is definitely a problem with the current code, so it's a
> matter of finding out exactly what's happening and what's the correct (and
> spec-compliant) way to fix it.
>
> Felipe
Pls review "Starting and stopping rings" (pls review all of it not just my
snippet) which has this text:
...
Client must start ring upon receiving a kick (that is, detecting that file
descriptor is readable) on the descriptor specified by
VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
VHOST_USER_GET_VRING_BASE.
> >
> >> ---
> >> 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
- [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/12
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Marc-André Lureau, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/17
Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/16