| Message ID | 20210204105232.834642-4-marcandre.lureau@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Various vhost-user-gpu & UI fixes | expand |
On 2/4/21 11:52 AM, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Introduce a pending state for commands which aren't finished yet, but > are being handled. See following patch. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > contrib/vhost-user-gpu/vugpu.h | 8 +++++++- > contrib/vhost-user-gpu/vhost-user-gpu.c | 8 ++++---- > contrib/vhost-user-gpu/virgl.c | 2 +- > 3 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h > index 86f3ac86aa..04d5615812 100644 > --- a/contrib/vhost-user-gpu/vugpu.h > +++ b/contrib/vhost-user-gpu/vugpu.h > @@ -129,12 +129,18 @@ typedef struct VuGpu { > QTAILQ_HEAD(, virtio_gpu_ctrl_command) fenceq; > } VuGpu; > > +enum { > + VG_CMD_STATE_NEW, Maybe VG_CMD_STATE_STARTING? > + VG_CMD_STATE_PENDING, Maybe introduce VG_CMD_STATE_PENDING in the patch using it. > + VG_CMD_STATE_FINISHED, > +}; Can we use a typedef ... > + > struct virtio_gpu_ctrl_command { > VuVirtqElement elem; > VuVirtq *vq; > struct virtio_gpu_ctrl_hdr cmd_hdr; > uint32_t error; > - bool finished; > + int state; ... and use it here? Or directly declare in place: enum { VG_CMD_STATE_STARTING, VG_CMD_STATE_PENDING, VG_CMD_STATE_FINISHED, } state; > QTAILQ_ENTRY(virtio_gpu_ctrl_command) next; > }; > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On 2/4/21 12:26 PM, Philippe Mathieu-Daudé wrote: > On 2/4/21 11:52 AM, marcandre.lureau@redhat.com wrote: >> From: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> Introduce a pending state for commands which aren't finished yet, but >> are being handled. See following patch. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> --- >> contrib/vhost-user-gpu/vugpu.h | 8 +++++++- >> contrib/vhost-user-gpu/vhost-user-gpu.c | 8 ++++---- >> contrib/vhost-user-gpu/virgl.c | 2 +- >> 3 files changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h >> index 86f3ac86aa..04d5615812 100644 >> --- a/contrib/vhost-user-gpu/vugpu.h >> +++ b/contrib/vhost-user-gpu/vugpu.h >> @@ -129,12 +129,18 @@ typedef struct VuGpu { >> QTAILQ_HEAD(, virtio_gpu_ctrl_command) fenceq; >> } VuGpu; >> >> +enum { >> + VG_CMD_STATE_NEW, > > Maybe VG_CMD_STATE_STARTING? > >> + VG_CMD_STATE_PENDING, > > Maybe introduce VG_CMD_STATE_PENDING in the > patch using it. > >> + VG_CMD_STATE_FINISHED, >> +}; > > Can we use a typedef ... > >> + >> struct virtio_gpu_ctrl_command { >> VuVirtqElement elem; >> VuVirtq *vq; >> struct virtio_gpu_ctrl_hdr cmd_hdr; >> uint32_t error; >> - bool finished; >> + int state; > > ... and use it here? > > Or directly declare in place: > > enum { > VG_CMD_STATE_STARTING, > VG_CMD_STATE_PENDING, > VG_CMD_STATE_FINISHED, > } state; > >> QTAILQ_ENTRY(virtio_gpu_ctrl_command) next; >> }; >> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Since Gerd was willing to queue v1, feel free to ignore my comments at this point. R-b stands.
diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h index 86f3ac86aa..04d5615812 100644 --- a/contrib/vhost-user-gpu/vugpu.h +++ b/contrib/vhost-user-gpu/vugpu.h @@ -129,12 +129,18 @@ typedef struct VuGpu { QTAILQ_HEAD(, virtio_gpu_ctrl_command) fenceq; } VuGpu; +enum { + VG_CMD_STATE_NEW, + VG_CMD_STATE_PENDING, + VG_CMD_STATE_FINISHED, +}; + struct virtio_gpu_ctrl_command { VuVirtqElement elem; VuVirtq *vq; struct virtio_gpu_ctrl_hdr cmd_hdr; uint32_t error; - bool finished; + int state; QTAILQ_ENTRY(virtio_gpu_ctrl_command) next; }; diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c index 85c16404fb..7dcc02966c 100644 --- a/contrib/vhost-user-gpu/vhost-user-gpu.c +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c @@ -246,7 +246,7 @@ vg_ctrl_response(VuGpu *g, } vu_queue_push(&g->dev.parent, cmd->vq, &cmd->elem, s); vu_queue_notify(&g->dev.parent, cmd->vq); - cmd->finished = true; + cmd->state = VG_CMD_STATE_FINISHED; } void @@ -800,7 +800,7 @@ vg_process_cmd(VuGpu *vg, struct virtio_gpu_ctrl_command *cmd) cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; break; } - if (!cmd->finished) { + if (cmd->state == VG_CMD_STATE_NEW) { vg_ctrl_response_nodata(vg, cmd, cmd->error ? cmd->error : VIRTIO_GPU_RESP_OK_NODATA); } @@ -825,7 +825,7 @@ vg_handle_ctrl(VuDev *dev, int qidx) } cmd->vq = vq; cmd->error = 0; - cmd->finished = false; + cmd->state = VG_CMD_STATE_NEW; len = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, &cmd->cmd_hdr, sizeof(cmd->cmd_hdr)); @@ -844,7 +844,7 @@ vg_handle_ctrl(VuDev *dev, int qidx) vg_process_cmd(vg, cmd); } - if (!cmd->finished) { + if (cmd->state != VG_CMD_STATE_FINISHED) { QTAILQ_INSERT_TAIL(&vg->fenceq, cmd, next); vg->inflight++; } else { diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c index e647278052..8bb3c563d9 100644 --- a/contrib/vhost-user-gpu/virgl.c +++ b/contrib/vhost-user-gpu/virgl.c @@ -482,7 +482,7 @@ void vg_virgl_process_cmd(VuGpu *g, struct virtio_gpu_ctrl_command *cmd) break; } - if (cmd->finished) { + if (cmd->state != VG_CMD_STATE_NEW) { return; }