Linux-RDMA Archive on lore.kernel.org
 help / color / Atom feed
From: Yishai Hadas <yishaih@mellanox.com>
To: linux-rdma@vger.kernel.org, jgg@mellanox.com, dledford@redhat.com
Cc: yishaih@mellanox.com, maorg@mellanox.com, michaelgur@mellanox.com
Subject: [PATCH rdma-next 01/14] RDMA/mlx5: Use RCU and direct refcounts to keep memory alive
Date: Wed,  8 Jan 2020 19:21:53 +0200
Message-ID: <1578504126-9400-2-git-send-email-yishaih@mellanox.com> (raw)
In-Reply-To: <1578504126-9400-1-git-send-email-yishaih@mellanox.com>

From: Jason Gunthorpe <jgg@mellanox.com>

dispatch_event_fd() runs from a notifier with minimal locking, and relies
on RCU and a file refcount to keep the uobject and eventfd alive.

As the next patch wants to remove the file_operations release function
from the drivers, re-organize things so that the devx_event_notifier()
path uses the existing RCU to manage the lifetime of the uobject and
eventfd.

Move the refcount puts to a call_rcu so that the objects are guaranteed to
exist and remove the indirect file refcount.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 drivers/infiniband/core/rdma_core.c | 11 ++++++-----
 drivers/infiniband/core/rdma_core.h | 15 ---------------
 drivers/infiniband/hw/mlx5/devx.c   | 34 +++++++++++++++++-----------------
 include/rdma/uverbs_types.h         | 12 ++++++++++++
 4 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 6c72773..a5c29e3 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -42,11 +42,6 @@
 #include "core_priv.h"
 #include "rdma_core.h"
 
-void uverbs_uobject_get(struct ib_uobject *uobject)
-{
-	kref_get(&uobject->ref);
-}
-
 static void uverbs_uobject_free(struct kref *ref)
 {
 	struct ib_uobject *uobj =
@@ -58,10 +53,16 @@ static void uverbs_uobject_free(struct kref *ref)
 		kfree(uobj);
 }
 
+/*
+ * In order to indicate we no longer needs this uobject, uverbs_uobject_put
+ * is called. When the reference count is decreased, the uobject is freed.
+ * For example, this is used when attaching a completion channel to a CQ.
+ */
 void uverbs_uobject_put(struct ib_uobject *uobject)
 {
 	kref_put(&uobject->ref, uverbs_uobject_free);
 }
+EXPORT_SYMBOL(uverbs_uobject_put);
 
 static int uverbs_try_lock_object(struct ib_uobject *uobj,
 				  enum rdma_lookup_mode mode)
diff --git a/drivers/infiniband/core/rdma_core.h b/drivers/infiniband/core/rdma_core.h
index e63fbda..d5d58a1 100644
--- a/drivers/infiniband/core/rdma_core.h
+++ b/drivers/infiniband/core/rdma_core.h
@@ -50,21 +50,6 @@ void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile,
 
 int uobj_destroy(struct ib_uobject *uobj, struct uverbs_attr_bundle *attrs);
 
-/*
- * uverbs_uobject_get is called in order to increase the reference count on
- * an uobject. This is useful when a handler wants to keep the uobject's memory
- * alive, regardless if this uobject is still alive in the context's objects
- * repository. Objects are put via uverbs_uobject_put.
- */
-void uverbs_uobject_get(struct ib_uobject *uobject);
-
-/*
- * In order to indicate we no longer needs this uobject, uverbs_uobject_put
- * is called. When the reference count is decreased, the uobject is freed.
- * For example, this is used when attaching a completion channel to a CQ.
- */
-void uverbs_uobject_put(struct ib_uobject *uobject);
-
 /* Indicate this fd is no longer used by this consumer, but its memory isn't
  * necessarily released yet. When the last reference is put, we release the
  * memory. After this call is executed, calling uverbs_uobject_get isn't
diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index 9d0a18c..968fff0 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -72,7 +72,6 @@ struct devx_event_subscription {
 	struct rcu_head	rcu;
 	u64 cookie;
 	struct devx_async_event_file *ev_file;
-	struct file *filp; /* Upon hot unplug we need a direct access to */
 	struct eventfd_ctx *eventfd;
 };
 
@@ -2032,6 +2031,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
 			goto err;
 
 		list_add_tail(&event_sub->event_list, &sub_list);
+		uverbs_uobject_get(&ev_file->uobj);
 		if (use_eventfd) {
 			event_sub->eventfd =
 				eventfd_ctx_fdget(redirect_fd);
@@ -2045,7 +2045,6 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
 
 		event_sub->cookie = cookie;
 		event_sub->ev_file = ev_file;
-		event_sub->filp = fd_uobj->object;
 		/* May be needed upon cleanup the devx object/subscription */
 		event_sub->xa_key_level1 = key_level1;
 		event_sub->xa_key_level2 = obj_id;
@@ -2099,7 +2098,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
 
 		if (event_sub->eventfd)
 			eventfd_ctx_put(event_sub->eventfd);
-
+		uverbs_uobject_put(&event_sub->ev_file->uobj);
 		kfree(event_sub);
 	}
 
@@ -2361,17 +2360,10 @@ static void dispatch_event_fd(struct list_head *fd_list,
 	struct devx_event_subscription *item;
 
 	list_for_each_entry_rcu(item, fd_list, xa_list) {
-		if (!get_file_rcu(item->filp))
-			continue;
-
-		if (item->eventfd) {
+		if (item->eventfd)
 			eventfd_signal(item->eventfd, 1);
-			fput(item->filp);
-			continue;
-		}
-
-		deliver_event(item, data);
-		fput(item->filp);
+		else
+			deliver_event(item, data);
 	}
 }
 
@@ -2653,6 +2645,17 @@ static __poll_t devx_async_event_poll(struct file *filp,
 	return pollflags;
 }
 
+static void devx_free_subscription(struct rcu_head *rcu)
+{
+	struct devx_event_subscription *event_sub =
+		container_of(rcu, struct devx_event_subscription, rcu);
+
+	if (event_sub->eventfd)
+		eventfd_ctx_put(event_sub->eventfd);
+	uverbs_uobject_put(&event_sub->ev_file->uobj);
+	kfree(event_sub);
+}
+
 static int devx_async_event_close(struct inode *inode, struct file *filp)
 {
 	struct devx_async_event_file *ev_file = filp->private_data;
@@ -2665,12 +2668,9 @@ static int devx_async_event_close(struct inode *inode, struct file *filp)
 	list_for_each_entry_safe(event_sub, event_sub_tmp,
 				 &ev_file->subscribed_events_list, file_list) {
 		devx_cleanup_subscription(dev, event_sub);
-		if (event_sub->eventfd)
-			eventfd_ctx_put(event_sub->eventfd);
-
 		list_del_rcu(&event_sub->file_list);
 		/* subscription may not be used by the read API any more */
-		kfree_rcu(event_sub, rcu);
+		call_rcu(&event_sub->rcu, devx_free_subscription);
 	}
 
 	mutex_unlock(&dev->devx_event_table.event_xa_lock);
diff --git a/include/rdma/uverbs_types.h b/include/rdma/uverbs_types.h
index d57a5ba..388ffee 100644
--- a/include/rdma/uverbs_types.h
+++ b/include/rdma/uverbs_types.h
@@ -145,6 +145,18 @@ void rdma_alloc_abort_uobject(struct ib_uobject *uobj,
 int __must_check rdma_alloc_commit_uobject(struct ib_uobject *uobj,
 					   struct uverbs_attr_bundle *attrs);
 
+/*
+ * uverbs_uobject_get is called in order to increase the reference count on
+ * an uobject. This is useful when a handler wants to keep the uobject's memory
+ * alive, regardless if this uobject is still alive in the context's objects
+ * repository. Objects are put via uverbs_uobject_put.
+ */
+static inline void uverbs_uobject_get(struct ib_uobject *uobject)
+{
+	kref_get(&uobject->ref);
+}
+void uverbs_uobject_put(struct ib_uobject *uobject);
+
 struct uverbs_obj_fd_type {
 	/*
 	 * In fd based objects, uverbs_obj_type_ops points to generic
-- 
1.8.3.1


  reply index

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
2020-01-08 17:21 ` Yishai Hadas [this message]
2020-01-08 17:21 ` [PATCH rdma-next 02/14] RDMA/core: Simplify destruction of FD uobjects Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 03/14] RDMA/mlx5: Simplify devx async commands Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 04/14] RDMA/core: Do not allow alloc_commit to fail Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 05/14] RDMA/core: Make ib_ucq_object use ib_uevent_object Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 06/14] RDMA/core: Do not erase the type of ib_cq.uobject Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 07/14] RDMA/core: Do not erase the type of ib_qp.uobject Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 08/14] RDMA/core: Do not erase the type of ib_srq.uobject Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 09/14] RDMA/core: Do not erase the type of ib_wq.uobject Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 10/14] RDMA/core: Simplify type usage for ib_uverbs_async_handler() Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 11/14] RDMA/core: Fix locking in ib_uverbs_event_read Yishai Hadas
2020-01-10 14:54   ` Jason Gunthorpe
2020-01-08 17:22 ` [PATCH rdma-next 12/14] RDMA/core: Remove the ufile arg from rdma_alloc_begin_uobject Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 13/14] RDMA/core: Make ib_uverbs_async_event_file into a uobject Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 14/14] RDMA/core: Use READ_ONCE for ib_ufile.async_file Yishai Hadas
2020-01-13 20:37 ` [PATCH rdma-next 00/14] Refactoring FD usage Jason Gunthorpe

Reply instructions:

You may reply publically 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=1578504126-9400-2-git-send-email-yishaih@mellanox.com \
    --to=yishaih@mellanox.com \
    --cc=dledford@redhat.com \
    --cc=jgg@mellanox.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=maorg@mellanox.com \
    --cc=michaelgur@mellanox.com \
    /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

Linux-RDMA Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rdma/0 linux-rdma/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 linux-rdma linux-rdma/ https://lore.kernel.org/linux-rdma \
		linux-rdma@vger.kernel.org
	public-inbox-index linux-rdma

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rdma


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