IB/uverbs: Add support for passing memory region invalidations to userspace

Hi everyone,

I think I've finally gotten my revision of ummunotify to live within userspace verbs into a reasonable state. I've done no testing beyond compiling it and reading over the code -- in fact, I haven't even implemented the corresponding support in libibverbs -- but I wanted to get this out so that people can take a look.

Given that we are probably no more than a week away from the 2.6.37 merge window, I don't think this is appropriate for 2.6.37, but I definitely plan to get this into 2.6.38 unless there is some big problem I don't see right now.

As usual, all comments and criticisms welcome!

---

IB/uverbs: Add support for passing memory region invalidations to userspace

As discussed in <http://article.gmane.org/gmane.linux.drivers.openib/61925>
and follow-up messages, libraries using RDMA would like to track precisely when application code changes memory mapping via free(), munmap(), etc. Current pure-userspace solutions using malloc hooks and other tricks are not robust, and the feeling among experts is that the issue is unfixable without kernel help.

We solve this not by implementing the full API proposed in the email linked above but rather with a simpler and more generic interface. Unlike previous attempts at this, which implemented a new generic character device, this patch works within the existing RDMA userspace verbs support. Specifically, we implement three new userspace operations:

1. A new version of the "register MR" operation, which creates an MR for which the kernel will notify userspace when the virtual mapping changes. We need a new operation for this (rather than, say, simply adding another flag to the access_flags field of the existing reg_mr operation) because we need to extend the ABI to allow userspace to pass in a cookie that will be returned as part of invalidation events.

2. A new version of the "deregister MR" operation that returns the number of invalidate events passed to userspace. Now that we generate events for MRs, we need this event count in the destroy operation to avoid unfixable races in userspace for exactly analogous reasons to the existing destroy CQ, QP and SRQ operations.

3. A new command to create an MMU notification file descriptor for a userspace verbs context. We require this FD to be created before allowing any MRs with invalidation notification to be registered. When an invalidation event occurs, the kernel queues an event on this FD that userspace can retrieve with read().

We also allow userspace to mmap() one page at offset 0 to map a kernel page that contains a generation counter that is incremented each time an event is queued. This allows userspace to have a fast path that checks that no events have occurred, without needing to do a system call.

Thanks to Jason Gunthorpe for suggestions on the interface design. Also thanks to Jeff Squyres for prototyping support for this in Open MPI, which helped find several bugs during development.

---
 drivers/infiniband/Kconfig            |    1 +
 drivers/infiniband/core/umem.c        |  141 +++++++++++++++++
 drivers/infiniband/core/uverbs.h      |   24 +++-
 drivers/infiniband/core/uverbs_cmd.c  |  279 ++++++++++++++++++++++++++-------
 drivers/infiniband/core/uverbs_main.c |  122 ++++++++++++++-
 include/rdma/ib_umem.h                |   58 +++++++
 include/rdma/ib_user_verbs.h          |   42 +++++-
 7 files changed, 601 insertions(+), 66 deletions(-)

index 89d70de..9e5c776 100644
@@ -21,6 +21,7 @@ config INFINIBAND_USER_MAD
 config INFINIBAND_USER_ACCESS
 	tristate "InfiniBand userspace access (verbs and CM)"
 	select ANON_INODES
+	select MMU_NOTIFIER
 	---help---
 	  Userspace InfiniBand access support.  This enables the
 	  kernel side of userspace verbs and the userspace

index a078e56..10fec27 100644
@@ -78,9 +78,15 @@ struct ib_uverbs_device {
 	struct cdev			        cdev;
 };
 
+enum ib_uverbs_event_file_type {
+	IB_UVERBS_EVENT_FILE_ASYNC,
+	IB_UVERBS_EVENT_FILE_COMP,
+	IB_UVERBS_EVENT_FILE_MMU_NOTIFY,
+};
+
 struct ib_uverbs_event_file {
 	struct kref				ref;
-	int					is_async;
+	enum ib_uverbs_event_file_type		type;
 	struct ib_uverbs_file		       *uverbs_file;
 	spinlock_t				lock;
 	int					is_closed;

@@ -95,13 +101,17 @@ struct ib_uverbs_file {
 	struct ib_uverbs_device		       *device;
 	struct ib_ucontext		       *ucontext;
 	struct ib_event_handler			event_handler;
+	struct ib_ummunotify_context	        mmu_notify_context;
+	u64				       *mmu_notify_counter;
 	struct ib_uverbs_event_file	       *async_file;
+	struct ib_uverbs_event_file	       *mmu_notify_file;
 };
 
 struct ib_uverbs_event {
 	union {
 		struct ib_uverbs_async_event_desc	async;
 		struct ib_uverbs_comp_event_desc	comp;
+		struct ib_uverbs_mmu_notify_event_desc	mmu_notify;
 	}					desc;
 	struct list_head			list;
 	struct list_head			obj_list;

@@ -120,6 +130,11 @@ struct ib_uevent_object {
 	u32			events_reported;
 };
 
+struct ib_umr_object {
+	struct ib_uevent_object	uevent;
+	struct ib_ummunotify_range range;
+};
+
 struct ib_uqp_object {
 	struct ib_uevent_object	uevent;
 	struct list_head 	mcast_list;

@@ -146,7 +161,7 @@ extern struct idr ib_uverbs_srq_idr;
 void idr_remove_uobj(struct idr *idp, struct ib_uobject *uobj);
 
 struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file *uverbs_file,
-					int is_async);
+					enum ib_uverbs_event_file_type type);
 struct ib_uverbs_event_file *ib_uverbs_lookup_comp_file(int fd);
 
 void ib_uverbs_release_ucq(struct ib_uverbs_file *file,

@@ -156,6 +171,8 @@ void ib_uverbs_release_uevent(struct ib_uverbs_file *file,
 			      struct ib_uevent_object *uobj);
 
 void ib_uverbs_comp_handler(struct ib_cq *cq, void *cq_context);
+void ib_uverbs_mr_event_handler(struct ib_ummunotify_context *context,
+				struct ib_ummunotify_range *range);
 void ib_uverbs_cq_event_handler(struct ib_event *event, void *context_ptr);
 void ib_uverbs_qp_event_handler(struct ib_event *event, void *context_ptr);
 void ib_uverbs_srq_event_handler(struct ib_event *event, void *context_ptr);

@@ -195,5 +212,8 @@ IB_UVERBS_DECLARE_CMD(create_srq);
 IB_UVERBS_DECLARE_CMD(modify_srq);
 IB_UVERBS_DECLARE_CMD(query_srq);
 IB_UVERBS_DECLARE_CMD(destroy_srq);
+IB_UVERBS_DECLARE_CMD(create_mmu_notify_channel);
+IB_UVERBS_DECLARE_CMD(reg_mmu_notify_mr);
+IB_UVERBS_DECLARE_CMD(dereg_mmu_notify_mr);
 
 #endif /* UVERBS_H */

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I made a review and overall it looks good. One comment though. Could we add an option to ib_uverbs_dereg_mr_common() to deregister all regions for which notifications have been received? This could relieve anyone who does not wish to read the notifications from the notifications channel but instead have it done automatically.