* [PATCH] net/vhost: add flag to control wait queuing @ 2022-06-01 14:25 Yuan Wang 2022-06-02 8:32 ` Ling, WeiX 2022-06-27 5:51 ` [PATCH v2] net/vhost: fix deadlock on vring state change Yuan Wang 0 siblings, 2 replies; 5+ messages in thread From: Yuan Wang @ 2022-06-01 14:25 UTC (permalink / raw) To: maxime.coquelin, chenbo.xia, dev Cc: jiayu.hu, xingguang.he, yuanx.wang, stable update_queuing_status prevents PMD queue operations from affecting the data plane by waiting for rx/tx_pkt_burst to stops accessing the vhost device. In fact, it is only necessary to wait when destroy/stop the device, new/start device and vring_state_changed cases do not need. Since vring is locked when vring state changes, unconditional waiting may also cause deadlocks. To avoid deadlocks and unnecessary waiting, this patch adds a flag to control whether waiting is required. Fixes: 9dc6bb0682 (net/vhost: fix access to freed memory) Fixes: 1ce3c7fe14 (net/vhost: emulate device start/stop behavior) Cc: stable@dpdk.org Signed-off-by: Yuan Wang <yuanx.wang@intel.com> --- drivers/net/vhost/rte_eth_vhost.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c index a248a65df4..a280e788fb 100644 --- a/drivers/net/vhost/rte_eth_vhost.c +++ b/drivers/net/vhost/rte_eth_vhost.c @@ -716,7 +716,7 @@ eth_vhost_install_intr(struct rte_eth_dev *dev) } static void -update_queuing_status(struct rte_eth_dev *dev) +update_queuing_status(struct rte_eth_dev *dev, bool wait_queuing) { struct pmd_internal *internal = dev->data->dev_private; struct vhost_queue *vq; @@ -742,7 +742,7 @@ update_queuing_status(struct rte_eth_dev *dev) rte_atomic32_set(&vq->allow_queuing, 1); else rte_atomic32_set(&vq->allow_queuing, 0); - while (rte_atomic32_read(&vq->while_queuing)) + while (wait_queuing && rte_atomic32_read(&vq->while_queuing)) rte_pause(); } @@ -754,7 +754,7 @@ update_queuing_status(struct rte_eth_dev *dev) rte_atomic32_set(&vq->allow_queuing, 1); else rte_atomic32_set(&vq->allow_queuing, 0); - while (rte_atomic32_read(&vq->while_queuing)) + while (wait_queuing && rte_atomic32_read(&vq->while_queuing)) rte_pause(); } } @@ -836,7 +836,7 @@ new_device(int vid) eth_dev->data->dev_link.link_status = RTE_ETH_LINK_UP; rte_atomic32_set(&internal->dev_attached, 1); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, false); VHOST_LOG(INFO, "Vhost device %d created\n", vid); @@ -866,7 +866,7 @@ destroy_device(int vid) internal = eth_dev->data->dev_private; rte_atomic32_set(&internal->dev_attached, 0); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, true); eth_dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN; @@ -976,7 +976,7 @@ vring_state_changed(int vid, uint16_t vring, int enable) state->max_vring = RTE_MAX(vring, state->max_vring); rte_spinlock_unlock(&state->lock); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, false); VHOST_LOG(INFO, "vring%u is %s\n", vring, enable ? "enabled" : "disabled"); @@ -1163,7 +1163,7 @@ eth_dev_start(struct rte_eth_dev *eth_dev) } rte_atomic32_set(&internal->started, 1); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, false); return 0; } @@ -1175,7 +1175,7 @@ eth_dev_stop(struct rte_eth_dev *dev) dev->data->dev_started = 0; rte_atomic32_set(&internal->started, 0); - update_queuing_status(dev); + update_queuing_status(dev, true); return 0; } -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net/vhost: add flag to control wait queuing 2022-06-01 14:25 [PATCH] net/vhost: add flag to control wait queuing Yuan Wang @ 2022-06-02 8:32 ` Ling, WeiX 2022-06-27 5:51 ` [PATCH v2] net/vhost: fix deadlock on vring state change Yuan Wang 1 sibling, 0 replies; 5+ messages in thread From: Ling, WeiX @ 2022-06-02 8:32 UTC (permalink / raw) To: Wang, YuanX, maxime.coquelin, Xia, Chenbo, dev Cc: Hu, Jiayu, He, Xingguang, Wang, YuanX, stable > -----Original Message----- > From: Yuan Wang <yuanx.wang@intel.com> > Sent: Wednesday, June 1, 2022 10:26 PM > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; > dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang > <xingguang.he@intel.com>; Wang, YuanX <yuanx.wang@intel.com>; > stable@dpdk.org > Subject: [PATCH] net/vhost: add flag to control wait queuing > > update_queuing_status prevents PMD queue operations from affecting the > data plane by waiting for rx/tx_pkt_burst to stops accessing the vhost device. > In fact, it is only necessary to wait when destroy/stop the device, new/start > device and vring_state_changed cases do not need. > > Since vring is locked when vring state changes, unconditional waiting may > also cause deadlocks. > > To avoid deadlocks and unnecessary waiting, this patch adds a flag to control > whether waiting is required. > > Fixes: 9dc6bb0682 (net/vhost: fix access to freed memory) > Fixes: 1ce3c7fe14 (net/vhost: emulate device start/stop behavior) > Cc: stable@dpdk.org > > Signed-off-by: Yuan Wang <yuanx.wang@intel.com> > --- Tested-by: Wei Ling <weix.ling@intel.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] net/vhost: fix deadlock on vring state change 2022-06-01 14:25 [PATCH] net/vhost: add flag to control wait queuing Yuan Wang 2022-06-02 8:32 ` Ling, WeiX @ 2022-06-27 5:51 ` Yuan Wang 2022-07-01 12:31 ` Xia, Chenbo 2022-07-01 13:58 ` Maxime Coquelin 1 sibling, 2 replies; 5+ messages in thread From: Yuan Wang @ 2022-06-27 5:51 UTC (permalink / raw) To: maxime.coquelin, chenbo.xia, dev Cc: jiayu.hu, xingguang.he, cheng1.jiang, weix.ling, Yuan Wang, stable If vring state changes after pmd starts working, the locked vring notifies pmd, thus calling update_queuing_status(), the latter will wait for pmd to finish accessing vring, while pmd is also waiting for vring to be unlocked, thus causing deadlock. Actually, update_queuing_status() only needs to wait while destroy/stopping the device, but not in other cases. This patch adds a flag for whether or not to wait to fix this issue. Fixes: 1ce3c7fe149f ("net/vhost: emulate device start/stop behavior") Cc: stable@dpdk.org Signed-off-by: Yuan Wang <yuanx.wang@intel.com> --- V2: rewrite the commit log. --- drivers/net/vhost/rte_eth_vhost.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c index d75d256040..7e512d94bf 100644 --- a/drivers/net/vhost/rte_eth_vhost.c +++ b/drivers/net/vhost/rte_eth_vhost.c @@ -741,7 +741,7 @@ eth_vhost_install_intr(struct rte_eth_dev *dev) } static void -update_queuing_status(struct rte_eth_dev *dev) +update_queuing_status(struct rte_eth_dev *dev, bool wait_queuing) { struct pmd_internal *internal = dev->data->dev_private; struct vhost_queue *vq; @@ -767,7 +767,7 @@ update_queuing_status(struct rte_eth_dev *dev) rte_atomic32_set(&vq->allow_queuing, 1); else rte_atomic32_set(&vq->allow_queuing, 0); - while (rte_atomic32_read(&vq->while_queuing)) + while (wait_queuing && rte_atomic32_read(&vq->while_queuing)) rte_pause(); } @@ -779,7 +779,7 @@ update_queuing_status(struct rte_eth_dev *dev) rte_atomic32_set(&vq->allow_queuing, 1); else rte_atomic32_set(&vq->allow_queuing, 0); - while (rte_atomic32_read(&vq->while_queuing)) + while (wait_queuing && rte_atomic32_read(&vq->while_queuing)) rte_pause(); } } @@ -868,7 +868,7 @@ new_device(int vid) vhost_dev_csum_configure(eth_dev); rte_atomic32_set(&internal->dev_attached, 1); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, false); VHOST_LOG(INFO, "Vhost device %d created\n", vid); @@ -898,7 +898,7 @@ destroy_device(int vid) internal = eth_dev->data->dev_private; rte_atomic32_set(&internal->dev_attached, 0); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, true); eth_dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN; @@ -1008,7 +1008,7 @@ vring_state_changed(int vid, uint16_t vring, int enable) state->max_vring = RTE_MAX(vring, state->max_vring); rte_spinlock_unlock(&state->lock); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, false); VHOST_LOG(INFO, "vring%u is %s\n", vring, enable ? "enabled" : "disabled"); @@ -1197,7 +1197,7 @@ eth_dev_start(struct rte_eth_dev *eth_dev) } rte_atomic32_set(&internal->started, 1); - update_queuing_status(eth_dev); + update_queuing_status(eth_dev, false); return 0; } @@ -1209,7 +1209,7 @@ eth_dev_stop(struct rte_eth_dev *dev) dev->data->dev_started = 0; rte_atomic32_set(&internal->started, 0); - update_queuing_status(dev); + update_queuing_status(dev, true); return 0; } -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] net/vhost: fix deadlock on vring state change 2022-06-27 5:51 ` [PATCH v2] net/vhost: fix deadlock on vring state change Yuan Wang @ 2022-07-01 12:31 ` Xia, Chenbo 2022-07-01 13:58 ` Maxime Coquelin 1 sibling, 0 replies; 5+ messages in thread From: Xia, Chenbo @ 2022-07-01 12:31 UTC (permalink / raw) To: Wang, YuanX, maxime.coquelin, dev Cc: Hu, Jiayu, He, Xingguang, Jiang, Cheng1, Ling, WeiX, stable > -----Original Message----- > From: Wang, YuanX <yuanx.wang@intel.com> > Sent: Monday, June 27, 2022 1:51 PM > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; > dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang <xingguang.he@intel.com>; > Jiang, Cheng1 <cheng1.jiang@intel.com>; Ling, WeiX <weix.ling@intel.com>; > Wang, YuanX <yuanx.wang@intel.com>; stable@dpdk.org > Subject: [PATCH v2] net/vhost: fix deadlock on vring state change > > If vring state changes after pmd starts working, the locked vring > notifies pmd, thus calling update_queuing_status(), the latter > will wait for pmd to finish accessing vring, while pmd is also > waiting for vring to be unlocked, thus causing deadlock. > > Actually, update_queuing_status() only needs to wait while > destroy/stopping the device, but not in other cases. > > This patch adds a flag for whether or not to wait to fix this issue. > > Fixes: 1ce3c7fe149f ("net/vhost: emulate device start/stop behavior") > Cc: stable@dpdk.org > > Signed-off-by: Yuan Wang <yuanx.wang@intel.com> > --- > V2: rewrite the commit log. > --- > drivers/net/vhost/rte_eth_vhost.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/vhost/rte_eth_vhost.c > b/drivers/net/vhost/rte_eth_vhost.c > index d75d256040..7e512d94bf 100644 > --- a/drivers/net/vhost/rte_eth_vhost.c > +++ b/drivers/net/vhost/rte_eth_vhost.c > @@ -741,7 +741,7 @@ eth_vhost_install_intr(struct rte_eth_dev *dev) > } > > static void > -update_queuing_status(struct rte_eth_dev *dev) > +update_queuing_status(struct rte_eth_dev *dev, bool wait_queuing) > { > struct pmd_internal *internal = dev->data->dev_private; > struct vhost_queue *vq; > @@ -767,7 +767,7 @@ update_queuing_status(struct rte_eth_dev *dev) > rte_atomic32_set(&vq->allow_queuing, 1); > else > rte_atomic32_set(&vq->allow_queuing, 0); > - while (rte_atomic32_read(&vq->while_queuing)) > + while (wait_queuing && rte_atomic32_read(&vq->while_queuing)) > rte_pause(); > } > > @@ -779,7 +779,7 @@ update_queuing_status(struct rte_eth_dev *dev) > rte_atomic32_set(&vq->allow_queuing, 1); > else > rte_atomic32_set(&vq->allow_queuing, 0); > - while (rte_atomic32_read(&vq->while_queuing)) > + while (wait_queuing && rte_atomic32_read(&vq->while_queuing)) > rte_pause(); > } > } > @@ -868,7 +868,7 @@ new_device(int vid) > vhost_dev_csum_configure(eth_dev); > > rte_atomic32_set(&internal->dev_attached, 1); > - update_queuing_status(eth_dev); > + update_queuing_status(eth_dev, false); > > VHOST_LOG(INFO, "Vhost device %d created\n", vid); > > @@ -898,7 +898,7 @@ destroy_device(int vid) > internal = eth_dev->data->dev_private; > > rte_atomic32_set(&internal->dev_attached, 0); > - update_queuing_status(eth_dev); > + update_queuing_status(eth_dev, true); > > eth_dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN; > > @@ -1008,7 +1008,7 @@ vring_state_changed(int vid, uint16_t vring, int > enable) > state->max_vring = RTE_MAX(vring, state->max_vring); > rte_spinlock_unlock(&state->lock); > > - update_queuing_status(eth_dev); > + update_queuing_status(eth_dev, false); > > VHOST_LOG(INFO, "vring%u is %s\n", > vring, enable ? "enabled" : "disabled"); > @@ -1197,7 +1197,7 @@ eth_dev_start(struct rte_eth_dev *eth_dev) > } > > rte_atomic32_set(&internal->started, 1); > - update_queuing_status(eth_dev); > + update_queuing_status(eth_dev, false); > > return 0; > } > @@ -1209,7 +1209,7 @@ eth_dev_stop(struct rte_eth_dev *dev) > > dev->data->dev_started = 0; > rte_atomic32_set(&internal->started, 0); > - update_queuing_status(dev); > + update_queuing_status(dev, true); > > return 0; > } > -- > 2.25.1 Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net/vhost: fix deadlock on vring state change 2022-06-27 5:51 ` [PATCH v2] net/vhost: fix deadlock on vring state change Yuan Wang 2022-07-01 12:31 ` Xia, Chenbo @ 2022-07-01 13:58 ` Maxime Coquelin 1 sibling, 0 replies; 5+ messages in thread From: Maxime Coquelin @ 2022-07-01 13:58 UTC (permalink / raw) To: Yuan Wang, chenbo.xia, dev Cc: jiayu.hu, xingguang.he, cheng1.jiang, weix.ling, stable On 6/27/22 07:51, Yuan Wang wrote: > If vring state changes after pmd starts working, the locked vring > notifies pmd, thus calling update_queuing_status(), the latter > will wait for pmd to finish accessing vring, while pmd is also > waiting for vring to be unlocked, thus causing deadlock. > > Actually, update_queuing_status() only needs to wait while > destroy/stopping the device, but not in other cases. > > This patch adds a flag for whether or not to wait to fix this issue. > > Fixes: 1ce3c7fe149f ("net/vhost: emulate device start/stop behavior") > Cc: stable@dpdk.org > > Signed-off-by: Yuan Wang <yuanx.wang@intel.com> > --- > V2: rewrite the commit log. > --- > drivers/net/vhost/rte_eth_vhost.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > Applied to dpdk-next-virtio/main. Thanks, Maxime ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-01 13:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-01 14:25 [PATCH] net/vhost: add flag to control wait queuing Yuan Wang 2022-06-02 8:32 ` Ling, WeiX 2022-06-27 5:51 ` [PATCH v2] net/vhost: fix deadlock on vring state change Yuan Wang 2022-07-01 12:31 ` Xia, Chenbo 2022-07-01 13:58 ` Maxime Coquelin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).