DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion
@ 2020-01-14 18:53 Maxime Coquelin
  2020-01-15  4:51 ` Tiwei Bie
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Maxime Coquelin @ 2020-01-14 18:53 UTC (permalink / raw)
  To: dev, tiwei.bie, amorenoz, zhihong.wang, echaudro; +Cc: Maxime Coquelin, stable

If the vhost-user application (e.g. OVS) deletes the vhost-user
port while Qemu sends a vhost-user request, a deadlock can
happen if the request handler tries to acquire vhost-user's
global mutex, which is also locked by the vhost-user port
deletion API (rte_vhost_driver_unregister).

This patch prevents the deadlock by making
rte_vhost_driver_unregister() to release the mutex and try
again if a request is being handled to give a chance to
the request handler to complete.

Fixes: 8b4b949144b8 ("vhost: fix dead lock on closing in server mode")
Fixes: 5fbb3941da9f ("vhost: introduce driver features related APIs")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/socket.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 633c2cbc27..c57a0c7cdd 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -1052,9 +1052,10 @@ rte_vhost_driver_unregister(const char *path)
 				next = TAILQ_NEXT(conn, next);
 
 				/*
-				 * If r/wcb is executing, release the
-				 * conn_mutex lock, and try again since
-				 * the r/wcb may use the conn_mutex lock.
+				 * If r/wcb is executing, release vsocket's
+				 * conn_mutex and vhost_user's mutex locks, and
+				 * try again since the r/wcb may use the
+				 * conn_mutex and mutex locks.
 				 */
 				if (fdset_try_del(&vhost_user.fdset,
 						  conn->connfd) == -1) {
@@ -1075,8 +1076,17 @@ rte_vhost_driver_unregister(const char *path)
 			pthread_mutex_unlock(&vsocket->conn_mutex);
 
 			if (vsocket->is_server) {
-				fdset_del(&vhost_user.fdset,
-						vsocket->socket_fd);
+				/*
+				 * If r/wcb is executing, release vhost_user's
+				 * mutex lock, and try again since the r/wcb
+				 * may use the mutex lock.
+				 */
+				if (fdset_try_del(&vhost_user.fdset,
+						vsocket->socket_fd) == -1) {
+					pthread_mutex_unlock(&vhost_user.mutex);
+					goto again;
+				}
+
 				close(vsocket->socket_fd);
 				unlink(path);
 			} else if (vsocket->reconnect) {
-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion
  2020-01-14 18:53 [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion Maxime Coquelin
@ 2020-01-15  4:51 ` Tiwei Bie
  2020-01-15  8:25 ` Eelco Chaudron
  2020-01-15 11:17 ` Maxime Coquelin
  2 siblings, 0 replies; 4+ messages in thread
From: Tiwei Bie @ 2020-01-15  4:51 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev, amorenoz, zhihong.wang, echaudro, stable

On Tue, Jan 14, 2020 at 07:53:57PM +0100, Maxime Coquelin wrote:
> If the vhost-user application (e.g. OVS) deletes the vhost-user
> port while Qemu sends a vhost-user request, a deadlock can
> happen if the request handler tries to acquire vhost-user's
> global mutex, which is also locked by the vhost-user port
> deletion API (rte_vhost_driver_unregister).
> 
> This patch prevents the deadlock by making
> rte_vhost_driver_unregister() to release the mutex and try
> again if a request is being handled to give a chance to
> the request handler to complete.
> 
> Fixes: 8b4b949144b8 ("vhost: fix dead lock on closing in server mode")
> Fixes: 5fbb3941da9f ("vhost: introduce driver features related APIs")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/socket.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion
  2020-01-14 18:53 [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion Maxime Coquelin
  2020-01-15  4:51 ` Tiwei Bie
@ 2020-01-15  8:25 ` Eelco Chaudron
  2020-01-15 11:17 ` Maxime Coquelin
  2 siblings, 0 replies; 4+ messages in thread
From: Eelco Chaudron @ 2020-01-15  8:25 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev, tiwei.bie, amorenoz, zhihong.wang, stable



On 14 Jan 2020, at 19:53, Maxime Coquelin wrote:

> If the vhost-user application (e.g. OVS) deletes the vhost-user
> port while Qemu sends a vhost-user request, a deadlock can
> happen if the request handler tries to acquire vhost-user's
> global mutex, which is also locked by the vhost-user port
> deletion API (rte_vhost_driver_unregister).
>
> This patch prevents the deadlock by making
> rte_vhost_driver_unregister() to release the mutex and try
> again if a request is being handled to give a chance to
> the request handler to complete.
>
> Fixes: 8b4b949144b8 ("vhost: fix dead lock on closing in server mode")
> Fixes: 5fbb3941da9f ("vhost: introduce driver features related APIs")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Acked-by: Eelco Chaudron <echaudro@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion
  2020-01-14 18:53 [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion Maxime Coquelin
  2020-01-15  4:51 ` Tiwei Bie
  2020-01-15  8:25 ` Eelco Chaudron
@ 2020-01-15 11:17 ` Maxime Coquelin
  2 siblings, 0 replies; 4+ messages in thread
From: Maxime Coquelin @ 2020-01-15 11:17 UTC (permalink / raw)
  To: dev, tiwei.bie, amorenoz, zhihong.wang, echaudro; +Cc: stable



On 1/14/20 7:53 PM, Maxime Coquelin wrote:
> If the vhost-user application (e.g. OVS) deletes the vhost-user
> port while Qemu sends a vhost-user request, a deadlock can
> happen if the request handler tries to acquire vhost-user's
> global mutex, which is also locked by the vhost-user port
> deletion API (rte_vhost_driver_unregister).
> 
> This patch prevents the deadlock by making
> rte_vhost_driver_unregister() to release the mutex and try
> again if a request is being handled to give a chance to
> the request handler to complete.
> 
> Fixes: 8b4b949144b8 ("vhost: fix dead lock on closing in server mode")
> Fixes: 5fbb3941da9f ("vhost: introduce driver features related APIs")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/socket.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)


Applied to dpdk-next-virtio/master

Thanks to the reviewers,
Maxime


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 18:53 [dpdk-dev] [PATCH] vhost: fix deadlock on port deletion Maxime Coquelin
2020-01-15  4:51 ` Tiwei Bie
2020-01-15  8:25 ` Eelco Chaudron
2020-01-15 11:17 ` Maxime Coquelin

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git