All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Leon Romanovsky <leonro@mellanox.com>,
	Jason Gunthorpe <jgg@nvidia.com>, Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.8 05/16] RDMA/cma: Remove unneeded locking for req paths
Date: Fri, 11 Sep 2020 14:47:22 +0200	[thread overview]
Message-ID: <20200911122459.843026940@linuxfoundation.org> (raw)
In-Reply-To: <20200911122459.585735377@linuxfoundation.org>

From: Jason Gunthorpe <jgg@nvidia.com>

[ Upstream commit cc9c037343898eb7a775e6b81d092ee21eeff218 ]

The REQ flows are concerned that once the handler is called on the new
cm_id the ULP can choose to trigger a rdma_destroy_id() concurrently at
any time.

However, this is not true, while the ULP can call rdma_destroy_id(), it
immediately blocks on the handler_mutex which prevents anything harmful
from running concurrently.

Remove the confusing extra locking and refcounts and make the
handler_mutex protecting state during destroy more clear.

Link: https://lore.kernel.org/r/20200723070707.1771101-4-leon@kernel.org
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/infiniband/core/cma.c | 31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 04151c301e851..11f43204fee77 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1831,21 +1831,21 @@ static void cma_leave_mc_groups(struct rdma_id_private *id_priv)
 
 void rdma_destroy_id(struct rdma_cm_id *id)
 {
-	struct rdma_id_private *id_priv;
+	struct rdma_id_private *id_priv =
+		container_of(id, struct rdma_id_private, id);
 	enum rdma_cm_state state;
 
-	id_priv = container_of(id, struct rdma_id_private, id);
-	trace_cm_id_destroy(id_priv);
-	state = cma_exch(id_priv, RDMA_CM_DESTROYING);
-	cma_cancel_operation(id_priv, state);
-
 	/*
 	 * Wait for any active callback to finish.  New callbacks will find
 	 * the id_priv state set to destroying and abort.
 	 */
 	mutex_lock(&id_priv->handler_mutex);
+	trace_cm_id_destroy(id_priv);
+	state = cma_exch(id_priv, RDMA_CM_DESTROYING);
 	mutex_unlock(&id_priv->handler_mutex);
 
+	cma_cancel_operation(id_priv, state);
+
 	rdma_restrack_del(&id_priv->res);
 	if (id_priv->cma_dev) {
 		if (rdma_cap_ib_cm(id_priv->id.device, 1)) {
@@ -2205,19 +2205,9 @@ static int cma_ib_req_handler(struct ib_cm_id *cm_id,
 	cm_id->context = conn_id;
 	cm_id->cm_handler = cma_ib_handler;
 
-	/*
-	 * Protect against the user destroying conn_id from another thread
-	 * until we're done accessing it.
-	 */
-	cma_id_get(conn_id);
 	ret = cma_cm_event_handler(conn_id, &event);
 	if (ret)
 		goto err3;
-	/*
-	 * Acquire mutex to prevent user executing rdma_destroy_id()
-	 * while we're accessing the cm_id.
-	 */
-	mutex_lock(&lock);
 	if (cma_comp(conn_id, RDMA_CM_CONNECT) &&
 	    (conn_id->id.qp_type != IB_QPT_UD)) {
 		trace_cm_send_mra(cm_id->context);
@@ -2226,13 +2216,11 @@ static int cma_ib_req_handler(struct ib_cm_id *cm_id,
 	mutex_unlock(&lock);
 	mutex_unlock(&conn_id->handler_mutex);
 	mutex_unlock(&listen_id->handler_mutex);
-	cma_id_put(conn_id);
 	if (net_dev)
 		dev_put(net_dev);
 	return 0;
 
 err3:
-	cma_id_put(conn_id);
 	/* Destroy the CM ID by returning a non-zero value. */
 	conn_id->cm_id.ib = NULL;
 err2:
@@ -2409,11 +2397,6 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
 	memcpy(cma_src_addr(conn_id), laddr, rdma_addr_size(laddr));
 	memcpy(cma_dst_addr(conn_id), raddr, rdma_addr_size(raddr));
 
-	/*
-	 * Protect against the user destroying conn_id from another thread
-	 * until we're done accessing it.
-	 */
-	cma_id_get(conn_id);
 	ret = cma_cm_event_handler(conn_id, &event);
 	if (ret) {
 		/* User wants to destroy the CM ID */
@@ -2421,13 +2404,11 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
 		cma_exch(conn_id, RDMA_CM_DESTROYING);
 		mutex_unlock(&conn_id->handler_mutex);
 		mutex_unlock(&listen_id->handler_mutex);
-		cma_id_put(conn_id);
 		rdma_destroy_id(&conn_id->id);
 		return ret;
 	}
 
 	mutex_unlock(&conn_id->handler_mutex);
-	cma_id_put(conn_id);
 
 out:
 	mutex_unlock(&listen_id->handler_mutex);
-- 
2.25.1




  parent reply	other threads:[~2020-09-11 16:32 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-11 12:47 [PATCH 5.8 00/16] 5.8.9-rc1 review Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 01/16] io_uring: fix cancel of deferred reqs with ->files Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 02/16] io_uring: fix linked deferred ->files cancellation Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 03/16] RDMA/cma: Simplify DEVICE_REMOVAL for internal_id Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 04/16] RDMA/cma: Using the standard locking pattern when delivering the removal event Greg Kroah-Hartman
2020-09-11 12:47 ` Greg Kroah-Hartman [this message]
2020-09-11 12:47 ` [PATCH 5.8 06/16] RDMA/cma: Execute rdma_cm destruction from a handler properly Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 07/16] ipv4: Silence suspicious RCU usage warning Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 08/16] ipv6: Fix sysctl max for fib_multipath_hash_policy Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 09/16] netlabel: fix problems with mapping removal Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 10/16] net: usb: dm9601: Add USB ID of Keenetic Plus DSL Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 11/16] sctp: not disable bh in the whole sctp_get_port_local() Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 12/16] taprio: Fix using wrong queues in gate mask Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 13/16] tipc: fix shutdown() of connectionless socket Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 14/16] tipc: fix using smp_processor_id() in preemptible Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 15/16] net: disable netpoll on fresh napis Greg Kroah-Hartman
2020-09-11 12:47 ` [PATCH 5.8 16/16] mptcp: free acked data before waiting for more memory Greg Kroah-Hartman
2020-09-11 17:10 ` [PATCH 5.8 00/16] 5.8.9-rc1 review Jon Hunter
2020-09-12 12:44   ` Greg Kroah-Hartman
2020-09-11 22:19 ` Shuah Khan
2020-09-12 12:44   ` Greg Kroah-Hartman
2020-09-12  2:19 ` Guenter Roeck
2020-09-12 12:44   ` Greg Kroah-Hartman
2020-09-12  7:27 ` Naresh Kamboju
2020-09-12 12:44   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200911122459.843026940@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=jgg@nvidia.com \
    --cc=leonro@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.