* [PATCH v2] vhost: fix deadlock when handling user messages [not found] <20220505134008.2865-1-wenwux.ma@intel.com> @ 2022-05-05 16:17 ` Wenwu Ma 2022-05-06 8:19 ` Xia, Chenbo 2022-05-07 13:27 ` [PATCH v3] vhost: fix deadlock when message handling failed Wenwu Ma 1 sibling, 1 reply; 11+ messages in thread From: Wenwu Ma @ 2022-05-05 16:17 UTC (permalink / raw) To: maxime.coquelin, chenbo.xia, dev Cc: jiayu.hu, yinan.wang, xingguang.he, Wenwu Ma, stable In vhost_user_msg_handler(), if vhost message handling failed, we should check whether the queue is locked and release the lock before returning. Or, it will cause a deadlock later. Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness notification") Cc: stable@dpdk.org Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> --- lib/vhost/vhost_user.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 1d390677fa..4baf969ee0 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2976,7 +2976,6 @@ vhost_user_msg_handler(int vid, int fd) return -1; } - ret = 0; request = ctx.msg.request.master; if (request > VHOST_USER_NONE && request < VHOST_USER_MAX && vhost_message_str[request]) { @@ -3113,9 +3112,11 @@ vhost_user_msg_handler(int vid, int fd) send_vhost_reply(dev, fd, &ctx); } else if (ret == RTE_VHOST_MSG_RESULT_ERR) { VHOST_LOG_CONFIG(ERR, "(%s) vhost message handling failed.\n", dev->ifname); - return -1; + ret = -1; + goto unlock; } + ret = 0; for (i = 0; i < dev->nr_vring; i++) { struct vhost_virtqueue *vq = dev->virtqueue[i]; bool cur_ready = vq_is_ready(dev, vq); @@ -3126,10 +3127,11 @@ vhost_user_msg_handler(int vid, int fd) } } +unlock: if (unlock_required) vhost_user_unlock_all_queue_pairs(dev); - if (!virtio_is_ready(dev)) + if (ret != 0 || !virtio_is_ready(dev)) goto out; /* @@ -3156,7 +3158,7 @@ vhost_user_msg_handler(int vid, int fd) } out: - return 0; + return ret; } static int process_slave_message_reply(struct virtio_net *dev, -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2] vhost: fix deadlock when handling user messages 2022-05-05 16:17 ` [PATCH v2] vhost: fix deadlock when handling user messages Wenwu Ma @ 2022-05-06 8:19 ` Xia, Chenbo 0 siblings, 0 replies; 11+ messages in thread From: Xia, Chenbo @ 2022-05-06 8:19 UTC (permalink / raw) To: Ma, WenwuX, maxime.coquelin, dev Cc: Hu, Jiayu, Wang, Yinan, He, Xingguang, stable > -----Original Message----- > From: Ma, WenwuX <wenwux.ma@intel.com> > Sent: Friday, May 6, 2022 12:17 AM > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; > dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He, > Xingguang <xingguang.he@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; > stable@dpdk.org > Subject: [PATCH v2] vhost: fix deadlock when handling user messages > > In vhost_user_msg_handler(), if vhost message handling > failed, we should check whether the queue is locked and > release the lock before returning. Or, it will cause a > deadlock later. > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness notification") > Cc: stable@dpdk.org > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > --- > lib/vhost/vhost_user.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > index 1d390677fa..4baf969ee0 100644 > --- a/lib/vhost/vhost_user.c > +++ b/lib/vhost/vhost_user.c > @@ -2976,7 +2976,6 @@ vhost_user_msg_handler(int vid, int fd) > return -1; > } > > - ret = 0; > request = ctx.msg.request.master; > if (request > VHOST_USER_NONE && request < VHOST_USER_MAX && > vhost_message_str[request]) { > @@ -3113,9 +3112,11 @@ vhost_user_msg_handler(int vid, int fd) > send_vhost_reply(dev, fd, &ctx); > } else if (ret == RTE_VHOST_MSG_RESULT_ERR) { > VHOST_LOG_CONFIG(ERR, "(%s) vhost message handling failed.\n", > dev->ifname); > - return -1; > + ret = -1; > + goto unlock; > } > > + ret = 0; > for (i = 0; i < dev->nr_vring; i++) { > struct vhost_virtqueue *vq = dev->virtqueue[i]; > bool cur_ready = vq_is_ready(dev, vq); > @@ -3126,10 +3127,11 @@ vhost_user_msg_handler(int vid, int fd) > } > } > > +unlock: > if (unlock_required) > vhost_user_unlock_all_queue_pairs(dev); > > - if (!virtio_is_ready(dev)) > + if (ret != 0 || !virtio_is_ready(dev)) > goto out; > > /* > @@ -3156,7 +3158,7 @@ vhost_user_msg_handler(int vid, int fd) > } > > out: > - return 0; > + return ret; > } > > static int process_slave_message_reply(struct virtio_net *dev, > -- > 2.25.1 'vhost: fix deadlock when message handling failed' may be a better title. Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] vhost: fix deadlock when message handling failed [not found] <20220505134008.2865-1-wenwux.ma@intel.com> 2022-05-05 16:17 ` [PATCH v2] vhost: fix deadlock when handling user messages Wenwu Ma @ 2022-05-07 13:27 ` Wenwu Ma 2022-05-07 6:02 ` Xia, Chenbo ` (3 more replies) 1 sibling, 4 replies; 11+ messages in thread From: Wenwu Ma @ 2022-05-07 13:27 UTC (permalink / raw) To: maxime.coquelin, chenbo.xia, dev Cc: jiayu.hu, yinan.wang, xingguang.he, Wenwu Ma, stable In vhost_user_msg_handler(), if vhost message handling failed, we should check whether the queue is locked and release the lock before returning. Or, it will cause a deadlock later. Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness notification") Cc: stable@dpdk.org Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> --- lib/vhost/vhost_user.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 1d390677fa..4baf969ee0 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2976,7 +2976,6 @@ vhost_user_msg_handler(int vid, int fd) return -1; } - ret = 0; request = ctx.msg.request.master; if (request > VHOST_USER_NONE && request < VHOST_USER_MAX && vhost_message_str[request]) { @@ -3113,9 +3112,11 @@ vhost_user_msg_handler(int vid, int fd) send_vhost_reply(dev, fd, &ctx); } else if (ret == RTE_VHOST_MSG_RESULT_ERR) { VHOST_LOG_CONFIG(ERR, "(%s) vhost message handling failed.\n", dev->ifname); - return -1; + ret = -1; + goto unlock; } + ret = 0; for (i = 0; i < dev->nr_vring; i++) { struct vhost_virtqueue *vq = dev->virtqueue[i]; bool cur_ready = vq_is_ready(dev, vq); @@ -3126,10 +3127,11 @@ vhost_user_msg_handler(int vid, int fd) } } +unlock: if (unlock_required) vhost_user_unlock_all_queue_pairs(dev); - if (!virtio_is_ready(dev)) + if (ret != 0 || !virtio_is_ready(dev)) goto out; /* @@ -3156,7 +3158,7 @@ vhost_user_msg_handler(int vid, int fd) } out: - return 0; + return ret; } static int process_slave_message_reply(struct virtio_net *dev, -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] vhost: fix deadlock when message handling failed 2022-05-07 13:27 ` [PATCH v3] vhost: fix deadlock when message handling failed Wenwu Ma @ 2022-05-07 6:02 ` Xia, Chenbo 2022-05-09 3:13 ` Ling, WeiX ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Xia, Chenbo @ 2022-05-07 6:02 UTC (permalink / raw) To: Ma, WenwuX, maxime.coquelin, dev Cc: Hu, Jiayu, Wang, Yinan, He, Xingguang, stable > -----Original Message----- > From: Ma, WenwuX <wenwux.ma@intel.com> > Sent: Saturday, May 7, 2022 9:28 PM > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; > dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He, > Xingguang <xingguang.he@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; > stable@dpdk.org > Subject: [PATCH v3] vhost: fix deadlock when message handling failed > > In vhost_user_msg_handler(), if vhost message handling > failed, we should check whether the queue is locked and > release the lock before returning. Or, it will cause a > deadlock later. > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness notification") > Cc: stable@dpdk.org > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > --- > lib/vhost/vhost_user.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > index 1d390677fa..4baf969ee0 100644 > --- a/lib/vhost/vhost_user.c > +++ b/lib/vhost/vhost_user.c > @@ -2976,7 +2976,6 @@ vhost_user_msg_handler(int vid, int fd) > return -1; > } > > - ret = 0; > request = ctx.msg.request.master; > if (request > VHOST_USER_NONE && request < VHOST_USER_MAX && > vhost_message_str[request]) { > @@ -3113,9 +3112,11 @@ vhost_user_msg_handler(int vid, int fd) > send_vhost_reply(dev, fd, &ctx); > } else if (ret == RTE_VHOST_MSG_RESULT_ERR) { > VHOST_LOG_CONFIG(ERR, "(%s) vhost message handling failed.\n", > dev->ifname); > - return -1; > + ret = -1; > + goto unlock; > } > > + ret = 0; > for (i = 0; i < dev->nr_vring; i++) { > struct vhost_virtqueue *vq = dev->virtqueue[i]; > bool cur_ready = vq_is_ready(dev, vq); > @@ -3126,10 +3127,11 @@ vhost_user_msg_handler(int vid, int fd) > } > } > > +unlock: > if (unlock_required) > vhost_user_unlock_all_queue_pairs(dev); > > - if (!virtio_is_ready(dev)) > + if (ret != 0 || !virtio_is_ready(dev)) > goto out; > > /* > @@ -3156,7 +3158,7 @@ vhost_user_msg_handler(int vid, int fd) > } > > out: > - return 0; > + return ret; > } > > static int process_slave_message_reply(struct virtio_net *dev, > -- > 2.25.1 Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] vhost: fix deadlock when message handling failed 2022-05-07 13:27 ` [PATCH v3] vhost: fix deadlock when message handling failed Wenwu Ma 2022-05-07 6:02 ` Xia, Chenbo @ 2022-05-09 3:13 ` Ling, WeiX 2022-05-17 13:15 ` David Marchand 2022-05-17 13:24 ` Maxime Coquelin 3 siblings, 0 replies; 11+ messages in thread From: Ling, WeiX @ 2022-05-09 3:13 UTC (permalink / raw) To: Ma, WenwuX, maxime.coquelin, Xia, Chenbo, dev Cc: Hu, Jiayu, Wang, Yinan, He, Xingguang, Ma, WenwuX, stable > -----Original Message----- > From: Wenwu Ma <wenwux.ma@intel.com> > Sent: Saturday, May 7, 2022 9:28 PM > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; > dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; > He, Xingguang <xingguang.he@intel.com>; Ma, WenwuX > <wenwux.ma@intel.com>; stable@dpdk.org > Subject: [PATCH v3] vhost: fix deadlock when message handling failed > Tested-by: Wei Ling <weix.ling@intel.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] vhost: fix deadlock when message handling failed 2022-05-07 13:27 ` [PATCH v3] vhost: fix deadlock when message handling failed Wenwu Ma 2022-05-07 6:02 ` Xia, Chenbo 2022-05-09 3:13 ` Ling, WeiX @ 2022-05-17 13:15 ` David Marchand 2022-05-17 13:24 ` Maxime Coquelin 3 siblings, 0 replies; 11+ messages in thread From: David Marchand @ 2022-05-17 13:15 UTC (permalink / raw) To: Wenwu Ma Cc: Maxime Coquelin, Xia, Chenbo, dev, Jiayu Hu, Wang, Yinan, xingguang.he, dpdk stable On Sat, May 7, 2022 at 7:30 AM Wenwu Ma <wenwux.ma@intel.com> wrote: > > In vhost_user_msg_handler(), if vhost message handling > failed, we should check whether the queue is locked and > release the lock before returning. Or, it will cause a > deadlock later. > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness notification") > Cc: stable@dpdk.org > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> Thanks, lgtm. Acked-by: David Marchand <david.marchand@redhat.com> -- David Marchand ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] vhost: fix deadlock when message handling failed 2022-05-07 13:27 ` [PATCH v3] vhost: fix deadlock when message handling failed Wenwu Ma ` (2 preceding siblings ...) 2022-05-17 13:15 ` David Marchand @ 2022-05-17 13:24 ` Maxime Coquelin 2022-07-11 8:42 ` Pei, Andy 3 siblings, 1 reply; 11+ messages in thread From: Maxime Coquelin @ 2022-05-17 13:24 UTC (permalink / raw) To: Wenwu Ma, chenbo.xia, dev; +Cc: jiayu.hu, yinan.wang, xingguang.he, stable On 5/7/22 15:27, Wenwu Ma wrote: > In vhost_user_msg_handler(), if vhost message handling > failed, we should check whether the queue is locked and > release the lock before returning. Or, it will cause a > deadlock later. > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness notification") > Cc: stable@dpdk.org > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > --- > lib/vhost/vhost_user.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > Applied to dpdk-next-virtio/main. Thanks, Maxime ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] vhost: fix deadlock when message handling failed 2022-05-17 13:24 ` Maxime Coquelin @ 2022-07-11 8:42 ` Pei, Andy 2022-07-11 8:54 ` Xia, Chenbo 0 siblings, 1 reply; 11+ messages in thread From: Pei, Andy @ 2022-07-11 8:42 UTC (permalink / raw) To: Maxime Coquelin, Ma, WenwuX, Xia, Chenbo, dev Cc: Hu, Jiayu, Wang, Yinan, He, Xingguang, stable HI ALL, I see that in function vhost_user_msg_handler. We use "ret" to store both vhost msg return code like "RTE_VHOST_MSG_RESULT_XXX" and function return value. I wonder if it is better to use two different variable to make it easy to read. > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Tuesday, May 17, 2022 9:24 PM > To: Ma, WenwuX <WenwuX.Ma@intel.com>; Xia, Chenbo > <Chenbo.Xia@intel.com>; dev@dpdk.org > Cc: Hu, Jiayu <Jiayu.Hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He, > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > Subject: Re: [PATCH v3] vhost: fix deadlock when message handling failed > > > > On 5/7/22 15:27, Wenwu Ma wrote: > > In vhost_user_msg_handler(), if vhost message handling failed, we > > should check whether the queue is locked and release the lock before > > returning. Or, it will cause a deadlock later. > > > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness > > notification") > > Cc: stable@dpdk.org > > > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > > --- > > lib/vhost/vhost_user.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > Applied to dpdk-next-virtio/main. > > Thanks, > Maxime ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] vhost: fix deadlock when message handling failed 2022-07-11 8:42 ` Pei, Andy @ 2022-07-11 8:54 ` Xia, Chenbo 2022-07-11 9:02 ` Pei, Andy 0 siblings, 1 reply; 11+ messages in thread From: Xia, Chenbo @ 2022-07-11 8:54 UTC (permalink / raw) To: Pei, Andy, Maxime Coquelin, Ma, WenwuX, dev Cc: Hu, Jiayu, Wang, Yinan, He, Xingguang, stable > -----Original Message----- > From: Pei, Andy <andy.pei@intel.com> > Sent: Monday, July 11, 2022 4:42 PM > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Ma, WenwuX > <wenwux.ma@intel.com>; Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He, > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > Subject: RE: [PATCH v3] vhost: fix deadlock when message handling failed > > HI ALL, > > I see that in function vhost_user_msg_handler. > We use "ret" to store both vhost msg return code like > "RTE_VHOST_MSG_RESULT_XXX" and function return value. > I wonder if it is better to use two different variable to make it easy to > read. By saying 'function return value', you mean which function? Can you elaborate more? Thanks, Chenbo > > > -----Original Message----- > > From: Maxime Coquelin <maxime.coquelin@redhat.com> > > Sent: Tuesday, May 17, 2022 9:24 PM > > To: Ma, WenwuX <WenwuX.Ma@intel.com>; Xia, Chenbo > > <Chenbo.Xia@intel.com>; dev@dpdk.org > > Cc: Hu, Jiayu <Jiayu.Hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; > He, > > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > > Subject: Re: [PATCH v3] vhost: fix deadlock when message handling failed > > > > > > > > On 5/7/22 15:27, Wenwu Ma wrote: > > > In vhost_user_msg_handler(), if vhost message handling failed, we > > > should check whether the queue is locked and release the lock before > > > returning. Or, it will cause a deadlock later. > > > > > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness > > > notification") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > > > --- > > > lib/vhost/vhost_user.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > > Applied to dpdk-next-virtio/main. > > > > Thanks, > > Maxime ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] vhost: fix deadlock when message handling failed 2022-07-11 8:54 ` Xia, Chenbo @ 2022-07-11 9:02 ` Pei, Andy 2022-07-11 9:14 ` Xia, Chenbo 0 siblings, 1 reply; 11+ messages in thread From: Pei, Andy @ 2022-07-11 9:02 UTC (permalink / raw) To: Xia, Chenbo, Maxime Coquelin, Ma, WenwuX, dev Cc: Hu, Jiayu, Wang, Yinan, He, Xingguang, stable HI Chenbo, See below example 1. ret = read_vhost_message(dev, fd, &ctx); if (ret <= 0) { if (ret < 0) VHOST_LOG_CONFIG(dev->ifname, ERR, "vhost read message failed\n"); else VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer closed\n"); return -1; } 2. ret = vhost_user_check_and_alloc_queue_pair(dev, &ctx); if (ret < 0) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to alloc queue\n"); return -1; } 3. ret = -1 } else if (ret == RTE_VHOST_MSG_RESULT_ERR) { VHOST_LOG_CONFIG(dev->ifname, ERR, "vhost message handling failed.\n"); ret = -1; goto unlock; } I mean ret is used to check the function is returned successfully or not while it is also used to store vhost msg return code like "RTE_VHOST_MSG_RESULT_XXX". > -----Original Message----- > From: Xia, Chenbo <chenbo.xia@intel.com> > Sent: Monday, July 11, 2022 4:54 PM > To: Pei, Andy <andy.pei@intel.com>; Maxime Coquelin > <maxime.coquelin@redhat.com>; Ma, WenwuX <wenwux.ma@intel.com>; > dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He, > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > Subject: RE: [PATCH v3] vhost: fix deadlock when message handling failed > > > -----Original Message----- > > From: Pei, Andy <andy.pei@intel.com> > > Sent: Monday, July 11, 2022 4:42 PM > > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Ma, WenwuX > > <wenwux.ma@intel.com>; Xia, Chenbo <chenbo.xia@intel.com>; > > dev@dpdk.org > > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan > > <yinan.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; > > stable@dpdk.org > > Subject: RE: [PATCH v3] vhost: fix deadlock when message handling > > failed > > > > HI ALL, > > > > I see that in function vhost_user_msg_handler. > > We use "ret" to store both vhost msg return code like > > "RTE_VHOST_MSG_RESULT_XXX" and function return value. > > I wonder if it is better to use two different variable to make it easy > > to read. > > By saying 'function return value', you mean which function? Can you elaborate > more? > > Thanks, > Chenbo > > > > > > -----Original Message----- > > > From: Maxime Coquelin <maxime.coquelin@redhat.com> > > > Sent: Tuesday, May 17, 2022 9:24 PM > > > To: Ma, WenwuX <WenwuX.Ma@intel.com>; Xia, Chenbo > > > <Chenbo.Xia@intel.com>; dev@dpdk.org > > > Cc: Hu, Jiayu <Jiayu.Hu@intel.com>; Wang, Yinan > > > <yinan.wang@intel.com>; > > He, > > > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > > > Subject: Re: [PATCH v3] vhost: fix deadlock when message handling > > > failed > > > > > > > > > > > > On 5/7/22 15:27, Wenwu Ma wrote: > > > > In vhost_user_msg_handler(), if vhost message handling failed, we > > > > should check whether the queue is locked and release the lock > > > > before returning. Or, it will cause a deadlock later. > > > > > > > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness > > > > notification") > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > > > > --- > > > > lib/vhost/vhost_user.c | 10 ++++++---- > > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > > > > > Applied to dpdk-next-virtio/main. > > > > > > Thanks, > > > Maxime ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] vhost: fix deadlock when message handling failed 2022-07-11 9:02 ` Pei, Andy @ 2022-07-11 9:14 ` Xia, Chenbo 0 siblings, 0 replies; 11+ messages in thread From: Xia, Chenbo @ 2022-07-11 9:14 UTC (permalink / raw) To: Pei, Andy, Maxime Coquelin, Ma, WenwuX, dev Cc: Hu, Jiayu, Wang, Yinan, He, Xingguang, stable Hi Andy, > -----Original Message----- > From: Pei, Andy <andy.pei@intel.com> > Sent: Monday, July 11, 2022 5:03 PM > To: Xia, Chenbo <chenbo.xia@intel.com>; Maxime Coquelin > <maxime.coquelin@redhat.com>; Ma, WenwuX <wenwux.ma@intel.com>; > dev@dpdk.org > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He, > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > Subject: RE: [PATCH v3] vhost: fix deadlock when message handling failed > > HI Chenbo, > > See below example > > 1. > ret = read_vhost_message(dev, fd, &ctx); > if (ret <= 0) { > if (ret < 0) > VHOST_LOG_CONFIG(dev->ifname, ERR, "vhost read message > failed\n"); > else > VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer > closed\n"); > > return -1; > } > > 2. > ret = vhost_user_check_and_alloc_queue_pair(dev, &ctx); > if (ret < 0) { > VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to alloc queue\n"); > return -1; > } > > > 3. ret = -1 > } else if (ret == RTE_VHOST_MSG_RESULT_ERR) { > VHOST_LOG_CONFIG(dev->ifname, ERR, "vhost message handling > failed.\n"); > ret = -1; > goto unlock; > } > > I mean ret is used to check the function is returned successfully or not > while it is also used to store vhost msg return code like > "RTE_VHOST_MSG_RESULT_XXX". It makes sense to make it two, one int for return value, one enum rte_vhost_msg_result only for message handling. IMHO, this could improve readability. Thanks, Chenbo > > > > > > -----Original Message----- > > From: Xia, Chenbo <chenbo.xia@intel.com> > > Sent: Monday, July 11, 2022 4:54 PM > > To: Pei, Andy <andy.pei@intel.com>; Maxime Coquelin > > <maxime.coquelin@redhat.com>; Ma, WenwuX <wenwux.ma@intel.com>; > > dev@dpdk.org > > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; > He, > > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > > Subject: RE: [PATCH v3] vhost: fix deadlock when message handling failed > > > > > -----Original Message----- > > > From: Pei, Andy <andy.pei@intel.com> > > > Sent: Monday, July 11, 2022 4:42 PM > > > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Ma, WenwuX > > > <wenwux.ma@intel.com>; Xia, Chenbo <chenbo.xia@intel.com>; > > > dev@dpdk.org > > > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan > > > <yinan.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; > > > stable@dpdk.org > > > Subject: RE: [PATCH v3] vhost: fix deadlock when message handling > > > failed > > > > > > HI ALL, > > > > > > I see that in function vhost_user_msg_handler. > > > We use "ret" to store both vhost msg return code like > > > "RTE_VHOST_MSG_RESULT_XXX" and function return value. > > > I wonder if it is better to use two different variable to make it easy > > > to read. > > > > By saying 'function return value', you mean which function? Can you > elaborate > > more? > > > > Thanks, > > Chenbo > > > > > > > > > -----Original Message----- > > > > From: Maxime Coquelin <maxime.coquelin@redhat.com> > > > > Sent: Tuesday, May 17, 2022 9:24 PM > > > > To: Ma, WenwuX <WenwuX.Ma@intel.com>; Xia, Chenbo > > > > <Chenbo.Xia@intel.com>; dev@dpdk.org > > > > Cc: Hu, Jiayu <Jiayu.Hu@intel.com>; Wang, Yinan > > > > <yinan.wang@intel.com>; > > > He, > > > > Xingguang <xingguang.he@intel.com>; stable@dpdk.org > > > > Subject: Re: [PATCH v3] vhost: fix deadlock when message handling > > > > failed > > > > > > > > > > > > > > > > On 5/7/22 15:27, Wenwu Ma wrote: > > > > > In vhost_user_msg_handler(), if vhost message handling failed, we > > > > > should check whether the queue is locked and release the lock > > > > > before returning. Or, it will cause a deadlock later. > > > > > > > > > > Fixes: 7f31d4ea05ca ("vhost: fix lock on device readiness > > > > > notification") > > > > > Cc: stable@dpdk.org > > > > > > > > > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > > > > > --- > > > > > lib/vhost/vhost_user.c | 10 ++++++---- > > > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > > > > > > > > Applied to dpdk-next-virtio/main. > > > > > > > > Thanks, > > > > Maxime ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-07-11 9:14 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220505134008.2865-1-wenwux.ma@intel.com>
2022-05-05 16:17 ` [PATCH v2] vhost: fix deadlock when handling user messages Wenwu Ma
2022-05-06 8:19 ` Xia, Chenbo
2022-05-07 13:27 ` [PATCH v3] vhost: fix deadlock when message handling failed Wenwu Ma
2022-05-07 6:02 ` Xia, Chenbo
2022-05-09 3:13 ` Ling, WeiX
2022-05-17 13:15 ` David Marchand
2022-05-17 13:24 ` Maxime Coquelin
2022-07-11 8:42 ` Pei, Andy
2022-07-11 8:54 ` Xia, Chenbo
2022-07-11 9:02 ` Pei, Andy
2022-07-11 9:14 ` Xia, Chenbo
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).