* [PATCH v3 0/2] Improve vhost-user VQ notifier unmap @ 2021-10-08 7:58 Xueming Li 2021-10-08 7:58 ` [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup Xueming Li 2021-10-08 7:58 ` [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore Xueming Li 0 siblings, 2 replies; 10+ messages in thread From: Xueming Li @ 2021-10-08 7:58 UTC (permalink / raw) To: qemu-devel; +Cc: xuemingl, qemu-stable When vDPA applicaiton in client mode shutdown, unmapped VQ notifier might being accessed by vCPU thread under high tx traffic, it will crash VM in rare conditon. This patch try to fix it with better RCU sychronization of new flatview. v2: no RCU draining on vCPU thread v3: minor fix on coding style and comments Xueming Li (2): vhost-user: fix VirtQ notifier cleanup vhost-user: remove VirtQ notifier restore hw/virtio/vhost-user.c | 41 +++++++++++++--------------------- include/hw/virtio/vhost-user.h | 1 - 2 files changed, 16 insertions(+), 26 deletions(-) -- 2.33.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup 2021-10-08 7:58 [PATCH v3 0/2] Improve vhost-user VQ notifier unmap Xueming Li @ 2021-10-08 7:58 ` Xueming Li 2021-10-19 6:15 ` Michael S. Tsirkin 2021-10-08 7:58 ` [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore Xueming Li 1 sibling, 1 reply; 10+ messages in thread From: Xueming Li @ 2021-10-08 7:58 UTC (permalink / raw) To: qemu-devel Cc: xuemingl, qemu-stable, tiwei.bie, Yuwei Zhang, Michael S. Tsirkin When vhost-user device cleanup and unmmap notifier address, VM cpu thread that writing the notifier failed with accessing invalid address. To avoid this concurrent issue, wait memory flatview update by draining rcu callbacks, then unmap notifiers. Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") Cc: tiwei.bie@intel.com Cc: qemu-stable@nongnu.org Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- hw/virtio/vhost-user.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index bf6e50223c..b2e948bdc7 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -1165,6 +1165,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, if (n->addr && n->set) { virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); + if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ + /* Wait for VM threads accessing old flatview which contains notifier. */ + drain_call_rcu(); + } + munmap(n->addr, qemu_real_host_page_size); + n->addr = NULL; n->set = false; } } @@ -1502,12 +1508,7 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, n = &user->notifier[queue_idx]; - if (n->addr) { - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); - object_unparent(OBJECT(&n->mr)); - munmap(n->addr, page_size); - n->addr = NULL; - } + vhost_user_host_notifier_remove(dev, queue_idx); if (area->u64 & VHOST_USER_VRING_NOFD_MASK) { return 0; @@ -2485,11 +2486,17 @@ void vhost_user_cleanup(VhostUserState *user) for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { if (user->notifier[i].addr) { object_unparent(OBJECT(&user->notifier[i].mr)); + } + } + memory_region_transaction_commit(); + /* Wait for VM threads accessing old flatview which contains notifier. */ + drain_call_rcu(); + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { + if (user->notifier[i].addr) { munmap(user->notifier[i].addr, qemu_real_host_page_size); user->notifier[i].addr = NULL; } } - memory_region_transaction_commit(); user->chr = NULL; } -- 2.33.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup 2021-10-08 7:58 ` [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup Xueming Li @ 2021-10-19 6:15 ` Michael S. Tsirkin 2021-10-19 6:45 ` Xueming(Steven) Li 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2021-10-19 6:15 UTC (permalink / raw) To: Xueming Li; +Cc: Yuwei Zhang, qemu-devel, tiwei.bie, qemu-stable On Fri, Oct 08, 2021 at 03:58:04PM +0800, Xueming Li wrote: > When vhost-user device cleanup and unmmap notifier address, VM cpu > thread that writing the notifier failed with accessing invalid address. > > To avoid this concurrent issue, wait memory flatview update by draining > rcu callbacks, then unmap notifiers. > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > Cc: tiwei.bie@intel.com > Cc: qemu-stable@nongnu.org > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > --- > hw/virtio/vhost-user.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index bf6e50223c..b2e948bdc7 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1165,6 +1165,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > if (n->addr && n->set) { > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > + if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > + /* Wait for VM threads accessing old flatview which contains notifier. */ > + drain_call_rcu(); > + } > + munmap(n->addr, qemu_real_host_page_size); > + n->addr = NULL; > n->set = false; > } > } ../hw/virtio/vhost-user.c: In function ‘vhost_user_host_notifier_remove’: ../hw/virtio/vhost-user.c:1168:14: error: implicit declaration of function ‘qemu_in_vcpu_thread’ [-Werror=implicit-function-declaration] 1168 | if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ | ^~~~~~~~~~~~~~~~~~~ ../hw/virtio/vhost-user.c:1168:14: error: nested extern declaration of ‘qemu_in_vcpu_thread’ [-Werror=nested-externs] cc1: all warnings being treated as errors ninja: build stopped: subcommand failed. make[1]: *** [Makefile:162: run-ninja] Error 1 make[1]: Leaving directory '/scm/qemu/build' make: *** [GNUmakefile:11: all] Error 2 Although the following patch fixes it, bisect is broken. > @@ -1502,12 +1508,7 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > n = &user->notifier[queue_idx]; > > - if (n->addr) { > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > - object_unparent(OBJECT(&n->mr)); > - munmap(n->addr, page_size); > - n->addr = NULL; > - } > + vhost_user_host_notifier_remove(dev, queue_idx); > > if (area->u64 & VHOST_USER_VRING_NOFD_MASK) { > return 0; > @@ -2485,11 +2486,17 @@ void vhost_user_cleanup(VhostUserState *user) > for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { > if (user->notifier[i].addr) { > object_unparent(OBJECT(&user->notifier[i].mr)); > + } > + } > + memory_region_transaction_commit(); > + /* Wait for VM threads accessing old flatview which contains notifier. */ > + drain_call_rcu(); > + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { > + if (user->notifier[i].addr) { > munmap(user->notifier[i].addr, qemu_real_host_page_size); > user->notifier[i].addr = NULL; > } > } > - memory_region_transaction_commit(); > user->chr = NULL; > } > > -- > 2.33.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup 2021-10-19 6:15 ` Michael S. Tsirkin @ 2021-10-19 6:45 ` Xueming(Steven) Li 2021-10-19 6:57 ` Michael S. Tsirkin 0 siblings, 1 reply; 10+ messages in thread From: Xueming(Steven) Li @ 2021-10-19 6:45 UTC (permalink / raw) To: mst; +Cc: zhangyuwei.9149, qemu-devel, qemu-stable, tiwei.bie On Tue, 2021-10-19 at 02:15 -0400, Michael S. Tsirkin wrote: > On Fri, Oct 08, 2021 at 03:58:04PM +0800, Xueming Li wrote: > > When vhost-user device cleanup and unmmap notifier address, VM cpu > > thread that writing the notifier failed with accessing invalid address. > > > > To avoid this concurrent issue, wait memory flatview update by draining > > rcu callbacks, then unmap notifiers. > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > Cc: tiwei.bie@intel.com > > Cc: qemu-stable@nongnu.org > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > --- > > hw/virtio/vhost-user.c | 21 ++++++++++++++------- > > 1 file changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index bf6e50223c..b2e948bdc7 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -1165,6 +1165,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > if (n->addr && n->set) { > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > + if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > > + /* Wait for VM threads accessing old flatview which contains notifier. */ > > + drain_call_rcu(); > > + } > > + munmap(n->addr, qemu_real_host_page_size); > > + n->addr = NULL; > > n->set = false; > > } > > } > > > ../hw/virtio/vhost-user.c: In function ‘vhost_user_host_notifier_remove’: > ../hw/virtio/vhost-user.c:1168:14: error: implicit declaration of function ‘qemu_in_vcpu_thread’ [-Werror=implicit-function-declaration] > 1168 | if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > | ^~~~~~~~~~~~~~~~~~~ > ../hw/virtio/vhost-user.c:1168:14: error: nested extern declaration of ‘qemu_in_vcpu_thread’ [-Werror=nested-externs] > cc1: all warnings being treated as errors > ninja: build stopped: subcommand failed. > make[1]: *** [Makefile:162: run-ninja] Error 1 > make[1]: Leaving directory '/scm/qemu/build' > make: *** [GNUmakefile:11: all] Error 2 > > > Although the following patch fixes it, bisect is broken. Yes, really an issue, v4 posted, thanks! > > > > @@ -1502,12 +1508,7 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > n = &user->notifier[queue_idx]; > > > > - if (n->addr) { > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > - object_unparent(OBJECT(&n->mr)); > > - munmap(n->addr, page_size); > > - n->addr = NULL; > > - } > > + vhost_user_host_notifier_remove(dev, queue_idx); > > > > if (area->u64 & VHOST_USER_VRING_NOFD_MASK) { > > return 0; > > @@ -2485,11 +2486,17 @@ void vhost_user_cleanup(VhostUserState *user) > > for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { > > if (user->notifier[i].addr) { > > object_unparent(OBJECT(&user->notifier[i].mr)); > > + } > > + } > > + memory_region_transaction_commit(); > > + /* Wait for VM threads accessing old flatview which contains notifier. */ > > + drain_call_rcu(); > > + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { > > + if (user->notifier[i].addr) { > > munmap(user->notifier[i].addr, qemu_real_host_page_size); > > user->notifier[i].addr = NULL; > > } > > } > > - memory_region_transaction_commit(); > > user->chr = NULL; > > } > > > > -- > > 2.33.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup 2021-10-19 6:45 ` Xueming(Steven) Li @ 2021-10-19 6:57 ` Michael S. Tsirkin 0 siblings, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2021-10-19 6:57 UTC (permalink / raw) To: Xueming(Steven) Li; +Cc: zhangyuwei.9149, qemu-devel, tiwei.bie, qemu-stable On Tue, Oct 19, 2021 at 06:45:19AM +0000, Xueming(Steven) Li wrote: > On Tue, 2021-10-19 at 02:15 -0400, Michael S. Tsirkin wrote: > > On Fri, Oct 08, 2021 at 03:58:04PM +0800, Xueming Li wrote: > > > When vhost-user device cleanup and unmmap notifier address, VM cpu > > > thread that writing the notifier failed with accessing invalid address. > > > > > > To avoid this concurrent issue, wait memory flatview update by draining > > > rcu callbacks, then unmap notifiers. > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > Cc: tiwei.bie@intel.com > > > Cc: qemu-stable@nongnu.org > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > --- > > > hw/virtio/vhost-user.c | 21 ++++++++++++++------- > > > 1 file changed, 14 insertions(+), 7 deletions(-) > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > index bf6e50223c..b2e948bdc7 100644 > > > --- a/hw/virtio/vhost-user.c > > > +++ b/hw/virtio/vhost-user.c > > > @@ -1165,6 +1165,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > > if (n->addr && n->set) { > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > + if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > > > + /* Wait for VM threads accessing old flatview which contains notifier. */ > > > + drain_call_rcu(); > > > + } > > > + munmap(n->addr, qemu_real_host_page_size); > > > + n->addr = NULL; > > > n->set = false; > > > } > > > } > > > > > > ../hw/virtio/vhost-user.c: In function ‘vhost_user_host_notifier_remove’: > > ../hw/virtio/vhost-user.c:1168:14: error: implicit declaration of function ‘qemu_in_vcpu_thread’ [-Werror=implicit-function-declaration] > > 1168 | if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > > | ^~~~~~~~~~~~~~~~~~~ > > ../hw/virtio/vhost-user.c:1168:14: error: nested extern declaration of ‘qemu_in_vcpu_thread’ [-Werror=nested-externs] > > cc1: all warnings being treated as errors > > ninja: build stopped: subcommand failed. > > make[1]: *** [Makefile:162: run-ninja] Error 1 > > make[1]: Leaving directory '/scm/qemu/build' > > make: *** [GNUmakefile:11: all] Error 2 > > > > > > Although the following patch fixes it, bisect is broken. > > Yes, really an issue, v4 posted, thanks! Pls address the comment on 2/2 too. > > > > > > > @@ -1502,12 +1508,7 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > > > n = &user->notifier[queue_idx]; > > > > > > - if (n->addr) { > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > - object_unparent(OBJECT(&n->mr)); > > > - munmap(n->addr, page_size); > > > - n->addr = NULL; > > > - } > > > + vhost_user_host_notifier_remove(dev, queue_idx); > > > > > > if (area->u64 & VHOST_USER_VRING_NOFD_MASK) { > > > return 0; > > > @@ -2485,11 +2486,17 @@ void vhost_user_cleanup(VhostUserState *user) > > > for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { > > > if (user->notifier[i].addr) { > > > object_unparent(OBJECT(&user->notifier[i].mr)); > > > + } > > > + } > > > + memory_region_transaction_commit(); > > > + /* Wait for VM threads accessing old flatview which contains notifier. */ > > > + drain_call_rcu(); > > > + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { > > > + if (user->notifier[i].addr) { > > > munmap(user->notifier[i].addr, qemu_real_host_page_size); > > > user->notifier[i].addr = NULL; > > > } > > > } > > > - memory_region_transaction_commit(); > > > user->chr = NULL; > > > } > > > > > > -- > > > 2.33.0 > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore 2021-10-08 7:58 [PATCH v3 0/2] Improve vhost-user VQ notifier unmap Xueming Li 2021-10-08 7:58 ` [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup Xueming Li @ 2021-10-08 7:58 ` Xueming Li 2021-10-19 6:37 ` Michael S. Tsirkin 1 sibling, 1 reply; 10+ messages in thread From: Xueming Li @ 2021-10-08 7:58 UTC (permalink / raw) To: qemu-devel Cc: xuemingl, qemu-stable, tiwei.bie, Yuwei Zhang, Michael S. Tsirkin When vhost-user vdpa client restart, VQ notifier resources become invalid, no need to keep mmap, vdpa client will set VQ notifier after reconnect. Removes VQ notifier restore and related flags. Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") Cc: tiwei.bie@intel.com Cc: qemu-stable@nongnu.org Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- hw/virtio/vhost-user.c | 20 ++------------------ include/hw/virtio/vhost-user.h | 1 - 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index b2e948bdc7..d127aa478a 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -22,6 +22,7 @@ #include "qemu/main-loop.h" #include "qemu/sockets.h" #include "sysemu/cryptodev.h" +#include "sysemu/cpus.h" #include "migration/migration.h" #include "migration/postcopy-ram.h" #include "trace.h" @@ -1143,19 +1144,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); } -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, - int queue_idx) -{ - struct vhost_user *u = dev->opaque; - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; - VirtIODevice *vdev = dev->vdev; - - if (n->addr && !n->set) { - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); - n->set = true; - } -} - static void vhost_user_host_notifier_remove(struct vhost_dev *dev, int queue_idx) { @@ -1163,7 +1151,7 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; VirtIODevice *vdev = dev->vdev; - if (n->addr && n->set) { + if (n->addr) { virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ /* Wait for VM threads accessing old flatview which contains notifier. */ @@ -1171,15 +1159,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, } munmap(n->addr, qemu_real_host_page_size); n->addr = NULL; - n->set = false; } } static int vhost_user_set_vring_base(struct vhost_dev *dev, struct vhost_vring_state *ring) { - vhost_user_host_notifier_restore(dev, ring->index); - return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); } @@ -1539,7 +1524,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, } n->addr = addr; - n->set = true; return 0; } diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h index a9abca3288..f6012b2078 100644 --- a/include/hw/virtio/vhost-user.h +++ b/include/hw/virtio/vhost-user.h @@ -14,7 +14,6 @@ typedef struct VhostUserHostNotifier { MemoryRegion mr; void *addr; - bool set; } VhostUserHostNotifier; typedef struct VhostUserState { -- 2.33.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore 2021-10-08 7:58 ` [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore Xueming Li @ 2021-10-19 6:37 ` Michael S. Tsirkin 2021-10-19 7:21 ` Xueming(Steven) Li 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2021-10-19 6:37 UTC (permalink / raw) To: Xueming Li Cc: Yuwei Zhang, Raphael Norwitz, qemu-devel, tiwei.bie, qemu-stable On Fri, Oct 08, 2021 at 03:58:05PM +0800, Xueming Li wrote: > When vhost-user vdpa client restart, VQ notifier resources become > invalid, no need to keep mmap, vdpa client will set VQ notifier after > reconnect. > > Removes VQ notifier restore and related flags. > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > Cc: tiwei.bie@intel.com > Cc: qemu-stable@nongnu.org > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > Signed-off-by: Xueming Li <xuemingl@nvidia.com> Pls fix up bisect and repost. Also, can you please clarify what does "no need" mean? You include a Fixes tag but is there a bug? What behaviour are you trying to fix? A resource leak? Or are you just simplifying code? If the later then no need for a Fixes tag. > --- > hw/virtio/vhost-user.c | 20 ++------------------ > include/hw/virtio/vhost-user.h | 1 - > 2 files changed, 2 insertions(+), 19 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index b2e948bdc7..d127aa478a 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -22,6 +22,7 @@ > #include "qemu/main-loop.h" > #include "qemu/sockets.h" > #include "sysemu/cryptodev.h" > +#include "sysemu/cpus.h" > #include "migration/migration.h" > #include "migration/postcopy-ram.h" > #include "trace.h" > @@ -1143,19 +1144,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > } > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > - int queue_idx) > -{ > - struct vhost_user *u = dev->opaque; > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > - VirtIODevice *vdev = dev->vdev; > - > - if (n->addr && !n->set) { > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > - n->set = true; > - } > -} > - > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > int queue_idx) > { > @@ -1163,7 +1151,7 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > VirtIODevice *vdev = dev->vdev; > > - if (n->addr && n->set) { > + if (n->addr) { > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > /* Wait for VM threads accessing old flatview which contains notifier. */ > @@ -1171,15 +1159,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > } > munmap(n->addr, qemu_real_host_page_size); > n->addr = NULL; > - n->set = false; > } > } > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > struct vhost_vring_state *ring) > { > - vhost_user_host_notifier_restore(dev, ring->index); > - > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > } > > @@ -1539,7 +1524,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > } > > n->addr = addr; > - n->set = true; > > return 0; > } > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > index a9abca3288..f6012b2078 100644 > --- a/include/hw/virtio/vhost-user.h > +++ b/include/hw/virtio/vhost-user.h > @@ -14,7 +14,6 @@ > typedef struct VhostUserHostNotifier { > MemoryRegion mr; > void *addr; > - bool set; > } VhostUserHostNotifier; > > typedef struct VhostUserState { > -- > 2.33.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore 2021-10-19 6:37 ` Michael S. Tsirkin @ 2021-10-19 7:21 ` Xueming(Steven) Li 2021-10-19 7:24 ` Michael S. Tsirkin 0 siblings, 1 reply; 10+ messages in thread From: Xueming(Steven) Li @ 2021-10-19 7:21 UTC (permalink / raw) To: mst; +Cc: zhangyuwei.9149, raphael.norwitz, qemu-devel, qemu-stable, tiwei.bie On Tue, 2021-10-19 at 02:37 -0400, Michael S. Tsirkin wrote: > On Fri, Oct 08, 2021 at 03:58:05PM +0800, Xueming Li wrote: > > When vhost-user vdpa client restart, VQ notifier resources become > > invalid, no need to keep mmap, vdpa client will set VQ notifier after > > reconnect. > > > > Removes VQ notifier restore and related flags. > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > Cc: tiwei.bie@intel.com > > Cc: qemu-stable@nongnu.org > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > Pls fix up bisect and repost. > Also, can you please clarify what does "no need" mean? mmap becomes invalid, need to unmap it. > You include a Fixes tag but is there a bug? What behaviour > are you trying to fix? A resource leak? > Or are you just simplifying code? > If the later then no need for a Fixes tag. Yes, just simplifying code, will remove it. > > > > > > --- > > hw/virtio/vhost-user.c | 20 ++------------------ > > include/hw/virtio/vhost-user.h | 1 - > > 2 files changed, 2 insertions(+), 19 deletions(-) > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index b2e948bdc7..d127aa478a 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -22,6 +22,7 @@ > > #include "qemu/main-loop.h" > > #include "qemu/sockets.h" > > #include "sysemu/cryptodev.h" > > +#include "sysemu/cpus.h" > > #include "migration/migration.h" > > #include "migration/postcopy-ram.h" > > #include "trace.h" > > @@ -1143,19 +1144,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > } > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > - int queue_idx) > > -{ > > - struct vhost_user *u = dev->opaque; > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > - VirtIODevice *vdev = dev->vdev; > > - > > - if (n->addr && !n->set) { > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > - n->set = true; > > - } > > -} > > - > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > int queue_idx) > > { > > @@ -1163,7 +1151,7 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > VirtIODevice *vdev = dev->vdev; > > > > - if (n->addr && n->set) { > > + if (n->addr) { > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > > /* Wait for VM threads accessing old flatview which contains notifier. */ > > @@ -1171,15 +1159,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > } > > munmap(n->addr, qemu_real_host_page_size); > > n->addr = NULL; > > - n->set = false; > > } > > } > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > struct vhost_vring_state *ring) > > { > > - vhost_user_host_notifier_restore(dev, ring->index); > > - > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > } > > > > @@ -1539,7 +1524,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > } > > > > n->addr = addr; > > - n->set = true; > > > > return 0; > > } > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > index a9abca3288..f6012b2078 100644 > > --- a/include/hw/virtio/vhost-user.h > > +++ b/include/hw/virtio/vhost-user.h > > @@ -14,7 +14,6 @@ > > typedef struct VhostUserHostNotifier { > > MemoryRegion mr; > > void *addr; > > - bool set; > > } VhostUserHostNotifier; > > > > typedef struct VhostUserState { > > -- > > 2.33.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore 2021-10-19 7:21 ` Xueming(Steven) Li @ 2021-10-19 7:24 ` Michael S. Tsirkin 2021-10-19 8:00 ` Xueming(Steven) Li 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2021-10-19 7:24 UTC (permalink / raw) To: Xueming(Steven) Li Cc: zhangyuwei.9149, qemu-stable, qemu-devel, tiwei.bie, raphael.norwitz On Tue, Oct 19, 2021 at 07:21:24AM +0000, Xueming(Steven) Li wrote: > On Tue, 2021-10-19 at 02:37 -0400, Michael S. Tsirkin wrote: > > On Fri, Oct 08, 2021 at 03:58:05PM +0800, Xueming Li wrote: > > > When vhost-user vdpa client restart, VQ notifier resources become > > > invalid, no need to keep mmap, vdpa client will set VQ notifier after > > > reconnect. > > > > > > Removes VQ notifier restore and related flags. > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > Cc: tiwei.bie@intel.com > > > Cc: qemu-stable@nongnu.org > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > Pls fix up bisect and repost. > > Also, can you please clarify what does "no need" mean? > > mmap becomes invalid, need to unmap it. confused even more. pls include explanation in commit log. > > You include a Fixes tag but is there a bug? What behaviour > > are you trying to fix? A resource leak? > > Or are you just simplifying code? > > If the later then no need for a Fixes tag. > > Yes, just simplifying code, will remove it. > > > > > > > > > > > > --- > > > hw/virtio/vhost-user.c | 20 ++------------------ > > > include/hw/virtio/vhost-user.h | 1 - > > > 2 files changed, 2 insertions(+), 19 deletions(-) > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > index b2e948bdc7..d127aa478a 100644 > > > --- a/hw/virtio/vhost-user.c > > > +++ b/hw/virtio/vhost-user.c > > > @@ -22,6 +22,7 @@ > > > #include "qemu/main-loop.h" > > > #include "qemu/sockets.h" > > > #include "sysemu/cryptodev.h" > > > +#include "sysemu/cpus.h" > > > #include "migration/migration.h" > > > #include "migration/postcopy-ram.h" > > > #include "trace.h" > > > @@ -1143,19 +1144,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > } > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > - int queue_idx) > > > -{ > > > - struct vhost_user *u = dev->opaque; > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > - VirtIODevice *vdev = dev->vdev; > > > - > > > - if (n->addr && !n->set) { > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > - n->set = true; > > > - } > > > -} > > > - > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > int queue_idx) > > > { > > > @@ -1163,7 +1151,7 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > VirtIODevice *vdev = dev->vdev; > > > > > > - if (n->addr && n->set) { > > > + if (n->addr) { > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > > > /* Wait for VM threads accessing old flatview which contains notifier. */ > > > @@ -1171,15 +1159,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > } > > > munmap(n->addr, qemu_real_host_page_size); > > > n->addr = NULL; > > > - n->set = false; > > > } > > > } > > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > struct vhost_vring_state *ring) > > > { > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > - > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > } > > > > > > @@ -1539,7 +1524,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > } > > > > > > n->addr = addr; > > > - n->set = true; > > > > > > return 0; > > > } > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > index a9abca3288..f6012b2078 100644 > > > --- a/include/hw/virtio/vhost-user.h > > > +++ b/include/hw/virtio/vhost-user.h > > > @@ -14,7 +14,6 @@ > > > typedef struct VhostUserHostNotifier { > > > MemoryRegion mr; > > > void *addr; > > > - bool set; > > > } VhostUserHostNotifier; > > > > > > typedef struct VhostUserState { > > > -- > > > 2.33.0 > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore 2021-10-19 7:24 ` Michael S. Tsirkin @ 2021-10-19 8:00 ` Xueming(Steven) Li 0 siblings, 0 replies; 10+ messages in thread From: Xueming(Steven) Li @ 2021-10-19 8:00 UTC (permalink / raw) To: mst; +Cc: qemu-devel, tiwei.bie, zhangyuwei.9149, raphael.norwitz, qemu-stable On Tue, 2021-10-19 at 03:24 -0400, Michael S. Tsirkin wrote: > On Tue, Oct 19, 2021 at 07:21:24AM +0000, Xueming(Steven) Li wrote: > > On Tue, 2021-10-19 at 02:37 -0400, Michael S. Tsirkin wrote: > > > On Fri, Oct 08, 2021 at 03:58:05PM +0800, Xueming Li wrote: > > > > When vhost-user vdpa client restart, VQ notifier resources become > > > > invalid, no need to keep mmap, vdpa client will set VQ notifier after > > > > reconnect. > > > > > > > > Removes VQ notifier restore and related flags. > > > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > > Cc: tiwei.bie@intel.com > > > > Cc: qemu-stable@nongnu.org > > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > > > Pls fix up bisect and repost. > > > Also, can you please clarify what does "no need" mean? > > > > mmap becomes invalid, need to unmap it. > > > confused even more. pls include explanation in commit log. My bad, updated in v5, please check. > > > > You include a Fixes tag but is there a bug? What behaviour > > > are you trying to fix? A resource leak? > > > Or are you just simplifying code? > > > If the later then no need for a Fixes tag. > > > > Yes, just simplifying code, will remove it. Sorry, it's a fix after another check. Will add it back. > > > > > > > > > > > > > > > > > > --- > > > > hw/virtio/vhost-user.c | 20 ++------------------ > > > > include/hw/virtio/vhost-user.h | 1 - > > > > 2 files changed, 2 insertions(+), 19 deletions(-) > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > > index b2e948bdc7..d127aa478a 100644 > > > > --- a/hw/virtio/vhost-user.c > > > > +++ b/hw/virtio/vhost-user.c > > > > @@ -22,6 +22,7 @@ > > > > #include "qemu/main-loop.h" > > > > #include "qemu/sockets.h" > > > > #include "sysemu/cryptodev.h" > > > > +#include "sysemu/cpus.h" > > > > #include "migration/migration.h" > > > > #include "migration/postcopy-ram.h" > > > > #include "trace.h" > > > > @@ -1143,19 +1144,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > > } > > > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > > - int queue_idx) > > > > -{ > > > > - struct vhost_user *u = dev->opaque; > > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > - VirtIODevice *vdev = dev->vdev; > > > > - > > > > - if (n->addr && !n->set) { > > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > > - n->set = true; > > > > - } > > > > -} > > > > - > > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > int queue_idx) > > > > { > > > > @@ -1163,7 +1151,7 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > VirtIODevice *vdev = dev->vdev; > > > > > > > > - if (n->addr && n->set) { > > > > + if (n->addr) { > > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > > if (!qemu_in_vcpu_thread()) { /* Avoid vCPU dead lock. */ > > > > /* Wait for VM threads accessing old flatview which contains notifier. */ > > > > @@ -1171,15 +1159,12 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > } > > > > munmap(n->addr, qemu_real_host_page_size); > > > > n->addr = NULL; > > > > - n->set = false; > > > > } > > > > } > > > > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > > struct vhost_vring_state *ring) > > > > { > > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > > - > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > > } > > > > > > > > @@ -1539,7 +1524,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > } > > > > > > > > n->addr = addr; > > > > - n->set = true; > > > > > > > > return 0; > > > > } > > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > > index a9abca3288..f6012b2078 100644 > > > > --- a/include/hw/virtio/vhost-user.h > > > > +++ b/include/hw/virtio/vhost-user.h > > > > @@ -14,7 +14,6 @@ > > > > typedef struct VhostUserHostNotifier { > > > > MemoryRegion mr; > > > > void *addr; > > > > - bool set; > > > > } VhostUserHostNotifier; > > > > > > > > typedef struct VhostUserState { > > > > -- > > > > 2.33.0 > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-10-19 8:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-08 7:58 [PATCH v3 0/2] Improve vhost-user VQ notifier unmap Xueming Li 2021-10-08 7:58 ` [PATCH v3 1/2] vhost-user: fix VirtQ notifier cleanup Xueming Li 2021-10-19 6:15 ` Michael S. Tsirkin 2021-10-19 6:45 ` Xueming(Steven) Li 2021-10-19 6:57 ` Michael S. Tsirkin 2021-10-08 7:58 ` [PATCH v3 2/2] vhost-user: remove VirtQ notifier restore Xueming Li 2021-10-19 6:37 ` Michael S. Tsirkin 2021-10-19 7:21 ` Xueming(Steven) Li 2021-10-19 7:24 ` Michael S. Tsirkin 2021-10-19 8:00 ` Xueming(Steven) Li
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.