Hi Chenbo, On 6/25/21 1:37 PM, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: stable <stable-boun...@dpdk.org> On Behalf Of Xia, Chenbo >> Sent: Friday, June 25, 2021 10:56 AM >> To: Maxime Coquelin <maxime.coque...@redhat.com>; dev@dpdk.org; >> david.march...@redhat.com >> Cc: sta...@dpdk.org >> Subject: Re: [dpdk-stable] [PATCH v6 4/7] vhost: fix NUMA reallocation >> with multiqueue >> >> Hi Maxime, >> >>> -----Original Message----- >>> From: Maxime Coquelin <maxime.coque...@redhat.com> >>> Sent: Friday, June 18, 2021 10:04 PM >>> To: dev@dpdk.org; david.march...@redhat.com; Xia, Chenbo >> <chenbo....@intel.com> >>> Cc: Maxime Coquelin <maxime.coque...@redhat.com>; sta...@dpdk.org >>> Subject: [PATCH v6 4/7] vhost: fix NUMA reallocation with multiqueue >>> >>> Since the Vhost-user device initialization has been reworked, >>> enabling the application to start using the device as soon as >>> the first queue pair is ready, NUMA reallocation no more >>> happened on queue pairs other than the first one since >>> numa_realloc() was returning early if the device was running. >>> >>> This patch fixes this issue by only preventing the device >>> metadata to be allocated if the device is running. For the >>> virtqueues, a vring state change notification is sent to >>> notify the application of its disablement. Since the callback >>> is supposed to be blocking, it is safe to reallocate it >>> afterwards. >>> >>> Fixes: d0fcc38f5fa4 ("vhost: improve device readiness notifications") >>> Cc: sta...@dpdk.org >>> >>> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com> >>> --- >>> lib/vhost/vhost_user.c | 13 ++++++++++--- >>> 1 file changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c >>> index 82adf80fe5..51b96a0716 100644 >>> --- a/lib/vhost/vhost_user.c >>> +++ b/lib/vhost/vhost_user.c >>> @@ -488,12 +488,16 @@ numa_realloc(struct virtio_net *dev, int index) >>> struct batch_copy_elem *new_batch_copy_elems; >>> int ret; >>> >>> - if (dev->flags & VIRTIO_DEV_RUNNING) >>> - return dev; >>> - >>> old_dev = dev; >>> vq = old_vq = dev->virtqueue[index]; >>> >>> + /* >>> + * If VQ is ready, it is too late to reallocate, it certainly >> already >>> + * happened anyway on VHOST_USER_SET_VRING_ADRR. >>> + */ >>> + if (vq->ready) >>> + return dev; >>> + >>> ret = get_mempolicy(&newnode, NULL, 0, old_vq->desc, >>> MPOL_F_NODE | MPOL_F_ADDR); >>> >>> @@ -558,6 +562,9 @@ numa_realloc(struct virtio_net *dev, int index) >>> rte_free(old_vq); >>> } >>> >>> + if (dev->flags & VIRTIO_DEV_RUNNING) >>> + goto out; >>> + >> >> Since we don't realloc when vq is ready, there is no case that vq not >> ready but >> device still running, right? > > Sorry, I forgot DEV_RUNNING now only requires 1 qpair ready now ☹ > Ignore above comments..
No problem, thanks for the review! > Thanks, > Chenbo > >> >> Thanks, >> Chenbo >> >>> /* check if we need to reallocate dev */ >>> ret = get_mempolicy(&oldnode, NULL, 0, old_dev, >>> MPOL_F_NODE | MPOL_F_ADDR); >>> -- >>> 2.31.1 >