* [PATCH rdma-next 01/14] RDMA/mlx5: Use RCU and direct refcounts to keep memory alive
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
@ 2020-01-08 17:21 ` Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 02/14] RDMA/core: Simplify destruction of FD uobjects Yishai Hadas
` (13 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:21 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 02/14] RDMA/core: Simplify destruction of FD uobjects
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 01/14] RDMA/mlx5: Use RCU and direct refcounts to keep memory alive Yishai Hadas
@ 2020-01-08 17:21 ` Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 03/14] RDMA/mlx5: Simplify devx async commands Yishai Hadas
` (12 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:21 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
FD uobjects have a weird split between the struct file and uobject
world. Simplify this to make them pure uobjects and use a generic release
method for all struct file operations.
This fixes the control flow so that mlx5_cmd_cleanup_async_ctx() is always
called before erasing the linked list contents to make the concurrancy
simpler to understand.
For this to work the uobject destruction must fence anything that it is
cleaning up - the design must not rely on struct file lifetime.
Only deliver_event() relies on the struct file to when adding new events
to the queue, add a is_destroyed check under lock to block it.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/rdma_core.c | 34 +++++----
drivers/infiniband/core/rdma_core.h | 8 ---
drivers/infiniband/core/uverbs_main.c | 23 +-----
drivers/infiniband/core/uverbs_std_types.c | 23 +++---
drivers/infiniband/core/uverbs_uapi.c | 6 +-
drivers/infiniband/hw/mlx5/devx.c | 111 +++++++++++++----------------
include/rdma/uverbs_types.h | 12 ++--
7 files changed, 94 insertions(+), 123 deletions(-)
diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index a5c29e3..e8ee89d 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -359,9 +359,9 @@ static int idr_add_uobj(struct ib_uobject *uobj)
uobject = f->private_data;
/*
- * fget(id) ensures we are not currently running uverbs_close_fd,
- * and the caller is expected to ensure that uverbs_close_fd is never
- * done while a call top lookup is possible.
+ * fget(id) ensures we are not currently running
+ * uverbs_uobject_fd_release(), and the caller is expected to ensure
+ * that release is never done while a call to lookup is possible.
*/
if (f->f_op != fd_type->fops) {
fput(f);
@@ -554,7 +554,7 @@ static int __must_check destroy_hw_fd_uobject(struct ib_uobject *uobj,
{
const struct uverbs_obj_fd_type *fd_type = container_of(
uobj->uapi_object->type_attrs, struct uverbs_obj_fd_type, type);
- int ret = fd_type->context_closed(uobj, why);
+ int ret = fd_type->destroy_object(uobj, why);
if (ib_is_destroy_retryable(ret, why, uobj))
return ret;
@@ -593,9 +593,9 @@ static int alloc_commit_fd_uobject(struct ib_uobject *uobj)
/*
* The kref for uobj is moved into filp->private data and put in
- * uverbs_close_fd(). Once alloc_commit() succeeds uverbs_close_fd()
- * must be guaranteed to be called from the provided fops release
- * callback.
+ * uverbs_close_fd(). Once alloc_commit() succeeds
+ * uverbs_uobject_fd_release() must be guaranteed to be called from
+ * the provided fops release callback.
*/
filp = anon_inode_getfile(fd_type->name,
fd_type->fops,
@@ -606,7 +606,7 @@ static int alloc_commit_fd_uobject(struct ib_uobject *uobj)
uobj->object = filp;
- /* Matching put will be done in uverbs_close_fd() */
+ /* Matching put will be done in uverbs_uobject_fd_release() */
kref_get(&uobj->ufile->ref);
/* This shouldn't be used anymore. Use the file object instead */
@@ -614,7 +614,7 @@ static int alloc_commit_fd_uobject(struct ib_uobject *uobj)
/*
* NOTE: Once we install the file we loose ownership of our kref on
- * uobj. It will be put by uverbs_close_fd()
+ * uobj. It will be put by uverbs_uobject_fd_release()
*/
fd_install(fd, filp);
@@ -682,7 +682,10 @@ static void lookup_put_fd_uobject(struct ib_uobject *uobj,
struct file *filp = uobj->object;
WARN_ON(mode != UVERBS_LOOKUP_READ);
- /* This indirectly calls uverbs_close_fd and free the object */
+ /*
+ * This indirectly calls uverbs_uobject_fd_release() and free the
+ * object
+ */
fput(filp);
}
@@ -762,9 +765,13 @@ void release_ufile_idr_uobject(struct ib_uverbs_file *ufile)
};
EXPORT_SYMBOL(uverbs_idr_class);
-void uverbs_close_fd(struct file *f)
+/*
+ * Users of UVERBS_TYPE_ALLOC_FD should set this function as the struct
+ * file_operations release method.
+ */
+int uverbs_uobject_fd_release(struct inode *inode, struct file *filp)
{
- struct ib_uobject *uobj = f->private_data;
+ struct ib_uobject *uobj = filp->private_data;
struct ib_uverbs_file *ufile = uobj->ufile;
struct uverbs_attr_bundle attrs = {
.context = uobj->context,
@@ -788,8 +795,9 @@ void uverbs_close_fd(struct file *f)
/* Pairs with filp->private_data in alloc_begin_fd_uobject */
uverbs_uobject_put(uobj);
+ return 0;
}
-EXPORT_SYMBOL(uverbs_close_fd);
+EXPORT_SYMBOL(uverbs_uobject_fd_release);
/*
* Drop the ucontext off the ufile and completely disconnect it from the
diff --git a/drivers/infiniband/core/rdma_core.h b/drivers/infiniband/core/rdma_core.h
index d5d58a1..9269425 100644
--- a/drivers/infiniband/core/rdma_core.h
+++ b/drivers/infiniband/core/rdma_core.h
@@ -50,14 +50,6 @@ void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile,
int uobj_destroy(struct ib_uobject *uobj, struct uverbs_attr_bundle *attrs);
-/* 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
- * allowed.
- * This must be called from the release file_operations of the file!
- */
-void uverbs_close_fd(struct file *f);
-
/*
* Get an ib_uobject that corresponds to the given id from ufile, assuming
* the object is from the given type. Lock it to the required access when
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 970d8e3..290a91e 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -377,32 +377,11 @@ static int ib_uverbs_async_event_close(struct inode *inode, struct file *filp)
return 0;
}
-static int ib_uverbs_comp_event_close(struct inode *inode, struct file *filp)
-{
- struct ib_uobject *uobj = filp->private_data;
- struct ib_uverbs_completion_event_file *file = container_of(
- uobj, struct ib_uverbs_completion_event_file, uobj);
- struct ib_uverbs_event *entry, *tmp;
-
- spin_lock_irq(&file->ev_queue.lock);
- list_for_each_entry_safe(entry, tmp, &file->ev_queue.event_list, list) {
- if (entry->counter)
- list_del(&entry->obj_list);
- kfree(entry);
- }
- file->ev_queue.is_closed = 1;
- spin_unlock_irq(&file->ev_queue.lock);
-
- uverbs_close_fd(filp);
-
- return 0;
-}
-
const struct file_operations uverbs_event_fops = {
.owner = THIS_MODULE,
.read = ib_uverbs_comp_event_read,
.poll = ib_uverbs_comp_event_poll,
- .release = ib_uverbs_comp_event_close,
+ .release = uverbs_uobject_fd_release,
.fasync = ib_uverbs_comp_event_fasync,
.llseek = no_llseek,
};
diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index 35b2e2c..def038a 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -202,22 +202,29 @@ static int uverbs_free_pd(struct ib_uobject *uobject,
return 0;
}
-static int uverbs_hot_unplug_completion_event_file(struct ib_uobject *uobj,
- enum rdma_remove_reason why)
+static int
+uverbs_completion_event_file_destroy_uobj(struct ib_uobject *uobj,
+ enum rdma_remove_reason why)
{
- struct ib_uverbs_completion_event_file *comp_event_file =
+ struct ib_uverbs_completion_event_file *file =
container_of(uobj, struct ib_uverbs_completion_event_file,
uobj);
- struct ib_uverbs_event_queue *event_queue = &comp_event_file->ev_queue;
+ struct ib_uverbs_event_queue *event_queue = &file->ev_queue;
+ struct ib_uverbs_event *entry, *tmp;
spin_lock_irq(&event_queue->lock);
event_queue->is_closed = 1;
spin_unlock_irq(&event_queue->lock);
+ wake_up_interruptible(&event_queue->poll_wait);
+ kill_fasync(&event_queue->async_queue, SIGIO, POLL_IN);
- if (why == RDMA_REMOVE_DRIVER_REMOVE) {
- wake_up_interruptible(&event_queue->poll_wait);
- kill_fasync(&event_queue->async_queue, SIGIO, POLL_IN);
+ spin_lock_irq(&event_queue->lock);
+ list_for_each_entry_safe(entry, tmp, &event_queue->event_list, list) {
+ if (entry->counter)
+ list_del(&entry->obj_list);
+ kfree(entry);
}
+ spin_unlock_irq(&event_queue->lock);
return 0;
};
@@ -230,7 +237,7 @@ int uverbs_destroy_def_handler(struct uverbs_attr_bundle *attrs)
DECLARE_UVERBS_NAMED_OBJECT(
UVERBS_OBJECT_COMP_CHANNEL,
UVERBS_TYPE_ALLOC_FD(sizeof(struct ib_uverbs_completion_event_file),
- uverbs_hot_unplug_completion_event_file,
+ uverbs_completion_event_file_destroy_uobj,
&uverbs_event_fops,
"[infinibandevent]",
O_RDONLY));
diff --git a/drivers/infiniband/core/uverbs_uapi.c b/drivers/infiniband/core/uverbs_uapi.c
index 00c54788..9b84a12 100644
--- a/drivers/infiniband/core/uverbs_uapi.c
+++ b/drivers/infiniband/core/uverbs_uapi.c
@@ -195,9 +195,9 @@ static int uapi_merge_obj_tree(struct uverbs_api *uapi,
* disassociation, and the FD types require the driver to use
* struct file_operations.owner to prevent the driver module
* code from unloading while the file is open. This provides
- * enough safety that uverbs_close_fd() will continue to work.
- * Drivers using FD are responsible to handle disassociation of
- * the device on their own.
+ * enough safety that uverbs_uobject_fd_release() will
+ * continue to work. Drivers using FD are responsible to
+ * handle disassociation of the device on their own.
*/
if (WARN_ON(is_driver &&
obj->type_attrs->type_class != &uverbs_idr_class &&
diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index 968fff0..02125d8 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -2328,6 +2328,9 @@ static int deliver_event(struct devx_event_subscription *event_sub,
return 0;
}
+ /* is_destroyed is ignored here because we don't have any memory
+ * allocation to clean up for the omit_data case
+ */
list_add_tail(&event_sub->event_list, &ev_file->event_list);
spin_unlock_irqrestore(&ev_file->lock, flags);
wake_up_interruptible(&ev_file->poll_wait);
@@ -2347,7 +2350,10 @@ static int deliver_event(struct devx_event_subscription *event_sub,
memcpy(event_data->hdr.out_data, data, sizeof(struct mlx5_eqe));
spin_lock_irqsave(&ev_file->lock, flags);
- list_add_tail(&event_data->list, &ev_file->event_list);
+ if (!ev_file->is_destroyed)
+ list_add_tail(&event_data->list, &ev_file->event_list);
+ else
+ kfree(event_data);
spin_unlock_irqrestore(&ev_file->lock, flags);
wake_up_interruptible(&ev_file->poll_wait);
@@ -2501,23 +2507,6 @@ static ssize_t devx_async_cmd_event_read(struct file *filp, char __user *buf,
return ret;
}
-static int devx_async_cmd_event_close(struct inode *inode, struct file *filp)
-{
- struct ib_uobject *uobj = filp->private_data;
- struct devx_async_cmd_event_file *comp_ev_file = container_of(
- uobj, struct devx_async_cmd_event_file, uobj);
- struct devx_async_data *entry, *tmp;
-
- spin_lock_irq(&comp_ev_file->ev_queue.lock);
- list_for_each_entry_safe(entry, tmp,
- &comp_ev_file->ev_queue.event_list, list)
- kvfree(entry);
- spin_unlock_irq(&comp_ev_file->ev_queue.lock);
-
- uverbs_close_fd(filp);
- return 0;
-}
-
static __poll_t devx_async_cmd_event_poll(struct file *filp,
struct poll_table_struct *wait)
{
@@ -2541,7 +2530,7 @@ static __poll_t devx_async_cmd_event_poll(struct file *filp,
.owner = THIS_MODULE,
.read = devx_async_cmd_event_read,
.poll = devx_async_cmd_event_poll,
- .release = devx_async_cmd_event_close,
+ .release = uverbs_uobject_fd_release,
.llseek = no_llseek,
};
@@ -2656,78 +2645,74 @@ static void devx_free_subscription(struct rcu_head *rcu)
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;
- struct devx_event_subscription *event_sub, *event_sub_tmp;
- struct devx_async_event_data *entry, *tmp;
- struct mlx5_ib_dev *dev = ev_file->dev;
-
- mutex_lock(&dev->devx_event_table.event_xa_lock);
- /* delete the subscriptions which are related to this FD */
- list_for_each_entry_safe(event_sub, event_sub_tmp,
- &ev_file->subscribed_events_list, file_list) {
- devx_cleanup_subscription(dev, event_sub);
- list_del_rcu(&event_sub->file_list);
- /* subscription may not be used by the read API any more */
- call_rcu(&event_sub->rcu, devx_free_subscription);
- }
-
- mutex_unlock(&dev->devx_event_table.event_xa_lock);
-
- /* free the pending events allocation */
- if (!ev_file->omit_data) {
- spin_lock_irq(&ev_file->lock);
- list_for_each_entry_safe(entry, tmp,
- &ev_file->event_list, list)
- kfree(entry); /* read can't come any more */
- spin_unlock_irq(&ev_file->lock);
- }
-
- uverbs_close_fd(filp);
- put_device(&dev->ib_dev.dev);
- return 0;
-}
-
static const struct file_operations devx_async_event_fops = {
.owner = THIS_MODULE,
.read = devx_async_event_read,
.poll = devx_async_event_poll,
- .release = devx_async_event_close,
+ .release = uverbs_uobject_fd_release,
.llseek = no_llseek,
};
-static int devx_hot_unplug_async_cmd_event_file(struct ib_uobject *uobj,
- enum rdma_remove_reason why)
+static int devx_async_cmd_event_destroy_uobj(struct ib_uobject *uobj,
+ enum rdma_remove_reason why)
{
struct devx_async_cmd_event_file *comp_ev_file =
container_of(uobj, struct devx_async_cmd_event_file,
uobj);
struct devx_async_event_queue *ev_queue = &comp_ev_file->ev_queue;
+ struct devx_async_data *entry, *tmp;
spin_lock_irq(&ev_queue->lock);
ev_queue->is_destroyed = 1;
spin_unlock_irq(&ev_queue->lock);
-
- if (why == RDMA_REMOVE_DRIVER_REMOVE)
- wake_up_interruptible(&ev_queue->poll_wait);
+ wake_up_interruptible(&ev_queue->poll_wait);
mlx5_cmd_cleanup_async_ctx(&comp_ev_file->async_ctx);
+
+ spin_lock_irq(&comp_ev_file->ev_queue.lock);
+ list_for_each_entry_safe(entry, tmp,
+ &comp_ev_file->ev_queue.event_list, list)
+ kvfree(entry);
+ spin_unlock_irq(&comp_ev_file->ev_queue.lock);
return 0;
};
-static int devx_hot_unplug_async_event_file(struct ib_uobject *uobj,
- enum rdma_remove_reason why)
+static int devx_async_event_destroy_uobj(struct ib_uobject *uobj,
+ enum rdma_remove_reason why)
{
struct devx_async_event_file *ev_file =
container_of(uobj, struct devx_async_event_file,
uobj);
+ struct devx_event_subscription *event_sub, *event_sub_tmp;
+ struct devx_async_event_data *entry, *tmp;
+ struct mlx5_ib_dev *dev = ev_file->dev;
spin_lock_irq(&ev_file->lock);
ev_file->is_destroyed = 1;
spin_unlock_irq(&ev_file->lock);
-
wake_up_interruptible(&ev_file->poll_wait);
+
+ mutex_lock(&dev->devx_event_table.event_xa_lock);
+ /* delete the subscriptions which are related to this FD */
+ list_for_each_entry_safe(event_sub, event_sub_tmp,
+ &ev_file->subscribed_events_list, file_list) {
+ devx_cleanup_subscription(dev, event_sub);
+ list_del_rcu(&event_sub->file_list);
+ /* subscription may not be used by the read API any more */
+ call_rcu(&event_sub->rcu, devx_free_subscription);
+ }
+ mutex_unlock(&dev->devx_event_table.event_xa_lock);
+
+ /* free the pending events allocation */
+ if (!ev_file->omit_data) {
+ spin_lock_irq(&ev_file->lock);
+ list_for_each_entry_safe(entry, tmp,
+ &ev_file->event_list, list)
+ kfree(entry); /* read can't come any more */
+ spin_unlock_irq(&ev_file->lock);
+ }
+
+ put_device(&dev->ib_dev.dev);
return 0;
};
@@ -2913,7 +2898,7 @@ static int devx_hot_unplug_async_event_file(struct ib_uobject *uobj,
DECLARE_UVERBS_NAMED_OBJECT(
MLX5_IB_OBJECT_DEVX_ASYNC_CMD_FD,
UVERBS_TYPE_ALLOC_FD(sizeof(struct devx_async_cmd_event_file),
- devx_hot_unplug_async_cmd_event_file,
+ devx_async_cmd_event_destroy_uobj,
&devx_async_cmd_event_fops, "[devx_async_cmd]",
O_RDONLY),
&UVERBS_METHOD(MLX5_IB_METHOD_DEVX_ASYNC_CMD_FD_ALLOC));
@@ -2931,7 +2916,7 @@ static int devx_hot_unplug_async_event_file(struct ib_uobject *uobj,
DECLARE_UVERBS_NAMED_OBJECT(
MLX5_IB_OBJECT_DEVX_ASYNC_EVENT_FD,
UVERBS_TYPE_ALLOC_FD(sizeof(struct devx_async_event_file),
- devx_hot_unplug_async_event_file,
+ devx_async_event_destroy_uobj,
&devx_async_event_fops, "[devx_async_event]",
O_RDONLY),
&UVERBS_METHOD(MLX5_IB_METHOD_DEVX_ASYNC_EVENT_FD_ALLOC));
diff --git a/include/rdma/uverbs_types.h b/include/rdma/uverbs_types.h
index 388ffee..657c313 100644
--- a/include/rdma/uverbs_types.h
+++ b/include/rdma/uverbs_types.h
@@ -162,11 +162,11 @@ struct uverbs_obj_fd_type {
* In fd based objects, uverbs_obj_type_ops points to generic
* fd operations. In order to specialize the underlying types (e.g.
* completion_channel), we use fops, name and flags for fd creation.
- * context_closed is called when the context is closed either when
- * the driver is removed or the process terminated.
+ * destroy_object is called when the uobject is to be destroyed,
+ * because the driver is removed or the FD is closed.
*/
struct uverbs_obj_type type;
- int (*context_closed)(struct ib_uobject *uobj,
+ int (*destroy_object)(struct ib_uobject *uobj,
enum rdma_remove_reason why);
const struct file_operations *fops;
const char *name;
@@ -175,11 +175,11 @@ struct uverbs_obj_fd_type {
extern const struct uverbs_obj_type_class uverbs_idr_class;
extern const struct uverbs_obj_type_class uverbs_fd_class;
-void uverbs_close_fd(struct file *f);
+int uverbs_uobject_fd_release(struct inode *inode, struct file *filp);
#define UVERBS_BUILD_BUG_ON(cond) (sizeof(char[1 - 2 * !!(cond)]) - \
sizeof(char))
-#define UVERBS_TYPE_ALLOC_FD(_obj_size, _context_closed, _fops, _name, _flags)\
+#define UVERBS_TYPE_ALLOC_FD(_obj_size, _destroy_object, _fops, _name, _flags) \
((&((const struct uverbs_obj_fd_type) \
{.type = { \
.type_class = &uverbs_fd_class, \
@@ -187,7 +187,7 @@ struct uverbs_obj_fd_type {
UVERBS_BUILD_BUG_ON((_obj_size) < \
sizeof(struct ib_uobject)), \
}, \
- .context_closed = _context_closed, \
+ .destroy_object = _destroy_object, \
.fops = _fops, \
.name = _name, \
.flags = _flags}))->type)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 03/14] RDMA/mlx5: Simplify devx async commands
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 01/14] RDMA/mlx5: Use RCU and direct refcounts to keep memory alive Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 02/14] RDMA/core: Simplify destruction of FD uobjects Yishai Hadas
@ 2020-01-08 17:21 ` Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 04/14] RDMA/core: Do not allow alloc_commit to fail Yishai Hadas
` (11 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:21 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
With the new FD structure the async commands do not need to hold any
references while running. The existing mlx5_cmd_exec_cb() and
mlx5_cmd_cleanup_async_ctx() provide enough synchronization to ensure
that all outstanding commands are completed before the uobject can be
destructed.
Remove the now confusing get_file() and the type erasure of the
devx_async_cmd_event_file.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/hw/mlx5/devx.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index 02125d8..7bb91d2 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -30,7 +30,7 @@ enum devx_obj_flags {
struct devx_async_data {
struct mlx5_ib_dev *mdev;
struct list_head list;
- struct ib_uobject *fd_uobj;
+ struct devx_async_cmd_event_file *ev_file;
struct mlx5_async_work cb_work;
u16 cmd_out_len;
/* must be last field in this structure */
@@ -1673,21 +1673,20 @@ static void devx_query_callback(int status, struct mlx5_async_work *context)
{
struct devx_async_data *async_data =
container_of(context, struct devx_async_data, cb_work);
- struct ib_uobject *fd_uobj = async_data->fd_uobj;
- struct devx_async_cmd_event_file *ev_file;
- struct devx_async_event_queue *ev_queue;
+ struct devx_async_cmd_event_file *ev_file = async_data->ev_file;
+ struct devx_async_event_queue *ev_queue = &ev_file->ev_queue;
unsigned long flags;
- ev_file = container_of(fd_uobj, struct devx_async_cmd_event_file,
- uobj);
- ev_queue = &ev_file->ev_queue;
-
+ /*
+ * Note that if the struct devx_async_cmd_event_file uobj begins to be
+ * destroyed it will block at mlx5_cmd_cleanup_async_ctx() until this
+ * routine returns, ensuring that it always remains valid here.
+ */
spin_lock_irqsave(&ev_queue->lock, flags);
list_add_tail(&async_data->list, &ev_queue->event_list);
spin_unlock_irqrestore(&ev_queue->lock, flags);
wake_up_interruptible(&ev_queue->poll_wait);
- fput(fd_uobj->object);
}
#define MAX_ASYNC_BYTES_IN_USE (1024 * 1024) /* 1MB */
@@ -1756,9 +1755,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
async_data->cmd_out_len = cmd_out_len;
async_data->mdev = mdev;
- async_data->fd_uobj = fd_uobj;
+ async_data->ev_file = ev_file;
- get_file(fd_uobj->object);
MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
err = mlx5_cmd_exec_cb(&ev_file->async_ctx, cmd_in,
uverbs_attr_get_len(attrs,
@@ -1768,12 +1766,10 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
devx_query_callback, &async_data->cb_work);
if (err)
- goto cb_err;
+ goto free_async;
return 0;
-cb_err:
- fput(fd_uobj->object);
free_async:
kvfree(async_data);
sub_bytes:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 04/14] RDMA/core: Do not allow alloc_commit to fail
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (2 preceding siblings ...)
2020-01-08 17:21 ` [PATCH rdma-next 03/14] RDMA/mlx5: Simplify devx async commands Yishai Hadas
@ 2020-01-08 17:21 ` Yishai Hadas
2020-01-08 17:21 ` [PATCH rdma-next 05/14] RDMA/core: Make ib_ucq_object use ib_uevent_object Yishai Hadas
` (10 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:21 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This is a left over from an earlier version that creates a lot of
complexity for error unwind, particularly for FD uobjects.
The only reason this was done is so that anon_inode_get_file() could be
called with the final fops and a fully setup uobject. Both need to be
setup since unwinding anon_inode_get_file() via fput will call the
driver's release().
Now that the driver does not provide release, we no longer need to worry
about this complicated sequence, simply create the struct file at the
start and allow the core code's release function to deal with the abort
case.
This allows all the confusing error paths around commit to be removed.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/rdma_core.c | 116 +++++++++++++++------------------
drivers/infiniband/core/rdma_core.h | 21 +-----
drivers/infiniband/core/uverbs_cmd.c | 40 +++++++-----
drivers/infiniband/core/uverbs_ioctl.c | 45 ++++---------
include/rdma/uverbs_std_types.h | 10 ---
include/rdma/uverbs_types.h | 6 +-
6 files changed, 97 insertions(+), 141 deletions(-)
diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index e8ee89d..8ad6883 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -136,7 +136,11 @@ static int uverbs_destroy_uobject(struct ib_uobject *uobj,
lockdep_assert_held(&ufile->hw_destroy_rwsem);
assert_uverbs_usecnt(uobj, UVERBS_LOOKUP_WRITE);
- if (uobj->object) {
+ if (reason == RDMA_REMOVE_ABORT) {
+ WARN_ON(!list_empty(&uobj->list));
+ WARN_ON(!uobj->context);
+ uobj->uapi_object->type_class->alloc_abort(uobj);
+ } else if (uobj->object) {
ret = uobj->uapi_object->type_class->destroy_hw(uobj, reason,
attrs);
if (ret) {
@@ -152,12 +156,6 @@ static int uverbs_destroy_uobject(struct ib_uobject *uobj,
uobj->object = NULL;
}
- if (reason == RDMA_REMOVE_ABORT) {
- WARN_ON(!list_empty(&uobj->list));
- WARN_ON(!uobj->context);
- uobj->uapi_object->type_class->alloc_abort(uobj);
- }
-
uobj->context = NULL;
/*
@@ -456,22 +454,40 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
alloc_begin_fd_uobject(const struct uverbs_api_object *obj,
struct ib_uverbs_file *ufile)
{
+ const struct uverbs_obj_fd_type *fd_type =
+ container_of(obj->type_attrs, struct uverbs_obj_fd_type, type);
int new_fd;
struct ib_uobject *uobj;
+ struct file *filp;
+
+ if (WARN_ON(fd_type->fops->release != &uverbs_uobject_fd_release))
+ return ERR_PTR(-EINVAL);
new_fd = get_unused_fd_flags(O_CLOEXEC);
if (new_fd < 0)
return ERR_PTR(new_fd);
uobj = alloc_uobj(ufile, obj);
- if (IS_ERR(uobj)) {
- put_unused_fd(new_fd);
- return uobj;
+ if (IS_ERR(uobj))
+ goto err_fd;
+
+ /* Note that uverbs_uobject_fd_release() is called during abort */
+ filp = anon_inode_getfile(fd_type->name, fd_type->fops, NULL,
+ fd_type->flags);
+ if (IS_ERR(filp)) {
+ uobj = ERR_CAST(filp);
+ goto err_uobj;
}
+ uobj->object = filp;
uobj->id = new_fd;
uobj->ufile = ufile;
+ return uobj;
+err_uobj:
+ uverbs_uobject_put(uobj);
+err_fd:
+ put_unused_fd(new_fd);
return uobj;
}
@@ -545,6 +561,9 @@ static void remove_handle_idr_uobject(struct ib_uobject *uobj)
static void alloc_abort_fd_uobject(struct ib_uobject *uobj)
{
+ struct file *filp = uobj->object;
+
+ fput(filp);
put_unused_fd(uobj->id);
}
@@ -566,7 +585,7 @@ static void remove_handle_fd_uobject(struct ib_uobject *uobj)
{
}
-static int alloc_commit_idr_uobject(struct ib_uobject *uobj)
+static void alloc_commit_idr_uobject(struct ib_uobject *uobj)
{
struct ib_uverbs_file *ufile = uobj->ufile;
void *old;
@@ -580,31 +599,12 @@ static int alloc_commit_idr_uobject(struct ib_uobject *uobj)
*/
old = xa_store(&ufile->idr, uobj->id, uobj, GFP_KERNEL);
WARN_ON(old != NULL);
-
- return 0;
}
-static int alloc_commit_fd_uobject(struct ib_uobject *uobj)
+static void alloc_commit_fd_uobject(struct ib_uobject *uobj)
{
- const struct uverbs_obj_fd_type *fd_type = container_of(
- uobj->uapi_object->type_attrs, struct uverbs_obj_fd_type, type);
int fd = uobj->id;
- struct file *filp;
-
- /*
- * The kref for uobj is moved into filp->private data and put in
- * uverbs_close_fd(). Once alloc_commit() succeeds
- * uverbs_uobject_fd_release() must be guaranteed to be called from
- * the provided fops release callback.
- */
- filp = anon_inode_getfile(fd_type->name,
- fd_type->fops,
- uobj,
- fd_type->flags);
- if (IS_ERR(filp))
- return PTR_ERR(filp);
-
- uobj->object = filp;
+ struct file *filp = uobj->object;
/* Matching put will be done in uverbs_uobject_fd_release() */
kref_get(&uobj->ufile->ref);
@@ -616,9 +616,8 @@ static int alloc_commit_fd_uobject(struct ib_uobject *uobj)
* NOTE: Once we install the file we loose ownership of our kref on
* uobj. It will be put by uverbs_uobject_fd_release()
*/
+ filp->private_data = uobj;
fd_install(fd, filp);
-
- return 0;
}
/*
@@ -626,19 +625,13 @@ static int alloc_commit_fd_uobject(struct ib_uobject *uobj)
* caller can no longer assume uobj is valid. If this function fails it
* destroys the uboject, including the attached HW object.
*/
-int __must_check rdma_alloc_commit_uobject(struct ib_uobject *uobj,
- struct uverbs_attr_bundle *attrs)
+void rdma_alloc_commit_uobject(struct ib_uobject *uobj,
+ struct uverbs_attr_bundle *attrs)
{
struct ib_uverbs_file *ufile = attrs->ufile;
- int ret;
/* alloc_commit consumes the uobj kref */
- ret = uobj->uapi_object->type_class->alloc_commit(uobj);
- if (ret) {
- uverbs_destroy_uobject(uobj, RDMA_REMOVE_ABORT, attrs);
- up_read(&ufile->hw_destroy_rwsem);
- return ret;
- }
+ uobj->uapi_object->type_class->alloc_commit(uobj);
/* kref is held so long as the uobj is on the uobj list. */
uverbs_uobject_get(uobj);
@@ -651,8 +644,6 @@ int __must_check rdma_alloc_commit_uobject(struct ib_uobject *uobj,
/* Matches the down_read in rdma_alloc_begin_uobject */
up_read(&ufile->hw_destroy_rwsem);
-
- return 0;
}
/*
@@ -664,7 +655,6 @@ void rdma_alloc_abort_uobject(struct ib_uobject *uobj,
{
struct ib_uverbs_file *ufile = uobj->ufile;
- uobj->object = NULL;
uverbs_destroy_uobject(uobj, RDMA_REMOVE_ABORT, attrs);
/* Matches the down_read in rdma_alloc_begin_uobject */
@@ -771,14 +761,21 @@ void release_ufile_idr_uobject(struct ib_uverbs_file *ufile)
*/
int uverbs_uobject_fd_release(struct inode *inode, struct file *filp)
{
- struct ib_uobject *uobj = filp->private_data;
- struct ib_uverbs_file *ufile = uobj->ufile;
- struct uverbs_attr_bundle attrs = {
- .context = uobj->context,
- .ufile = ufile,
- };
+ struct ib_uverbs_file *ufile;
+ struct ib_uobject *uobj;
+
+ /* This can only happen if the fput came from alloc_abort_fd_uobject() */
+ if (!filp->private_data)
+ return 0;
+ uobj = filp->private_data;
+ ufile = uobj->ufile;
if (down_read_trylock(&ufile->hw_destroy_rwsem)) {
+ struct uverbs_attr_bundle attrs = {
+ .context = uobj->context,
+ .ufile = ufile,
+ };
+
/*
* lookup_get_fd_uobject holds the kref on the struct file any
* time a FD uobj is locked, which prevents this release
@@ -790,7 +787,7 @@ int uverbs_uobject_fd_release(struct inode *inode, struct file *filp)
up_read(&ufile->hw_destroy_rwsem);
}
- /* Matches the get in alloc_begin_fd_uobject */
+ /* Matches the get in alloc_commit_fd_uobject() */
kref_put(&ufile->ref, ib_uverbs_release_file);
/* Pairs with filp->private_data in alloc_begin_fd_uobject */
@@ -959,12 +956,10 @@ struct ib_uobject *
}
}
-int uverbs_finalize_object(struct ib_uobject *uobj,
- enum uverbs_obj_access access, bool commit,
- struct uverbs_attr_bundle *attrs)
+void uverbs_finalize_object(struct ib_uobject *uobj,
+ enum uverbs_obj_access access, bool commit,
+ struct uverbs_attr_bundle *attrs)
{
- int ret = 0;
-
/*
* refcounts should be handled at the object level and not at the
* uobject level. Refcounts of the objects themselves are done in
@@ -984,14 +979,11 @@ int uverbs_finalize_object(struct ib_uobject *uobj,
break;
case UVERBS_ACCESS_NEW:
if (commit)
- ret = rdma_alloc_commit_uobject(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
else
rdma_alloc_abort_uobject(uobj, attrs);
break;
default:
WARN_ON(true);
- ret = -EOPNOTSUPP;
}
-
- return ret;
}
diff --git a/drivers/infiniband/core/rdma_core.h b/drivers/infiniband/core/rdma_core.h
index 9269425..29f905e 100644
--- a/drivers/infiniband/core/rdma_core.h
+++ b/drivers/infiniband/core/rdma_core.h
@@ -63,24 +63,9 @@ struct ib_uobject *
uverbs_get_uobject_from_file(u16 object_id, enum uverbs_obj_access access,
s64 id, struct uverbs_attr_bundle *attrs);
-/*
- * Note that certain finalize stages could return a status:
- * (a) alloc_commit could return a failure if the object is committed at the
- * same time when the context is destroyed.
- * (b) remove_commit could fail if the object wasn't destroyed successfully.
- * Since multiple objects could be finalized in one transaction, it is very NOT
- * recommended to have several finalize actions which have side effects.
- * For example, it's NOT recommended to have a certain action which has both
- * a commit action and a destroy action or two destroy objects in the same
- * action. The rule of thumb is to have one destroy or commit action with
- * multiple lookups.
- * The first non zero return value of finalize_object is returned from this
- * function. For example, this could happen when we couldn't destroy an
- * object.
- */
-int uverbs_finalize_object(struct ib_uobject *uobj,
- enum uverbs_obj_access access, bool commit,
- struct uverbs_attr_bundle *attrs);
+void uverbs_finalize_object(struct ib_uobject *uobj,
+ enum uverbs_obj_access access, bool commit,
+ struct uverbs_attr_bundle *attrs);
int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 06ed32c..74f6ae4 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -446,7 +446,8 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
if (ret)
goto err_copy;
- return uobj_alloc_commit(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
+ return 0;
err_copy:
ib_dealloc_pd_user(pd, uverbs_get_cleared_udata(attrs));
@@ -642,7 +643,8 @@ static int ib_uverbs_open_xrcd(struct uverbs_attr_bundle *attrs)
mutex_unlock(&ibudev->xrcd_tree_mutex);
- return uobj_alloc_commit(&obj->uobject, attrs);
+ rdma_alloc_commit_uobject(&obj->uobject, attrs);
+ return 0;
err_copy:
if (inode) {
@@ -774,7 +776,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
uobj_put_obj_read(pd);
- return uobj_alloc_commit(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
+ return 0;
err_copy:
ib_dereg_mr_user(mr, uverbs_get_cleared_udata(attrs));
@@ -928,7 +931,8 @@ static int ib_uverbs_alloc_mw(struct uverbs_attr_bundle *attrs)
goto err_copy;
uobj_put_obj_read(pd);
- return uobj_alloc_commit(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
+ return 0;
err_copy:
uverbs_dealloc_mw(mw);
@@ -980,7 +984,8 @@ static int ib_uverbs_create_comp_channel(struct uverbs_attr_bundle *attrs)
return ret;
}
- return uobj_alloc_commit(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
+ return 0;
}
static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
@@ -1049,9 +1054,7 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
if (ret)
goto err_cb;
- ret = uobj_alloc_commit(&obj->uobject, attrs);
- if (ret)
- return ERR_PTR(ret);
+ rdma_alloc_commit_uobject(&obj->uobject, attrs);
return obj;
err_cb:
@@ -1491,7 +1494,8 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
if (ind_tbl)
uobj_put_obj_read(ind_tbl);
- return uobj_alloc_commit(&obj->uevent.uobject, attrs);
+ rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
+ return 0;
err_cb:
ib_destroy_qp_user(qp, uverbs_get_cleared_udata(attrs));
@@ -1623,7 +1627,8 @@ static int ib_uverbs_open_qp(struct uverbs_attr_bundle *attrs)
qp->uobject = &obj->uevent.uobject;
uobj_put_read(xrcd_uobj);
- return uobj_alloc_commit(&obj->uevent.uobject, attrs);
+ rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
+ return 0;
err_destroy:
ib_destroy_qp_user(qp, uverbs_get_cleared_udata(attrs));
@@ -2465,7 +2470,8 @@ static int ib_uverbs_create_ah(struct uverbs_attr_bundle *attrs)
goto err_copy;
uobj_put_obj_read(pd);
- return uobj_alloc_commit(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
+ return 0;
err_copy:
rdma_destroy_ah_user(ah, RDMA_DESTROY_AH_SLEEPABLE,
@@ -2977,7 +2983,8 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)
uobj_put_obj_read(pd);
uobj_put_obj_read(cq);
- return uobj_alloc_commit(&obj->uevent.uobject, attrs);
+ rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
+ return 0;
err_copy:
ib_destroy_wq(wq, uverbs_get_cleared_udata(attrs));
@@ -3151,7 +3158,8 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
for (j = 0; j < num_read_wqs; j++)
uobj_put_obj_read(wqs[j]);
- return uobj_alloc_commit(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
+ return 0;
err_copy:
ib_destroy_rwq_ind_table(rwq_ind_tbl);
@@ -3329,7 +3337,8 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
kfree(flow_attr);
if (cmd.flow_attr.num_of_specs)
kfree(kern_flow_attr);
- return uobj_alloc_commit(uobj, attrs);
+ rdma_alloc_commit_uobject(uobj, attrs);
+ return 0;
err_copy:
if (!qp->device->ops.destroy_flow(flow_id))
atomic_dec(&qp->usecnt);
@@ -3477,7 +3486,8 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs,
uobj_put_obj_read(attr.ext.cq);
uobj_put_obj_read(pd);
- return uobj_alloc_commit(&obj->uevent.uobject, attrs);
+ rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
+ return 0;
err_copy:
ib_destroy_srq_user(srq, uverbs_get_cleared_udata(attrs));
diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index 269938f..538affb 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -220,24 +220,17 @@ static int uverbs_process_idrs_array(struct bundle_priv *pbundle,
return ret;
}
-static int uverbs_free_idrs_array(const struct uverbs_api_attr *attr_uapi,
- struct uverbs_objs_arr_attr *attr,
- bool commit, struct uverbs_attr_bundle *attrs)
+static void uverbs_free_idrs_array(const struct uverbs_api_attr *attr_uapi,
+ struct uverbs_objs_arr_attr *attr,
+ bool commit,
+ struct uverbs_attr_bundle *attrs)
{
const struct uverbs_attr_spec *spec = &attr_uapi->spec;
- int current_ret;
- int ret = 0;
size_t i;
- for (i = 0; i != attr->len; i++) {
- current_ret = uverbs_finalize_object(attr->uobjects[i],
- spec->u2.objs_arr.access,
- commit, attrs);
- if (!ret)
- ret = current_ret;
- }
-
- return ret;
+ for (i = 0; i != attr->len; i++)
+ uverbs_finalize_object(attr->uobjects[i],
+ spec->u2.objs_arr.access, commit, attrs);
}
static int uverbs_process_attr(struct bundle_priv *pbundle,
@@ -495,26 +488,22 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
return ret;
}
-static int bundle_destroy(struct bundle_priv *pbundle, bool commit)
+static void bundle_destroy(struct bundle_priv *pbundle, bool commit)
{
unsigned int key_bitmap_len = pbundle->method_elm->key_bitmap_len;
struct bundle_alloc_head *memblock;
unsigned int i;
- int ret = 0;
/* fast path for simple uobjects */
i = -1;
while ((i = find_next_bit(pbundle->uobj_finalize, key_bitmap_len,
i + 1)) < key_bitmap_len) {
struct uverbs_attr *attr = &pbundle->bundle.attrs[i];
- int current_ret;
- current_ret = uverbs_finalize_object(
+ uverbs_finalize_object(
attr->obj_attr.uobject,
attr->obj_attr.attr_elm->spec.u.obj.access, commit,
&pbundle->bundle);
- if (!ret)
- ret = current_ret;
}
i = -1;
@@ -523,7 +512,6 @@ static int bundle_destroy(struct bundle_priv *pbundle, bool commit)
struct uverbs_attr *attr = &pbundle->bundle.attrs[i];
const struct uverbs_api_attr *attr_uapi;
void __rcu **slot;
- int current_ret;
slot = uapi_get_attr_for_method(
pbundle,
@@ -534,11 +522,8 @@ static int bundle_destroy(struct bundle_priv *pbundle, bool commit)
attr_uapi = rcu_dereference_protected(*slot, true);
if (attr_uapi->spec.type == UVERBS_ATTR_TYPE_IDRS_ARRAY) {
- current_ret = uverbs_free_idrs_array(
- attr_uapi, &attr->objs_arr_attr, commit,
- &pbundle->bundle);
- if (!ret)
- ret = current_ret;
+ uverbs_free_idrs_array(attr_uapi, &attr->objs_arr_attr,
+ commit, &pbundle->bundle);
}
}
@@ -548,8 +533,6 @@ static int bundle_destroy(struct bundle_priv *pbundle, bool commit)
memblock = memblock->next;
kvfree(tmp);
}
-
- return ret;
}
static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile,
@@ -562,7 +545,6 @@ static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile,
struct bundle_priv *pbundle;
struct bundle_priv onstack;
void __rcu **slot;
- int destroy_ret;
int ret;
if (unlikely(hdr->driver_id != uapi->driver_id))
@@ -610,10 +592,7 @@ static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile,
memset(pbundle->spec_finalize, 0, sizeof(pbundle->spec_finalize));
ret = ib_uverbs_run_method(pbundle, hdr->num_attrs);
- destroy_ret = bundle_destroy(pbundle, ret == 0);
- if (unlikely(destroy_ret && !ret))
- return destroy_ret;
-
+ bundle_destroy(pbundle, ret == 0);
return ret;
}
diff --git a/include/rdma/uverbs_std_types.h b/include/rdma/uverbs_std_types.h
index 05eabfd..c6bcaad 100644
--- a/include/rdma/uverbs_std_types.h
+++ b/include/rdma/uverbs_std_types.h
@@ -104,16 +104,6 @@ static inline void uobj_put_write(struct ib_uobject *uobj)
rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_WRITE);
}
-static inline int __must_check
-uobj_alloc_commit(struct ib_uobject *uobj, struct uverbs_attr_bundle *attrs)
-{
- int ret = rdma_alloc_commit_uobject(uobj, attrs);
-
- if (ret)
- return ret;
- return 0;
-}
-
static inline void uobj_alloc_abort(struct ib_uobject *uobj,
struct uverbs_attr_bundle *attrs)
{
diff --git a/include/rdma/uverbs_types.h b/include/rdma/uverbs_types.h
index 657c313..0532588 100644
--- a/include/rdma/uverbs_types.h
+++ b/include/rdma/uverbs_types.h
@@ -85,7 +85,7 @@ struct uverbs_obj_type_class {
struct ib_uobject *(*alloc_begin)(const struct uverbs_api_object *obj,
struct ib_uverbs_file *ufile);
/* This consumes the kref on uobj */
- int (*alloc_commit)(struct ib_uobject *uobj);
+ void (*alloc_commit)(struct ib_uobject *uobj);
/* This does not consume the kref on uobj */
void (*alloc_abort)(struct ib_uobject *uobj);
@@ -142,8 +142,8 @@ struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_api_object *obj,
struct uverbs_attr_bundle *attrs);
void rdma_alloc_abort_uobject(struct ib_uobject *uobj,
struct uverbs_attr_bundle *attrs);
-int __must_check rdma_alloc_commit_uobject(struct ib_uobject *uobj,
- struct uverbs_attr_bundle *attrs);
+void 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
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 05/14] RDMA/core: Make ib_ucq_object use ib_uevent_object
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (3 preceding siblings ...)
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 ` 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
` (9 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:21 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
Any uobject that sends events into the async_event_file should be using
ib_uevent_object so it can use the standard uevent based helper
functions. CQ pushes events into both the async_event and the comp_channel
in an open coded way. Move the async events related stuff to
ib_uevent_object.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/uverbs.h | 5 ++---
drivers/infiniband/core/uverbs_cmd.c | 20 +++++++++-----------
drivers/infiniband/core/uverbs_main.c | 17 ++++++-----------
drivers/infiniband/core/uverbs_std_types_cq.c | 18 ++++++++----------
4 files changed, 25 insertions(+), 35 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 63f7f7d..9fa0446 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -183,6 +183,7 @@ struct ib_uverbs_mcast_entry {
struct ib_uevent_object {
struct ib_uobject uobject;
+ /* List member for ib_uverbs_async_event_file list */
struct list_head event_list;
u32 events_reported;
};
@@ -210,11 +211,9 @@ struct ib_uwq_object {
};
struct ib_ucq_object {
- struct ib_uobject uobject;
+ struct ib_uevent_object uevent;
struct list_head comp_list;
- struct list_head async_list;
u32 comp_events_reported;
- u32 async_events_reported;
};
extern const struct file_operations uverbs_event_fops;
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 74f6ae4..3a2a278 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1015,11 +1015,9 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
}
}
- obj->uobject.user_handle = cmd->user_handle;
- obj->comp_events_reported = 0;
- obj->async_events_reported = 0;
+ obj->uevent.uobject.user_handle = cmd->user_handle;
INIT_LIST_HEAD(&obj->comp_list);
- INIT_LIST_HEAD(&obj->async_list);
+ INIT_LIST_HEAD(&obj->uevent.event_list);
attr.cqe = cmd->cqe;
attr.comp_vector = cmd->comp_vector;
@@ -1031,7 +1029,7 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
goto err_file;
}
cq->device = ib_dev;
- cq->uobject = &obj->uobject;
+ cq->uobject = &obj->uevent.uobject;
cq->comp_handler = ib_uverbs_comp_handler;
cq->event_handler = ib_uverbs_cq_event_handler;
cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;
@@ -1041,9 +1039,9 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
if (ret)
goto err_free;
- obj->uobject.object = cq;
+ obj->uevent.uobject.object = cq;
memset(&resp, 0, sizeof resp);
- resp.base.cq_handle = obj->uobject.id;
+ resp.base.cq_handle = obj->uevent.uobject.id;
resp.base.cqe = cq->cqe;
resp.response_length = uverbs_response_length(attrs, sizeof(resp));
@@ -1054,7 +1052,7 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
if (ret)
goto err_cb;
- rdma_alloc_commit_uobject(&obj->uobject, attrs);
+ rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
return obj;
err_cb:
@@ -1067,7 +1065,7 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
ib_uverbs_release_ucq(attrs->ufile, ev_file, obj);
err:
- uobj_alloc_abort(&obj->uobject, attrs);
+ uobj_alloc_abort(&obj->uevent.uobject, attrs);
return ERR_PTR(ret);
}
@@ -1261,10 +1259,10 @@ static int ib_uverbs_destroy_cq(struct uverbs_attr_bundle *attrs)
if (IS_ERR(uobj))
return PTR_ERR(uobj);
- obj = container_of(uobj, struct ib_ucq_object, uobject);
+ obj = container_of(uobj, struct ib_ucq_object, uevent.uobject);
memset(&resp, 0, sizeof(resp));
resp.comp_events_reported = obj->comp_events_reported;
- resp.async_events_reported = obj->async_events_reported;
+ resp.async_events_reported = obj->uevent.events_reported;
uobj_put_destroy(uobj);
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 290a91e..8fd8b48 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -150,12 +150,7 @@ void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
uverbs_uobject_put(&ev_file->uobj);
}
- spin_lock_irq(&file->async_file->ev_queue.lock);
- list_for_each_entry_safe(evt, tmp, &uobj->async_list, obj_list) {
- list_del(&evt->list);
- kfree(evt);
- }
- spin_unlock_irq(&file->async_file->ev_queue.lock);
+ ib_uverbs_release_uevent(file, &uobj->uevent);
}
void ib_uverbs_release_uevent(struct ib_uverbs_file *file,
@@ -417,7 +412,7 @@ void ib_uverbs_comp_handler(struct ib_cq *cq, void *cq_context)
return;
}
- uobj = container_of(cq->uobject, struct ib_ucq_object, uobject);
+ uobj = container_of(cq->uobject, struct ib_ucq_object, uevent.uobject);
entry->desc.comp.cq_handle = cq->uobject->user_handle;
entry->counter = &uobj->comp_events_reported;
@@ -466,12 +461,12 @@ static void ib_uverbs_async_handler(struct ib_uverbs_file *file,
void ib_uverbs_cq_event_handler(struct ib_event *event, void *context_ptr)
{
- struct ib_ucq_object *uobj = container_of(event->element.cq->uobject,
- struct ib_ucq_object, uobject);
+ struct ib_uevent_object *uobj = container_of(
+ event->element.cq->uobject, struct ib_uevent_object, uobject);
ib_uverbs_async_handler(uobj->uobject.ufile, uobj->uobject.user_handle,
- event->event, &uobj->async_list,
- &uobj->async_events_reported);
+ event->event, &uobj->event_list,
+ &uobj->events_reported);
}
void ib_uverbs_qp_event_handler(struct ib_event *event, void *context_ptr)
diff --git a/drivers/infiniband/core/uverbs_std_types_cq.c b/drivers/infiniband/core/uverbs_std_types_cq.c
index e39fe6a..fbc605a 100644
--- a/drivers/infiniband/core/uverbs_std_types_cq.c
+++ b/drivers/infiniband/core/uverbs_std_types_cq.c
@@ -41,7 +41,7 @@ static int uverbs_free_cq(struct ib_uobject *uobject,
struct ib_cq *cq = uobject->object;
struct ib_uverbs_event_queue *ev_queue = cq->cq_context;
struct ib_ucq_object *ucq =
- container_of(uobject, struct ib_ucq_object, uobject);
+ container_of(uobject, struct ib_ucq_object, uevent.uobject);
int ret;
ret = ib_destroy_cq_user(cq, &attrs->driver_udata);
@@ -63,7 +63,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
{
struct ib_ucq_object *obj = container_of(
uverbs_attr_get_uobject(attrs, UVERBS_ATTR_CREATE_CQ_HANDLE),
- typeof(*obj), uobject);
+ typeof(*obj), uevent.uobject);
struct ib_device *ib_dev = attrs->context->device;
int ret;
u64 user_handle;
@@ -106,10 +106,8 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
goto err_event_file;
}
- obj->comp_events_reported = 0;
- obj->async_events_reported = 0;
INIT_LIST_HEAD(&obj->comp_list);
- INIT_LIST_HEAD(&obj->async_list);
+ INIT_LIST_HEAD(&obj->uevent.event_list);
cq = rdma_zalloc_drv_obj(ib_dev, ib_cq);
if (!cq) {
@@ -118,7 +116,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
}
cq->device = ib_dev;
- cq->uobject = &obj->uobject;
+ cq->uobject = &obj->uevent.uobject;
cq->comp_handler = ib_uverbs_comp_handler;
cq->event_handler = ib_uverbs_cq_event_handler;
cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;
@@ -129,8 +127,8 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
if (ret)
goto err_free;
- obj->uobject.object = cq;
- obj->uobject.user_handle = user_handle;
+ obj->uevent.uobject.object = cq;
+ obj->uevent.uobject.user_handle = user_handle;
rdma_restrack_uadd(&cq->res);
ret = uverbs_copy_to(attrs, UVERBS_ATTR_CREATE_CQ_RESP_CQE, &cq->cqe,
@@ -182,10 +180,10 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_DESTROY)(
struct ib_uobject *uobj =
uverbs_attr_get_uobject(attrs, UVERBS_ATTR_DESTROY_CQ_HANDLE);
struct ib_ucq_object *obj =
- container_of(uobj, struct ib_ucq_object, uobject);
+ container_of(uobj, struct ib_ucq_object, uevent.uobject);
struct ib_uverbs_destroy_cq_resp resp = {
.comp_events_reported = obj->comp_events_reported,
- .async_events_reported = obj->async_events_reported
+ .async_events_reported = obj->uevent.events_reported
};
return uverbs_copy_to(attrs, UVERBS_ATTR_DESTROY_CQ_RESP, &resp,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 06/14] RDMA/core: Do not erase the type of ib_cq.uobject
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (4 preceding siblings ...)
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 ` 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
` (8 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:21 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This is a struct ib_ucq_object pointer, instead of using container_of()
all over the place just store it with its actual type.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/nldev.c | 3 +-
drivers/infiniband/core/uverbs_cmd.c | 40 +++++++++++++++++----------
drivers/infiniband/core/uverbs_main.c | 7 ++---
drivers/infiniband/core/uverbs_std_types_cq.c | 2 +-
include/rdma/ib_verbs.h | 4 ++-
5 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index cbf6041..37b433aa 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -41,6 +41,7 @@
#include "core_priv.h"
#include "cma_priv.h"
#include "restrack.h"
+#include "uverbs.h"
typedef int (*res_fill_func_t)(struct sk_buff*, bool,
struct rdma_restrack_entry*, uint32_t);
@@ -599,7 +600,7 @@ static int fill_res_cq_entry(struct sk_buff *msg, bool has_cap_net_admin,
goto err;
if (!rdma_is_kernel_res(res) &&
nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_CTXN,
- cq->uobject->context->res.id))
+ cq->uobject->uevent.uobject.context->res.id))
goto err;
if (fill_res_name_pid(msg, res))
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 3a2a278..b08679a 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1029,7 +1029,7 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
goto err_file;
}
cq->device = ib_dev;
- cq->uobject = &obj->uevent.uobject;
+ cq->uobject = obj;
cq->comp_handler = ib_uverbs_comp_handler;
cq->event_handler = ib_uverbs_cq_event_handler;
cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;
@@ -1134,7 +1134,8 @@ static int ib_uverbs_resize_cq(struct uverbs_attr_bundle *attrs)
ret = uverbs_response(attrs, &resp, sizeof(resp));
out:
- uobj_put_obj_read(cq);
+ rdma_lookup_put_uobject(&cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return ret;
}
@@ -1217,7 +1218,8 @@ static int ib_uverbs_poll_cq(struct uverbs_attr_bundle *attrs)
ret = uverbs_output_written(attrs, UVERBS_ATTR_CORE_OUT);
out_put:
- uobj_put_obj_read(cq);
+ rdma_lookup_put_uobject(&cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return ret;
}
@@ -1238,8 +1240,8 @@ static int ib_uverbs_req_notify_cq(struct uverbs_attr_bundle *attrs)
ib_req_notify_cq(cq, cmd.solicited_only ?
IB_CQ_SOLICITED : IB_CQ_NEXT_COMP);
- uobj_put_obj_read(cq);
-
+ rdma_lookup_put_uobject(&cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return 0;
}
@@ -1484,9 +1486,11 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
if (pd)
uobj_put_obj_read(pd);
if (scq)
- uobj_put_obj_read(scq);
+ rdma_lookup_put_uobject(&scq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (rcq && rcq != scq)
- uobj_put_obj_read(rcq);
+ rdma_lookup_put_uobject(&rcq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (srq)
uobj_put_obj_read(srq);
if (ind_tbl)
@@ -1503,9 +1507,11 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
if (pd)
uobj_put_obj_read(pd);
if (scq)
- uobj_put_obj_read(scq);
+ rdma_lookup_put_uobject(&scq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (rcq && rcq != scq)
- uobj_put_obj_read(rcq);
+ rdma_lookup_put_uobject(&rcq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (srq)
uobj_put_obj_read(srq);
if (ind_tbl)
@@ -2980,14 +2986,16 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)
goto err_copy;
uobj_put_obj_read(pd);
- uobj_put_obj_read(cq);
+ rdma_lookup_put_uobject(&cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
return 0;
err_copy:
ib_destroy_wq(wq, uverbs_get_cleared_udata(attrs));
err_put_cq:
- uobj_put_obj_read(cq);
+ rdma_lookup_put_uobject(&cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
err_put_pd:
uobj_put_obj_read(pd);
err_uobj:
@@ -3481,7 +3489,8 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs,
uobj_put_read(xrcd_uobj);
if (ib_srq_has_cq(cmd->srq_type))
- uobj_put_obj_read(attr.ext.cq);
+ rdma_lookup_put_uobject(&attr.ext.cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
uobj_put_obj_read(pd);
rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
@@ -3498,7 +3507,8 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs,
err_put_cq:
if (ib_srq_has_cq(cmd->srq_type))
- uobj_put_obj_read(attr.ext.cq);
+ rdma_lookup_put_uobject(&attr.ext.cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
err_put_xrcd:
if (cmd->srq_type == IB_SRQT_XRC) {
@@ -3714,8 +3724,8 @@ static int ib_uverbs_ex_modify_cq(struct uverbs_attr_bundle *attrs)
ret = rdma_set_cq_moderation(cq, cmd.attr.cq_count, cmd.attr.cq_period);
- uobj_put_obj_read(cq);
-
+ rdma_lookup_put_uobject(&cq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return ret;
}
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 8fd8b48..bf13591 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -412,9 +412,9 @@ void ib_uverbs_comp_handler(struct ib_cq *cq, void *cq_context)
return;
}
- uobj = container_of(cq->uobject, struct ib_ucq_object, uevent.uobject);
+ uobj = cq->uobject;
- entry->desc.comp.cq_handle = cq->uobject->user_handle;
+ entry->desc.comp.cq_handle = cq->uobject->uevent.uobject.user_handle;
entry->counter = &uobj->comp_events_reported;
list_add_tail(&entry->list, &ev_queue->event_list);
@@ -461,8 +461,7 @@ static void ib_uverbs_async_handler(struct ib_uverbs_file *file,
void ib_uverbs_cq_event_handler(struct ib_event *event, void *context_ptr)
{
- struct ib_uevent_object *uobj = container_of(
- event->element.cq->uobject, struct ib_uevent_object, uobject);
+ struct ib_uevent_object *uobj = &event->element.cq->uobject->uevent;
ib_uverbs_async_handler(uobj->uobject.ufile, uobj->uobject.user_handle,
event->event, &uobj->event_list,
diff --git a/drivers/infiniband/core/uverbs_std_types_cq.c b/drivers/infiniband/core/uverbs_std_types_cq.c
index fbc605a..a41c758 100644
--- a/drivers/infiniband/core/uverbs_std_types_cq.c
+++ b/drivers/infiniband/core/uverbs_std_types_cq.c
@@ -116,7 +116,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
}
cq->device = ib_dev;
- cq->uobject = &obj->uevent.uobject;
+ cq->uobject = obj;
cq->comp_handler = ib_uverbs_comp_handler;
cq->event_handler = ib_uverbs_cq_event_handler;
cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 5608e14..81bc259 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -77,6 +77,8 @@
extern struct workqueue_struct *ib_comp_wq;
extern struct workqueue_struct *ib_comp_unbound_wq;
+struct ib_ucq_object;
+
__printf(3, 4) __cold
void ibdev_printk(const char *level, const struct ib_device *ibdev,
const char *format, ...);
@@ -1544,7 +1546,7 @@ enum ib_poll_context {
struct ib_cq {
struct ib_device *device;
- struct ib_uobject *uobject;
+ struct ib_ucq_object *uobject;
ib_comp_handler comp_handler;
void (*event_handler)(struct ib_event *, void *);
void *cq_context;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 07/14] RDMA/core: Do not erase the type of ib_qp.uobject
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (5 preceding siblings ...)
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 ` 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
` (7 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:21 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This is a struct ib_uqp_object pointer, instead of using container_of()
all over the place just store it with its actual type.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/core_priv.h | 2 +-
drivers/infiniband/core/uverbs_cmd.c | 34 +++++++++++++++++++++-------------
drivers/infiniband/core/uverbs_main.c | 3 +--
include/rdma/ib_verbs.h | 3 ++-
4 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 3645e09..34feb68 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -320,7 +320,7 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
struct ib_pd *pd,
struct ib_qp_init_attr *attr,
struct ib_udata *udata,
- struct ib_uobject *uobj)
+ struct ib_uqp_object *uobj)
{
enum ib_qp_type qp_type = attr->qp_type;
struct ib_qp *qp;
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index b08679a..4d84d08 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1424,7 +1424,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
qp = ib_create_qp(pd, &attr);
else
qp = _ib_create_qp(device, pd, &attr, &attrs->driver_udata,
- &obj->uevent.uobject);
+ obj);
if (IS_ERR(qp)) {
ret = PTR_ERR(qp);
@@ -1457,7 +1457,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
atomic_inc(&ind_tbl->usecnt);
} else {
/* It is done in _ib_create_qp for other QP types */
- qp->uobject = &obj->uevent.uobject;
+ qp->uobject = obj;
}
obj->uevent.uobject.object = qp;
@@ -1628,7 +1628,7 @@ static int ib_uverbs_open_qp(struct uverbs_attr_bundle *attrs)
obj->uxrcd = container_of(xrcd_uobj, struct ib_uxrcd_object, uobject);
atomic_inc(&obj->uxrcd->refcnt);
- qp->uobject = &obj->uevent.uobject;
+ qp->uobject = obj;
uobj_put_read(xrcd_uobj);
rdma_alloc_commit_uobject(&obj->uevent.uobject, attrs);
@@ -1693,7 +1693,8 @@ static int ib_uverbs_query_qp(struct uverbs_attr_bundle *attrs)
ret = ib_query_qp(qp, attr, cmd.attr_mask, init_attr);
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (ret)
goto out;
@@ -1930,7 +1931,8 @@ static int modify_qp(struct uverbs_attr_bundle *attrs,
&attrs->driver_udata);
release_qp:
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
out:
kfree(attr);
@@ -2194,7 +2196,8 @@ static int ib_uverbs_post_send(struct uverbs_attr_bundle *attrs)
ret = ret2;
out_put:
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
while (wr) {
if (is_ud && ud_wr(wr)->ah)
@@ -2336,7 +2339,8 @@ static int ib_uverbs_post_recv(struct uverbs_attr_bundle *attrs)
resp.bad_wr = 0;
ret = qp->device->ops.post_recv(qp->real_qp, wr, &bad_wr);
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (ret) {
for (next = wr; next; next = next->next) {
++resp.bad_wr;
@@ -2517,7 +2521,7 @@ static int ib_uverbs_attach_mcast(struct uverbs_attr_bundle *attrs)
if (!qp)
return -EINVAL;
- obj = container_of(qp->uobject, struct ib_uqp_object, uevent.uobject);
+ obj = qp->uobject;
mutex_lock(&obj->mcast_lock);
list_for_each_entry(mcast, &obj->mcast_list, list)
@@ -2544,7 +2548,8 @@ static int ib_uverbs_attach_mcast(struct uverbs_attr_bundle *attrs)
out_put:
mutex_unlock(&obj->mcast_lock);
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return ret;
}
@@ -2566,7 +2571,7 @@ static int ib_uverbs_detach_mcast(struct uverbs_attr_bundle *attrs)
if (!qp)
return -EINVAL;
- obj = container_of(qp->uobject, struct ib_uqp_object, uevent.uobject);
+ obj = qp->uobject;
mutex_lock(&obj->mcast_lock);
list_for_each_entry(mcast, &obj->mcast_list, list)
@@ -2587,7 +2592,8 @@ static int ib_uverbs_detach_mcast(struct uverbs_attr_bundle *attrs)
out_put:
mutex_unlock(&obj->mcast_lock);
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return ret;
}
@@ -3339,7 +3345,8 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
if (err)
goto err_copy;
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
kfree(flow_attr);
if (cmd.flow_attr.num_of_specs)
kfree(kern_flow_attr);
@@ -3353,7 +3360,8 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
err_free_flow_attr:
kfree(flow_attr);
err_put:
- uobj_put_obj_read(qp);
+ rdma_lookup_put_uobject(&qp->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
err_uobj:
uobj_alloc_abort(uobj, attrs);
err_free_attr:
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index bf13591..b0e7daa 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -476,8 +476,7 @@ void ib_uverbs_qp_event_handler(struct ib_event *event, void *context_ptr)
if (!event->element.qp->uobject)
return;
- uobj = container_of(event->element.qp->uobject,
- struct ib_uevent_object, uobject);
+ uobj = &event->element.qp->uobject->uevent;
ib_uverbs_async_handler(context_ptr, uobj->uobject.user_handle,
event->event, &uobj->event_list,
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 81bc259..12e0c92 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -72,6 +72,7 @@
#define IB_FW_VERSION_NAME_MAX ETHTOOL_FWVERS_LEN
struct ib_umem_odp;
+struct ib_uqp_object;
extern struct workqueue_struct *ib_wq;
extern struct workqueue_struct *ib_comp_wq;
@@ -1730,7 +1731,7 @@ struct ib_qp {
atomic_t usecnt;
struct list_head open_list;
struct ib_qp *real_qp;
- struct ib_uobject *uobject;
+ struct ib_uqp_object *uobject;
void (*event_handler)(struct ib_event *, void *);
void *qp_context;
/* sgid_attrs associated with the AV's */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 08/14] RDMA/core: Do not erase the type of ib_srq.uobject
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (6 preceding siblings ...)
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 ` 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
` (6 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:22 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This is a struct ib_usrq_object pointer, instead of using container_of()
all over the place just store it with its actual type.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/uverbs_cmd.c | 17 +++++++++++------
drivers/infiniband/core/uverbs_main.c | 3 +--
include/rdma/ib_verbs.h | 3 ++-
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 4d84d08..8350e02 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1492,7 +1492,8 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
rdma_lookup_put_uobject(&rcq->uobject->uevent.uobject,
UVERBS_LOOKUP_READ);
if (srq)
- uobj_put_obj_read(srq);
+ rdma_lookup_put_uobject(&srq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (ind_tbl)
uobj_put_obj_read(ind_tbl);
@@ -1513,7 +1514,8 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
rdma_lookup_put_uobject(&rcq->uobject->uevent.uobject,
UVERBS_LOOKUP_READ);
if (srq)
- uobj_put_obj_read(srq);
+ rdma_lookup_put_uobject(&srq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (ind_tbl)
uobj_put_obj_read(ind_tbl);
@@ -2390,7 +2392,8 @@ static int ib_uverbs_post_srq_recv(struct uverbs_attr_bundle *attrs)
resp.bad_wr = 0;
ret = srq->device->ops.post_srq_recv(srq, wr, &bad_wr);
- uobj_put_obj_read(srq);
+ rdma_lookup_put_uobject(&srq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (ret)
for (next = wr; next; next = next->next) {
@@ -3458,7 +3461,7 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs,
srq->device = pd->device;
srq->pd = pd;
srq->srq_type = cmd->srq_type;
- srq->uobject = &obj->uevent.uobject;
+ srq->uobject = obj;
srq->event_handler = attr.event_handler;
srq->srq_context = attr.srq_context;
@@ -3584,7 +3587,8 @@ static int ib_uverbs_modify_srq(struct uverbs_attr_bundle *attrs)
ret = srq->device->ops.modify_srq(srq, &attr, cmd.attr_mask,
&attrs->driver_udata);
- uobj_put_obj_read(srq);
+ rdma_lookup_put_uobject(&srq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return ret;
}
@@ -3607,7 +3611,8 @@ static int ib_uverbs_query_srq(struct uverbs_attr_bundle *attrs)
ret = ib_query_srq(srq, &attr);
- uobj_put_obj_read(srq);
+ rdma_lookup_put_uobject(&srq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
if (ret)
return ret;
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index b0e7daa..b7bee1f 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -497,8 +497,7 @@ void ib_uverbs_srq_event_handler(struct ib_event *event, void *context_ptr)
{
struct ib_uevent_object *uobj;
- uobj = container_of(event->element.srq->uobject,
- struct ib_uevent_object, uobject);
+ uobj = &event->element.srq->uobject->uevent;
ib_uverbs_async_handler(context_ptr, uobj->uobject.user_handle,
event->event, &uobj->event_list,
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 12e0c92..cfa871b 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -73,6 +73,7 @@
struct ib_umem_odp;
struct ib_uqp_object;
+struct ib_usrq_object;
extern struct workqueue_struct *ib_wq;
extern struct workqueue_struct *ib_comp_wq;
@@ -1570,7 +1571,7 @@ struct ib_cq {
struct ib_srq {
struct ib_device *device;
struct ib_pd *pd;
- struct ib_uobject *uobject;
+ struct ib_usrq_object *uobject;
void (*event_handler)(struct ib_event *, void *);
void *srq_context;
enum ib_srq_type srq_type;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 09/14] RDMA/core: Do not erase the type of ib_wq.uobject
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (7 preceding siblings ...)
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 ` Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 10/14] RDMA/core: Simplify type usage for ib_uverbs_async_handler() Yishai Hadas
` (5 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:22 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This is a struct ib_uwq_object pointer, instead of using container_of()
all over the place just store it with its actual type.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/uverbs_cmd.c | 13 ++++++++-----
drivers/infiniband/core/uverbs_main.c | 3 +--
include/rdma/ib_verbs.h | 3 ++-
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 8350e02..66f86b4 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2971,7 +2971,7 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)
goto err_put_cq;
}
- wq->uobject = &obj->uevent.uobject;
+ wq->uobject = obj;
obj->uevent.uobject.object = wq;
wq->wq_type = wq_init_attr.wq_type;
wq->cq = cq;
@@ -2981,7 +2981,7 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)
atomic_set(&wq->usecnt, 0);
atomic_inc(&pd->usecnt);
atomic_inc(&cq->usecnt);
- wq->uobject = &obj->uevent.uobject;
+ wq->uobject = obj;
obj->uevent.uobject.object = wq;
memset(&resp, 0, sizeof(resp));
@@ -3070,7 +3070,8 @@ static int ib_uverbs_ex_modify_wq(struct uverbs_attr_bundle *attrs)
}
ret = wq->device->ops.modify_wq(wq, &wq_attr, cmd.attr_mask,
&attrs->driver_udata);
- uobj_put_obj_read(wq);
+ rdma_lookup_put_uobject(&wq->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
return ret;
}
@@ -3171,7 +3172,8 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
kfree(wqs_handles);
for (j = 0; j < num_read_wqs; j++)
- uobj_put_obj_read(wqs[j]);
+ rdma_lookup_put_uobject(&wqs[j]->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
rdma_alloc_commit_uobject(uobj, attrs);
return 0;
@@ -3182,7 +3184,8 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
uobj_alloc_abort(uobj, attrs);
put_wqs:
for (j = 0; j < num_read_wqs; j++)
- uobj_put_obj_read(wqs[j]);
+ rdma_lookup_put_uobject(&wqs[j]->uobject->uevent.uobject,
+ UVERBS_LOOKUP_READ);
err_free:
kfree(wqs_handles);
kfree(wqs);
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index b7bee1f..b0aad2e 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -485,8 +485,7 @@ void ib_uverbs_qp_event_handler(struct ib_event *event, void *context_ptr)
void ib_uverbs_wq_event_handler(struct ib_event *event, void *context_ptr)
{
- struct ib_uevent_object *uobj = container_of(event->element.wq->uobject,
- struct ib_uevent_object, uobject);
+ struct ib_uevent_object *uobj = &event->element.wq->uobject->uevent;
ib_uverbs_async_handler(context_ptr, uobj->uobject.user_handle,
event->event, &uobj->event_list,
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index cfa871b..08cc7dc 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -74,6 +74,7 @@
struct ib_umem_odp;
struct ib_uqp_object;
struct ib_usrq_object;
+struct ib_uwq_object;
extern struct workqueue_struct *ib_wq;
extern struct workqueue_struct *ib_comp_wq;
@@ -1616,7 +1617,7 @@ enum ib_wq_state {
struct ib_wq {
struct ib_device *device;
- struct ib_uobject *uobject;
+ struct ib_uwq_object *uobject;
void *wq_context;
void (*event_handler)(struct ib_event *, void *);
struct ib_pd *pd;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 10/14] RDMA/core: Simplify type usage for ib_uverbs_async_handler()
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (8 preceding siblings ...)
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 ` Yishai Hadas
2020-01-08 17:22 ` [PATCH rdma-next 11/14] RDMA/core: Fix locking in ib_uverbs_event_read Yishai Hadas
` (4 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:22 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This function works on an ib_uverbs_async_file. Accept that as a parameter
instead of the struct ib_uverbs_file.
Consoldiate all the callers working from an ib_uevent_object to a single
function and locate the async_file directly from the struct ib_uobject
instead of using context_ptr.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/uverbs.h | 2 -
drivers/infiniband/core/uverbs_cmd.c | 9 +---
drivers/infiniband/core/uverbs_main.c | 79 +++++++++++++++--------------------
3 files changed, 34 insertions(+), 56 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 9fa0446..8384b66 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -235,8 +235,6 @@ void ib_uverbs_release_uevent(struct ib_uverbs_file *file,
void ib_uverbs_qp_event_handler(struct ib_event *event, void *context_ptr);
void ib_uverbs_wq_event_handler(struct ib_event *event, void *context_ptr);
void ib_uverbs_srq_event_handler(struct ib_event *event, void *context_ptr);
-void ib_uverbs_event_handler(struct ib_event_handler *handler,
- struct ib_event *event);
int ib_uverbs_dealloc_xrcd(struct ib_uobject *uobject, struct ib_xrcd *xrcd,
enum rdma_remove_reason why,
struct uverbs_attr_bundle *attrs);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 66f86b4..51117e7 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1378,7 +1378,6 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
}
attr.event_handler = ib_uverbs_qp_event_handler;
- attr.qp_context = attrs->ufile;
attr.send_cq = scq;
attr.recv_cq = rcq;
attr.srq = srq;
@@ -1394,7 +1393,6 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
attr.cap.max_recv_sge = cmd->max_recv_sge;
attr.cap.max_inline_data = cmd->max_inline_data;
- obj->uevent.events_reported = 0;
INIT_LIST_HEAD(&obj->uevent.event_list);
INIT_LIST_HEAD(&obj->mcast_list);
@@ -1442,7 +1440,6 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
qp->srq = attr.srq;
qp->rwq_ind_tbl = ind_tbl;
qp->event_handler = attr.event_handler;
- qp->qp_context = attr.qp_context;
qp->qp_type = attr.qp_type;
atomic_set(&qp->usecnt, 0);
atomic_inc(&pd->usecnt);
@@ -1577,7 +1574,7 @@ static int ib_uverbs_open_qp(struct uverbs_attr_bundle *attrs)
struct ib_xrcd *xrcd;
struct ib_uobject *uninitialized_var(xrcd_uobj);
struct ib_qp *qp;
- struct ib_qp_open_attr attr;
+ struct ib_qp_open_attr attr = {};
int ret;
struct ib_device *ib_dev;
@@ -1603,11 +1600,9 @@ static int ib_uverbs_open_qp(struct uverbs_attr_bundle *attrs)
}
attr.event_handler = ib_uverbs_qp_event_handler;
- attr.qp_context = attrs->ufile;
attr.qp_num = cmd.qpn;
attr.qp_type = cmd.qp_type;
- obj->uevent.events_reported = 0;
INIT_LIST_HEAD(&obj->uevent.event_list);
INIT_LIST_HEAD(&obj->mcast_list);
@@ -2962,7 +2957,6 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)
wq_init_attr.wq_type = cmd.wq_type;
wq_init_attr.event_handler = ib_uverbs_wq_event_handler;
wq_init_attr.create_flags = cmd.create_flags;
- obj->uevent.events_reported = 0;
INIT_LIST_HEAD(&obj->uevent.event_list);
wq = pd->device->ops.create_wq(pd, &wq_init_attr, &attrs->driver_udata);
@@ -3452,7 +3446,6 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs,
attr.attr.max_sge = cmd->max_sge;
attr.attr.srq_limit = cmd->srq_limit;
- obj->uevent.events_reported = 0;
INIT_LIST_HEAD(&obj->uevent.event_list);
srq = rdma_zalloc_drv_obj(ib_dev, ib_srq);
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index b0aad2e..85dbd81 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -425,92 +425,79 @@ void ib_uverbs_comp_handler(struct ib_cq *cq, void *cq_context)
kill_fasync(&ev_queue->async_queue, SIGIO, POLL_IN);
}
-static void ib_uverbs_async_handler(struct ib_uverbs_file *file,
- __u64 element, __u64 event,
- struct list_head *obj_list,
- u32 *counter)
+static void
+ib_uverbs_async_handler(struct ib_uverbs_async_event_file *async_file,
+ __u64 element, __u64 event, struct list_head *obj_list,
+ u32 *counter)
{
struct ib_uverbs_event *entry;
unsigned long flags;
- spin_lock_irqsave(&file->async_file->ev_queue.lock, flags);
- if (file->async_file->ev_queue.is_closed) {
- spin_unlock_irqrestore(&file->async_file->ev_queue.lock, flags);
+ spin_lock_irqsave(&async_file->ev_queue.lock, flags);
+ if (async_file->ev_queue.is_closed) {
+ spin_unlock_irqrestore(&async_file->ev_queue.lock, flags);
return;
}
entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
if (!entry) {
- spin_unlock_irqrestore(&file->async_file->ev_queue.lock, flags);
+ spin_unlock_irqrestore(&async_file->ev_queue.lock, flags);
return;
}
- entry->desc.async.element = element;
+ entry->desc.async.element = element;
entry->desc.async.event_type = event;
- entry->desc.async.reserved = 0;
- entry->counter = counter;
+ entry->desc.async.reserved = 0;
+ entry->counter = counter;
- list_add_tail(&entry->list, &file->async_file->ev_queue.event_list);
+ list_add_tail(&entry->list, &async_file->ev_queue.event_list);
if (obj_list)
list_add_tail(&entry->obj_list, obj_list);
- spin_unlock_irqrestore(&file->async_file->ev_queue.lock, flags);
+ spin_unlock_irqrestore(&async_file->ev_queue.lock, flags);
- wake_up_interruptible(&file->async_file->ev_queue.poll_wait);
- kill_fasync(&file->async_file->ev_queue.async_queue, SIGIO, POLL_IN);
+ wake_up_interruptible(&async_file->ev_queue.poll_wait);
+ kill_fasync(&async_file->ev_queue.async_queue, SIGIO, POLL_IN);
}
-void ib_uverbs_cq_event_handler(struct ib_event *event, void *context_ptr)
+static void uverbs_uobj_event(struct ib_uevent_object *eobj,
+ struct ib_event *event)
{
- struct ib_uevent_object *uobj = &event->element.cq->uobject->uevent;
+ ib_uverbs_async_handler(eobj->uobject.ufile->async_file,
+ eobj->uobject.user_handle, event->event,
+ &eobj->event_list, &eobj->events_reported);
+}
- ib_uverbs_async_handler(uobj->uobject.ufile, uobj->uobject.user_handle,
- event->event, &uobj->event_list,
- &uobj->events_reported);
+void ib_uverbs_cq_event_handler(struct ib_event *event, void *context_ptr)
+{
+ uverbs_uobj_event(&event->element.cq->uobject->uevent, event);
}
void ib_uverbs_qp_event_handler(struct ib_event *event, void *context_ptr)
{
- struct ib_uevent_object *uobj;
-
/* for XRC target qp's, check that qp is live */
if (!event->element.qp->uobject)
return;
- uobj = &event->element.qp->uobject->uevent;
-
- ib_uverbs_async_handler(context_ptr, uobj->uobject.user_handle,
- event->event, &uobj->event_list,
- &uobj->events_reported);
+ uverbs_uobj_event(&event->element.qp->uobject->uevent, event);
}
void ib_uverbs_wq_event_handler(struct ib_event *event, void *context_ptr)
{
- struct ib_uevent_object *uobj = &event->element.wq->uobject->uevent;
-
- ib_uverbs_async_handler(context_ptr, uobj->uobject.user_handle,
- event->event, &uobj->event_list,
- &uobj->events_reported);
+ uverbs_uobj_event(&event->element.wq->uobject->uevent, event);
}
void ib_uverbs_srq_event_handler(struct ib_event *event, void *context_ptr)
{
- struct ib_uevent_object *uobj;
-
- uobj = &event->element.srq->uobject->uevent;
-
- ib_uverbs_async_handler(context_ptr, uobj->uobject.user_handle,
- event->event, &uobj->event_list,
- &uobj->events_reported);
+ uverbs_uobj_event(&event->element.srq->uobject->uevent, event);
}
-void ib_uverbs_event_handler(struct ib_event_handler *handler,
- struct ib_event *event)
+static void ib_uverbs_event_handler(struct ib_event_handler *handler,
+ struct ib_event *event)
{
- struct ib_uverbs_file *file =
- container_of(handler, struct ib_uverbs_file, event_handler);
-
- ib_uverbs_async_handler(file, event->element.port_num, event->event,
- NULL, NULL);
+ ib_uverbs_async_handler(
+ container_of(handler, struct ib_uverbs_file, event_handler)
+ ->async_file,
+ event->element.port_num, event->event, NULL, NULL);
}
void ib_uverbs_free_async_event_file(struct ib_uverbs_file *file)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 11/14] RDMA/core: Fix locking in ib_uverbs_event_read
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (9 preceding siblings ...)
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 ` 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
` (3 subsequent siblings)
14 siblings, 1 reply; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:22 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This should not be using ib_dev to test for disassociation, during
disassociation is_closed is set under lock and the waitq is triggered.
Instead check is_closed and be sure to re-obtain the lock to test the
value after the wait_event returns.
Fixes: 036b10635739 ("IB/uverbs: Enable device removal when there are active user space applications")
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/uverbs_main.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 85dbd81..97770e7 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -215,7 +215,6 @@ void ib_uverbs_release_file(struct kref *ref)
}
static ssize_t ib_uverbs_event_read(struct ib_uverbs_event_queue *ev_queue,
- struct ib_uverbs_file *uverbs_file,
struct file *filp, char __user *buf,
size_t count, loff_t *pos,
size_t eventsz)
@@ -233,19 +232,16 @@ static ssize_t ib_uverbs_event_read(struct ib_uverbs_event_queue *ev_queue,
if (wait_event_interruptible(ev_queue->poll_wait,
(!list_empty(&ev_queue->event_list) ||
- /* The barriers built into wait_event_interruptible()
- * and wake_up() guarentee this will see the null set
- * without using RCU
- */
- !uverbs_file->device->ib_dev)))
+ ev_queue->is_closed)))
return -ERESTARTSYS;
+ spin_lock_irq(&ev_queue->lock);
+
/* If device was disassociated and no event exists set an error */
- if (list_empty(&ev_queue->event_list) &&
- !uverbs_file->device->ib_dev)
+ if (list_empty(&ev_queue->event_list) && ev_queue->is_closed) {
+ spin_unlock_irq(&ev_queue->lock);
return -EIO;
-
- spin_lock_irq(&ev_queue->lock);
+ }
}
event = list_entry(ev_queue->event_list.next, struct ib_uverbs_event, list);
@@ -280,8 +276,7 @@ static ssize_t ib_uverbs_async_event_read(struct file *filp, char __user *buf,
{
struct ib_uverbs_async_event_file *file = filp->private_data;
- return ib_uverbs_event_read(&file->ev_queue, file->uverbs_file, filp,
- buf, count, pos,
+ return ib_uverbs_event_read(&file->ev_queue, filp, buf, count, pos,
sizeof(struct ib_uverbs_async_event_desc));
}
@@ -291,9 +286,8 @@ static ssize_t ib_uverbs_comp_event_read(struct file *filp, char __user *buf,
struct ib_uverbs_completion_event_file *comp_ev_file =
filp->private_data;
- return ib_uverbs_event_read(&comp_ev_file->ev_queue,
- comp_ev_file->uobj.ufile, filp,
- buf, count, pos,
+ return ib_uverbs_event_read(&comp_ev_file->ev_queue, filp, buf, count,
+ pos,
sizeof(struct ib_uverbs_comp_event_desc));
}
@@ -316,7 +310,9 @@ static __poll_t ib_uverbs_event_poll(struct ib_uverbs_event_queue *ev_queue,
static __poll_t ib_uverbs_async_event_poll(struct file *filp,
struct poll_table_struct *wait)
{
- return ib_uverbs_event_poll(filp->private_data, filp, wait);
+ struct ib_uverbs_async_event_file *file = filp->private_data;
+
+ return ib_uverbs_event_poll(&file->ev_queue, filp, wait);
}
static __poll_t ib_uverbs_comp_event_poll(struct file *filp,
@@ -330,9 +326,9 @@ static __poll_t ib_uverbs_comp_event_poll(struct file *filp,
static int ib_uverbs_async_event_fasync(int fd, struct file *filp, int on)
{
- struct ib_uverbs_event_queue *ev_queue = filp->private_data;
+ struct ib_uverbs_async_event_file *file = filp->private_data;
- return fasync_helper(fd, filp, on, &ev_queue->async_queue);
+ return fasync_helper(fd, filp, on, &file->ev_queue.async_queue);
}
static int ib_uverbs_comp_event_fasync(int fd, struct file *filp, int on)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH rdma-next 11/14] RDMA/core: Fix locking in ib_uverbs_event_read
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
0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2020-01-10 14:54 UTC (permalink / raw)
To: Yishai Hadas; +Cc: linux-rdma, dledford, maorg, michaelgur
On Wed, Jan 08, 2020 at 07:22:03PM +0200, Yishai Hadas wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> This should not be using ib_dev to test for disassociation, during
> disassociation is_closed is set under lock and the waitq is triggered.
>
> Instead check is_closed and be sure to re-obtain the lock to test the
> value after the wait_event returns.
>
> Fixes: 036b10635739 ("IB/uverbs: Enable device removal when there are active user space applications")
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
> drivers/infiniband/core/uverbs_main.c | 32 ++++++++++++++------------------
> 1 file changed, 14 insertions(+), 18 deletions(-)
Applied to for-next instead of the version from Hakon
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH rdma-next 12/14] RDMA/core: Remove the ufile arg from rdma_alloc_begin_uobject
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (10 preceding siblings ...)
2020-01-08 17:22 ` [PATCH rdma-next 11/14] RDMA/core: Fix locking in ib_uverbs_event_read Yishai Hadas
@ 2020-01-08 17:22 ` 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
` (2 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:22 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
Now that all callers provide a non-NULL attrs the ufile is redundant.
Adjust things so that the context handling is done inside alloc_uobj,
and the ib_uverbs_get_ucontext_file() is avoided if we already have the
context.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/rdma_core.c | 36 +++++++++++++++++++-----------------
include/rdma/uverbs_std_types.h | 3 +--
include/rdma/uverbs_types.h | 3 +--
3 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 8ad6883..f839b93 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -262,15 +262,20 @@ int __uobj_perform_destroy(const struct uverbs_api_object *obj, u32 id,
}
/* alloc_uobj must be undone by uverbs_destroy_uobject() */
-static struct ib_uobject *alloc_uobj(struct ib_uverbs_file *ufile,
+static struct ib_uobject *alloc_uobj(struct uverbs_attr_bundle *attrs,
const struct uverbs_api_object *obj)
{
+ struct ib_uverbs_file *ufile = attrs->ufile;
struct ib_uobject *uobj;
- struct ib_ucontext *ucontext;
- ucontext = ib_uverbs_get_ucontext_file(ufile);
- if (IS_ERR(ucontext))
- return ERR_CAST(ucontext);
+ if (!attrs->context) {
+ struct ib_ucontext *ucontext =
+ ib_uverbs_get_ucontext_file(ufile);
+
+ if (IS_ERR(ucontext))
+ return ERR_CAST(ucontext);
+ attrs->context = ucontext;
+ }
uobj = kzalloc(obj->type_attrs->obj_size, GFP_KERNEL);
if (!uobj)
@@ -280,7 +285,7 @@ static struct ib_uobject *alloc_uobj(struct ib_uverbs_file *ufile,
* The object is added to the list in the commit stage.
*/
uobj->ufile = ufile;
- uobj->context = ucontext;
+ uobj->context = attrs->context;
INIT_LIST_HEAD(&uobj->list);
uobj->uapi_object = obj;
/*
@@ -423,12 +428,12 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
static struct ib_uobject *
alloc_begin_idr_uobject(const struct uverbs_api_object *obj,
- struct ib_uverbs_file *ufile)
+ struct uverbs_attr_bundle *attrs)
{
int ret;
struct ib_uobject *uobj;
- uobj = alloc_uobj(ufile, obj);
+ uobj = alloc_uobj(attrs, obj);
if (IS_ERR(uobj))
return uobj;
@@ -444,7 +449,7 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
return uobj;
remove:
- xa_erase(&ufile->idr, uobj->id);
+ xa_erase(&attrs->ufile->idr, uobj->id);
uobj_put:
uverbs_uobject_put(uobj);
return ERR_PTR(ret);
@@ -452,7 +457,7 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
static struct ib_uobject *
alloc_begin_fd_uobject(const struct uverbs_api_object *obj,
- struct ib_uverbs_file *ufile)
+ struct uverbs_attr_bundle *attrs)
{
const struct uverbs_obj_fd_type *fd_type =
container_of(obj->type_attrs, struct uverbs_obj_fd_type, type);
@@ -467,7 +472,7 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
if (new_fd < 0)
return ERR_PTR(new_fd);
- uobj = alloc_uobj(ufile, obj);
+ uobj = alloc_uobj(attrs, obj);
if (IS_ERR(uobj))
goto err_fd;
@@ -481,7 +486,6 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
uobj->object = filp;
uobj->id = new_fd;
- uobj->ufile = ufile;
return uobj;
err_uobj:
@@ -492,9 +496,9 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
}
struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_api_object *obj,
- struct ib_uverbs_file *ufile,
struct uverbs_attr_bundle *attrs)
{
+ struct ib_uverbs_file *ufile = attrs->ufile;
struct ib_uobject *ret;
if (IS_ERR(obj))
@@ -508,13 +512,11 @@ struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_api_object *obj,
if (!down_read_trylock(&ufile->hw_destroy_rwsem))
return ERR_PTR(-EIO);
- ret = obj->type_class->alloc_begin(obj, ufile);
+ ret = obj->type_class->alloc_begin(obj, attrs);
if (IS_ERR(ret)) {
up_read(&ufile->hw_destroy_rwsem);
return ret;
}
- if (attrs)
- attrs->context = ret->context;
return ret;
}
@@ -949,7 +951,7 @@ struct ib_uobject *
return rdma_lookup_get_uobject(obj, attrs->ufile, id,
UVERBS_LOOKUP_WRITE, attrs);
case UVERBS_ACCESS_NEW:
- return rdma_alloc_begin_uobject(obj, attrs->ufile, attrs);
+ return rdma_alloc_begin_uobject(obj, attrs);
default:
WARN_ON(true);
return ERR_PTR(-EOPNOTSUPP);
diff --git a/include/rdma/uverbs_std_types.h b/include/rdma/uverbs_std_types.h
index c6bcaad..1b28ce1 100644
--- a/include/rdma/uverbs_std_types.h
+++ b/include/rdma/uverbs_std_types.h
@@ -114,8 +114,7 @@ static inline void uobj_alloc_abort(struct ib_uobject *uobj,
__uobj_alloc(const struct uverbs_api_object *obj,
struct uverbs_attr_bundle *attrs, struct ib_device **ib_dev)
{
- struct ib_uobject *uobj =
- rdma_alloc_begin_uobject(obj, attrs->ufile, attrs);
+ struct ib_uobject *uobj = rdma_alloc_begin_uobject(obj, attrs);
if (!IS_ERR(uobj))
*ib_dev = attrs->context->device;
diff --git a/include/rdma/uverbs_types.h b/include/rdma/uverbs_types.h
index 0532588..ded608b 100644
--- a/include/rdma/uverbs_types.h
+++ b/include/rdma/uverbs_types.h
@@ -83,7 +83,7 @@ enum rdma_lookup_mode {
*/
struct uverbs_obj_type_class {
struct ib_uobject *(*alloc_begin)(const struct uverbs_api_object *obj,
- struct ib_uverbs_file *ufile);
+ struct uverbs_attr_bundle *attrs);
/* This consumes the kref on uobj */
void (*alloc_commit)(struct ib_uobject *uobj);
/* This does not consume the kref on uobj */
@@ -138,7 +138,6 @@ struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
void rdma_lookup_put_uobject(struct ib_uobject *uobj,
enum rdma_lookup_mode mode);
struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_api_object *obj,
- struct ib_uverbs_file *ufile,
struct uverbs_attr_bundle *attrs);
void rdma_alloc_abort_uobject(struct ib_uobject *uobj,
struct uverbs_attr_bundle *attrs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 13/14] RDMA/core: Make ib_uverbs_async_event_file into a uobject
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (11 preceding siblings ...)
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 ` 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
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:22 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
This makes async events aligned with completion events as both are full
uobjects of FD type and use the same uobject lifecycle.
A bunch of duplicate code is consolidated and the general flow between the
two FDs is now very similar.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/Makefile | 3 +-
drivers/infiniband/core/rdma_core.h | 1 +
drivers/infiniband/core/uverbs.h | 15 +--
drivers/infiniband/core/uverbs_cmd.c | 30 ++---
drivers/infiniband/core/uverbs_main.c | 130 ++++-----------------
drivers/infiniband/core/uverbs_std_types.c | 25 ++--
.../infiniband/core/uverbs_std_types_async_fd.c | 33 ++++++
drivers/infiniband/core/uverbs_uapi.c | 1 +
include/uapi/rdma/ib_user_ioctl_cmds.h | 1 +
9 files changed, 96 insertions(+), 143 deletions(-)
create mode 100644 drivers/infiniband/core/uverbs_std_types_async_fd.c
diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index 9a8871e..e109c82 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -33,6 +33,7 @@ ib_uverbs-y := uverbs_main.o uverbs_cmd.o uverbs_marshall.o \
uverbs_std_types_cq.o \
uverbs_std_types_flow_action.o uverbs_std_types_dm.o \
uverbs_std_types_mr.o uverbs_std_types_counters.o \
- uverbs_uapi.o uverbs_std_types_device.o
+ uverbs_uapi.o uverbs_std_types_device.o \
+ uverbs_std_types_async_fd.o
ib_uverbs-$(CONFIG_INFINIBAND_USER_MEM) += umem.o
ib_uverbs-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o
diff --git a/drivers/infiniband/core/rdma_core.h b/drivers/infiniband/core/rdma_core.h
index 29f905e..33978e0 100644
--- a/drivers/infiniband/core/rdma_core.h
+++ b/drivers/infiniband/core/rdma_core.h
@@ -151,6 +151,7 @@ void uapi_compute_bundle_size(struct uverbs_api_ioctl_method *method_elm,
unsigned int num_attrs);
void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile);
+extern const struct uapi_definition uverbs_def_obj_async_fd[];
extern const struct uapi_definition uverbs_def_obj_counters[];
extern const struct uapi_definition uverbs_def_obj_cq[];
extern const struct uapi_definition uverbs_def_obj_device[];
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 8384b66..ccde5d2 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -111,7 +111,6 @@ struct ib_uverbs_device {
struct srcu_struct disassociate_srcu;
struct mutex lists_mutex; /* protect lists */
struct list_head uverbs_file_list;
- struct list_head uverbs_events_file_list;
struct uverbs_api *uapi;
};
@@ -124,10 +123,9 @@ struct ib_uverbs_event_queue {
};
struct ib_uverbs_async_event_file {
+ struct ib_uobject uobj;
struct ib_uverbs_event_queue ev_queue;
- struct ib_uverbs_file *uverbs_file;
- struct kref ref;
- struct list_head list;
+ struct ib_event_handler event_handler;
};
struct ib_uverbs_completion_event_file {
@@ -144,8 +142,7 @@ struct ib_uverbs_file {
* ucontext_lock held
*/
struct ib_ucontext *ucontext;
- struct ib_event_handler event_handler;
- struct ib_uverbs_async_event_file *async_file;
+ struct ib_uverbs_async_event_file *async_file;
struct list_head list;
/*
@@ -217,10 +214,10 @@ struct ib_ucq_object {
};
extern const struct file_operations uverbs_event_fops;
+extern const struct file_operations uverbs_async_event_fops;
void ib_uverbs_init_event_queue(struct ib_uverbs_event_queue *ev_queue);
-struct file *ib_uverbs_alloc_async_event_file(struct ib_uverbs_file *uverbs_file,
- struct ib_device *ib_dev);
-void ib_uverbs_free_async_event_file(struct ib_uverbs_file *uverbs_file);
+void ib_uverbs_init_async_event_file(struct ib_uverbs_async_event_file *ev_file);
+void ib_uverbs_free_event_queue(struct ib_uverbs_event_queue *event_queue);
void ib_uverbs_flow_resources_free(struct ib_uflow_resources *uflow_res);
void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 51117e7..ced1384 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -209,9 +209,9 @@ static int ib_uverbs_get_context(struct uverbs_attr_bundle *attrs)
struct ib_uverbs_get_context cmd;
struct ib_uverbs_get_context_resp resp;
struct ib_ucontext *ucontext;
- struct file *filp;
struct ib_rdmacg_object cg_obj;
struct ib_device *ib_dev;
+ struct ib_uobject *uobj;
int ret;
ret = uverbs_request(attrs, &cmd, sizeof(cmd));
@@ -254,30 +254,28 @@ static int ib_uverbs_get_context(struct uverbs_attr_bundle *attrs)
xa_init_flags(&ucontext->mmap_xa, XA_FLAGS_ALLOC);
- ret = get_unused_fd_flags(O_CLOEXEC);
- if (ret < 0)
+ uobj = uobj_alloc(UVERBS_OBJECT_ASYNC_EVENT, attrs, &ib_dev);
+ if (IS_ERR(uobj)) {
+ ret = PTR_ERR(uobj);
goto err_free;
- resp.async_fd = ret;
-
- filp = ib_uverbs_alloc_async_event_file(file, ib_dev);
- if (IS_ERR(filp)) {
- ret = PTR_ERR(filp);
- goto err_fd;
}
+ resp.async_fd = uobj->id;
resp.num_comp_vectors = file->device->num_comp_vectors;
ret = uverbs_response(attrs, &resp, sizeof(resp));
if (ret)
- goto err_file;
+ goto err_uobj;
ret = ib_dev->ops.alloc_ucontext(ucontext, &attrs->driver_udata);
if (ret)
- goto err_file;
+ goto err_uobj;
rdma_restrack_uadd(&ucontext->res);
- fd_install(resp.async_fd, filp);
+ ib_uverbs_init_async_event_file(
+ container_of(uobj, struct ib_uverbs_async_event_file, uobj));
+ rdma_alloc_commit_uobject(uobj, attrs);
/*
* Make sure that ib_uverbs_get_ucontext() sees the pointer update
@@ -289,12 +287,8 @@ static int ib_uverbs_get_context(struct uverbs_attr_bundle *attrs)
return 0;
-err_file:
- ib_uverbs_free_async_event_file(file);
- fput(filp);
-
-err_fd:
- put_unused_fd(resp.async_fd);
+err_uobj:
+ rdma_alloc_abort_uobject(uobj, attrs);
err_free:
kfree(ucontext);
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 97770e7..121e65f 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -125,14 +125,6 @@ static void ib_uverbs_release_dev(struct device *device)
kfree(dev);
}
-static void ib_uverbs_release_async_event_file(struct kref *ref)
-{
- struct ib_uverbs_async_event_file *file =
- container_of(ref, struct ib_uverbs_async_event_file, ref);
-
- kfree(file);
-}
-
void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
struct ib_uverbs_completion_event_file *ev_file,
struct ib_ucq_object *uobj)
@@ -203,8 +195,7 @@ void ib_uverbs_release_file(struct kref *ref)
ib_uverbs_comp_dev(file->device);
if (file->async_file)
- kref_put(&file->async_file->ref,
- ib_uverbs_release_async_event_file);
+ uverbs_uobject_put(&file->async_file->uobj);
put_device(&file->device->dev);
if (file->disassociate_page)
@@ -339,35 +330,6 @@ static int ib_uverbs_comp_event_fasync(int fd, struct file *filp, int on)
return fasync_helper(fd, filp, on, &comp_ev_file->ev_queue.async_queue);
}
-static int ib_uverbs_async_event_close(struct inode *inode, struct file *filp)
-{
- struct ib_uverbs_async_event_file *file = filp->private_data;
- struct ib_uverbs_file *uverbs_file = file->uverbs_file;
- struct ib_uverbs_event *entry, *tmp;
- int closed_already = 0;
-
- mutex_lock(&uverbs_file->device->lists_mutex);
- spin_lock_irq(&file->ev_queue.lock);
- closed_already = file->ev_queue.is_closed;
- file->ev_queue.is_closed = 1;
- list_for_each_entry_safe(entry, tmp, &file->ev_queue.event_list, list) {
- if (entry->counter)
- list_del(&entry->obj_list);
- kfree(entry);
- }
- spin_unlock_irq(&file->ev_queue.lock);
- if (!closed_already) {
- list_del(&file->list);
- ib_unregister_event_handler(&uverbs_file->event_handler);
- }
- mutex_unlock(&uverbs_file->device->lists_mutex);
-
- kref_put(&uverbs_file->ref, ib_uverbs_release_file);
- kref_put(&file->ref, ib_uverbs_release_async_event_file);
-
- return 0;
-}
-
const struct file_operations uverbs_event_fops = {
.owner = THIS_MODULE,
.read = ib_uverbs_comp_event_read,
@@ -377,11 +339,11 @@ static int ib_uverbs_async_event_close(struct inode *inode, struct file *filp)
.llseek = no_llseek,
};
-static const struct file_operations uverbs_async_event_fops = {
+const struct file_operations uverbs_async_event_fops = {
.owner = THIS_MODULE,
.read = ib_uverbs_async_event_read,
.poll = ib_uverbs_async_event_poll,
- .release = ib_uverbs_async_event_close,
+ .release = uverbs_uobject_fd_release,
.fasync = ib_uverbs_async_event_fasync,
.llseek = no_llseek,
};
@@ -491,17 +453,11 @@ static void ib_uverbs_event_handler(struct ib_event_handler *handler,
struct ib_event *event)
{
ib_uverbs_async_handler(
- container_of(handler, struct ib_uverbs_file, event_handler)
- ->async_file,
+ container_of(handler, struct ib_uverbs_async_event_file,
+ event_handler),
event->element.port_num, event->event, NULL, NULL);
}
-void ib_uverbs_free_async_event_file(struct ib_uverbs_file *file)
-{
- kref_put(&file->async_file->ref, ib_uverbs_release_async_event_file);
- file->async_file = NULL;
-}
-
void ib_uverbs_init_event_queue(struct ib_uverbs_event_queue *ev_queue)
{
spin_lock_init(&ev_queue->lock);
@@ -511,45 +467,23 @@ void ib_uverbs_init_event_queue(struct ib_uverbs_event_queue *ev_queue)
ev_queue->async_queue = NULL;
}
-struct file *ib_uverbs_alloc_async_event_file(struct ib_uverbs_file *uverbs_file,
- struct ib_device *ib_dev)
+void ib_uverbs_init_async_event_file(
+ struct ib_uverbs_async_event_file *async_file)
{
- struct ib_uverbs_async_event_file *ev_file;
- struct file *filp;
-
- ev_file = kzalloc(sizeof(*ev_file), GFP_KERNEL);
- if (!ev_file)
- return ERR_PTR(-ENOMEM);
-
- ib_uverbs_init_event_queue(&ev_file->ev_queue);
- ev_file->uverbs_file = uverbs_file;
- kref_get(&ev_file->uverbs_file->ref);
- kref_init(&ev_file->ref);
- filp = anon_inode_getfile("[infinibandevent]", &uverbs_async_event_fops,
- ev_file, O_RDONLY);
- if (IS_ERR(filp))
- goto err_put_refs;
-
- mutex_lock(&uverbs_file->device->lists_mutex);
- list_add_tail(&ev_file->list,
- &uverbs_file->device->uverbs_events_file_list);
- mutex_unlock(&uverbs_file->device->lists_mutex);
-
- WARN_ON(uverbs_file->async_file);
- uverbs_file->async_file = ev_file;
- kref_get(&uverbs_file->async_file->ref);
- INIT_IB_EVENT_HANDLER(&uverbs_file->event_handler,
- ib_dev,
- ib_uverbs_event_handler);
- ib_register_event_handler(&uverbs_file->event_handler);
- /* At that point async file stuff was fully set */
+ struct ib_uverbs_file *uverbs_file = async_file->uobj.ufile;
+ struct ib_device *ib_dev = async_file->uobj.context->device;
- return filp;
+ ib_uverbs_init_event_queue(&async_file->ev_queue);
-err_put_refs:
- kref_put(&ev_file->uverbs_file->ref, ib_uverbs_release_file);
- kref_put(&ev_file->ref, ib_uverbs_release_async_event_file);
- return filp;
+ if (!WARN_ON(uverbs_file->async_file)) {
+ uverbs_file->async_file = async_file;
+ /* Pairs with the put in ib_uverbs_release_file */
+ uverbs_uobject_get(&async_file->uobj);
+ }
+
+ INIT_IB_EVENT_HANDLER(&async_file->event_handler, ib_dev,
+ ib_uverbs_event_handler);
+ ib_register_event_handler(&async_file->event_handler);
}
static ssize_t verify_hdr(struct ib_uverbs_cmd_hdr *hdr,
@@ -1178,7 +1112,6 @@ static void ib_uverbs_add_one(struct ib_device *device)
mutex_init(&uverbs_dev->xrcd_tree_mutex);
mutex_init(&uverbs_dev->lists_mutex);
INIT_LIST_HEAD(&uverbs_dev->uverbs_file_list);
- INIT_LIST_HEAD(&uverbs_dev->uverbs_events_file_list);
rcu_assign_pointer(uverbs_dev->ib_dev, device);
uverbs_dev->num_comp_vectors = device->num_comp_vectors;
@@ -1223,7 +1156,6 @@ static void ib_uverbs_free_hw_resources(struct ib_uverbs_device *uverbs_dev,
struct ib_device *ib_dev)
{
struct ib_uverbs_file *file;
- struct ib_uverbs_async_event_file *event_file;
struct ib_event event;
/* Pending running commands to terminate */
@@ -1246,31 +1178,15 @@ static void ib_uverbs_free_hw_resources(struct ib_uverbs_device *uverbs_dev,
*/
mutex_unlock(&uverbs_dev->lists_mutex);
- ib_uverbs_event_handler(&file->event_handler, &event);
+ if (file->async_file)
+ ib_uverbs_event_handler(
+ &file->async_file->event_handler, &event);
+
uverbs_destroy_ufile_hw(file, RDMA_REMOVE_DRIVER_REMOVE);
kref_put(&file->ref, ib_uverbs_release_file);
mutex_lock(&uverbs_dev->lists_mutex);
}
-
- while (!list_empty(&uverbs_dev->uverbs_events_file_list)) {
- event_file = list_first_entry(&uverbs_dev->
- uverbs_events_file_list,
- struct ib_uverbs_async_event_file,
- list);
- spin_lock_irq(&event_file->ev_queue.lock);
- event_file->ev_queue.is_closed = 1;
- spin_unlock_irq(&event_file->ev_queue.lock);
-
- list_del(&event_file->list);
- ib_unregister_event_handler(
- &event_file->uverbs_file->event_handler);
- event_file->uverbs_file->event_handler.device =
- NULL;
-
- wake_up_interruptible(&event_file->ev_queue.poll_wait);
- kill_fasync(&event_file->ev_queue.async_queue, SIGIO, POLL_IN);
- }
mutex_unlock(&uverbs_dev->lists_mutex);
uverbs_disassociate_api(uverbs_dev->uapi);
diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index def038a..efe70bc 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -202,17 +202,15 @@ static int uverbs_free_pd(struct ib_uobject *uobject,
return 0;
}
-static int
-uverbs_completion_event_file_destroy_uobj(struct ib_uobject *uobj,
- enum rdma_remove_reason why)
+void ib_uverbs_free_event_queue(struct ib_uverbs_event_queue *event_queue)
{
- struct ib_uverbs_completion_event_file *file =
- container_of(uobj, struct ib_uverbs_completion_event_file,
- uobj);
- struct ib_uverbs_event_queue *event_queue = &file->ev_queue;
struct ib_uverbs_event *entry, *tmp;
spin_lock_irq(&event_queue->lock);
+ /*
+ * The user must ensure that no new items are added to the event_list
+ * once is_closed is set.
+ */
event_queue->is_closed = 1;
spin_unlock_irq(&event_queue->lock);
wake_up_interruptible(&event_queue->poll_wait);
@@ -225,8 +223,19 @@ static int uverbs_free_pd(struct ib_uobject *uobject,
kfree(entry);
}
spin_unlock_irq(&event_queue->lock);
+}
+
+static int
+uverbs_completion_event_file_destroy_uobj(struct ib_uobject *uobj,
+ enum rdma_remove_reason why)
+{
+ struct ib_uverbs_completion_event_file *file =
+ container_of(uobj, struct ib_uverbs_completion_event_file,
+ uobj);
+
+ ib_uverbs_free_event_queue(&file->ev_queue);
return 0;
-};
+}
int uverbs_destroy_def_handler(struct uverbs_attr_bundle *attrs)
{
diff --git a/drivers/infiniband/core/uverbs_std_types_async_fd.c b/drivers/infiniband/core/uverbs_std_types_async_fd.c
new file mode 100644
index 0000000..31ff968
--- /dev/null
+++ b/drivers/infiniband/core/uverbs_std_types_async_fd.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/*
+ * Copyright (c) 2019, Mellanox Technologies inc. All rights reserved.
+ */
+
+#include <rdma/uverbs_std_types.h>
+#include <rdma/uverbs_ioctl.h>
+#include "rdma_core.h"
+#include "uverbs.h"
+
+static int uverbs_async_event_destroy_uobj(struct ib_uobject *uobj,
+ enum rdma_remove_reason why)
+{
+ struct ib_uverbs_async_event_file *event_file =
+ container_of(uobj, struct ib_uverbs_async_event_file, uobj);
+
+ ib_unregister_event_handler(&event_file->event_handler);
+ ib_uverbs_free_event_queue(&event_file->ev_queue);
+ return 0;
+}
+
+DECLARE_UVERBS_NAMED_OBJECT(
+ UVERBS_OBJECT_ASYNC_EVENT,
+ UVERBS_TYPE_ALLOC_FD(sizeof(struct ib_uverbs_async_event_file),
+ uverbs_async_event_destroy_uobj,
+ &uverbs_async_event_fops,
+ "[infinibandevent]",
+ O_RDONLY));
+
+const struct uapi_definition uverbs_def_obj_async_fd[] = {
+ UAPI_DEF_CHAIN_OBJ_TREE_NAMED(UVERBS_OBJECT_ASYNC_EVENT),
+ {}
+};
diff --git a/drivers/infiniband/core/uverbs_uapi.c b/drivers/infiniband/core/uverbs_uapi.c
index 9b84a12..3f121ac 100644
--- a/drivers/infiniband/core/uverbs_uapi.c
+++ b/drivers/infiniband/core/uverbs_uapi.c
@@ -626,6 +626,7 @@ void uverbs_destroy_api(struct uverbs_api *uapi)
}
static const struct uapi_definition uverbs_core_api[] = {
+ UAPI_DEF_CHAIN(uverbs_def_obj_async_fd),
UAPI_DEF_CHAIN(uverbs_def_obj_counters),
UAPI_DEF_CHAIN(uverbs_def_obj_cq),
UAPI_DEF_CHAIN(uverbs_def_obj_device),
diff --git a/include/uapi/rdma/ib_user_ioctl_cmds.h b/include/uapi/rdma/ib_user_ioctl_cmds.h
index 64f0e3a..9cfadb5 100644
--- a/include/uapi/rdma/ib_user_ioctl_cmds.h
+++ b/include/uapi/rdma/ib_user_ioctl_cmds.h
@@ -56,6 +56,7 @@ enum uverbs_default_objects {
UVERBS_OBJECT_FLOW_ACTION,
UVERBS_OBJECT_DM,
UVERBS_OBJECT_COUNTERS,
+ UVERBS_OBJECT_ASYNC_EVENT,
};
enum {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH rdma-next 14/14] RDMA/core: Use READ_ONCE for ib_ufile.async_file
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (12 preceding siblings ...)
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 ` Yishai Hadas
2020-01-13 20:37 ` [PATCH rdma-next 00/14] Refactoring FD usage Jason Gunthorpe
14 siblings, 0 replies; 17+ messages in thread
From: Yishai Hadas @ 2020-01-08 17:22 UTC (permalink / raw)
To: linux-rdma, jgg, dledford; +Cc: yishaih, maorg, michaelgur
From: Jason Gunthorpe <jgg@mellanox.com>
The writer for async_file holds the ucontext_lock, while the readers are
left unlocked. Most readers rely on an implicit locking, either by having
a uobject (which cannot be created before a context) or by holding the
ib_ufile kref.
However ib_uverbs_free_hw_resources() has no implicit lock and has a
possible race. Make this all clear and sane by using READ_ONCE
consistently.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
drivers/infiniband/core/uverbs.h | 6 ++----
drivers/infiniband/core/uverbs_cmd.c | 2 +-
drivers/infiniband/core/uverbs_main.c | 29 +++++++++++----------------
drivers/infiniband/core/uverbs_std_types.c | 6 +++---
drivers/infiniband/core/uverbs_std_types_cq.c | 1 -
5 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index ccde5d2..aaa5c75 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -220,11 +220,9 @@ struct ib_ucq_object {
void ib_uverbs_free_event_queue(struct ib_uverbs_event_queue *event_queue);
void ib_uverbs_flow_resources_free(struct ib_uflow_resources *uflow_res);
-void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
- struct ib_uverbs_completion_event_file *ev_file,
+void ib_uverbs_release_ucq(struct ib_uverbs_completion_event_file *ev_file,
struct ib_ucq_object *uobj);
-void ib_uverbs_release_uevent(struct ib_uverbs_file *file,
- struct ib_uevent_object *uobj);
+void ib_uverbs_release_uevent(struct ib_uevent_object *uobj);
void ib_uverbs_release_file(struct kref *ref);
void ib_uverbs_comp_handler(struct ib_cq *cq, void *cq_context);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index ced1384..29b1b5a 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1056,7 +1056,7 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
kfree(cq);
err_file:
if (ev_file)
- ib_uverbs_release_ucq(attrs->ufile, ev_file, obj);
+ ib_uverbs_release_ucq(ev_file, obj);
err:
uobj_alloc_abort(&obj->uevent.uobject, attrs);
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 121e65f..1f279b0a 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -125,9 +125,8 @@ static void ib_uverbs_release_dev(struct device *device)
kfree(dev);
}
-void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
- struct ib_uverbs_completion_event_file *ev_file,
- struct ib_ucq_object *uobj)
+void ib_uverbs_release_ucq(struct ib_uverbs_completion_event_file *ev_file,
+ struct ib_ucq_object *uobj)
{
struct ib_uverbs_event *evt, *tmp;
@@ -142,20 +141,21 @@ void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
uverbs_uobject_put(&ev_file->uobj);
}
- ib_uverbs_release_uevent(file, &uobj->uevent);
+ ib_uverbs_release_uevent(&uobj->uevent);
}
-void ib_uverbs_release_uevent(struct ib_uverbs_file *file,
- struct ib_uevent_object *uobj)
+void ib_uverbs_release_uevent(struct ib_uevent_object *uobj)
{
+ struct ib_uverbs_async_event_file *async_file =
+ READ_ONCE(uobj->uobject.ufile->async_file);
struct ib_uverbs_event *evt, *tmp;
- spin_lock_irq(&file->async_file->ev_queue.lock);
+ spin_lock_irq(&async_file->ev_queue.lock);
list_for_each_entry_safe(evt, tmp, &uobj->event_list, obj_list) {
list_del(&evt->list);
kfree(evt);
}
- spin_unlock_irq(&file->async_file->ev_queue.lock);
+ spin_unlock_irq(&async_file->ev_queue.lock);
}
void ib_uverbs_detach_umcast(struct ib_qp *qp,
@@ -420,7 +420,7 @@ void ib_uverbs_comp_handler(struct ib_cq *cq, void *cq_context)
static void uverbs_uobj_event(struct ib_uevent_object *eobj,
struct ib_event *event)
{
- ib_uverbs_async_handler(eobj->uobject.ufile->async_file,
+ ib_uverbs_async_handler(READ_ONCE(eobj->uobject.ufile->async_file),
eobj->uobject.user_handle, event->event,
&eobj->event_list, &eobj->events_reported);
}
@@ -476,9 +476,9 @@ void ib_uverbs_init_async_event_file(
ib_uverbs_init_event_queue(&async_file->ev_queue);
if (!WARN_ON(uverbs_file->async_file)) {
- uverbs_file->async_file = async_file;
/* Pairs with the put in ib_uverbs_release_file */
uverbs_uobject_get(&async_file->uobj);
+ smp_store_release(&uverbs_file->async_file, async_file);
}
INIT_IB_EVENT_HANDLER(&async_file->event_handler, ib_dev,
@@ -1156,13 +1156,9 @@ static void ib_uverbs_free_hw_resources(struct ib_uverbs_device *uverbs_dev,
struct ib_device *ib_dev)
{
struct ib_uverbs_file *file;
- struct ib_event event;
/* Pending running commands to terminate */
uverbs_disassociate_api_pre(uverbs_dev);
- event.event = IB_EVENT_DEVICE_FATAL;
- event.element.port_num = 0;
- event.device = ib_dev;
mutex_lock(&uverbs_dev->lists_mutex);
while (!list_empty(&uverbs_dev->uverbs_file_list)) {
@@ -1178,9 +1174,8 @@ static void ib_uverbs_free_hw_resources(struct ib_uverbs_device *uverbs_dev,
*/
mutex_unlock(&uverbs_dev->lists_mutex);
- if (file->async_file)
- ib_uverbs_event_handler(
- &file->async_file->event_handler, &event);
+ ib_uverbs_async_handler(READ_ONCE(file->async_file), 0,
+ IB_EVENT_DEVICE_FATAL, NULL, NULL);
uverbs_destroy_ufile_hw(file, RDMA_REMOVE_DRIVER_REMOVE);
kref_put(&file->ref, ib_uverbs_release_file);
diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index efe70bc..994d874 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -105,7 +105,7 @@ static int uverbs_free_qp(struct ib_uobject *uobject,
if (uqp->uxrcd)
atomic_dec(&uqp->uxrcd->refcnt);
- ib_uverbs_release_uevent(attrs->ufile, &uqp->uevent);
+ ib_uverbs_release_uevent(&uqp->uevent);
return ret;
}
@@ -138,7 +138,7 @@ static int uverbs_free_wq(struct ib_uobject *uobject,
if (ib_is_destroy_retryable(ret, why, uobject))
return ret;
- ib_uverbs_release_uevent(attrs->ufile, &uwq->uevent);
+ ib_uverbs_release_uevent(&uwq->uevent);
return ret;
}
@@ -163,7 +163,7 @@ static int uverbs_free_srq(struct ib_uobject *uobject,
atomic_dec(&us->uxrcd->refcnt);
}
- ib_uverbs_release_uevent(attrs->ufile, uevent);
+ ib_uverbs_release_uevent(uevent);
return ret;
}
diff --git a/drivers/infiniband/core/uverbs_std_types_cq.c b/drivers/infiniband/core/uverbs_std_types_cq.c
index a41c758..da4110a 100644
--- a/drivers/infiniband/core/uverbs_std_types_cq.c
+++ b/drivers/infiniband/core/uverbs_std_types_cq.c
@@ -49,7 +49,6 @@ static int uverbs_free_cq(struct ib_uobject *uobject,
return ret;
ib_uverbs_release_ucq(
- attrs->ufile,
ev_queue ? container_of(ev_queue,
struct ib_uverbs_completion_event_file,
ev_queue) :
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH rdma-next 00/14] Refactoring FD usage
2020-01-08 17:21 [PATCH rdma-next 00/14] Refactoring FD usage Yishai Hadas
` (13 preceding siblings ...)
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 ` Jason Gunthorpe
14 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2020-01-13 20:37 UTC (permalink / raw)
To: Yishai Hadas; +Cc: linux-rdma, dledford, maorg, michaelgur
On Wed, Jan 08, 2020 at 07:21:52PM +0200, Yishai Hadas wrote:
> This series refactors the usage of FDs in both IB core and mlx5 driver.
> It includes:
> - Simplify destruction of FD uobjects by making them pure uobjects and use
> a generic release method for all struct file operations.
> - Make ib_uverbs_async_event_file into a uobject.
> - Improve locking in few related areas.
> - Simplify type usage for ib_uverbs_async_handler().
>
> This refactoring series may be followed by some other series that will allow
> the async FD to be allocated separately from the context and then enables
> having the alloc_context command over ioctl.
>
> Yishai
>
> Jason Gunthorpe (14):
> RDMA/mlx5: Use RCU and direct refcounts to keep memory alive
> RDMA/core: Simplify destruction of FD uobjects
> RDMA/mlx5: Simplify devx async commands
> RDMA/core: Do not allow alloc_commit to fail
> RDMA/core: Make ib_ucq_object use ib_uevent_object
> RDMA/core: Do not erase the type of ib_cq.uobject
> RDMA/core: Do not erase the type of ib_qp.uobject
> RDMA/core: Do not erase the type of ib_srq.uobject
> RDMA/core: Do not erase the type of ib_wq.uobject
> RDMA/core: Simplify type usage for ib_uverbs_async_handler()
> RDMA/core: Fix locking in ib_uverbs_event_read
> RDMA/core: Remove the ufile arg from rdma_alloc_begin_uobject
> RDMA/core: Make ib_uverbs_async_event_file into a uobject
> RDMA/core: Use READ_ONCE for ib_ufile.async_file
Applied to for-next
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread