Fixes to IB_EVENT_DEVICE_FATAL logic

PATCH Applied Leon Romanovsky 2 participants 4 messages May 7, 2020 – May 12, 2020 InfiniBand

lore

Share & tools

lore

Leon Romanovsky

author

2020-05-07 06:33 UTC #1 lore raw

PATCH 1/2 RDMA/uverbs: Do not discard the IB_EVENT_DEVICE_FATAL event

From: Jason Gunthorpe <[email protected]>

The commit below moved all of the destruction to the disassociate step and cleaned up the event channel during destroy_uobj.

However, when ib_uverbs_free_hw_resources() pushes IB_EVENT_DEVICE_FATAL and then immediately goes to destroy all uobjects this causes ib_uverbs_free_event_queue() to discard the queued event if userspace hasn't already read() it.

Unlike all other event queues async FD needs to defer the ib_uverbs_free_event_queue() until FD release. This still unregisters the handler from the IB device during disassociation.

Fixes 3e032c0e92aa ("RDMA/core: Make ib_uverbs_async_event_file into a uobject")

Signed-off-by Yishai Hadas <[email protected]>

Signed-off-by Jason Gunthorpe <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/rdma_core.c           |  3 ++-
 drivers/infiniband/core/uverbs.h              |  1 +
 drivers/infiniband/core/uverbs_main.c         |  2 +-
 .../core/uverbs_std_types_async_fd.c          | 26 ++++++++++++++++++-
 4 files changed, 29 insertions(+), 3 deletions(-)

drivers/infiniband/core/rdma_core.c +2 -1

index 177333d8bcda..bf8e149d3191 100644
@@ -459,7 +459,8 @@ alloc_begin_fd_uobject(const struct uverbs_api_object *obj,
 	struct ib_uobject *uobj;
 	struct file *filp;
 
-	if (WARN_ON(fd_type->fops->release != &uverbs_uobject_fd_release))
+	if (WARN_ON(fd_type->fops->release != &uverbs_uobject_fd_release &&
+		    fd_type->fops->release != &uverbs_async_event_release))
 		return ERR_PTR(-EINVAL);
 
 	new_fd = get_unused_fd_flags(O_CLOEXEC);

drivers/infiniband/core/uverbs.h +1 -0

index 7df71983212d..2673cb1cd655 100644
@@ -219,6 +219,7 @@ void ib_uverbs_init_event_queue(struct ib_uverbs_event_queue *ev_queue);
 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);
+int uverbs_async_event_release(struct inode *inode, struct file *filp);
 
 int ib_alloc_ucontext(struct uverbs_attr_bundle *attrs);
 int ib_init_ucontext(struct uverbs_attr_bundle *attrs);

drivers/infiniband/core/uverbs_main.c +1 -1

index 17fc25db0311..cb5b59123d8f 100644
@@ -346,7 +346,7 @@ const struct file_operations uverbs_async_event_fops = {
 	.owner	 = THIS_MODULE,
 	.read	 = ib_uverbs_async_event_read,
 	.poll    = ib_uverbs_async_event_poll,
-	.release = uverbs_uobject_fd_release,
+	.release = uverbs_async_event_release,
 	.fasync  = ib_uverbs_async_event_fasync,
 	.llseek	 = no_llseek,
 };

drivers/infiniband/core/uverbs_std_types_async_fd.c +25 -1

index 82ec0806b34b..462deb506b16 100644
@@ -26,10 +26,34 @@ static int uverbs_async_event_destroy_uobj(struct ib_uobject *uobj,
 		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;
 }
 
+int uverbs_async_event_release(struct inode *inode, struct file *filp)
+{
+	struct ib_uverbs_async_event_file *event_file;
+	struct ib_uobject *uobj = filp->private_data;
+	int ret;
+
+	if (!uobj)
+		return uverbs_uobject_fd_release(inode, filp);
+
+	event_file =
+		container_of(uobj, struct ib_uverbs_async_event_file, uobj);
+
+	/*
+	 * The async event FD has to deliver IB_EVENT_DEVICE_FATAL even after
+	 * disassociation, so cleaning the event list must only happen after
+	 * release. The user knows it has reached the end of the event stream
+	 * when it sees IB_EVENT_DEVICE_FATAL.
+	 */
+	uverbs_uobject_get(uobj);
+	ret = uverbs_uobject_fd_release(inode, filp);
+	ib_uverbs_free_event_queue(&event_file->ev_queue);
+	uverbs_uobject_put(uobj);
+	return ret;
+}
+
 DECLARE_UVERBS_NAMED_METHOD(
 	UVERBS_METHOD_ASYNC_EVENT_ALLOC,
 	UVERBS_ATTR_FD(UVERBS_ATTR_ASYNC_EVENT_ALLOC_FD_HANDLE,

Signature · 1 line

2.26.2

Leon Romanovsky

author

2020-05-07 06:34 UTC #2 lore raw

PATCH 2/2 RDMA/uverbs: Move IB_EVENT_DEVICE_FATAL to destroy_uobj

In reply to Leon Romanovsky

From: Jason Gunthorpe <[email protected]>

When multiple async FDs were allowed to exist the idea what for all broadcast events to be delivered to all async FDs, however IB_EVENT_DEVICE_FATAL was missed.

Instead of having ib_uverbs_free_hw_resources() special case the global async_fd, have it cause the event during the uobject destruction. Every async fd is now a uobject so simply generate the IB_EVENT_DEVICE_FATAL while destroying the async fd uobject. This ensures every async FD gets a copy of the event.

Fixes d680e88e2013 ("RDMA/core: Add UVERBS_METHOD_ASYNC_EVENT_ALLOC")

Signed-off-by Yishai Hadas <[email protected]>

Signed-off-by Jason Gunthorpe <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/uverbs.h                    |  3 +++
 drivers/infiniband/core/uverbs_main.c               | 10 +++-------
 drivers/infiniband/core/uverbs_std_types_async_fd.c |  4 ++++
 3 files changed, 10 insertions(+), 7 deletions(-)

drivers/infiniband/core/uverbs.h +3 -0

index 2673cb1cd655..3d189c7ee59e 100644
@@ -228,6 +228,9 @@ 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_uevent_object *uobj);
 void ib_uverbs_release_file(struct kref *ref);
+void ib_uverbs_async_handler(struct ib_uverbs_async_event_file *async_file,
+			     __u64 element, __u64 event,
+			     struct list_head *obj_list, u32 *counter);
 
 void ib_uverbs_comp_handler(struct ib_cq *cq, void *cq_context);
 void ib_uverbs_cq_event_handler(struct ib_event *event, void *context_ptr);

drivers/infiniband/core/uverbs_main.c +3 -7

index cb5b59123d8f..1bab8de14757 100644
@@ -386,10 +386,9 @@ 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_async_event_file *async_file,
-			__u64 element, __u64 event, struct list_head *obj_list,
-			u32 *counter)
+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;

@@ -1187,9 +1186,6 @@ static void ib_uverbs_free_hw_resources(struct ib_uverbs_device *uverbs_dev,
 		 */
 		mutex_unlock(&uverbs_dev->lists_mutex);
 
-		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);

drivers/infiniband/core/uverbs_std_types_async_fd.c +4 -0

index 462deb506b16..61899eaf1f91 100644
@@ -26,6 +26,10 @@ static int uverbs_async_event_destroy_uobj(struct ib_uobject *uobj,
 		container_of(uobj, struct ib_uverbs_async_event_file, uobj);
 
 	ib_unregister_event_handler(&event_file->event_handler);
+
+	if (why == RDMA_REMOVE_DRIVER_REMOVE)
+		ib_uverbs_async_handler(event_file, 0, IB_EVENT_DEVICE_FATAL,
+					NULL, NULL);
 	return 0;
 }

Signature · 1 line

2.26.2

Jason Gunthorpe

Applied

2020-05-12 20:05 UTC #3 lore raw

In reply to Leon Romanovsky

Quoted 11 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>
Hi,
Two changes, pretty straightforward.
Thanks
Jason Gunthorpe (2):
RDMA/uverbs: Do not discard the IB_EVENT_DEVICE_FATAL event RDMA/uverbs: Move IB_EVENT_DEVICE_FATAL to destroy_uobj

Applied to for-rc

Jason