hw/virtio/vhost-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: BillXiang <xiangwencheng@dayudpu.com>
Currently, we have added VHOST_USER_SET_LOG_BASE to
vhost_user_per_device_request in commit 7c211eb078c4
("vhost-user: Skip unnecessary duplicated VHOST_USER_SET_LOG_BASE requests"),
as a result, VHOST_USER_SET_LOG_BASE will be sent only once
when 'vq_index == 0'.
In this patch we add the check of 'vq_index == 0' before
vhost_user_read, such that we do not wait for reply for not
sent VHOST_USER_SET_LOG_BASE.
Signed-off-by: BillXiang <xiangwencheng@dayudpu.com>
---
hw/virtio/vhost-user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 00561daa06..fd12992d15 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -460,7 +460,7 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
return ret;
}
- if (shmfd) {
+ if (shmfd && (dev->vq_index == 0)) {
msg.hdr.size = 0;
ret = vhost_user_read(dev, &msg);
if (ret < 0) {
--
2.30.0On Thu, Aug 01, 2024 at 08:45:40PM +0800, BillXiang wrote:
> From: BillXiang <xiangwencheng@dayudpu.com>
>
> Currently, we have added VHOST_USER_SET_LOG_BASE to
> vhost_user_per_device_request in commit 7c211eb078c4
> ("vhost-user: Skip unnecessary duplicated VHOST_USER_SET_LOG_BASE requests"),
> as a result, VHOST_USER_SET_LOG_BASE will be sent only once
> when 'vq_index == 0'.
> In this patch we add the check of 'vq_index == 0' before
> vhost_user_read, such that we do not wait for reply for not
> sent VHOST_USER_SET_LOG_BASE.
>
> Signed-off-by: BillXiang <xiangwencheng@dayudpu.com>
If you still want this in, pls rewrite the commit log to make
it clear wat is going on: e.g. "cleanup X which does not do
Y with Z, which does" and repost.
> ---
> hw/virtio/vhost-user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 00561daa06..fd12992d15 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -460,7 +460,7 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
> return ret;
> }
>
> - if (shmfd) {
> + if (shmfd && (dev->vq_index == 0)) {
> msg.hdr.size = 0;
> ret = vhost_user_read(dev, &msg);
> if (ret < 0) {
> --
> 2.30.0
On Thu, Aug 01, 2024 at 08:45:40PM +0800, BillXiang wrote:
> From: BillXiang <xiangwencheng@dayudpu.com>
>
> Currently, we have added VHOST_USER_SET_LOG_BASE to
> vhost_user_per_device_request in commit 7c211eb078c4
> ("vhost-user: Skip unnecessary duplicated VHOST_USER_SET_LOG_BASE requests"),
> as a result, VHOST_USER_SET_LOG_BASE will be sent only once
> when 'vq_index == 0'.
> In this patch we add the check of 'vq_index == 0' before
> vhost_user_read, such that we do not wait for reply for not
> sent VHOST_USER_SET_LOG_BASE.
>
> Signed-off-by: BillXiang <xiangwencheng@dayudpu.com>
> ---
> hw/virtio/vhost-user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 00561daa06..fd12992d15 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -460,7 +460,7 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
> return ret;
> }
>
> - if (shmfd) {
> + if (shmfd && (dev->vq_index == 0)) {
> msg.hdr.size = 0;
> ret = vhost_user_read(dev, &msg);
> if (ret < 0) {
How do things work now after 7c211eb078c4 then?
> --
> 2.30.0
> From: "Michael S. Tsirkin"<mst@redhat.com>
> Date: Thu, Aug 1, 2024, 22:13
> Subject: Re: [PATCH v3] vhost-user: Do not wait for reply for not sent VHOST_USER_SET_LOG_BASE
> To: "BillXiang"<xiangwencheng@dayudpu.com>
> Cc: <qemu-devel@nongnu.org>
> On Thu, Aug 01, 2024 at 08:45:40PM +0800, BillXiang wrote:
> > From: BillXiang <xiangwencheng@dayudpu.com>
> >
> > Currently, we have added VHOST_USER_SET_LOG_BASE to
> > vhost_user_per_device_request in commit 7c211eb078c4
> > ("vhost-user: Skip unnecessary duplicated VHOST_USER_SET_LOG_BASE requests"),
> > as a result, VHOST_USER_SET_LOG_BASE will be sent only once
> > when 'vq_index == 0'.
> > In this patch we add the check of 'vq_index == 0' before
> > vhost_user_read, such that we do not wait for reply for not
> > sent VHOST_USER_SET_LOG_BASE.
> >
> > Signed-off-by: BillXiang <xiangwencheng@dayudpu.com>
> > ---
> > hw/virtio/vhost-user.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 00561daa06..fd12992d15 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -460,7 +460,7 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
> > return ret;
> > }
> >
> > - if (shmfd) {
> > + if (shmfd && (dev->vq_index == 0)) {
> > msg.hdr.size = 0;
> > ret = vhost_user_read(dev, &msg);
> > if (ret < 0) {
>
>
>
> How do things work now after 7c211eb078c4 then?
It does not really work after 7c211eb078c4 and it's my mistake.
I forgot to submit the code to check vq_index in 7c211eb078c4.
>
> > --
> > 2.30.0
On Thu, 1 Aug 2024 at 20:32, BillXiang <xiangwencheng@dayudpu.com> wrote:
> > From: "Michael S. Tsirkin"<mst@redhat.com>
> > How do things work now after 7c211eb078c4 then?
>
> It does not really work after 7c211eb078c4 and it's my mistake.
> I forgot to submit the code to check vq_index in 7c211eb078c4.
>
* vhost_user_set_log_base() sends set log message only when (vq_index == 0),
-> https://github.com/qemu/qemu/commit/c98ac64cfb53ccb862a80e818c3a19bdd386e61e
===
+ /* Send only once with first queue pair */
+ if (dev->vq_index != 0) {
+ return 0;
+ }
===
This should help to keep things working, no?
---
- Prasad
> From: "Prasad Pandit"<ppandit@redhat.com>
> Date: Tue, Aug 27, 2024, 19:06
> Subject: Re: [PATCH v3] vhost-user: Do not wait for reply for not sent VHOST_USER_SET_LOG_BASE
> To: "BillXiang"<xiangwencheng@dayudpu.com>
> Cc: "Michael S. Tsirkin"<mst@redhat.com>, <qemu-devel@nongnu.org>
> On Thu, 1 Aug 2024 at 20:32, BillXiang <xiangwencheng@dayudpu.com> wrote:
> > > From: "Michael S. Tsirkin"<mst@redhat.com>
> > > How do things work now after 7c211eb078c4 then?
> >
> > It does not really work after 7c211eb078c4 and it's my mistake.
> > I forgot to submit the code to check vq_index in 7c211eb078c4.
> >
>
> * vhost_user_set_log_base() sends set log message only when (vq_index == 0),
> -> https://github.com/qemu/qemu/commit/c98ac64cfb53ccb862a80e818c3a19bdd386e61e
> ===
> + /* Send only once with first queue pair */
> + if (dev->vq_index != 0) {
> + return 0;
> + }
> ===
>
> This should help to keep things working, no?
> ---
> - Prasad
Yes, that works. But I think it's better to be consistent to use vhost_user_per_device_request for those per-device messages, right?
On Tue, 27 Aug 2024 at 16:50, BillXiang <xiangwencheng@dayudpu.com> wrote: > it's better to be consistent to use vhost_user_per_device_request for those per-device messages, right? * ...consistent to use? Could you please elaborate a little? Thank you. --- - Prasad
> From: "Prasad Pandit"<ppandit@redhat.com>
> Date: Tue, Aug 27, 2024, 20:37
> Subject: Re: [PATCH v3] vhost-user: Do not wait for reply for not sent VHOST_USER_SET_LOG_BASE
> To: "BillXiang"<xiangwencheng@dayudpu.com>
> Cc: "Michael S. Tsirkin"<mst@redhat.com>, <qemu-devel@nongnu.org>
> On Tue, 27 Aug 2024 at 16:50, BillXiang <xiangwencheng@dayudpu.com> wrote:
> > it's better to be consistent to use vhost_user_per_device_request for those per-device messages, right?
>
> * ...consistent to use? Could you please elaborate a little?
>
> Thank you.
> ---
> - Prasad
That was elaborated in commit b931bfbf0429 (" vhost-user: add multiple queue support ").
We have added vhost_user_one_time_request() to send those per-device messages only once
for multi-queue device. Which was then changed to vhost_user_per_device_request() in
commit 0dcb4172f2ce ("vhost-user: Change one_time to per_device request").
And VHOST_USER_SET_LOG_BASE should be one of those per-device messages that only
be sent once for multi-queue device.
On Tue, Aug 27, 2024 at 09:00:35PM +0800, BillXiang wrote:
>
> > From: "Prasad Pandit"<ppandit@redhat.com>
> > Date: Tue, Aug 27, 2024, 20:37
> > Subject: Re: [PATCH v3] vhost-user: Do not wait for reply for not sent VHOST_USER_SET_LOG_BASE
> > To: "BillXiang"<xiangwencheng@dayudpu.com>
> > Cc: "Michael S. Tsirkin"<mst@redhat.com>, <qemu-devel@nongnu.org>
> > On Tue, 27 Aug 2024 at 16:50, BillXiang <xiangwencheng@dayudpu.com> wrote:
> > > it's better to be consistent to use vhost_user_per_device_request for those per-device messages, right?
> >
> > * ...consistent to use? Could you please elaborate a little?
> >
> > Thank you.
> > ---
> > - Prasad
>
> That was elaborated in commit b931bfbf0429 (" vhost-user: add multiple queue support ").
> We have added vhost_user_one_time_request() to send those per-device messages only once
> for multi-queue device. Which was then changed to vhost_user_per_device_request() in
> commit 0dcb4172f2ce ("vhost-user: Change one_time to per_device request").
> And VHOST_USER_SET_LOG_BASE should be one of those per-device messages that only
> be sent once for multi-queue device.
Bill,
it's important to make it clear, in the commit message, what is the
current behaviour and what is the effect of the patch.
For example: currently qemu hangs waiting for ...., to fix,
.... so we never wait for .... .
At the moment, I'm not really sure if this is a bugfix, or
a cleanup, or what.
--
MST
© 2016 - 2024 Red Hat, Inc.