[RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Leon Romanovsky]

The following patch set comes to enrich security model as a follow up
to commit e6bd18f57aad ('IB/security: Restrict use of the write() interface').

This patch series add ioctl() interface to the existing write() interface and
provide an easy route to backport this change to legacy supported systems.

The proposed code was initially tested with ibv_devinfo application
and supplementary part of libibverbs, which will be posted next.

In a response to the ABI discussion [1] and the summary posted [2], two ioctl()
calls were introduced: IB_IOCTL_VERBS and IB_IOCTL_DIRECT.

* IB_IOCTL_VERBS command will perform all validation and parsing
  supplied by IB CORE logic, before serving the request.
* IB_IOCTL_DIRECT command will bypass IB core logic and it
  is intended for vendor specific channel operations.

Such separation will simplify the user space development by working with
limited and constant number of ioctl numbers without limiting the
ability to expand the vendor's interface in the future.

In order to fully utilize such opportunity to extend ABI while converting
write() to ioctl() interfaces, the community decision was to introduce
netlink-based ABI for ioctl() data [3].

IB_IOCTL_VERBS commands start with generic header:

struct ib_uverbs_ioctl_hdr {
	__u32 length;
	__u16 flags;
	__u16 object_type;
	__u16 reserved;
	/* First 8 actions are common to all objects */
	__u16 action;
	__u32 user_handler;
	/*
	 * These fields represent core response only,
	 * provider's response is given as a netlink attribute.
	 */
	struct ib_uverbs_uptr resp;
};

struct ib_uverbs_uptr {
	__u64 ptr;
	__u32 len;
};

After the header, a stream of netlink attributes (TLVs) are given. These
attributes represent the command itself.

Every command validates its attributes using a generic way by ensure:
1. Attributes are known
2. Attributes' sizes are correct
3. Mandatory attributes exist

The actual uverbs implementation get an array of pointers to
netlink attributes represents the core command, a udata attribute
which the response will be written to (the response starts with a
header identical to netlink nested attribute header) and a udata
represents the vendor data.

Commands and responses are now extensible by nature, as new netlink
attributes could be added when required.

Vendor commands and responses which have udata will use pointers to a
different buffer (using a new netlink uptr attribute). Such separation
will allow to retain backward compatibility with current user-space vendor
drivers.

Thanks,
	Matan, Haggai and Leon.

[1] http://www.spinics.net/lists/linux-rdma/msg36295.html
[2] http://marc.info/?l=linux-rdma&m=146403269100691&w=2
[3] http://lists.openfabrics.org/pipermail/ofvwg/attachments/20160517/0ddbded9/attachment-0002.pdf

Leon Romanovsky (2):
  IB/core: Export RDMA IOCTL declarations
  IB/core: Add DIRECT ioctl call to vendor

Matan Barak (6):
  lib/nlattr: Add parsing netlink and validate using callback
  IB/core: Adding netlink based udata
  IB/core: Add new ioctl for VERBS commands with netlink style parsing
  IB/core: Add outptr to udata in order to track the output size
  IB/core: Refactor idr to a shared file
  IB/core: Implement device_create with the new ABI

 drivers/infiniband/core/Makefile            |   3 +-
 drivers/infiniband/core/user_mad.c          |   2 +-
 drivers/infiniband/core/uverbs.h            |  64 +++-
 drivers/infiniband/core/uverbs_cmd.c        | 558 ++++++++--------------------
 drivers/infiniband/core/uverbs_cmd_common.c | 280 ++++++++++++++
 drivers/infiniband/core/uverbs_cmd_nl.c     | 151 ++++++++
 drivers/infiniband/core/uverbs_main.c       | 322 +++++++++++++++-
 drivers/infiniband/core/uverbs_nl.c         |  92 +++++
 include/linux/netlink.h                     |   1 -
 include/net/netlink.h                       |   5 +
 include/rdma/ib_ioctl.h                     |  49 +++
 include/rdma/ib_verbs.h                     |  10 +-
 include/uapi/rdma/Kbuild                    |   1 +
 include/uapi/rdma/ib_user_ioctl.h           | 126 +++++++
 include/uapi/rdma/ib_user_mad.h             |  12 -
 lib/nlattr.c                                |  43 ++-
 16 files changed, 1290 insertions(+), 429 deletions(-)
 create mode 100644 drivers/infiniband/core/uverbs_cmd_common.c
 create mode 100644 drivers/infiniband/core/uverbs_cmd_nl.c
 create mode 100644 drivers/infiniband/core/uverbs_nl.c
 create mode 100644 include/rdma/ib_ioctl.h
 create mode 100644 include/uapi/rdma/ib_user_ioctl.h

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC ABI 1/8] IB/core: Export RDMA IOCTL declarations

[Leon Romanovsky]

Place all external RDMA IOCTL declarations into one UAPI exported
header file and move all legacy MAD commands to that file.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/user_mad.c    |  2 +-
 drivers/infiniband/core/uverbs_main.c |  1 +
 include/rdma/ib_ioctl.h               | 38 ++++++++++++++++++++++++
 include/uapi/rdma/Kbuild              |  1 +
 include/uapi/rdma/ib_user_ioctl.h     | 55 +++++++++++++++++++++++++++++++++++
 include/uapi/rdma/ib_user_mad.h       | 12 --------
 6 files changed, 96 insertions(+), 13 deletions(-)
 create mode 100644 include/rdma/ib_ioctl.h
 create mode 100644 include/uapi/rdma/ib_user_ioctl.h

diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 415a318..bee2a34 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -52,8 +52,8 @@
 
 #include <asm/uaccess.h>
 
+#include <rdma/ib_ioctl.h>
 #include <rdma/ib_mad.h>
-#include <rdma/ib_user_mad.h>
 
 MODULE_AUTHOR("Roland Dreier");
 MODULE_DESCRIPTION("InfiniBand userspace MAD packet access");
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 31f422a..42fbb6d 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -49,6 +49,7 @@
 #include <asm/uaccess.h>
 
 #include <rdma/ib.h>
+#include <rdma/ib_ioctl.h>
 
 #include "uverbs.h"
 
diff --git a/include/rdma/ib_ioctl.h b/include/rdma/ib_ioctl.h
new file mode 100644
index 0000000..2d8db54
--- /dev/null
+++ b/include/rdma/ib_ioctl.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies, LTD. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef IB_IOCTL_H
+#define IB_IOCTL_H
+
+#include <uapi/rdma/ib_user_ioctl.h>
+
+#endif /* IB_IOCTL_H */
diff --git a/include/uapi/rdma/Kbuild b/include/uapi/rdma/Kbuild
index 231901b..c9344b3 100644
--- a/include/uapi/rdma/Kbuild
+++ b/include/uapi/rdma/Kbuild
@@ -1,5 +1,6 @@
 # UAPI Header export list
 header-y += ib_user_cm.h
+header-y += ib_user_ioctl.h
 header-y += ib_user_mad.h
 header-y += ib_user_sa.h
 header-y += ib_user_verbs.h
diff --git a/include/uapi/rdma/ib_user_ioctl.h b/include/uapi/rdma/ib_user_ioctl.h
new file mode 100644
index 0000000..d1928dea
--- /dev/null
+++ b/include/uapi/rdma/ib_user_ioctl.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies, LTD. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef IB_USER_IOCTL_H
+#define IB_USER_IOCTL_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+#define IB_IOCTL_MAGIC		0x1b
+
+/* Legacy part
+ * !!!! NOTE: It uses the same command index as VERBS
+ */
+#include <rdma/ib_user_mad.h>
+#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
+					      struct ib_user_mad_reg_req)
+
+#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
+
+#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
+
+#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
+					      struct ib_user_mad_reg_req2)
+
+#endif /* IB_USER_IOCTL_H */
diff --git a/include/uapi/rdma/ib_user_mad.h b/include/uapi/rdma/ib_user_mad.h
index 09f809f..7e722e7 100644
--- a/include/uapi/rdma/ib_user_mad.h
+++ b/include/uapi/rdma/ib_user_mad.h
@@ -230,16 +230,4 @@ struct ib_user_mad_reg_req2 {
 	__u8	reserved[3];
 };
 
-#define IB_IOCTL_MAGIC		0x1b
-
-#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
-					      struct ib_user_mad_reg_req)
-
-#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
-
-#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
-
-#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
-					      struct ib_user_mad_reg_req2)
-
 #endif /* IB_USER_MAD_H */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 1/8] IB/core: Export RDMA IOCTL declarations

[Hefty, Sean]

> Place all external RDMA IOCTL declarations into one UAPI exported
> header file and move all legacy MAD commands to that file.

Are you expecting umad ioctl's to come through the same file as uverbs?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 1/8] IB/core: Export RDMA IOCTL declarations

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

On Tue, May 24, 2016 at 07:46:32PM +0000, Hefty, Sean wrote:
> > Place all external RDMA IOCTL declarations into one UAPI exported
> > header file and move all legacy MAD commands to that file.
> 
> Are you expecting umad ioctl's to come through the same file as uverbs?

No, just wanted to export one common file.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[RFC ABI 2/8] lib/nlattr: Add parsing netlink and validate using callback

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

When parsing netlink packets, users may want to do some extra
validations on the netlink attributes. Adding nla_parse_cb, which
gets an extra callback parameter. This function is invoked for every
netlink attribute. If the callback fails, parsing will fail as well.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/linux/netlink.h |  1 -
 include/net/netlink.h   |  5 +++++
 lib/nlattr.c            | 43 ++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index da14ab6..a08b63b 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -117,7 +117,6 @@ netlink_skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
 
 #define NLMSG_DEFAULT_SIZE (NLMSG_GOODSIZE - NLMSG_HDRLEN)
 
-
 struct netlink_callback {
 	struct sk_buff		*skb;
 	const struct nlmsghdr	*nlh;
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 0e31727..70ba851 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -149,6 +149,8 @@
  *   nla_find()				find attribute in stream of attributes
  *   nla_find_nested()			find attribute in nested attributes
  *   nla_parse()			parse and validate stream of attrs
+ *   nla_parse_cb()			parse stream of attrs and call to
+ *						callback for every nlattr
  *   nla_parse_nested()			parse nested attribuets
  *   nla_for_each_attr()		loop over all attributes
  *   nla_for_each_nested()		loop over the nested attributes
@@ -237,6 +239,9 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
 		 const struct nla_policy *policy);
 int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 	      int len, const struct nla_policy *policy);
+int nla_parse_cb(struct nlattr **tb, int maxtype, const struct nlattr *head,
+		 int len, const struct nla_policy *policy,
+		 int (*cb)(const struct nlattr *, void *), void *cb_priv);
 int nla_policy_len(const struct nla_policy *, int);
 struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
 size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
diff --git a/lib/nlattr.c b/lib/nlattr.c
index f5907d2..ec79387 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -165,22 +165,28 @@ nla_policy_len(const struct nla_policy *p, int n)
 EXPORT_SYMBOL(nla_policy_len);
 
 /**
- * nla_parse - Parse a stream of attributes into a tb buffer
+ * nla_parse_cb - Parse a stream of attributes into a tb buffer with callback
  * @tb: destination array with maxtype+1 elements
  * @maxtype: maximum attribute type to be expected
  * @head: head of attribute stream
  * @len: length of attribute stream
  * @policy: validation policy
+ * @cb: callback function and data, must not be NULL
+ * @cb_priv: private data for the callback
  *
  * Parses a stream of attributes and stores a pointer to each attribute in
  * the tb array accessible via the attribute type. Attributes with a type
  * exceeding maxtype will be silently ignored for backwards compatibility
  * reasons. policy may be set to NULL if no validation is required.
+ * For every validated nla, the function calls the user-space callback.
+ * If the callback fails (returns with non-zero value), parsing is terminated
+ * and this value is returned as error.
  *
  * Returns 0 on success or a negative error code.
  */
-int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
-	      int len, const struct nla_policy *policy)
+int nla_parse_cb(struct nlattr **tb, int maxtype, const struct nlattr *head,
+		 int len, const struct nla_policy *policy,
+		 int (*cb)(const struct nlattr *, void *), void *cb_priv)
 {
 	const struct nlattr *nla;
 	int rem, err;
@@ -196,6 +202,9 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 				if (err < 0)
 					goto errout;
 			}
+			err = cb(nla, cb_priv);
+			if (err)
+				goto errout;
 
 			tb[type] = (struct nlattr *)nla;
 		}
@@ -209,6 +218,34 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 errout:
 	return err;
 }
+EXPORT_SYMBOL(nla_parse_cb);
+
+static int nla_pass_all_cb(const struct nlattr *nla, void *priv)
+{
+	return 0;
+}
+
+/**
+ * nla_parse - Parse a stream of attributes into a tb buffer
+ * @tb: destination array with maxtype+1 elements
+ * @maxtype: maximum attribute type to be expected
+ * @head: head of attribute stream
+ * @len: length of attribute stream
+ * @policy: validation policy
+ *
+ * Parses a stream of attributes and stores a pointer to each attribute in
+ * the tb array accessible via the attribute type. Attributes with a type
+ * exceeding maxtype will be silently ignored for backwards compatibility
+ * reasons. policy may be set to NULL if no validation is required.
+ *
+ * Returns 0 on success or a negative error code.
+ */
+int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
+	      int len, const struct nla_policy *policy)
+{
+	return nla_parse_cb(tb, maxtype, head, len, policy, nla_pass_all_cb,
+			    NULL);
+}
 EXPORT_SYMBOL(nla_parse);
 
 /**
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC ABI 3/8] IB/core: Adding netlink based udata

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The new ABI is based on netlink attributes. Currently, the netlink
implementation uses a skb in order to write data into. Adding an
ability to serialize netlink attributes straight to udata.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/Makefile    |  3 +-
 drivers/infiniband/core/uverbs.h    |  8 ++++
 drivers/infiniband/core/uverbs_nl.c | 92 +++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100644 drivers/infiniband/core/uverbs_nl.c

diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index 26987d9..3d8a423 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -34,4 +34,5 @@ ib_umad-y :=			user_mad.o
 
 ib_ucm-y :=			ucm.o
 
-ib_uverbs-y :=			uverbs_main.o uverbs_cmd.o uverbs_marshall.o
+ib_uverbs-y :=			uverbs_main.o uverbs_cmd.o uverbs_marshall.o \
+				uverbs_nl.o
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 612ccfd..a46cb29 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -84,6 +84,14 @@
  * released when the CQ is destroyed.
  */
 
+struct nlattr __user *ib_uverbs_nla_put(struct ib_udata *udata,
+					int attrtype, int attrlen,
+					const void *data);
+void ib_uverbs_nla_nest_end(struct ib_udata *udata,
+			    struct nlattr __user *nla);
+struct nlattr __user *ib_uverbs_nla_nest_start(struct ib_udata *udata,
+					       uint16_t type);
+
 struct ib_uverbs_device {
 	atomic_t				refcount;
 	int					num_comp_vectors;
diff --git a/drivers/infiniband/core/uverbs_nl.c b/drivers/infiniband/core/uverbs_nl.c
new file mode 100644
index 0000000..abf2d35
--- /dev/null
+++ b/drivers/infiniband/core/uverbs_nl.c
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies, LTD. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <net/netlink.h>
+#include <rdma/ib_verbs.h>
+
+static struct nlattr __user *ib_uverbs_nla_reserve(struct ib_udata *udata,
+						   int attrtype, int attrlen)
+{
+	struct nlattr __user *nla;
+	unsigned int i;
+
+	if (nla_total_size(attrlen) > udata->outlen)
+		return ERR_PTR(-ENOSPC);
+
+	nla = (struct nlattr __user *)udata->outbuf;
+	udata->outbuf += nla_total_size(attrlen);
+	udata->outlen -= nla_total_size(attrlen);
+	put_user(attrtype, &nla->nla_type);
+	put_user(nla_attr_size(attrlen), &nla->nla_len);
+
+	/* TODO: optimize */
+	for (i = 0; i < nla_padlen(attrlen); i++)
+		put_user(0,
+			 (unsigned char __user *)nla + nla_attr_size(attrlen));
+
+	return nla;
+}
+
+struct nlattr __user *ib_uverbs_nla_put(struct ib_udata *udata,
+					int attrtype, int attrlen,
+					const void *data)
+{
+	struct nlattr __user *nla;
+	int ret;
+
+	nla = ib_uverbs_nla_reserve(udata, attrtype, attrlen);
+	if (IS_ERR(nla))
+		return nla;
+
+	ret = copy_to_user(nla_data(nla), data, attrlen);
+	if (ret) {
+		udata->outbuf -= nla_total_size(attrlen);
+		udata->outlen += nla_total_size(attrlen);
+		return ERR_PTR(ret);
+	}
+
+	return nla;
+}
+
+void ib_uverbs_nla_nest_end(struct ib_udata *udata,  struct nlattr __user *nla)
+{
+	put_user(udata->outbuf - (void __user *)nla, &nla->nla_len);
+}
+
+struct nlattr __user *ib_uverbs_nla_nest_start(struct ib_udata *udata,
+					       uint16_t type)
+{
+	struct nlattr __user *nla = ib_uverbs_nla_put(udata, type, 0, NULL);
+
+	return nla;
+}
+
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC ABI 4/8] IB/core: Add DIRECT ioctl call to vendor

[Leon Romanovsky]

Introduce a new RDMA IOCTL command, which will be visible to
consumers, which would like to execute commands straight on the
vendor's driver. It bypasses IB core logic and is intended for
vendor specific channel operations.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/uverbs_main.c | 33 +++++++++++++++++++++++++++++++++
 include/rdma/ib_verbs.h               |  2 ++
 include/uapi/rdma/ib_user_ioctl.h     |  5 +++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 42fbb6d..68cf1fd 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -850,6 +850,37 @@ out:
 	return ret;
 }
 
+static long ib_uverbs_ioctl(struct file *filp, unsigned int cmd,
+			    unsigned long arg)
+{
+	struct ib_uverbs_file *file = filp->private_data;
+	struct ib_device *ib_dev;
+	int srcu_key;
+
+	if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+		return -EACCES;
+
+	if (cmd == IB_IOCTL_DIRECT) {
+		long ret = 0;
+
+		srcu_key = srcu_read_lock(&file->device->disassociate_srcu);
+		ib_dev = srcu_dereference(file->device->ib_dev,
+					  &file->device->disassociate_srcu);
+		if (!ib_dev) {
+			srcu_read_unlock(&file->device->disassociate_srcu, srcu_key);
+			return -EIO;
+		}
+
+		if (ib_dev->direct_fn)
+			ret = ib_dev->direct_fn(filp, arg);
+
+		srcu_read_unlock(&file->device->disassociate_srcu, srcu_key);
+		return ret;
+	}
+
+	return -ENOIOCTLCMD;
+}
+
 static int ib_uverbs_mmap(struct file *filp, struct vm_area_struct *vma)
 {
 	struct ib_uverbs_file *file = filp->private_data;
@@ -985,6 +1016,7 @@ static const struct file_operations uverbs_fops = {
 	.open	 = ib_uverbs_open,
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
+	.unlocked_ioctl = ib_uverbs_ioctl,
 };
 
 static const struct file_operations uverbs_mmap_fops = {
@@ -994,6 +1026,7 @@ static const struct file_operations uverbs_mmap_fops = {
 	.open	 = ib_uverbs_open,
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
+	.unlocked_ioctl = ib_uverbs_ioctl,
 };
 
 static struct ib_client uverbs_client = {
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index fc0320c..1904c02 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1707,6 +1707,8 @@ struct ib_device {
 
 	struct iw_cm_verbs	     *iwcm;
 
+	long		           (*direct_fn)(struct file *filp,
+						unsigned long arg);
 	int		           (*get_protocol_stats)(struct ib_device *device,
 							 union rdma_protocol_stats *stats);
 	int		           (*query_device)(struct ib_device *device,
diff --git a/include/uapi/rdma/ib_user_ioctl.h b/include/uapi/rdma/ib_user_ioctl.h
index d1928dea..f17e51a 100644
--- a/include/uapi/rdma/ib_user_ioctl.h
+++ b/include/uapi/rdma/ib_user_ioctl.h
@@ -38,6 +38,11 @@
 
 #define IB_IOCTL_MAGIC		0x1b
 
+#define IB_CMD_DIRECT		0x2
+
+#define IB_IOCTL_DIRECT \
+	_IOWR(IB_IOCTL_MAGIC, IB_CMD_DIRECT, unsigned long)
+
 /* Legacy part
  * !!!! NOTE: It uses the same command index as VERBS
  */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC ABI 5/8] IB/core: Add new ioctl for VERBS commands with netlink style parsing

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Introduce a new RDMA IOCTL command. The VERBS ioctl command will
perform all validation and parsing supplied by IB CORE logic,
before serving the request.

The verbs ioctl is the base of all generic commands. A command starts
with a generic header:

struct ib_uverbs_uptr {
       __u64 ptr;
       __u32 len;
};

struct ib_uverbs_ioctl_hdr {
       __u32 length;
       __u16 flags;
       __u16 object_type;
       __u16 reserved;
       __u16 action;
       __u32 user_handler;
       /*
        * These fields represent core response only,
        * provider's response is given as a netlink attribute.
        */
       struct ib_uverbs_uptr resp;
};

This header declares the action and the object type. The first 8
actions on every object are reserved for common actions.

After the header, a stream of netlink attributes are given. These
attributes represent the command itself.

Every command validates its attributes using a generic way by:
1. Ensuring all attributes are known
2. Ensuring all attributes' sizes are correct
3. Ensuring all mandatory attributes exist

The actual uverbs implementation get an array of pointers to
netlink attributes represents the core command, a udata attribute
which the response will be written to (the response starts with a
header identical to netlink nested attribute header) and a udata
represents the vendor data.

Commands and responses are now extensible by nature, as new netlink
attributes could be added when required.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/uverbs_main.c | 247 +++++++++++++++++++++++++++++++++-
 include/rdma/ib_ioctl.h               |  11 ++
 include/uapi/rdma/ib_user_ioctl.h     |  61 +++++++++
 3 files changed, 318 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 68cf1fd..2f49eb7 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -850,9 +850,230 @@ out:
 	return ret;
 }
 
+struct mandatory_fields {
+	unsigned long *bitmap;
+};
+
+struct validate_op {
+	const struct nla_policy		*policy;
+	struct mandatory_fields		mandatory_fields;
+	size_t				resp_min_sz;
+};
+
+void ib_build_udata_from_nl(struct ib_udata *udata, struct nlattr **tb,
+			    uint16_t resp_type)
+{
+	struct ib_uverbs_uptr ucmd = {};
+	struct ib_uverbs_uptr uresp = {};
+	struct ib_uverbs_uptr *pucmd = &ucmd;
+	struct ib_uverbs_uptr *puresp = &uresp;
+
+	if (tb[IBNL_PROVIDER_CMD_UPTR])
+		pucmd = (struct ib_uverbs_uptr *)nla_data(tb[IBNL_PROVIDER_CMD_UPTR]);
+
+	if (tb[IBNL_PROVIDER_RESP_UPTR])
+		puresp = (struct ib_uverbs_uptr *)nla_data(tb[IBNL_PROVIDER_RESP_UPTR]);
+
+	INIT_UDATA_BUF_OR_NULL(udata, (const void __user *)pucmd->ptr,
+			       (void __user *)puresp->ptr, pucmd->len,
+			       puresp->len);
+}
+
+#define IBNL_VENDOR_POLICY_ATTRS					  \
+	[IBNL_PROVIDER_CMD_UPTR]	 = {.type = NLA_BINARY,		  \
+				    .len = sizeof(struct ib_uverbs_uptr)},\
+	[IBNL_PROVIDER_RESP_UPTR]	 = {.type = NLA_BINARY,		  \
+				    .len = sizeof(struct ib_uverbs_uptr)}
+struct object_action {
+	struct {
+		struct validate_op validator;
+		long (*fn)(struct ib_uverbs_file *filp,
+			   struct ib_device *ib_dev,
+			   struct ib_uverbs_ioctl_hdr *hdr,
+			   struct nlattr **tb, struct ib_udata *resp,
+			   struct ib_udata *uhw);
+		unsigned int max_attrs;
+	} create;
+	/* other ops */
+	struct {
+		struct validate_op validator;
+		long (*fn)(struct ib_uverbs_file *filp,
+			   struct ib_device *ib_dev,
+			   struct ib_uverbs_ioctl_hdr *hdr, uint32_t id,
+			   struct nlattr **tb, struct ib_udata *resp,
+			   struct ib_udata *uhw);
+		unsigned int max_attrs;
+	} ops[];
+};
+
+static const struct object_action object_actions[IB_OBJ_TYPE_MAX];
+
+struct nla_validator_cb_priv {
+	int maxtype;
+	unsigned long *fields;
+};
+
+static int ib_set_bit(const struct nlattr *nla, void *priv)
+{
+	struct nla_validator_cb_priv *validator =
+		(struct nla_validator_cb_priv *)priv;
+	u16 type = nla_type(nla);
+
+	if (type >= validator->maxtype)
+		return -EOPNOTSUPP;
+
+	set_bit(type, validator->fields);
+
+	return 0;
+};
+
+static int nla_strict_parse(struct nlattr **tb, int maxtype,
+			    const struct nlattr *head, int len,
+			    const struct validate_op *validate)
+{
+	int err = 0;
+	const struct nla_policy	*policy = validate->policy;
+	DECLARE_BITMAP(fields, maxtype);
+	struct nla_validator_cb_priv validate_priv = {.maxtype = maxtype,
+						      .fields = fields};
+
+	err = nla_validate(head, len, maxtype, policy);
+	if (err)
+		return err;
+
+	err = nla_parse_cb(tb, maxtype, head, len, policy, ib_set_bit,
+			   &validate_priv);
+	if (err) {
+		err = -EINVAL;
+		goto errout;
+	}
+
+	bitmap_and(fields, fields, validate->mandatory_fields.bitmap,
+		   IB_UVERBS_MAX_SUPPORTED_ATTRS);
+	if (!bitmap_equal(fields, validate->mandatory_fields.bitmap,
+			  IB_UVERBS_MAX_SUPPORTED_ATTRS)) {
+		err = -EINVAL;
+		goto errout;
+	}
+
+errout:
+	return err;
+}
+
+#define IB_UVERBS_MAX_CMD_SZ	4096
+
+static long ib_uverbs_cmd_verbs(struct file *filp,
+				struct ib_uverbs_ioctl_hdr *hdr,
+				void __user *buf)
+{
+	struct ib_uverbs_file *file = filp->private_data;
+	enum ib_uverbs_object_type obj_type = hdr->object_type;
+	void *cmd_buf = NULL;
+	struct nlattr **tb = NULL;
+	struct ib_device *ib_dev;
+	int srcu_key;
+	int err = 0;
+
+	if (obj_type >= IB_OBJ_TYPE_MAX)
+		return -EOPNOTSUPP;
+
+	cmd_buf = kmalloc(hdr->length, GFP_KERNEL);
+	if (!cmd_buf)
+		return -ENOMEM;
+
+	if (copy_from_user(cmd_buf, buf, hdr->length - sizeof(*hdr))) {
+		pr_debug("copy_from_user failed while attempting to read uverbs command buffer: %p, %zd bytes\n",
+			 buf, hdr->length - sizeof(*hdr));
+		err = -EFAULT;
+		goto err1;
+	}
+
+	srcu_key = srcu_read_lock(&file->device->disassociate_srcu);
+	ib_dev = srcu_dereference(file->device->ib_dev,
+				  &file->device->disassociate_srcu);
+	if (!ib_dev) {
+		err = -EIO;
+		goto err;
+	}
+
+	/* TODO: handle flags, for example user specific object */
+
+	switch (hdr->action) {
+	case IBNL_OBJECT_CREATE: {
+		struct ib_udata uhw;
+		struct ib_udata uresp;
+		struct nlattr __user *nla_resp = NULL;
+
+		if (!object_actions[obj_type].create.fn) {
+			err = -EOPNOTSUPP;
+			goto err;
+		}
+
+		if (hdr->action || hdr->user_handler) {
+			pr_debug("invalid uverbs command. action=%d, user_handler=%d\n",
+				 hdr->action, hdr->user_handler);
+			err = -EINVAL;
+			goto err;
+		}
+
+		/* validate response */
+		if (hdr->resp.len < object_actions[obj_type].create.validator.resp_min_sz) {
+			err = -ENOSPC;
+			goto err;
+		}
+
+		tb = kcalloc(object_actions[obj_type].create.max_attrs + 1,
+			     sizeof(*tb), GFP_KERNEL);
+		if (!tb) {
+			err = -ENOMEM;
+			goto err;
+		}
+
+		err = nla_strict_parse(tb, object_actions[obj_type].create.max_attrs,
+				       (const struct nlattr *)cmd_buf,
+				       hdr->length - sizeof(*hdr),
+				       &object_actions[obj_type].create.validator);
+		if (err)
+			goto err;
+
+		ib_build_udata_from_nl(&uhw, tb,
+				       IBNL_RESPONSE_TYPE_VENDOR);
+
+		if (hdr->resp.len) {
+			INIT_UDATA_BUF_OR_NULL(&uresp, NULL,
+					       (void __user *)hdr->resp.ptr,
+					       0, hdr->resp.len);
+			nla_resp = ib_uverbs_nla_nest_start(&uresp,
+							    IBNL_RESPONSE_TYPE_RESP);
+			if (err)
+				goto err;
+		}
+
+		err = object_actions[obj_type].create.fn(file, ib_dev, hdr, tb,
+							 &uresp, &uhw);
+
+		if (nla_resp)
+			ib_uverbs_nla_nest_end(&uresp, nla_resp);
+		break;
+	}
+	default:
+		/* TODO: check object's specific actions */
+		return -ENOIOCTLCMD;
+	}
+err:
+	srcu_read_unlock(&file->device->disassociate_srcu, srcu_key);
+err1:
+	kfree(cmd_buf);
+	kfree(tb);
+	return err;
+}
+
 static long ib_uverbs_ioctl(struct file *filp, unsigned int cmd,
 			    unsigned long arg)
 {
+	struct ib_uverbs_ioctl_hdr __user *user_hdr =
+		(struct ib_uverbs_ioctl_hdr __user *)arg;
+	struct ib_uverbs_ioctl_hdr hdr;
 	struct ib_uverbs_file *file = filp->private_data;
 	struct ib_device *ib_dev;
 	int srcu_key;
@@ -878,7 +1099,31 @@ static long ib_uverbs_ioctl(struct file *filp, unsigned int cmd,
 		return ret;
 	}
 
-	return -ENOIOCTLCMD;
+	/*
+	 * Right now, we are supporting two possible calls
+	 * DIRECT	- go to driver, no parsing, no verification
+	 * VERBS	- pass verification, TLV parsing
+	 */
+	if (cmd != IB_IOCTL_VERBS)
+		return -ENOIOCTLCMD;
+
+	if (copy_from_user(&hdr, user_hdr, sizeof(hdr))) {
+		pr_debug("copy_from_user failed while attempting to read uverbs command header: %p. %zd bytes\n",
+			 user_hdr, sizeof(hdr));
+		return -EFAULT;
+	}
+
+	if (hdr.length > IB_UVERBS_MAX_CMD_SZ || hdr.length <= sizeof(hdr)) {
+		pr_debug("invalid uverbs ioctl command length %d\n", hdr.length);
+		return -EINVAL;
+	}
+
+	/* currently there are no flags supported */
+	if (hdr.flags || hdr.reserved)
+		return -EOPNOTSUPP;
+
+	return ib_uverbs_cmd_verbs(filp, &hdr,
+				   (__user void *)arg + sizeof(hdr));
 }
 
 static int ib_uverbs_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/include/rdma/ib_ioctl.h b/include/rdma/ib_ioctl.h
index 2d8db54..c1806ef 100644
--- a/include/rdma/ib_ioctl.h
+++ b/include/rdma/ib_ioctl.h
@@ -35,4 +35,15 @@
 
 #include <uapi/rdma/ib_user_ioctl.h>
 
+#define IB_UVERBS_MANDATORY_FIELDS(mandatory_bitmap)		\
+	{.bitmap = (unsigned long []){(mandatory_bitmap)} }
+
+#define IB_UVERBS_MAX_ATTRS(max_attr)				\
+	((max_attr) + IB_UVERBS_CHECK_MAX(max_attr) -		\
+		IB_UVERBS_CHECK_MAX(max_attr))
+
+#define IB_UVERBS_MAX_SUPPORTED_ATTRS	(BITS_PER_LONG)
+#define IB_UVERBS_CHECK_MAX(val) \
+	(sizeof(char[1 - 2 * !!((val) > IB_UVERBS_MAX_SUPPORTED_ATTRS)]))
+
 #endif /* IB_IOCTL_H */
diff --git a/include/uapi/rdma/ib_user_ioctl.h b/include/uapi/rdma/ib_user_ioctl.h
index f17e51a..3edb623 100644
--- a/include/uapi/rdma/ib_user_ioctl.h
+++ b/include/uapi/rdma/ib_user_ioctl.h
@@ -36,10 +36,71 @@
 #include <linux/types.h>
 #include <linux/ioctl.h>
 
+struct ib_uverbs_uptr {
+	__u64 ptr;
+	__u32 len;
+};
+
+struct ib_uverbs_ioctl_hdr {
+	__u32 length;
+	__u16 flags;
+	__u16 object_type;
+	__u16 reserved;
+	__u16 action;
+	__u32 user_handler;
+	/*
+	 * These fields represent core response only,
+	 * provider's response is given as a netlink attribute.
+	 */
+	struct ib_uverbs_uptr resp;
+};
+
+enum ib_uverbs_object_type {
+	IB_OBJ_TYPE_OBJECT, /* query supported types */
+	IB_OBJ_TYPE_DEVICE,
+	IB_OBJ_TYPE_QP,
+	IB_OBJ_TYPE_CQ,
+	IB_OBJ_TYPE_PD,
+	IB_OBJ_TYPE_MR,
+	IB_OBJ_TYPE_MW,
+	IB_OBJ_TYPE_FLOW,
+	IB_OBJ_TYPE_MAX
+};
+
+enum ib_uverbs_object_type_flags {
+	/* vendor flag should go here */
+	IB_UVERBS_OBJECT_TYPE_FLAGS_MAX = 1 << 0,
+};
+
+enum ib_uverbs_common_actions {
+	IBNL_OBJECT_CREATE,
+	IBNL_OBJECT_DESTROY,
+	IBNL_OBJECT_QUERY,
+	IBNL_OBJECT_MODIFY,
+	IBNL_OBJECT_MAX = 8
+};
+
+/* Couldn't be extended! */
+enum ibnl_vendor_attrs {
+	IBNL_PROVIDER_CMD_UPTR,
+	IBNL_PROVIDER_RESP_UPTR,
+	IBNL_VENDOR_ATTRS_MAX
+};
+
+enum ib_uverbs_common_resp_types {
+	IBNL_RESPONSE_TYPE_RESP,
+	IBNL_RESPONSE_TYPE_VENDOR,
+	IBNL_RESPONSE_TYPE_MAX = 8
+};
+
 #define IB_IOCTL_MAGIC		0x1b
 
+#define IB_CMD_VERBS		0x1
 #define IB_CMD_DIRECT		0x2
 
+#define IB_IOCTL_VERBS \
+	_IOWR(IB_IOCTL_MAGIC, IB_CMD_VERBS, struct ib_uverbs_ioctl_hdr)
+
 #define IB_IOCTL_DIRECT \
 	_IOWR(IB_IOCTL_MAGIC, IB_CMD_DIRECT, unsigned long)
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC ABI 6/8] IB/core: Add outptr to udata in order to track the output size

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Sometimes, a netlink attribute describes a response buffer.
For example, when a vendor writes a response, the uptr netlink
attribute in the core response describe the vendor's response length.
Similarly, the core response length is described by its header, which
is actually a netsted netlink attribute.
Therefore, we need to have a way to count how much data was written
to the udata. Adding an outptr to track this data.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/uverbs.h | 3 +++
 include/rdma/ib_verbs.h          | 8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index a46cb29..c8d8700 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -46,11 +46,13 @@
 #include <rdma/ib_verbs.h>
 #include <rdma/ib_umem.h>
 #include <rdma/ib_user_verbs.h>
+#include <rdma/ib_user_ioctl.h>
 
 #define INIT_UDATA(udata, ibuf, obuf, ilen, olen)			\
 	do {								\
 		(udata)->inbuf  = (const void __user *) (ibuf);		\
 		(udata)->outbuf = (void __user *) (obuf);		\
+		(udata)->outptr = (void __user *)(obuf);		\
 		(udata)->inlen  = (ilen);				\
 		(udata)->outlen = (olen);				\
 	} while (0)
@@ -59,6 +61,7 @@
 	do {									\
 		(udata)->inbuf  = (ilen) ? (const void __user *) (ibuf) : NULL;	\
 		(udata)->outbuf = (olen) ? (void __user *) (obuf) : NULL;	\
+		(udata)->outptr = (olen) ? (void __user *)(obuf) : NULL;	\
 		(udata)->inlen  = (ilen);					\
 		(udata)->outlen = (olen);					\
 	} while (0)
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 1904c02..f5275b9 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1360,6 +1360,7 @@ struct ib_uobject {
 struct ib_udata {
 	const void __user *inbuf;
 	void __user *outbuf;
+	void __user *outptr;
 	size_t       inlen;
 	size_t       outlen;
 };
@@ -1990,7 +1991,12 @@ static inline int ib_copy_from_udata(void *dest, struct ib_udata *udata, size_t
 
 static inline int ib_copy_to_udata(struct ib_udata *udata, void *src, size_t len)
 {
-	return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0;
+	int ret = copy_to_user(udata->outptr, src, len) ? -EFAULT : 0;
+
+	if (!ret)
+		udata->outptr += len;
+
+	return ret;
 }
 
 static inline bool ib_is_udata_cleared(struct ib_udata *udata,
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC ABI 7/8] IB/core: Refactor idr to a shared file

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Currently, there are two code paths that could get user-space object
handles and use them. In order to share code and avoid duplications,
we export the idr management to an external file.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/Makefile            |   2 +-
 drivers/infiniband/core/uverbs.h            |  44 ++-
 drivers/infiniband/core/uverbs_cmd.c        | 558 ++++++++--------------------
 drivers/infiniband/core/uverbs_cmd_common.c | 280 ++++++++++++++
 drivers/infiniband/core/uverbs_main.c       |  18 +-
 5 files changed, 491 insertions(+), 411 deletions(-)
 create mode 100644 drivers/infiniband/core/uverbs_cmd_common.c

diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index 3d8a423..2d976e3f 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -35,4 +35,4 @@ ib_umad-y :=			user_mad.o
 ib_ucm-y :=			ucm.o
 
 ib_uverbs-y :=			uverbs_main.o uverbs_cmd.o uverbs_marshall.o \
-				uverbs_nl.o
+				uverbs_nl.o uverbs_cmd_common.o
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index c8d8700..1f121dc 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -193,8 +193,6 @@ extern struct idr ib_uverbs_srq_idr;
 extern struct idr ib_uverbs_xrcd_idr;
 extern struct idr ib_uverbs_rule_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,
 					struct ib_device *ib_dev,
 					int is_async);
@@ -233,6 +231,48 @@ struct ib_uverbs_flow_spec {
 	};
 };
 
+struct uverbs_lock_class {
+	struct lock_class_key	key;
+	char			name[16];
+};
+
+extern struct uverbs_lock_class pd_lock_class;
+extern struct uverbs_lock_class mr_lock_class;
+extern struct uverbs_lock_class mw_lock_class;
+extern struct uverbs_lock_class cq_lock_class;
+extern struct uverbs_lock_class qp_lock_class;
+extern struct uverbs_lock_class ah_lock_class;
+extern struct uverbs_lock_class srq_lock_class;
+extern struct uverbs_lock_class xrcd_lock_class;
+extern struct uverbs_lock_class rule_lock_class;
+
+void ib_init_uobj(struct ib_uobject *uobj, u64 user_handle,
+		  struct ib_ucontext *context, struct uverbs_lock_class *c);
+void ib_put_uobj(struct ib_uobject *uobj);
+void ib_put_uobj_read(struct ib_uobject *uobj);
+void ib_put_uobj_write(struct ib_uobject *uobj);
+int ib_idr_add_uobj(struct idr *idr, struct ib_uobject *uobj);
+void ib_idr_remove_uobj(struct idr *idr, struct ib_uobject *uobj);
+struct ib_uobject *ib_idr_read_uobj(struct idr *idr, int id,
+				    struct ib_ucontext *context, int nested);
+struct ib_uobject *ib_idr_write_uobj(struct idr *idr, int id,
+				     struct ib_ucontext *context);
+struct ib_pd *ib_idr_read_pd(int pd_handle, struct ib_ucontext *context);
+void ib_put_read_pd(struct ib_pd *pd);
+struct ib_cq *ib_idr_read_cq(int cq_handle, struct ib_ucontext *context, int nested);
+void ib_put_read_cq(struct ib_cq *cq);
+struct ib_ah *ib_idr_read_ah(int ah_handle, struct ib_ucontext *context);
+void ib_put_read_ah(struct ib_ah *ah);
+struct ib_qp *ib_idr_read_qp(int qp_handle, struct ib_ucontext *context);
+struct ib_qp *ib_idr_write_qp(int qp_handle, struct ib_ucontext *context);
+void ib_put_read_qp(struct ib_qp *qp);
+void ib_put_write_qp(struct ib_qp *qp);
+struct ib_srq *ib_idr_read_srq(int srq_handle, struct ib_ucontext *context);
+void ib_put_read_srq(struct ib_srq *srq);
+struct ib_xrcd *ib_idr_read_xrcd(int xrcd_handle, struct ib_ucontext *context,
+				 struct ib_uobject **uobj);
+void ib_put_xrcd_read(struct ib_uobject *uobj);
+
 #define IB_UVERBS_DECLARE_CMD(name)					\
 	ssize_t ib_uverbs_##name(struct ib_uverbs_file *file,		\
 				 struct ib_device *ib_dev,              \
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 03e39c2..c5a31b3 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -43,246 +43,6 @@
 #include "uverbs.h"
 #include "core_priv.h"
 
-struct uverbs_lock_class {
-	struct lock_class_key	key;
-	char			name[16];
-};
-
-static struct uverbs_lock_class pd_lock_class	= { .name = "PD-uobj" };
-static struct uverbs_lock_class mr_lock_class	= { .name = "MR-uobj" };
-static struct uverbs_lock_class mw_lock_class	= { .name = "MW-uobj" };
-static struct uverbs_lock_class cq_lock_class	= { .name = "CQ-uobj" };
-static struct uverbs_lock_class qp_lock_class	= { .name = "QP-uobj" };
-static struct uverbs_lock_class ah_lock_class	= { .name = "AH-uobj" };
-static struct uverbs_lock_class srq_lock_class	= { .name = "SRQ-uobj" };
-static struct uverbs_lock_class xrcd_lock_class = { .name = "XRCD-uobj" };
-static struct uverbs_lock_class rule_lock_class = { .name = "RULE-uobj" };
-
-/*
- * The ib_uobject locking scheme is as follows:
- *
- * - ib_uverbs_idr_lock protects the uverbs idrs themselves, so it
- *   needs to be held during all idr write operations.  When an object is
- *   looked up, a reference must be taken on the object's kref before
- *   dropping this lock.  For read operations, the rcu_read_lock()
- *   and rcu_write_lock() but similarly the kref reference is grabbed
- *   before the rcu_read_unlock().
- *
- * - Each object also has an rwsem.  This rwsem must be held for
- *   reading while an operation that uses the object is performed.
- *   For example, while registering an MR, the associated PD's
- *   uobject.mutex must be held for reading.  The rwsem must be held
- *   for writing while initializing or destroying an object.
- *
- * - In addition, each object has a "live" flag.  If this flag is not
- *   set, then lookups of the object will fail even if it is found in
- *   the idr.  This handles a reader that blocks and does not acquire
- *   the rwsem until after the object is destroyed.  The destroy
- *   operation will set the live flag to 0 and then drop the rwsem;
- *   this will allow the reader to acquire the rwsem, see that the
- *   live flag is 0, and then drop the rwsem and its reference to
- *   object.  The underlying storage will not be freed until the last
- *   reference to the object is dropped.
- */
-
-static void init_uobj(struct ib_uobject *uobj, u64 user_handle,
-		      struct ib_ucontext *context, struct uverbs_lock_class *c)
-{
-	uobj->user_handle = user_handle;
-	uobj->context     = context;
-	kref_init(&uobj->ref);
-	init_rwsem(&uobj->mutex);
-	lockdep_set_class_and_name(&uobj->mutex, &c->key, c->name);
-	uobj->live        = 0;
-}
-
-static void release_uobj(struct kref *kref)
-{
-	kfree_rcu(container_of(kref, struct ib_uobject, ref), rcu);
-}
-
-static void put_uobj(struct ib_uobject *uobj)
-{
-	kref_put(&uobj->ref, release_uobj);
-}
-
-static void put_uobj_read(struct ib_uobject *uobj)
-{
-	up_read(&uobj->mutex);
-	put_uobj(uobj);
-}
-
-static void put_uobj_write(struct ib_uobject *uobj)
-{
-	up_write(&uobj->mutex);
-	put_uobj(uobj);
-}
-
-static int idr_add_uobj(struct idr *idr, struct ib_uobject *uobj)
-{
-	int ret;
-
-	idr_preload(GFP_KERNEL);
-	spin_lock(&ib_uverbs_idr_lock);
-
-	ret = idr_alloc(idr, uobj, 0, 0, GFP_NOWAIT);
-	if (ret >= 0)
-		uobj->id = ret;
-
-	spin_unlock(&ib_uverbs_idr_lock);
-	idr_preload_end();
-
-	return ret < 0 ? ret : 0;
-}
-
-void idr_remove_uobj(struct idr *idr, struct ib_uobject *uobj)
-{
-	spin_lock(&ib_uverbs_idr_lock);
-	idr_remove(idr, uobj->id);
-	spin_unlock(&ib_uverbs_idr_lock);
-}
-
-static struct ib_uobject *__idr_get_uobj(struct idr *idr, int id,
-					 struct ib_ucontext *context)
-{
-	struct ib_uobject *uobj;
-
-	rcu_read_lock();
-	uobj = idr_find(idr, id);
-	if (uobj) {
-		if (uobj->context == context)
-			kref_get(&uobj->ref);
-		else
-			uobj = NULL;
-	}
-	rcu_read_unlock();
-
-	return uobj;
-}
-
-static struct ib_uobject *idr_read_uobj(struct idr *idr, int id,
-					struct ib_ucontext *context, int nested)
-{
-	struct ib_uobject *uobj;
-
-	uobj = __idr_get_uobj(idr, id, context);
-	if (!uobj)
-		return NULL;
-
-	if (nested)
-		down_read_nested(&uobj->mutex, SINGLE_DEPTH_NESTING);
-	else
-		down_read(&uobj->mutex);
-	if (!uobj->live) {
-		put_uobj_read(uobj);
-		return NULL;
-	}
-
-	return uobj;
-}
-
-static struct ib_uobject *idr_write_uobj(struct idr *idr, int id,
-					 struct ib_ucontext *context)
-{
-	struct ib_uobject *uobj;
-
-	uobj = __idr_get_uobj(idr, id, context);
-	if (!uobj)
-		return NULL;
-
-	down_write(&uobj->mutex);
-	if (!uobj->live) {
-		put_uobj_write(uobj);
-		return NULL;
-	}
-
-	return uobj;
-}
-
-static void *idr_read_obj(struct idr *idr, int id, struct ib_ucontext *context,
-			  int nested)
-{
-	struct ib_uobject *uobj;
-
-	uobj = idr_read_uobj(idr, id, context, nested);
-	return uobj ? uobj->object : NULL;
-}
-
-static struct ib_pd *idr_read_pd(int pd_handle, struct ib_ucontext *context)
-{
-	return idr_read_obj(&ib_uverbs_pd_idr, pd_handle, context, 0);
-}
-
-static void put_pd_read(struct ib_pd *pd)
-{
-	put_uobj_read(pd->uobject);
-}
-
-static struct ib_cq *idr_read_cq(int cq_handle, struct ib_ucontext *context, int nested)
-{
-	return idr_read_obj(&ib_uverbs_cq_idr, cq_handle, context, nested);
-}
-
-static void put_cq_read(struct ib_cq *cq)
-{
-	put_uobj_read(cq->uobject);
-}
-
-static struct ib_ah *idr_read_ah(int ah_handle, struct ib_ucontext *context)
-{
-	return idr_read_obj(&ib_uverbs_ah_idr, ah_handle, context, 0);
-}
-
-static void put_ah_read(struct ib_ah *ah)
-{
-	put_uobj_read(ah->uobject);
-}
-
-static struct ib_qp *idr_read_qp(int qp_handle, struct ib_ucontext *context)
-{
-	return idr_read_obj(&ib_uverbs_qp_idr, qp_handle, context, 0);
-}
-
-static struct ib_qp *idr_write_qp(int qp_handle, struct ib_ucontext *context)
-{
-	struct ib_uobject *uobj;
-
-	uobj = idr_write_uobj(&ib_uverbs_qp_idr, qp_handle, context);
-	return uobj ? uobj->object : NULL;
-}
-
-static void put_qp_read(struct ib_qp *qp)
-{
-	put_uobj_read(qp->uobject);
-}
-
-static void put_qp_write(struct ib_qp *qp)
-{
-	put_uobj_write(qp->uobject);
-}
-
-static struct ib_srq *idr_read_srq(int srq_handle, struct ib_ucontext *context)
-{
-	return idr_read_obj(&ib_uverbs_srq_idr, srq_handle, context, 0);
-}
-
-static void put_srq_read(struct ib_srq *srq)
-{
-	put_uobj_read(srq->uobject);
-}
-
-static struct ib_xrcd *idr_read_xrcd(int xrcd_handle, struct ib_ucontext *context,
-				     struct ib_uobject **uobj)
-{
-	*uobj = idr_read_uobj(&ib_uverbs_xrcd_idr, xrcd_handle, context, 0);
-	return *uobj ? (*uobj)->object : NULL;
-}
-
-static void put_xrcd_read(struct ib_uobject *uobj)
-{
-	put_uobj_read(uobj);
-}
-
 ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 			      struct ib_device *ib_dev,
 			      const char __user *buf,
@@ -535,7 +295,7 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	if (!uobj)
 		return -ENOMEM;
 
-	init_uobj(uobj, 0, file->ucontext, &pd_lock_class);
+	ib_init_uobj(uobj, 0, file->ucontext, &pd_lock_class);
 	down_write(&uobj->mutex);
 
 	pd = ib_dev->alloc_pd(ib_dev, file->ucontext, &udata);
@@ -550,7 +310,7 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	atomic_set(&pd->usecnt, 0);
 
 	uobj->object = pd;
-	ret = idr_add_uobj(&ib_uverbs_pd_idr, uobj);
+	ret = ib_idr_add_uobj(&ib_uverbs_pd_idr, uobj);
 	if (ret)
 		goto err_idr;
 
@@ -574,13 +334,13 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	return in_len;
 
 err_copy:
-	idr_remove_uobj(&ib_uverbs_pd_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_pd_idr, uobj);
 
 err_idr:
 	ib_dealloc_pd(pd);
 
 err:
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 	return ret;
 }
 
@@ -597,7 +357,7 @@ ssize_t ib_uverbs_dealloc_pd(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	uobj = idr_write_uobj(&ib_uverbs_pd_idr, cmd.pd_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_pd_idr, cmd.pd_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 	pd = uobj->object;
@@ -613,20 +373,20 @@ ssize_t ib_uverbs_dealloc_pd(struct ib_uverbs_file *file,
 		goto err_put;
 
 	uobj->live = 0;
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
-	idr_remove_uobj(&ib_uverbs_pd_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_pd_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
 	mutex_unlock(&file->mutex);
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	return in_len;
 
 err_put:
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 	return ret;
 }
 
@@ -770,7 +530,7 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 		goto err_tree_mutex_unlock;
 	}
 
-	init_uobj(&obj->uobject, 0, file->ucontext, &xrcd_lock_class);
+	ib_init_uobj(&obj->uobject, 0, file->ucontext, &xrcd_lock_class);
 
 	down_write(&obj->uobject.mutex);
 
@@ -791,7 +551,7 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 
 	atomic_set(&obj->refcnt, 0);
 	obj->uobject.object = xrcd;
-	ret = idr_add_uobj(&ib_uverbs_xrcd_idr, &obj->uobject);
+	ret = ib_idr_add_uobj(&ib_uverbs_xrcd_idr, &obj->uobject);
 	if (ret)
 		goto err_idr;
 
@@ -835,13 +595,13 @@ err_copy:
 	}
 
 err_insert_xrcd:
-	idr_remove_uobj(&ib_uverbs_xrcd_idr, &obj->uobject);
+	ib_idr_remove_uobj(&ib_uverbs_xrcd_idr, &obj->uobject);
 
 err_idr:
 	ib_dealloc_xrcd(xrcd);
 
 err:
-	put_uobj_write(&obj->uobject);
+	ib_put_uobj_write(&obj->uobject);
 
 err_tree_mutex_unlock:
 	if (f.file)
@@ -869,7 +629,7 @@ ssize_t ib_uverbs_close_xrcd(struct ib_uverbs_file *file,
 		return -EFAULT;
 
 	mutex_lock(&file->device->xrcd_tree_mutex);
-	uobj = idr_write_uobj(&ib_uverbs_xrcd_idr, cmd.xrcd_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_xrcd_idr, cmd.xrcd_handle, file->ucontext);
 	if (!uobj) {
 		ret = -EINVAL;
 		goto out;
@@ -879,7 +639,7 @@ ssize_t ib_uverbs_close_xrcd(struct ib_uverbs_file *file,
 	inode = xrcd->inode;
 	obj   = container_of(uobj, struct ib_uxrcd_object, uobject);
 	if (atomic_read(&obj->refcnt)) {
-		put_uobj_write(uobj);
+		ib_put_uobj_write(uobj);
 		ret = -EBUSY;
 		goto out;
 	}
@@ -894,7 +654,7 @@ ssize_t ib_uverbs_close_xrcd(struct ib_uverbs_file *file,
 	if (inode && ret)
 		atomic_inc(&xrcd->usecnt);
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
 	if (ret)
 		goto out;
@@ -902,12 +662,12 @@ ssize_t ib_uverbs_close_xrcd(struct ib_uverbs_file *file,
 	if (inode && !live)
 		xrcd_table_delete(file->device, inode);
 
-	idr_remove_uobj(&ib_uverbs_xrcd_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_xrcd_idr, uobj);
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
 	mutex_unlock(&file->mutex);
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 	ret = in_len;
 
 out:
@@ -964,10 +724,10 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	if (!uobj)
 		return -ENOMEM;
 
-	init_uobj(uobj, 0, file->ucontext, &mr_lock_class);
+	ib_init_uobj(uobj, 0, file->ucontext, &mr_lock_class);
 	down_write(&uobj->mutex);
 
-	pd = idr_read_pd(cmd.pd_handle, file->ucontext);
+	pd = ib_idr_read_pd(cmd.pd_handle, file->ucontext);
 	if (!pd) {
 		ret = -EINVAL;
 		goto err_free;
@@ -995,7 +755,7 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	atomic_inc(&pd->usecnt);
 
 	uobj->object = mr;
-	ret = idr_add_uobj(&ib_uverbs_mr_idr, uobj);
+	ret = ib_idr_add_uobj(&ib_uverbs_mr_idr, uobj);
 	if (ret)
 		goto err_unreg;
 
@@ -1010,7 +770,7 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 		goto err_copy;
 	}
 
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 	mutex_lock(&file->mutex);
 	list_add_tail(&uobj->list, &file->ucontext->mr_list);
@@ -1023,16 +783,16 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	return in_len;
 
 err_copy:
-	idr_remove_uobj(&ib_uverbs_mr_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_mr_idr, uobj);
 
 err_unreg:
 	ib_dereg_mr(mr);
 
 err_put:
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 err_free:
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 	return ret;
 }
 
@@ -1068,8 +828,8 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 	     (cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK)))
 			return -EINVAL;
 
-	uobj = idr_write_uobj(&ib_uverbs_mr_idr, cmd.mr_handle,
-			      file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_mr_idr, cmd.mr_handle,
+				 file->ucontext);
 
 	if (!uobj)
 		return -EINVAL;
@@ -1079,14 +839,14 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 	if (cmd.flags & IB_MR_REREG_ACCESS) {
 		ret = ib_check_mr_access(cmd.access_flags);
 		if (ret)
-			goto put_uobjs;
+			goto ib_put_uobjs;
 	}
 
 	if (cmd.flags & IB_MR_REREG_PD) {
-		pd = idr_read_pd(cmd.pd_handle, file->ucontext);
+		pd = ib_idr_read_pd(cmd.pd_handle, file->ucontext);
 		if (!pd) {
 			ret = -EINVAL;
-			goto put_uobjs;
+			goto ib_put_uobjs;
 		}
 	}
 
@@ -1101,7 +861,7 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 			atomic_dec(&old_pd->usecnt);
 		}
 	} else {
-		goto put_uobj_pd;
+		goto ib_put_uobj_pd;
 	}
 
 	memset(&resp, 0, sizeof(resp));
@@ -1114,13 +874,13 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 	else
 		ret = in_len;
 
-put_uobj_pd:
+ib_put_uobj_pd:
 	if (cmd.flags & IB_MR_REREG_PD)
-		put_pd_read(pd);
+		ib_put_read_pd(pd);
 
-put_uobjs:
+ib_put_uobjs:
 
-	put_uobj_write(mr->uobject);
+	ib_put_uobj_write(mr->uobject);
 
 	return ret;
 }
@@ -1138,7 +898,7 @@ ssize_t ib_uverbs_dereg_mr(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	uobj = idr_write_uobj(&ib_uverbs_mr_idr, cmd.mr_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_mr_idr, cmd.mr_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 
@@ -1148,18 +908,18 @@ ssize_t ib_uverbs_dereg_mr(struct ib_uverbs_file *file,
 	if (!ret)
 		uobj->live = 0;
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
 	if (ret)
 		return ret;
 
-	idr_remove_uobj(&ib_uverbs_mr_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_mr_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
 	mutex_unlock(&file->mutex);
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	return in_len;
 }
@@ -1187,10 +947,10 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	if (!uobj)
 		return -ENOMEM;
 
-	init_uobj(uobj, 0, file->ucontext, &mw_lock_class);
+	ib_init_uobj(uobj, 0, file->ucontext, &mw_lock_class);
 	down_write(&uobj->mutex);
 
-	pd = idr_read_pd(cmd.pd_handle, file->ucontext);
+	pd = ib_idr_read_pd(cmd.pd_handle, file->ucontext);
 	if (!pd) {
 		ret = -EINVAL;
 		goto err_free;
@@ -1213,7 +973,7 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	atomic_inc(&pd->usecnt);
 
 	uobj->object = mw;
-	ret = idr_add_uobj(&ib_uverbs_mw_idr, uobj);
+	ret = ib_idr_add_uobj(&ib_uverbs_mw_idr, uobj);
 	if (ret)
 		goto err_unalloc;
 
@@ -1227,7 +987,7 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 		goto err_copy;
 	}
 
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 	mutex_lock(&file->mutex);
 	list_add_tail(&uobj->list, &file->ucontext->mw_list);
@@ -1240,16 +1000,16 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	return in_len;
 
 err_copy:
-	idr_remove_uobj(&ib_uverbs_mw_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_mw_idr, uobj);
 
 err_unalloc:
 	uverbs_dealloc_mw(mw);
 
 err_put:
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 err_free:
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 	return ret;
 }
 
@@ -1266,7 +1026,7 @@ ssize_t ib_uverbs_dealloc_mw(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
-	uobj = idr_write_uobj(&ib_uverbs_mw_idr, cmd.mw_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_mw_idr, cmd.mw_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 
@@ -1276,18 +1036,18 @@ ssize_t ib_uverbs_dealloc_mw(struct ib_uverbs_file *file,
 	if (!ret)
 		uobj->live = 0;
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
 	if (ret)
 		return ret;
 
-	idr_remove_uobj(&ib_uverbs_mw_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_mw_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
 	mutex_unlock(&file->mutex);
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	return in_len;
 }
@@ -1357,7 +1117,7 @@ static struct ib_ucq_object *create_cq(struct ib_uverbs_file *file,
 	if (!obj)
 		return ERR_PTR(-ENOMEM);
 
-	init_uobj(&obj->uobject, cmd->user_handle, file->ucontext, &cq_lock_class);
+	ib_init_uobj(&obj->uobject, cmd->user_handle, file->ucontext, &cq_lock_class);
 	down_write(&obj->uobject.mutex);
 
 	if (cmd->comp_channel >= 0) {
@@ -1395,7 +1155,7 @@ static struct ib_ucq_object *create_cq(struct ib_uverbs_file *file,
 	atomic_set(&cq->usecnt, 0);
 
 	obj->uobject.object = cq;
-	ret = idr_add_uobj(&ib_uverbs_cq_idr, &obj->uobject);
+	ret = ib_idr_add_uobj(&ib_uverbs_cq_idr, &obj->uobject);
 	if (ret)
 		goto err_free;
 
@@ -1421,7 +1181,7 @@ static struct ib_ucq_object *create_cq(struct ib_uverbs_file *file,
 	return obj;
 
 err_cb:
-	idr_remove_uobj(&ib_uverbs_cq_idr, &obj->uobject);
+	ib_idr_remove_uobj(&ib_uverbs_cq_idr, &obj->uobject);
 
 err_free:
 	ib_destroy_cq(cq);
@@ -1431,7 +1191,7 @@ err_file:
 		ib_uverbs_release_ucq(file, ev_file, obj);
 
 err:
-	put_uobj_write(&obj->uobject);
+	ib_put_uobj_write(&obj->uobject);
 
 	return ERR_PTR(ret);
 }
@@ -1554,7 +1314,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 		   (unsigned long) cmd.response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
-	cq = idr_read_cq(cmd.cq_handle, file->ucontext, 0);
+	cq = ib_idr_read_cq(cmd.cq_handle, file->ucontext, 0);
 	if (!cq)
 		return -EINVAL;
 
@@ -1569,7 +1329,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 		ret = -EFAULT;
 
 out:
-	put_cq_read(cq);
+	ib_put_read_cq(cq);
 
 	return ret ? ret : in_len;
 }
@@ -1616,7 +1376,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	cq = idr_read_cq(cmd.cq_handle, file->ucontext, 0);
+	cq = ib_idr_read_cq(cmd.cq_handle, file->ucontext, 0);
 	if (!cq)
 		return -EINVAL;
 
@@ -1648,7 +1408,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	ret = in_len;
 
 out_put:
-	put_cq_read(cq);
+	ib_put_read_cq(cq);
 	return ret;
 }
 
@@ -1663,14 +1423,14 @@ ssize_t ib_uverbs_req_notify_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	cq = idr_read_cq(cmd.cq_handle, file->ucontext, 0);
+	cq = ib_idr_read_cq(cmd.cq_handle, file->ucontext, 0);
 	if (!cq)
 		return -EINVAL;
 
 	ib_req_notify_cq(cq, cmd.solicited_only ?
 			 IB_CQ_SOLICITED : IB_CQ_NEXT_COMP);
 
-	put_cq_read(cq);
+	ib_put_read_cq(cq);
 
 	return in_len;
 }
@@ -1691,7 +1451,7 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	uobj = idr_write_uobj(&ib_uverbs_cq_idr, cmd.cq_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_cq_idr, cmd.cq_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 	cq      = uobj->object;
@@ -1702,12 +1462,12 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	if (!ret)
 		uobj->live = 0;
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
 	if (ret)
 		return ret;
 
-	idr_remove_uobj(&ib_uverbs_cq_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_cq_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
@@ -1719,7 +1479,7 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	resp.comp_events_reported  = obj->comp_events_reported;
 	resp.async_events_reported = obj->async_events_reported;
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	if (copy_to_user((void __user *) (unsigned long) cmd.response,
 			 &resp, sizeof resp))
@@ -1758,13 +1518,13 @@ static int create_qp(struct ib_uverbs_file *file,
 	if (!obj)
 		return -ENOMEM;
 
-	init_uobj(&obj->uevent.uobject, cmd->user_handle, file->ucontext,
-		  &qp_lock_class);
+	ib_init_uobj(&obj->uevent.uobject, cmd->user_handle, file->ucontext,
+		     &qp_lock_class);
 	down_write(&obj->uevent.uobject.mutex);
 
 	if (cmd->qp_type == IB_QPT_XRC_TGT) {
-		xrcd = idr_read_xrcd(cmd->pd_handle, file->ucontext,
-				     &xrcd_uobj);
+		xrcd = ib_idr_read_xrcd(cmd->pd_handle, file->ucontext,
+					&xrcd_uobj);
 		if (!xrcd) {
 			ret = -EINVAL;
 			goto err_put;
@@ -1776,8 +1536,8 @@ static int create_qp(struct ib_uverbs_file *file,
 			cmd->max_recv_sge = 0;
 		} else {
 			if (cmd->is_srq) {
-				srq = idr_read_srq(cmd->srq_handle,
-						   file->ucontext);
+				srq = ib_idr_read_srq(cmd->srq_handle,
+						      file->ucontext);
 				if (!srq || srq->srq_type != IB_SRQT_BASIC) {
 					ret = -EINVAL;
 					goto err_put;
@@ -1785,8 +1545,8 @@ static int create_qp(struct ib_uverbs_file *file,
 			}
 
 			if (cmd->recv_cq_handle != cmd->send_cq_handle) {
-				rcq = idr_read_cq(cmd->recv_cq_handle,
-						  file->ucontext, 0);
+				rcq = ib_idr_read_cq(cmd->recv_cq_handle,
+						     file->ucontext, 0);
 				if (!rcq) {
 					ret = -EINVAL;
 					goto err_put;
@@ -1794,9 +1554,9 @@ static int create_qp(struct ib_uverbs_file *file,
 			}
 		}
 
-		scq = idr_read_cq(cmd->send_cq_handle, file->ucontext, !!rcq);
+		scq = ib_idr_read_cq(cmd->send_cq_handle, file->ucontext, !!rcq);
 		rcq = rcq ?: scq;
-		pd  = idr_read_pd(cmd->pd_handle, file->ucontext);
+		pd  = ib_idr_read_pd(cmd->pd_handle, file->ucontext);
 		if (!pd || !scq) {
 			ret = -EINVAL;
 			goto err_put;
@@ -1878,7 +1638,7 @@ static int create_qp(struct ib_uverbs_file *file,
 	qp->uobject = &obj->uevent.uobject;
 
 	obj->uevent.uobject.object = qp;
-	ret = idr_add_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
+	ret = ib_idr_add_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
 	if (ret)
 		goto err_destroy;
 
@@ -1902,17 +1662,17 @@ static int create_qp(struct ib_uverbs_file *file,
 		obj->uxrcd = container_of(xrcd_uobj, struct ib_uxrcd_object,
 					  uobject);
 		atomic_inc(&obj->uxrcd->refcnt);
-		put_xrcd_read(xrcd_uobj);
+		ib_put_xrcd_read(xrcd_uobj);
 	}
 
 	if (pd)
-		put_pd_read(pd);
+		ib_put_read_pd(pd);
 	if (scq)
-		put_cq_read(scq);
+		ib_put_read_cq(scq);
 	if (rcq && rcq != scq)
-		put_cq_read(rcq);
+		ib_put_read_cq(rcq);
 	if (srq)
-		put_srq_read(srq);
+		ib_put_read_srq(srq);
 
 	mutex_lock(&file->mutex);
 	list_add_tail(&obj->uevent.uobject.list, &file->ucontext->qp_list);
@@ -1924,24 +1684,24 @@ static int create_qp(struct ib_uverbs_file *file,
 
 	return 0;
 err_cb:
-	idr_remove_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
+	ib_idr_remove_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
 
 err_destroy:
 	ib_destroy_qp(qp);
 
 err_put:
 	if (xrcd)
-		put_xrcd_read(xrcd_uobj);
+		ib_put_xrcd_read(xrcd_uobj);
 	if (pd)
-		put_pd_read(pd);
+		ib_put_read_pd(pd);
 	if (scq)
-		put_cq_read(scq);
+		ib_put_read_cq(scq);
 	if (rcq && rcq != scq)
-		put_cq_read(rcq);
+		ib_put_read_cq(rcq);
 	if (srq)
-		put_srq_read(srq);
+		ib_put_read_srq(srq);
 
-	put_uobj_write(&obj->uevent.uobject);
+	ib_put_uobj_write(&obj->uevent.uobject);
 	return ret;
 }
 
@@ -2081,10 +1841,10 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	if (!obj)
 		return -ENOMEM;
 
-	init_uobj(&obj->uevent.uobject, cmd.user_handle, file->ucontext, &qp_lock_class);
+	ib_init_uobj(&obj->uevent.uobject, cmd.user_handle, file->ucontext, &qp_lock_class);
 	down_write(&obj->uevent.uobject.mutex);
 
-	xrcd = idr_read_xrcd(cmd.pd_handle, file->ucontext, &xrcd_uobj);
+	xrcd = ib_idr_read_xrcd(cmd.pd_handle, file->ucontext, &xrcd_uobj);
 	if (!xrcd) {
 		ret = -EINVAL;
 		goto err_put;
@@ -2108,7 +1868,7 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	qp->uobject = &obj->uevent.uobject;
 
 	obj->uevent.uobject.object = qp;
-	ret = idr_add_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
+	ret = ib_idr_add_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
 	if (ret)
 		goto err_destroy;
 
@@ -2124,7 +1884,7 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 
 	obj->uxrcd = container_of(xrcd_uobj, struct ib_uxrcd_object, uobject);
 	atomic_inc(&obj->uxrcd->refcnt);
-	put_xrcd_read(xrcd_uobj);
+	ib_put_xrcd_read(xrcd_uobj);
 
 	mutex_lock(&file->mutex);
 	list_add_tail(&obj->uevent.uobject.list, &file->ucontext->qp_list);
@@ -2137,14 +1897,14 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	return in_len;
 
 err_remove:
-	idr_remove_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
+	ib_idr_remove_uobj(&ib_uverbs_qp_idr, &obj->uevent.uobject);
 
 err_destroy:
 	ib_destroy_qp(qp);
 
 err_put:
-	put_xrcd_read(xrcd_uobj);
-	put_uobj_write(&obj->uevent.uobject);
+	ib_put_xrcd_read(xrcd_uobj);
+	ib_put_uobj_write(&obj->uevent.uobject);
 	return ret;
 }
 
@@ -2170,7 +1930,7 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 		goto out;
 	}
 
-	qp = idr_read_qp(cmd.qp_handle, file->ucontext);
+	qp = ib_idr_read_qp(cmd.qp_handle, file->ucontext);
 	if (!qp) {
 		ret = -EINVAL;
 		goto out;
@@ -2178,7 +1938,7 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 
 	ret = ib_query_qp(qp, attr, cmd.attr_mask, init_attr);
 
-	put_qp_read(qp);
+	ib_put_read_qp(qp);
 
 	if (ret)
 		goto out;
@@ -2284,7 +2044,7 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 	if (!attr)
 		return -ENOMEM;
 
-	qp = idr_read_qp(cmd.qp_handle, file->ucontext);
+	qp = ib_idr_read_qp(cmd.qp_handle, file->ucontext);
 	if (!qp) {
 		ret = -EINVAL;
 		goto out;
@@ -2352,7 +2112,7 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 	ret = in_len;
 
 release_qp:
-	put_qp_read(qp);
+	ib_put_read_qp(qp);
 
 out:
 	kfree(attr);
@@ -2377,14 +2137,14 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 
 	memset(&resp, 0, sizeof resp);
 
-	uobj = idr_write_uobj(&ib_uverbs_qp_idr, cmd.qp_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_qp_idr, cmd.qp_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 	qp  = uobj->object;
 	obj = container_of(uobj, struct ib_uqp_object, uevent.uobject);
 
 	if (!list_empty(&obj->mcast_list)) {
-		put_uobj_write(uobj);
+		ib_put_uobj_write(uobj);
 		return -EBUSY;
 	}
 
@@ -2392,7 +2152,7 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	if (!ret)
 		uobj->live = 0;
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
 	if (ret)
 		return ret;
@@ -2400,7 +2160,7 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	if (obj->uxrcd)
 		atomic_dec(&obj->uxrcd->refcnt);
 
-	idr_remove_uobj(&ib_uverbs_qp_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_qp_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
@@ -2410,7 +2170,7 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 
 	resp.events_reported = obj->uevent.events_reported;
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	if (copy_to_user((void __user *) (unsigned long) cmd.response,
 			 &resp, sizeof resp))
@@ -2454,7 +2214,7 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 	if (!user_wr)
 		return -ENOMEM;
 
-	qp = idr_read_qp(cmd.qp_handle, file->ucontext);
+	qp = ib_idr_read_qp(cmd.qp_handle, file->ucontext);
 	if (!qp)
 		goto out;
 
@@ -2490,7 +2250,7 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 				goto out_put;
 			}
 
-			ud->ah = idr_read_ah(user_wr->wr.ud.ah, file->ucontext);
+			ud->ah = ib_idr_read_ah(user_wr->wr.ud.ah, file->ucontext);
 			if (!ud->ah) {
 				kfree(ud);
 				ret = -EINVAL;
@@ -2597,11 +2357,11 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 		ret = -EFAULT;
 
 out_put:
-	put_qp_read(qp);
+	ib_put_read_qp(qp);
 
 	while (wr) {
 		if (is_ud && ud_wr(wr)->ah)
-			put_ah_read(ud_wr(wr)->ah);
+			ib_put_read_ah(ud_wr(wr)->ah);
 		next = wr->next;
 		kfree(wr);
 		wr = next;
@@ -2718,14 +2478,14 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 	if (IS_ERR(wr))
 		return PTR_ERR(wr);
 
-	qp = idr_read_qp(cmd.qp_handle, file->ucontext);
+	qp = ib_idr_read_qp(cmd.qp_handle, file->ucontext);
 	if (!qp)
 		goto out;
 
 	resp.bad_wr = 0;
 	ret = qp->device->post_recv(qp->real_qp, wr, &bad_wr);
 
-	put_qp_read(qp);
+	ib_put_read_qp(qp);
 
 	if (ret)
 		for (next = wr; next; next = next->next) {
@@ -2768,14 +2528,14 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 	if (IS_ERR(wr))
 		return PTR_ERR(wr);
 
-	srq = idr_read_srq(cmd.srq_handle, file->ucontext);
+	srq = ib_idr_read_srq(cmd.srq_handle, file->ucontext);
 	if (!srq)
 		goto out;
 
 	resp.bad_wr = 0;
 	ret = srq->device->post_srq_recv(srq, wr, &bad_wr);
 
-	put_srq_read(srq);
+	ib_put_read_srq(srq);
 
 	if (ret)
 		for (next = wr; next; next = next->next) {
@@ -2821,10 +2581,10 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 	if (!uobj)
 		return -ENOMEM;
 
-	init_uobj(uobj, cmd.user_handle, file->ucontext, &ah_lock_class);
+	ib_init_uobj(uobj, cmd.user_handle, file->ucontext, &ah_lock_class);
 	down_write(&uobj->mutex);
 
-	pd = idr_read_pd(cmd.pd_handle, file->ucontext);
+	pd = ib_idr_read_pd(cmd.pd_handle, file->ucontext);
 	if (!pd) {
 		ret = -EINVAL;
 		goto err;
@@ -2852,7 +2612,7 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 	ah->uobject  = uobj;
 	uobj->object = ah;
 
-	ret = idr_add_uobj(&ib_uverbs_ah_idr, uobj);
+	ret = ib_idr_add_uobj(&ib_uverbs_ah_idr, uobj);
 	if (ret)
 		goto err_destroy;
 
@@ -2864,7 +2624,7 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 		goto err_copy;
 	}
 
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 	mutex_lock(&file->mutex);
 	list_add_tail(&uobj->list, &file->ucontext->ah_list);
@@ -2877,16 +2637,16 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 	return in_len;
 
 err_copy:
-	idr_remove_uobj(&ib_uverbs_ah_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_ah_idr, uobj);
 
 err_destroy:
 	ib_destroy_ah(ah);
 
 err_put:
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 err:
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 	return ret;
 }
 
@@ -2902,7 +2662,7 @@ ssize_t ib_uverbs_destroy_ah(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	uobj = idr_write_uobj(&ib_uverbs_ah_idr, cmd.ah_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_ah_idr, cmd.ah_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 	ah = uobj->object;
@@ -2911,18 +2671,18 @@ ssize_t ib_uverbs_destroy_ah(struct ib_uverbs_file *file,
 	if (!ret)
 		uobj->live = 0;
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
 	if (ret)
 		return ret;
 
-	idr_remove_uobj(&ib_uverbs_ah_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_ah_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
 	mutex_unlock(&file->mutex);
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	return in_len;
 }
@@ -2941,7 +2701,7 @@ ssize_t ib_uverbs_attach_mcast(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	qp = idr_write_qp(cmd.qp_handle, file->ucontext);
+	qp = ib_idr_write_qp(cmd.qp_handle, file->ucontext);
 	if (!qp)
 		return -EINVAL;
 
@@ -2970,7 +2730,7 @@ ssize_t ib_uverbs_attach_mcast(struct ib_uverbs_file *file,
 		kfree(mcast);
 
 out_put:
-	put_qp_write(qp);
+	ib_put_write_qp(qp);
 
 	return ret ? ret : in_len;
 }
@@ -2989,7 +2749,7 @@ ssize_t ib_uverbs_detach_mcast(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	qp = idr_write_qp(cmd.qp_handle, file->ucontext);
+	qp = ib_idr_write_qp(cmd.qp_handle, file->ucontext);
 	if (!qp)
 		return -EINVAL;
 
@@ -3008,7 +2768,7 @@ ssize_t ib_uverbs_detach_mcast(struct ib_uverbs_file *file,
 		}
 
 out_put:
-	put_qp_write(qp);
+	ib_put_write_qp(qp);
 
 	return ret ? ret : in_len;
 }
@@ -3133,10 +2893,10 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
 		err = -ENOMEM;
 		goto err_free_attr;
 	}
-	init_uobj(uobj, 0, file->ucontext, &rule_lock_class);
+	ib_init_uobj(uobj, 0, file->ucontext, &rule_lock_class);
 	down_write(&uobj->mutex);
 
-	qp = idr_read_qp(cmd.qp_handle, file->ucontext);
+	qp = ib_idr_read_qp(cmd.qp_handle, file->ucontext);
 	if (!qp) {
 		err = -EINVAL;
 		goto err_uobj;
@@ -3185,7 +2945,7 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
 	flow_id->uobject = uobj;
 	uobj->object = flow_id;
 
-	err = idr_add_uobj(&ib_uverbs_rule_idr, uobj);
+	err = ib_idr_add_uobj(&ib_uverbs_rule_idr, uobj);
 	if (err)
 		goto destroy_flow;
 
@@ -3197,7 +2957,7 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
 	if (err)
 		goto err_copy;
 
-	put_qp_read(qp);
+	ib_put_read_qp(qp);
 	mutex_lock(&file->mutex);
 	list_add_tail(&uobj->list, &file->ucontext->rule_list);
 	mutex_unlock(&file->mutex);
@@ -3210,15 +2970,15 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
 		kfree(kern_flow_attr);
 	return 0;
 err_copy:
-	idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
 destroy_flow:
 	ib_destroy_flow(flow_id);
 err_free:
 	kfree(flow_attr);
 err_put:
-	put_qp_read(qp);
+	ib_put_read_qp(qp);
 err_uobj:
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 err_free_attr:
 	if (cmd.flow_attr.num_of_specs)
 		kfree(kern_flow_attr);
@@ -3245,8 +3005,8 @@ int ib_uverbs_ex_destroy_flow(struct ib_uverbs_file *file,
 	if (cmd.comp_mask)
 		return -EINVAL;
 
-	uobj = idr_write_uobj(&ib_uverbs_rule_idr, cmd.flow_handle,
-			      file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_rule_idr, cmd.flow_handle,
+				 file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 	flow_id = uobj->object;
@@ -3255,15 +3015,15 @@ int ib_uverbs_ex_destroy_flow(struct ib_uverbs_file *file,
 	if (!ret)
 		uobj->live = 0;
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
-	idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
 	mutex_unlock(&file->mutex);
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	return ret;
 }
@@ -3285,11 +3045,11 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 	if (!obj)
 		return -ENOMEM;
 
-	init_uobj(&obj->uevent.uobject, cmd->user_handle, file->ucontext, &srq_lock_class);
+	ib_init_uobj(&obj->uevent.uobject, cmd->user_handle, file->ucontext, &srq_lock_class);
 	down_write(&obj->uevent.uobject.mutex);
 
 	if (cmd->srq_type == IB_SRQT_XRC) {
-		attr.ext.xrc.xrcd  = idr_read_xrcd(cmd->xrcd_handle, file->ucontext, &xrcd_uobj);
+		attr.ext.xrc.xrcd  = ib_idr_read_xrcd(cmd->xrcd_handle, file->ucontext, &xrcd_uobj);
 		if (!attr.ext.xrc.xrcd) {
 			ret = -EINVAL;
 			goto err;
@@ -3298,14 +3058,14 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 		obj->uxrcd = container_of(xrcd_uobj, struct ib_uxrcd_object, uobject);
 		atomic_inc(&obj->uxrcd->refcnt);
 
-		attr.ext.xrc.cq  = idr_read_cq(cmd->cq_handle, file->ucontext, 0);
+		attr.ext.xrc.cq  = ib_idr_read_cq(cmd->cq_handle, file->ucontext, 0);
 		if (!attr.ext.xrc.cq) {
 			ret = -EINVAL;
 			goto err_put_xrcd;
 		}
 	}
 
-	pd  = idr_read_pd(cmd->pd_handle, file->ucontext);
+	pd  = ib_idr_read_pd(cmd->pd_handle, file->ucontext);
 	if (!pd) {
 		ret = -EINVAL;
 		goto err_put_cq;
@@ -3345,7 +3105,7 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 	atomic_set(&srq->usecnt, 0);
 
 	obj->uevent.uobject.object = srq;
-	ret = idr_add_uobj(&ib_uverbs_srq_idr, &obj->uevent.uobject);
+	ret = ib_idr_add_uobj(&ib_uverbs_srq_idr, &obj->uevent.uobject);
 	if (ret)
 		goto err_destroy;
 
@@ -3363,10 +3123,10 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 	}
 
 	if (cmd->srq_type == IB_SRQT_XRC) {
-		put_uobj_read(xrcd_uobj);
-		put_cq_read(attr.ext.xrc.cq);
+		ib_put_uobj_read(xrcd_uobj);
+		ib_put_read_cq(attr.ext.xrc.cq);
 	}
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 	mutex_lock(&file->mutex);
 	list_add_tail(&obj->uevent.uobject.list, &file->ucontext->srq_list);
@@ -3379,26 +3139,26 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 	return 0;
 
 err_copy:
-	idr_remove_uobj(&ib_uverbs_srq_idr, &obj->uevent.uobject);
+	ib_idr_remove_uobj(&ib_uverbs_srq_idr, &obj->uevent.uobject);
 
 err_destroy:
 	ib_destroy_srq(srq);
 
 err_put:
-	put_pd_read(pd);
+	ib_put_read_pd(pd);
 
 err_put_cq:
 	if (cmd->srq_type == IB_SRQT_XRC)
-		put_cq_read(attr.ext.xrc.cq);
+		ib_put_read_cq(attr.ext.xrc.cq);
 
 err_put_xrcd:
 	if (cmd->srq_type == IB_SRQT_XRC) {
 		atomic_dec(&obj->uxrcd->refcnt);
-		put_uobj_read(xrcd_uobj);
+		ib_put_uobj_read(xrcd_uobj);
 	}
 
 err:
-	put_uobj_write(&obj->uevent.uobject);
+	ib_put_uobj_write(&obj->uevent.uobject);
 	return ret;
 }
 
@@ -3483,7 +3243,7 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
 	INIT_UDATA(&udata, buf + sizeof cmd, NULL, in_len - sizeof cmd,
 		   out_len);
 
-	srq = idr_read_srq(cmd.srq_handle, file->ucontext);
+	srq = ib_idr_read_srq(cmd.srq_handle, file->ucontext);
 	if (!srq)
 		return -EINVAL;
 
@@ -3492,7 +3252,7 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
 
 	ret = srq->device->modify_srq(srq, &attr, cmd.attr_mask, &udata);
 
-	put_srq_read(srq);
+	ib_put_read_srq(srq);
 
 	return ret ? ret : in_len;
 }
@@ -3514,13 +3274,13 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	srq = idr_read_srq(cmd.srq_handle, file->ucontext);
+	srq = ib_idr_read_srq(cmd.srq_handle, file->ucontext);
 	if (!srq)
 		return -EINVAL;
 
 	ret = ib_query_srq(srq, &attr);
 
-	put_srq_read(srq);
+	ib_put_read_srq(srq);
 
 	if (ret)
 		return ret;
@@ -3555,7 +3315,7 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	uobj = idr_write_uobj(&ib_uverbs_srq_idr, cmd.srq_handle, file->ucontext);
+	uobj = ib_idr_write_uobj(&ib_uverbs_srq_idr, cmd.srq_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
 	srq = uobj->object;
@@ -3566,7 +3326,7 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	if (!ret)
 		uobj->live = 0;
 
-	put_uobj_write(uobj);
+	ib_put_uobj_write(uobj);
 
 	if (ret)
 		return ret;
@@ -3576,7 +3336,7 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 		atomic_dec(&us->uxrcd->refcnt);
 	}
 
-	idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
+	ib_idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
 
 	mutex_lock(&file->mutex);
 	list_del(&uobj->list);
@@ -3587,7 +3347,7 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	memset(&resp, 0, sizeof resp);
 	resp.events_reported = obj->events_reported;
 
-	put_uobj(uobj);
+	ib_put_uobj(uobj);
 
 	if (copy_to_user((void __user *) (unsigned long) cmd.response,
 			 &resp, sizeof resp))
diff --git a/drivers/infiniband/core/uverbs_cmd_common.c b/drivers/infiniband/core/uverbs_cmd_common.c
new file mode 100644
index 0000000..f2526f5
--- /dev/null
+++ b/drivers/infiniband/core/uverbs_cmd_common.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (c) 2005 Topspin Communications.  All rights reserved.
+ * Copyright (c) 2005, 2006, 2007 Cisco Systems.  All rights reserved.
+ * Copyright (c) 2005 PathScale, Inc.  All rights reserved.
+ * Copyright (c) 2006 Mellanox Technologies.  All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/sched.h>
+
+#include <linux/uaccess.h>
+
+#include "uverbs.h"
+#include "core_priv.h"
+
+struct uverbs_lock_class pd_lock_class	= { .name = "PD-uobj" };
+struct uverbs_lock_class mr_lock_class	= { .name = "MR-uobj" };
+struct uverbs_lock_class mw_lock_class	= { .name = "MW-uobj" };
+struct uverbs_lock_class cq_lock_class	= { .name = "CQ-uobj" };
+struct uverbs_lock_class qp_lock_class	= { .name = "QP-uobj" };
+struct uverbs_lock_class ah_lock_class	= { .name = "AH-uobj" };
+struct uverbs_lock_class srq_lock_class	= { .name = "SRQ-uobj" };
+struct uverbs_lock_class xrcd_lock_class = { .name = "XRCD-uobj" };
+struct uverbs_lock_class rule_lock_class = { .name = "RULE-uobj" };
+
+/*
+ * The ib_uobject locking scheme is as follows:
+ *
+ * - ib_uverbs_idr_lock protects the uverbs idrs themselves, so it
+ *   needs to be held during all idr write operations.  When an object is
+ *   looked up, a reference must be taken on the object's kref before
+ *   dropping this lock.  For read operations, the rcu_read_lock()
+ *   and rcu_write_lock() but similarly the kref reference is grabbed
+ *   before the rcu_read_unlock().
+ *
+ * - Each object also has an rwsem.  This rwsem must be held for
+ *   reading while an operation that uses the object is performed.
+ *   For example, while registering an MR, the associated PD's
+ *   uobject.mutex must be held for reading.  The rwsem must be held
+ *   for writing while initializing or destroying an object.
+ *
+ * - In addition, each object has a "live" flag.  If this flag is not
+ *   set, then lookups of the object will fail even if it is found in
+ *   the idr.  This handles a reader that blocks and does not acquire
+ *   the rwsem until after the object is destroyed.  The destroy
+ *   operation will set the live flag to 0 and then drop the rwsem;
+ *   this will allow the reader to acquire the rwsem, see that the
+ *   live flag is 0, and then drop the rwsem and its reference to
+ *   object.  The underlying storage will not be freed until the last
+ *   reference to the object is dropped.
+ */
+
+void ib_init_uobj(struct ib_uobject *uobj, u64 user_handle,
+		  struct ib_ucontext *context, struct uverbs_lock_class *c)
+{
+	uobj->user_handle = user_handle;
+	uobj->context     = context;
+	kref_init(&uobj->ref);
+	init_rwsem(&uobj->mutex);
+	lockdep_set_class_and_name(&uobj->mutex, &c->key, c->name);
+	uobj->live        = 0;
+}
+
+static void release_uobj(struct kref *kref)
+{
+	kfree_rcu(container_of(kref, struct ib_uobject, ref), rcu);
+}
+
+void ib_put_uobj(struct ib_uobject *uobj)
+{
+	kref_put(&uobj->ref, release_uobj);
+}
+
+void ib_put_uobj_read(struct ib_uobject *uobj)
+{
+	up_read(&uobj->mutex);
+	ib_put_uobj(uobj);
+}
+
+void ib_put_uobj_write(struct ib_uobject *uobj)
+{
+	up_write(&uobj->mutex);
+	ib_put_uobj(uobj);
+}
+
+int ib_idr_add_uobj(struct idr *idr, struct ib_uobject *uobj)
+{
+	int ret;
+
+	idr_preload(GFP_KERNEL);
+	spin_lock(&ib_uverbs_idr_lock);
+
+	ret = idr_alloc(idr, uobj, 0, 0, GFP_NOWAIT);
+	if (ret >= 0)
+		uobj->id = ret;
+
+	spin_unlock(&ib_uverbs_idr_lock);
+	idr_preload_end();
+
+	return ret < 0 ? ret : 0;
+}
+
+void ib_idr_remove_uobj(struct idr *idr, struct ib_uobject *uobj)
+{
+	spin_lock(&ib_uverbs_idr_lock);
+	idr_remove(idr, uobj->id);
+	spin_unlock(&ib_uverbs_idr_lock);
+}
+
+static struct ib_uobject *__idr_get_uobj(struct idr *idr, int id,
+					 struct ib_ucontext *context)
+{
+	struct ib_uobject *uobj;
+
+	rcu_read_lock();
+	uobj = idr_find(idr, id);
+	if (uobj) {
+		if (uobj->context == context)
+			kref_get(&uobj->ref);
+		else
+			uobj = NULL;
+	}
+	rcu_read_unlock();
+
+	return uobj;
+}
+
+struct ib_uobject *ib_idr_read_uobj(struct idr *idr, int id,
+				    struct ib_ucontext *context, int nested)
+{
+	struct ib_uobject *uobj;
+
+	uobj = __idr_get_uobj(idr, id, context);
+	if (!uobj)
+		return NULL;
+
+	if (nested)
+		down_read_nested(&uobj->mutex, SINGLE_DEPTH_NESTING);
+	else
+		down_read(&uobj->mutex);
+	if (!uobj->live) {
+		ib_put_uobj_read(uobj);
+		return NULL;
+	}
+
+	return uobj;
+}
+
+struct ib_uobject *ib_idr_write_uobj(struct idr *idr, int id,
+				     struct ib_ucontext *context)
+{
+	struct ib_uobject *uobj;
+
+	uobj = __idr_get_uobj(idr, id, context);
+	if (!uobj)
+		return NULL;
+
+	down_write(&uobj->mutex);
+	if (!uobj->live) {
+		ib_put_uobj_write(uobj);
+		return NULL;
+	}
+
+	return uobj;
+}
+
+static void *idr_read_obj(struct idr *idr, int id, struct ib_ucontext *context,
+			  int nested)
+{
+	struct ib_uobject *uobj;
+
+	uobj = ib_idr_read_uobj(idr, id, context, nested);
+	return uobj ? uobj->object : NULL;
+}
+
+struct ib_pd *ib_idr_read_pd(int pd_handle, struct ib_ucontext *context)
+{
+	return idr_read_obj(&ib_uverbs_pd_idr, pd_handle, context, 0);
+}
+
+void ib_put_read_pd(struct ib_pd *pd)
+{
+	ib_put_uobj_read(pd->uobject);
+}
+
+struct ib_cq *ib_idr_read_cq(int cq_handle, struct ib_ucontext *context, int nested)
+{
+	return idr_read_obj(&ib_uverbs_cq_idr, cq_handle, context, nested);
+}
+
+void ib_put_read_cq(struct ib_cq *cq)
+{
+	ib_put_uobj_read(cq->uobject);
+}
+
+struct ib_ah *ib_idr_read_ah(int ah_handle, struct ib_ucontext *context)
+{
+	return idr_read_obj(&ib_uverbs_ah_idr, ah_handle, context, 0);
+}
+
+void ib_put_read_ah(struct ib_ah *ah)
+{
+	ib_put_uobj_read(ah->uobject);
+}
+
+struct ib_qp *ib_idr_read_qp(int qp_handle, struct ib_ucontext *context)
+{
+	return idr_read_obj(&ib_uverbs_qp_idr, qp_handle, context, 0);
+}
+
+struct ib_qp *ib_idr_write_qp(int qp_handle, struct ib_ucontext *context)
+{
+	struct ib_uobject *uobj;
+
+	uobj = ib_idr_write_uobj(&ib_uverbs_qp_idr, qp_handle, context);
+	return uobj ? uobj->object : NULL;
+}
+
+void ib_put_read_qp(struct ib_qp *qp)
+{
+	ib_put_uobj_read(qp->uobject);
+}
+
+void ib_put_write_qp(struct ib_qp *qp)
+{
+	ib_put_uobj_write(qp->uobject);
+}
+
+struct ib_srq *ib_idr_read_srq(int srq_handle, struct ib_ucontext *context)
+{
+	return idr_read_obj(&ib_uverbs_srq_idr, srq_handle, context, 0);
+}
+
+void ib_put_read_srq(struct ib_srq *srq)
+{
+	ib_put_uobj_read(srq->uobject);
+}
+
+struct ib_xrcd *ib_idr_read_xrcd(int xrcd_handle, struct ib_ucontext *context,
+				 struct ib_uobject **uobj)
+{
+	*uobj = ib_idr_read_uobj(&ib_uverbs_xrcd_idr, xrcd_handle, context, 0);
+	return *uobj ? (*uobj)->object : NULL;
+}
+
+void ib_put_xrcd_read(struct ib_uobject *uobj)
+{
+	ib_put_uobj_read(uobj);
+}
+
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 2f49eb7..f5dc276 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -228,7 +228,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 	list_for_each_entry_safe(uobj, tmp, &context->ah_list, list) {
 		struct ib_ah *ah = uobj->object;
 
-		idr_remove_uobj(&ib_uverbs_ah_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_ah_idr, uobj);
 		ib_destroy_ah(ah);
 		kfree(uobj);
 	}
@@ -237,7 +237,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 	list_for_each_entry_safe(uobj, tmp, &context->mw_list, list) {
 		struct ib_mw *mw = uobj->object;
 
-		idr_remove_uobj(&ib_uverbs_mw_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_mw_idr, uobj);
 		uverbs_dealloc_mw(mw);
 		kfree(uobj);
 	}
@@ -245,7 +245,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 	list_for_each_entry_safe(uobj, tmp, &context->rule_list, list) {
 		struct ib_flow *flow_id = uobj->object;
 
-		idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
 		ib_destroy_flow(flow_id);
 		kfree(uobj);
 	}
@@ -255,7 +255,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 		struct ib_uqp_object *uqp =
 			container_of(uobj, struct ib_uqp_object, uevent.uobject);
 
-		idr_remove_uobj(&ib_uverbs_qp_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_qp_idr, uobj);
 		if (qp != qp->real_qp) {
 			ib_close_qp(qp);
 		} else {
@@ -271,7 +271,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 		struct ib_uevent_object *uevent =
 			container_of(uobj, struct ib_uevent_object, uobject);
 
-		idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
 		ib_destroy_srq(srq);
 		ib_uverbs_release_uevent(file, uevent);
 		kfree(uevent);
@@ -283,7 +283,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 		struct ib_ucq_object *ucq =
 			container_of(uobj, struct ib_ucq_object, uobject);
 
-		idr_remove_uobj(&ib_uverbs_cq_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_cq_idr, uobj);
 		ib_destroy_cq(cq);
 		ib_uverbs_release_ucq(file, ev_file, ucq);
 		kfree(ucq);
@@ -292,7 +292,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 	list_for_each_entry_safe(uobj, tmp, &context->mr_list, list) {
 		struct ib_mr *mr = uobj->object;
 
-		idr_remove_uobj(&ib_uverbs_mr_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_mr_idr, uobj);
 		ib_dereg_mr(mr);
 		kfree(uobj);
 	}
@@ -303,7 +303,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 		struct ib_uxrcd_object *uxrcd =
 			container_of(uobj, struct ib_uxrcd_object, uobject);
 
-		idr_remove_uobj(&ib_uverbs_xrcd_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_xrcd_idr, uobj);
 		ib_uverbs_dealloc_xrcd(file->device, xrcd);
 		kfree(uxrcd);
 	}
@@ -312,7 +312,7 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 	list_for_each_entry_safe(uobj, tmp, &context->pd_list, list) {
 		struct ib_pd *pd = uobj->object;
 
-		idr_remove_uobj(&ib_uverbs_pd_idr, uobj);
+		ib_idr_remove_uobj(&ib_uverbs_pd_idr, uobj);
 		ib_dealloc_pd(pd);
 		kfree(uobj);
 	}
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Implement get_context by using CREATE action on the DEVICE object.
Currently, the command not requires any mandatory attribute, but could
get an optional vendor command and response descriptors.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/Makefile        |   2 +-
 drivers/infiniband/core/uverbs.h        |   9 ++
 drivers/infiniband/core/uverbs_cmd_nl.c | 151 ++++++++++++++++++++++++++++++++
 drivers/infiniband/core/uverbs_main.c   |  27 +++++-
 include/uapi/rdma/ib_user_ioctl.h       |   5 ++
 5 files changed, 192 insertions(+), 2 deletions(-)
 create mode 100644 drivers/infiniband/core/uverbs_cmd_nl.c

diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index 2d976e3f..c764fdb 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -35,4 +35,4 @@ ib_umad-y :=			user_mad.o
 ib_ucm-y :=			ucm.o
 
 ib_uverbs-y :=			uverbs_main.o uverbs_cmd.o uverbs_marshall.o \
-				uverbs_nl.o uverbs_cmd_common.o
+				uverbs_nl.o uverbs_cmd_nl.o uverbs_cmd_common.o
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 1f121dc..c4d1a49 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -327,4 +327,13 @@ IB_UVERBS_DECLARE_EX_CMD(query_device);
 IB_UVERBS_DECLARE_EX_CMD(create_cq);
 IB_UVERBS_DECLARE_EX_CMD(create_qp);
 
+#define IB_UVERBS_DECLARE_CMD_NL_CREATE(name)				       \
+	ssize_t ib_uverbs_nl_##name(struct ib_uverbs_file *filp,	       \
+				    struct ib_device *ib_dev,		       \
+				    struct ib_uverbs_ioctl_hdr *hdr,	       \
+				    struct nlattr **tb, struct ib_udata *uresp,\
+				    struct ib_udata *uhw)
+
+IB_UVERBS_DECLARE_CMD_NL_CREATE(context_create);
+
 #endif /* UVERBS_H */
diff --git a/drivers/infiniband/core/uverbs_cmd_nl.c b/drivers/infiniband/core/uverbs_cmd_nl.c
new file mode 100644
index 0000000..d12c54b
--- /dev/null
+++ b/drivers/infiniband/core/uverbs_cmd_nl.c
@@ -0,0 +1,151 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies, LTD. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <net/netlink.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/sched.h>
+
+#include <linux/uaccess.h>
+
+#include "uverbs.h"
+#include "core_priv.h"
+
+long ib_uverbs_nl_context_create(struct ib_uverbs_file *file,
+				 struct ib_device *ib_dev,
+				 struct ib_uverbs_ioctl_hdr *hdr,
+				 struct nlattr **tb, struct ib_udata *uresp,
+				 struct ib_udata *uhw)
+{
+	struct ib_uverbs_get_context_resp resp;
+	struct ib_ucontext		 *ucontext;
+	int ret;
+	struct file			 *filp;
+	struct nlattr __user *nla;
+
+	mutex_lock(&file->mutex);
+
+	if (file->ucontext) {
+		pr_debug("uverbs context create with already existing context\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	ucontext = ib_dev->alloc_ucontext(ib_dev, uhw);
+	if (IS_ERR(ucontext)) {
+		ret = PTR_ERR(ucontext);
+		goto err;
+	}
+	if (uhw->outptr - uhw->outbuf) {
+		__u32 vendor_len = uhw->outptr - uhw->outbuf;
+
+		nla = ib_uverbs_nla_put(uresp, IBNL_RESPONSE_TYPE_VENDOR,
+					sizeof(vendor_len), &vendor_len);
+		if (IS_ERR(nla)) {
+			ret = PTR_ERR(nla);
+			goto err_ctx;
+		}
+	}
+
+	ucontext->device = ib_dev;
+	INIT_LIST_HEAD(&ucontext->pd_list);
+	INIT_LIST_HEAD(&ucontext->mr_list);
+	INIT_LIST_HEAD(&ucontext->mw_list);
+	INIT_LIST_HEAD(&ucontext->cq_list);
+	INIT_LIST_HEAD(&ucontext->qp_list);
+	INIT_LIST_HEAD(&ucontext->srq_list);
+	INIT_LIST_HEAD(&ucontext->ah_list);
+	INIT_LIST_HEAD(&ucontext->xrcd_list);
+	INIT_LIST_HEAD(&ucontext->rule_list);
+	rcu_read_lock();
+	ucontext->tgid = get_task_pid(current->group_leader, PIDTYPE_PID);
+	rcu_read_unlock();
+	ucontext->closing = 0;
+
+#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
+	ucontext->umem_tree = RB_ROOT;
+	init_rwsem(&ucontext->umem_rwsem);
+	ucontext->odp_mrs_count = 0;
+	INIT_LIST_HEAD(&ucontext->no_private_counters);
+
+	if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_ON_DEMAND_PAGING))
+		ucontext->invalidate_range = NULL;
+
+#endif
+
+	resp.num_comp_vectors = file->device->num_comp_vectors;
+
+	ret = get_unused_fd_flags(O_CLOEXEC);
+	if (ret < 0)
+		goto err_free;
+	resp.async_fd = ret;
+
+	filp = ib_uverbs_alloc_event_file(file, ib_dev, 1);
+	if (IS_ERR(filp)) {
+		ret = PTR_ERR(filp);
+		goto err_fd;
+	}
+
+	nla = ib_uverbs_nla_put(uresp, IBNL_RESPONSE_TYPE_RESP,
+				sizeof(resp), &resp);
+	if (IS_ERR(nla)) {
+		ret = PTR_ERR(nla);
+		goto err_file;
+	}
+
+	file->ucontext = ucontext;
+
+	fd_install(resp.async_fd, filp);
+
+	mutex_unlock(&file->mutex);
+
+	return 0;
+
+err_file:
+	ib_uverbs_free_async_event_file(file);
+	fput(filp);
+
+err_fd:
+	put_unused_fd(resp.async_fd);
+
+err_free:
+	put_pid(ucontext->tgid);
+err_ctx:
+	ib_dev->dealloc_ucontext(ucontext);
+
+err:
+	mutex_unlock(&file->mutex);
+	return ret;
+	return 0;
+};
+
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index f5dc276..10c2412 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -884,6 +884,15 @@ void ib_build_udata_from_nl(struct ib_udata *udata, struct nlattr **tb,
 				    .len = sizeof(struct ib_uverbs_uptr)},\
 	[IBNL_PROVIDER_RESP_UPTR]	 = {.type = NLA_BINARY,		  \
 				    .len = sizeof(struct ib_uverbs_uptr)}
+
+static const struct nla_policy ibnl_create_device_policy[] = {
+	IBNL_VENDOR_POLICY_ATTRS,
+      /*
+       * If there are other command attributes:
+       * [IBNL_CREATE_DEVICE_CORE]		 = {.type = NLA_BINARY, .len = sizeof(cmd)},
+       */
+};
+
 struct object_action {
 	struct {
 		struct validate_op validator;
@@ -906,7 +915,23 @@ struct object_action {
 	} ops[];
 };
 
-static const struct object_action object_actions[IB_OBJ_TYPE_MAX];
+static const struct object_action object_actions[IB_OBJ_TYPE_MAX] = {
+	[IB_OBJ_TYPE_DEVICE] = {
+		.create = {
+			.fn = ib_uverbs_nl_context_create,
+			.validator = {.policy = ibnl_create_device_policy,
+				      /*
+				       * TODO: Mandatory fields validator
+				       * .mandatory_fields =
+				       *	IB_UVERBS_MANDATORY_FIELDS(IBNL_CREATE_DEVICE_CORE),
+				       */
+
+					.resp_min_sz = sizeof(struct ib_uverbs_get_context_resp),
+				},
+			.max_attrs = IB_UVERBS_MAX_ATTRS(IBNL_CREATE_DEVICE_MAX),
+		}
+	}
+};
 
 struct nla_validator_cb_priv {
 	int maxtype;
diff --git a/include/uapi/rdma/ib_user_ioctl.h b/include/uapi/rdma/ib_user_ioctl.h
index 3edb623..9e3db1e 100644
--- a/include/uapi/rdma/ib_user_ioctl.h
+++ b/include/uapi/rdma/ib_user_ioctl.h
@@ -118,4 +118,9 @@ enum ib_uverbs_common_resp_types {
 #define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
 					      struct ib_user_mad_reg_req2)
 
+enum ibnl_create_device {
+	IBNL_CREATE_DEVICE_CORE = IBNL_VENDOR_ATTRS_MAX,
+	IBNL_CREATE_DEVICE_MAX
+};
+
 #endif /* IB_USER_IOCTL_H */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Hefty, Sean]

> +	filp = ib_uverbs_alloc_event_file(file, ib_dev, 1);
> +	if (IS_ERR(filp)) {
> +		ret = PTR_ERR(filp);
> +		goto err_fd;
> +	}

I was considering events being reported directly on the file that's already opened.  Any objects opened through a file would report their events on that file, with possibly a way to redirect events to a different file (similar to rdma_migrate_id processing).

I want to limit the number of open files that are needed -- to reduce resources and simplify things for apps. 

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Jason Gunthorpe]

On Tue, May 24, 2016 at 08:04:41PM +0000, Hefty, Sean wrote:
> > +	filp = ib_uverbs_alloc_event_file(file, ib_dev, 1);
> > +	if (IS_ERR(filp)) {
> > +		ret = PTR_ERR(filp);
> > +		goto err_fd;
> > +	}
> 
> I was considering events being reported directly on the file that's
> already opened.  Any objects opened through a file would report
> their events on that file, with possibly a way to redirect events to
> a different file (similar to rdma_migrate_id processing).

Is this code then just an optional compat for libibverbs 1.0?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Hefty, Sean]

> > > +	filp = ib_uverbs_alloc_event_file(file, ib_dev, 1);
> > > +	if (IS_ERR(filp)) {
> > > +		ret = PTR_ERR(filp);
> > > +		goto err_fd;
> > > +	}
> >
> > I was considering events being reported directly on the file that's
> > already opened.  Any objects opened through a file would report
> > their events on that file, with possibly a way to redirect events to
> > a different file (similar to rdma_migrate_id processing).
> 
> Is this code then just an optional compat for libibverbs 1.0?

It can work for compat, but migrate id had other use cases, which might be applicable in other situations.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Jason Gunthorpe]

On Tue, May 24, 2016 at 10:02:26PM +0000, Hefty, Sean wrote:
> > > > +	filp = ib_uverbs_alloc_event_file(file, ib_dev, 1);
> > > > +	if (IS_ERR(filp)) {
> > > > +		ret = PTR_ERR(filp);
> > > > +		goto err_fd;
> > > > +	}
> > >
> > > I was considering events being reported directly on the file that's
> > > already opened.  Any objects opened through a file would report
> > > their events on that file, with possibly a way to redirect events to
> > > a different file (similar to rdma_migrate_id processing).
> > 
> > Is this code then just an optional compat for libibverbs 1.0?
> 
> It can work for compat, but migrate id had other use cases, which
> might be applicable in other situations.

Sorry, I ment the code you are quoting.

Eg, should the above have the 'struct ib_uverbs_get_context_resp'
redone to not include the 'resp.async_fd' and a new method call be required
to get the events filp?

compat libibverbs 1.0 would always have to call the new method.

If we had a new libibverbs 2.0 event delivery scheme like you envision
then 2.0 would call a different method instead to setup the new
scheme.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Hefty, Sean]

> > > > > +	filp = ib_uverbs_alloc_event_file(file, ib_dev, 1);
> > > > > +	if (IS_ERR(filp)) {
> > > > > +		ret = PTR_ERR(filp);
> > > > > +		goto err_fd;
> > > > > +	}
> > > >
> > > > I was considering events being reported directly on the file that's
> > > > already opened.  Any objects opened through a file would report
> > > > their events on that file, with possibly a way to redirect events to
> > > > a different file (similar to rdma_migrate_id processing).
> > >
> > > Is this code then just an optional compat for libibverbs 1.0?
> >
> > It can work for compat, but migrate id had other use cases, which
> > might be applicable in other situations.
> 
> Sorry, I ment the code you are quoting.
> 
> Eg, should the above have the 'struct ib_uverbs_get_context_resp'
> redone to not include the 'resp.async_fd' and a new method call be required
> to get the events filp?

That's likely to be more generic.

> compat libibverbs 1.0 would always have to call the new method.
> 
> If we had a new libibverbs 2.0 event delivery scheme like you envision
> then 2.0 would call a different method instead to setup the new
> scheme.

I was not attempting to provide a compatibility layer to verbs.  I was developing a generic framework to support all devices, verbs or otherwise.  If done correctly, compatibility should fall out naturally.  My immediate objective was to hook hfi1 into this framework.  Hooking verbs into it is a much lower priority to me, as we already have a working solution.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Jason Gunthorpe]

On Tue, May 24, 2016 at 05:35:26PM +0300, Leon Romanovsky wrote:

> +	ucontext->device = ib_dev;
> +	INIT_LIST_HEAD(&ucontext->pd_list);
> +	INIT_LIST_HEAD(&ucontext->mr_list);
> +	INIT_LIST_HEAD(&ucontext->mw_list);
> +	INIT_LIST_HEAD(&ucontext->cq_list);
> +	INIT_LIST_HEAD(&ucontext->qp_list);
> +	INIT_LIST_HEAD(&ucontext->srq_list);
> +	INIT_LIST_HEAD(&ucontext->ah_list);
> +	INIT_LIST_HEAD(&ucontext->xrcd_list);
> +	INIT_LIST_HEAD(&ucontext->rule_list);
[..]

Please don't cut and paste like this.

Considering your approach I'd suggest just go ahead and immediately
implement a compat wrapper for each replaced write() style call.

eg replace ib_uverbs_get_context with something that forms a
new-style-command and calls the netlink version directly.

This also then serves as the example on how to implement the user
space migration.

> +		nla = ib_uverbs_nla_put(uresp, IBNL_RESPONSE_TYPE_VENDOR,
> +					sizeof(vendor_len), &vendor_len);

Please don't use the word 'vendor' in any of this.

There is only common and driver-specific.

Maybe call this IBNL_RESPONSE_DRIVER_UDATA

> +	nla = ib_uverbs_nla_put(uresp, IBNL_RESPONSE_TYPE_RESP,
> +				sizeof(resp), &resp);

And this IBNL_RESPONSE_COMMON ?

> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index f5dc276..10c2412 100644
> +++ b/drivers/infiniband/core/uverbs_main.c

> +static const struct nla_policy ibnl_create_device_policy[] = {

Why isn't this stuff in a _nl file?

> +enum ibnl_create_device {
> +	IBNL_CREATE_DEVICE_CORE = IBNL_VENDOR_ATTRS_MAX,
> +	IBNL_CREATE_DEVICE_MAX
> +};

Whats all this about? Looks very strange.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

On Tue, May 24, 2016 at 04:07:20PM -0600, Jason Gunthorpe wrote:
> On Tue, May 24, 2016 at 05:35:26PM +0300, Leon Romanovsky wrote:
> 
> > +	ucontext->device = ib_dev;
> > +	INIT_LIST_HEAD(&ucontext->pd_list);
> > +	INIT_LIST_HEAD(&ucontext->mr_list);
> > +	INIT_LIST_HEAD(&ucontext->mw_list);
> > +	INIT_LIST_HEAD(&ucontext->cq_list);
> > +	INIT_LIST_HEAD(&ucontext->qp_list);
> > +	INIT_LIST_HEAD(&ucontext->srq_list);
> > +	INIT_LIST_HEAD(&ucontext->ah_list);
> > +	INIT_LIST_HEAD(&ucontext->xrcd_list);
> > +	INIT_LIST_HEAD(&ucontext->rule_list);
> [..]
> 
> Please don't cut and paste like this.
> 
> Considering your approach I'd suggest just go ahead and immediately
> implement a compat wrapper for each replaced write() style call.
> 
> eg replace ib_uverbs_get_context with something that forms a
> new-style-command and calls the netlink version directly.
> 

Thank you and Sean for reviewing it.

We will do our best to address all comments from this discussion in our
next RFC version.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Matan Barak]

On Wed, May 25, 2016 at 1:07 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Tue, May 24, 2016 at 05:35:26PM +0300, Leon Romanovsky wrote:
>
>> +     ucontext->device = ib_dev;
>> +     INIT_LIST_HEAD(&ucontext->pd_list);
>> +     INIT_LIST_HEAD(&ucontext->mr_list);
>> +     INIT_LIST_HEAD(&ucontext->mw_list);
>> +     INIT_LIST_HEAD(&ucontext->cq_list);
>> +     INIT_LIST_HEAD(&ucontext->qp_list);
>> +     INIT_LIST_HEAD(&ucontext->srq_list);
>> +     INIT_LIST_HEAD(&ucontext->ah_list);
>> +     INIT_LIST_HEAD(&ucontext->xrcd_list);
>> +     INIT_LIST_HEAD(&ucontext->rule_list);
> [..]
>
> Please don't cut and paste like this.
>
> Considering your approach I'd suggest just go ahead and immediately
> implement a compat wrapper for each replaced write() style call.
>
> eg replace ib_uverbs_get_context with something that forms a
> new-style-command and calls the netlink version directly.
>

Of course, this is just the first step of the ABI. It's only intended
to show some concepts regarding the new ABI and not as a final ABI
conversion.
I agree that we don't want code duplication and the old ABI handling
code should just wrap the parameters and call the new code path.

> This also then serves as the example on how to implement the user
> space migration.
>
>> +             nla = ib_uverbs_nla_put(uresp, IBNL_RESPONSE_TYPE_VENDOR,
>> +                                     sizeof(vendor_len), &vendor_len);
>
> Please don't use the word 'vendor' in any of this.
>
> There is only common and driver-specific.
>
> Maybe call this IBNL_RESPONSE_DRIVER_UDATA
>

Ok

>> +     nla = ib_uverbs_nla_put(uresp, IBNL_RESPONSE_TYPE_RESP,
>> +                             sizeof(resp), &resp);
>
> And this IBNL_RESPONSE_COMMON ?
>

Yeah

>> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
>> index f5dc276..10c2412 100644
>> +++ b/drivers/infiniband/core/uverbs_main.c
>
>> +static const struct nla_policy ibnl_create_device_policy[] = {
>
> Why isn't this stuff in a _nl file?
>

All validators and function pointers should move into uverbs_cmd_nl file.

>> +enum ibnl_create_device {
>> +     IBNL_CREATE_DEVICE_CORE = IBNL_VENDOR_ATTRS_MAX,
>> +     IBNL_CREATE_DEVICE_MAX
>> +};
>
> Whats all this about? Looks very strange.
>

The first attribute ids of every command are reserved for the vendor part.

> Jason

Thanks for taking a look. The patches are still WIP and we'll fix the
above issues.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 8/8] IB/core: Implement device_create with the new ABI

[Jason Gunthorpe]

On Thu, May 26, 2016 at 04:24:06PM +0300, Matan Barak wrote:

> >> +enum ibnl_create_device {
> >> +     IBNL_CREATE_DEVICE_CORE = IBNL_VENDOR_ATTRS_MAX,
> >> +     IBNL_CREATE_DEVICE_MAX
> >> +};
> >
> > Whats all this about? Looks very strange.
> 
> The first attribute ids of every command are reserved for the vendor part.

I'd still like to see if we can be happy with globally unique
attribute ID's. I would like to see each driver have it's own range of
global IDs that it can use. Much like an ioctl number each attribute
id should be associated with exactly one structure layout.

The obvious downside is how to index them as large ID's cannot be
array'd anymore, but I wonder if that approach is overkill anyhow, if
there is only going to be one or two arguments then a linear search
is OK, or we can require them to be sorted by ID or something.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> struct ib_uverbs_ioctl_hdr {
> 	__u32 length;

FWIW, I limited length to 16-bits, to prevent passing massive amounts of data between user space and the kernel as a single transfer.

> 	__u16 flags;

16-bit flags seems limiting.

> 	__u16 object_type;
> 	__u16 reserved;
> 	/* First 8 actions are common to all objects */
> 	__u16 action;
> 	__u32 user_handler;
> 	/*
> 	 * These fields represent core response only,
> 	 * provider's response is given as a netlink attribute.
> 	 */
> 	struct ib_uverbs_uptr resp;

This is associating the kernel ABI with a specific implementation of user space components.

Ioctl implementations I looked at treated the ioctl buffer as input-output (except for read only or write only ioctl's).  Maybe there's examples of redirecting the response to separate buffer(s)?

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Jason Gunthorpe]

On Tue, May 24, 2016 at 07:37:13PM +0000, Hefty, Sean wrote:

> Ioctl implementations I looked at treated the ioctl buffer as
> input-output (except for read only or write only ioctl's).  Maybe
> there's examples of redirecting the response to separate buffer(s)?

There are examples of this, eg SG_IO uses scatter/gather lists

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> > Ioctl implementations I looked at treated the ioctl buffer as
> > input-output (except for read only or write only ioctl's).  Maybe
> > there's examples of redirecting the response to separate buffer(s)?
> 
> There are examples of this, eg SG_IO uses scatter/gather lists

I did come across a use of sgl's (don't recall where), but I found the documentation so convoluted that I gave up trying to make sense of what it was attempting to do.  :)

SGLs for input or output add complexity, so for myself, I would need to be convinced that adding it to a common framework makes sense.  (SGLs which are part of command data are a separate issue.)  Though to be fair, I find netlink based data to be convoluted...

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Steve Wise]

> 
> > > Ioctl implementations I looked at treated the ioctl buffer as
> > > input-output (except for read only or write only ioctl's).  Maybe
> > > there's examples of redirecting the response to separate buffer(s)?
> >
> > There are examples of this, eg SG_IO uses scatter/gather lists
> 
> I did come across a use of sgl's (don't recall where), but I found the
documentation
> so convoluted that I gave up trying to make sense of what it was attempting to
do.
> :)
> 
> SGLs for input or output add complexity, so for myself, I would need to be
convinced
> that adding it to a common framework makes sense.  (SGLs which are part of
> command data are a separate issue.)  Though to be fair, I find netlink based
data to
> be convoluted...
> 

Makes my eyes cross... ;)


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Jason Gunthorpe]

On Tue, May 24, 2016 at 09:16:24PM +0000, Hefty, Sean wrote:

> SGLs for input or output add complexity, so for myself, I would need
> to be convinced that adding it to a common framework makes sense.
> (SGLs which are part of command data are a separate issue.)  Though
> to be fair, I find netlink based data to be convoluted...

Your patch didn't explore how to encode the arguments to the
operations - what did you want to do? (ie the stuff that trails the urdma_ioctl)

Netlink is just a very simple tag+length+data scheme.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> Your patch didn't explore how to encode the arguments to the
> operations - what did you want to do? (ie the stuff that trails the urdma_ioctl)

I hadn't considered that aspect yet.  I planned to look at the netlink proposal as a first step, but as an API it looks painful.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Jason Gunthorpe]

On Tue, May 24, 2016 at 09:59:46PM +0000, Hefty, Sean wrote:
> > Your patch didn't explore how to encode the arguments to the
> > operations - what did you want to do? (ie the stuff that trails the urdma_ioctl)
> 
> I hadn't considered that aspect yet.  I planned to look at the
> netlink proposal as a first step, but as an API it looks painful.

I agree.

If we are using SGL for the response then why not use SGL for
everything? We've already lost the advantage of the packet-like
encoding netlink uses.

eg:

struct urdma_ioctl {
	u64	flags;
	u16	domain;
	u8	object_count;
	u8	in_arg_count;
	u8	out_arg_count;
	u8      reserved[3];
	// union urdma_object objects[object_count];
	// struct arg_sgl in_args[in_arg_count];
	// struct arg_sgl out_args[out_arg_count];
};

union urdma_object {
      struct urdma_obj_id obj_id;
      u64 data;
};

struct urdma_arg_sgl {
      void *ptr;
      u32 id;
      u32 length;
};

(same comment goes for Mellanox's version)

This looks like it would make the decode and validation steps simpler
and avoid some of those ugly memory allocations in the Mellanox patch
as well...

This would also address your concern about hard-wiring the libibverbs
1.0 needs, and meet mellanox's concern to have a 0 copy path for the
compat udata/etc.

The user caller then uses the simplifed on-stack setup I was
suggesting:

int ibv_open_device(...)
{
    struct xxx resp;
    struct kern_call
    {
        struct urdma_ioctl hdr;
	struct urdma_arg common_out;
	struct urdma_arg udata_out;
    } call = {
       .hdr = {.domain = URDMA_DEVICE, .out_arg_count = 2},
       .common_out = {.ptr = &resp, id = CREATE_DEVICE_COMMON_RESP, .length = sizeof(resp)},
       .udata_out = {.ptr = &udata_resp, id = CREATE_DEVICE_MLX4_UDATA_RESP, .length = sizeof(udata_resp)},
    };

    ioctl(fd, URDMA_IOCTL_OPEN, &kern_call);

    // Kernel 0's the resp ID when it writes so user space knows it was supported
    if (call.common_out.id != 0)
       return EINVAL;
}

This outline is a mash up of ideas from both patches.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> If we are using SGL for the response then why not use SGL for
> everything? We've already lost the advantage of the packet-like
> encoding netlink uses.
> 
> eg:
> 
> struct urdma_ioctl {
> 	u64	flags;
> 	u16	domain;
> 	u8	object_count;
> 	u8	in_arg_count;
> 	u8	out_arg_count;
> 	u8      reserved[3];
> 	// union urdma_object objects[object_count];
> 	// struct arg_sgl in_args[in_arg_count];
> 	// struct arg_sgl out_args[out_arg_count];
> };
> 
> union urdma_object {
>       struct urdma_obj_id obj_id;
>       u64 data;
> };
> 
> struct urdma_arg_sgl {
>       void *ptr;
>       u32 id;
>       u32 length;
> };

Yes, this looks like it would work and be generic for any purpose.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Matan Barak]

On Wed, May 25, 2016 at 1:30 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Tue, May 24, 2016 at 09:59:46PM +0000, Hefty, Sean wrote:
>> > Your patch didn't explore how to encode the arguments to the
>> > operations - what did you want to do? (ie the stuff that trails the urdma_ioctl)
>>
>> I hadn't considered that aspect yet.  I planned to look at the
>> netlink proposal as a first step, but as an API it looks painful.
>
> I agree.
>
> If we are using SGL for the response then why not use SGL for
> everything? We've already lost the advantage of the packet-like
> encoding netlink uses.
>
> eg:
>
> struct urdma_ioctl {
>         u64     flags;
>         u16     domain;
>         u8      object_count;
>         u8      in_arg_count;
>         u8      out_arg_count;
>         u8      reserved[3];
>         // union urdma_object objects[object_count];
>         // struct arg_sgl in_args[in_arg_count];
>         // struct arg_sgl out_args[out_arg_count];
> };
>
> union urdma_object {
>       struct urdma_obj_id obj_id;
>       u64 data;
> };
>
> struct urdma_arg_sgl {
>       void *ptr;
>       u32 id;
>       u32 length;
> };
>
> (same comment goes for Mellanox's version)
>
> This looks like it would make the decode and validation steps simpler
> and avoid some of those ugly memory allocations in the Mellanox patch
> as well...
>
> This would also address your concern about hard-wiring the libibverbs
> 1.0 needs, and meet mellanox's concern to have a 0 copy path for the
> compat udata/etc.
>
> The user caller then uses the simplifed on-stack setup I was
> suggesting:
>
> int ibv_open_device(...)
> {
>     struct xxx resp;
>     struct kern_call
>     {
>         struct urdma_ioctl hdr;
>         struct urdma_arg common_out;
>         struct urdma_arg udata_out;
>     } call = {
>        .hdr = {.domain = URDMA_DEVICE, .out_arg_count = 2},
>        .common_out = {.ptr = &resp, id = CREATE_DEVICE_COMMON_RESP, .length = sizeof(resp)},
>        .udata_out = {.ptr = &udata_resp, id = CREATE_DEVICE_MLX4_UDATA_RESP, .length = sizeof(udata_resp)},
>     };
>
>     ioctl(fd, URDMA_IOCTL_OPEN, &kern_call);
>
>     // Kernel 0's the resp ID when it writes so user space knows it was supported
>     if (call.common_out.id != 0)
>        return EINVAL;
> }
>
> This outline is a mash up of ideas from both patches.
>

The proposal treat every pointer as a typed pointer (which pretty much
resembles the netlink style of doing things, but instead of
serializing it into the buffer, we just scatter it to multiple
buffers). I'm not sure it makes the parsing and validations easier.
Actually, I was thinking about ditching the common response buffer and
just overwriting the command buffer. The typed pointers are now only
used for the vendor part, as we want to retain comparability with old
user-space drivers (with zero copy). We could allow inlined vendor
data in the future.

> Jason

Matan

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Jason Gunthorpe]

On Thu, May 26, 2016 at 06:18:02PM +0300, Matan Barak wrote:
> The proposal treat every pointer as a typed pointer (which pretty much
> resembles the netlink style of doing things, but instead of
> serializing it into the buffer, we just scatter it to multiple
> buffers).

Yes

> I'm not sure it makes the parsing and validations easier.

It means less parsing work, you don't need to validate message
structure for instance, the header can be copy'd without allocation,
size validation can be deferred to the reader, we don't need alignment
padding complexity.

It also means less parsing work in user space because the reply
message is already fully scattered where the caller wants it, so it
doesn't need to parse netlink stuff in the reply either.

The downside is that for the most part the reply structure must be
known in advance

> Actually, I was thinking about ditching the common response buffer and
> just overwriting the command buffer.

The sgl approach allows this by letting user-space set the in and out
pointers to the same thing, without having problems with specifying
the out length or requiring user space to parse netlink to find the
reply.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> > Actually, I was thinking about ditching the common response buffer and
> > just overwriting the command buffer.
> 
> The sgl approach allows this by letting user-space set the in and out
> pointers to the same thing, without having problems with specifying
> the out length or requiring user space to parse netlink to find the
> reply.

For common handling, the framework needs to know how much memory to allocate for copy in/out purposes.  The location of the ioctl input data, and the resulting output response data, in the allocated buffer also need to be agreed upon by the framework and kernel client.

Ideally, the number of calls to copy to/from user should be minimized.

In my proposal, each ioctl is matched with a descriptor which defines the size of the kernel buffer needed for input/output, which is then allocated by the framework.  I had the response buffer overwrite the command buffer, with the kernel client writing the response at the start of the kernel buffer.  This is an area that needs discussion, but is at least separate from the uABI. 

Using an sgl for input would increase the number of copy from user calls.  I anticipate the common case of the command header and command data located as part of the same memory buffer.  If so, an input sgl would be less efficient.  I want to avoid copying the header to get a length, then copying the sgl, and then copying the sgl data.

Unless I'm missing something, using an sgl for output should work fine, and only adds a simple iterative loop into the code path.

If we want to support more optimal ioctl handling, we could use separate ioctl commands for sgl versus non-sgl, with all other aspects of the two commands the same.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Jason Gunthorpe]

On Thu, May 26, 2016 at 10:53:35PM +0000, Hefty, Sean wrote:
> Ideally, the number of calls to copy to/from user should be minimized.

I'd say we should avoid memory allocation first and if that takes more
copy calls then so be it. Our existing model uses many copy from user
calls and that is considered OK. Memory allocation is clearly more
expensive than a few copy setups.

I would be very sad if our new API required allocations on every
ioctl.

The idea with a sgl is that the core would check it with access_ok,
just like we do today, and then the implementation would
copy_from_user directly to the stack, like how the header scheme vs
data scheme works today. The attraction is we can keep with an
attribute type model without having to copy the whole buffer to parse
the attribute structure.

The main problem with the current scheme is that it is hard to add
variable sized things like addresses without opencoding that
everywhere. It is also hard to express what the kernel supports, and
doesn't well support 'optional' things.

This is where I see the attraction of the attribute id labeled buffer.

If we don't want to have attributes, then copy the basic current
scheme of single shared in/out buffer..

Or maybe do both <shrug>

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> I would be very sad if our new API required allocations on every
> ioctl.

Are you referring to kernel or user space allocations?

For the kernel, I declared a small amount of data on the stack, and allocate memory only if the ioctl length is larger.  We can probably adjust the stack size to support most operations.

For user space, do we have a guess wrt the common number of sgl entries that would be used as input data?  Knowing an expected number of objects also seems needed to come up with an optimal design here.

> The idea with a sgl is that the core would check it with access_ok,
> just like we do today, and then the implementation would
> copy_from_user directly to the stack, like how the header scheme vs
> data scheme works today. The attraction is we can keep with an
> attribute type model without having to copy the whole buffer to parse
> the attribute structure.
> 
> The main problem with the current scheme is that it is hard to add
> variable sized things like addresses without opencoding that
> everywhere. It is also hard to express what the kernel supports, and
> doesn't well support 'optional' things.

If the sgl contained offsets from the start of the memory buffer, rather than pointers to different buffers, we could support variable sized attributes without needing to fetch multiple buffers.

As it stands, I don't see how we get away from calling copy from user twice in most circumstances.  Keeping most ioctls to this requires some sort of trade off.  But I think we can keep enough flexibility to support everything we want.

> This is where I see the attraction of the attribute id labeled buffer.

I like the idea of an attribute id labeled buffer myself.  But I think the processing of the ioctl data is better handled by the kernel client, rather than the framework.  The framework just needs to know enough to copy the data efficiently into/out from the kernel, and I don't know what's the optimal design here.

- Sean 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

On Fri, May 27, 2016 at 12:46:48AM +0000, Hefty, Sean wrote:
> > This is where I see the attraction of the attribute id labeled buffer.
> 
> I like the idea of an attribute id labeled buffer myself.  But I think the processing of the ioctl data is better handled by the kernel client, rather than the framework.  The framework just needs to know enough to copy the data efficiently into/out from the kernel, and I don't know what's the optimal design here.

Sean,
What do you mean by "the kernel client"?

Maybe you are referring for something else, but I think about driver's
implementations of callbacks which will cause to code duplications.

> 
> - Sean 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> What do you mean by "the kernel client"?
> 
> Maybe you are referring for something else, but I think about driver's
> implementations of callbacks which will cause to code duplications.

I am referring to the creation of an ioctl handling framework that is suitable for use by the rdma cm, ib cm (if kept), verb devices, and non-verbs drivers that do not plug into the kernel verbs framework.  The kernel clients are the items I mentioned.  Having a common ioctl handling framework will maximize code reuse.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

On Fri, May 27, 2016 at 03:04:00PM +0000, Hefty, Sean wrote:
> > What do you mean by "the kernel client"?
> > 
> > Maybe you are referring for something else, but I think about driver's
> > implementations of callbacks which will cause to code duplications.
> 
> I am referring to the creation of an ioctl handling framework that is suitable for use by the rdma cm, ib cm (if kept), verb devices, and non-verbs drivers that do not plug into the kernel verbs framework.  The kernel clients are the items I mentioned.  Having a common ioctl handling framework will maximize code reuse.

It looks too generic for RDMA subsystem. What does it make this IOCTL framework specific to RDMA?

> 
> - Sean

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> > I am referring to the creation of an ioctl handling framework that is suitable for use by the rdma
> cm, ib cm (if kept), verb devices, and non-verbs drivers that do not plug into the kernel verbs
> framework.  The kernel clients are the items I mentioned.  Having a common ioctl handling framework
> will maximize code reuse.
> 
> It looks too generic for RDMA subsystem. What does it make this IOCTL framework specific to RDMA?

I'm not sure how something can be too generic.  Everything I mentioned above exists as part of the RDMA subsystem.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Tue, May 31, 2016 at 05:56:29PM +0000, Hefty, Sean wrote:
> > > I am referring to the creation of an ioctl handling framework that is suitable for use by the rdma
> > cm, ib cm (if kept), verb devices, and non-verbs drivers that do not plug into the kernel verbs
> > framework.  The kernel clients are the items I mentioned.  Having a common ioctl handling framework
> > will maximize code reuse.
> > 
> > It looks too generic for RDMA subsystem. What does it make this IOCTL framework specific to RDMA?
> 
> I'm not sure how something can be too generic.  Everything I mentioned above exists as part of the RDMA subsystem.

If I understand your proposal correctly, the proposed IOCTL framework will be
able to handle all types of objects without any limitations in order and locking,
everything is going to be processed by the driver, which will decide if
such object/method is supported or not.

What will stop the GPU with DRMs to use it?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Hefty, Sean]

> If I understand your proposal correctly, the proposed IOCTL framework will be
> able to handle all types of objects without any limitations in order and locking,
> everything is going to be processed by the driver, which will decide if
> such object/method is supported or not.

For the core ioctl framework - yes.

An ioctl can be dispatched to some other common handling, which can validate object ordering and type.

> What will stop the GPU with DRMs to use it?

It would have to hook into the same file.  But if it's generic enough, nothing would prevent other subsystems (e.g. Xeon Phi or NVM) from re-using the code.  I don't see this as a negative.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

On Tue, May 31, 2016 at 08:01:31PM +0000, Hefty, Sean wrote:
> > If I understand your proposal correctly, the proposed IOCTL framework will be
> > able to handle all types of objects without any limitations in order and locking,
> > everything is going to be processed by the driver, which will decide if
> > such object/method is supported or not.
> 
> For the core ioctl framework - yes.
> 
> An ioctl can be dispatched to some other common handling, which can validate object ordering and type.

Agree and IMHO it is where we should be focused.

> 
> > What will stop the GPU with DRMs to use it?
> 
> It would have to hook into the same file.  But if it's generic enough, nothing would prevent other subsystems (e.g. Xeon Phi or NVM) from re-using the code.  I don't see this as a negative.

It is not negative, just out of the scope of RDMA subsystem.

> 
> - Sean

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC ABI 0/8] Netlink-based IOCTLs RDMA ABI

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

On Tue, May 24, 2016 at 07:37:13PM +0000, Hefty, Sean wrote:
> > struct ib_uverbs_ioctl_hdr {
> > 	__u32 length;
> 
> FWIW, I limited length to 16-bits, to prevent passing massive amounts of data between user space and the kernel as a single transfer.

We are preventing it in the code
 988 #define IB_UVERBS_MAX_CMD_SZ    4096
....
1096 static long ib_uverbs_ioctl(struct file *filp, unsigned int cmd,
1097                             unsigned long arg)
1098 {
...
1141         if (hdr.length > IB_UVERBS_MAX_CMD_SZ || hdr.length <= sizeof(hdr)) {
1142                 pr_debug("invalid uverbs ioctl command length %d\n", hdr.length);
1143                 return -EINVAL;
1144         }

> 
> > 	__u16 flags;
> 
> 16-bit flags seems limiting.

These flags will be relevant to IOCTL header only, and if we consume them all, It will be sign
to move to new IOCTL number.

> 
> > 	__u16 object_type;
> > 	__u16 reserved;
> > 	/* First 8 actions are common to all objects */
> > 	__u16 action;
> > 	__u32 user_handler;
> > 	/*
> > 	 * These fields represent core response only,
> > 	 * provider's response is given as a netlink attribute.
> > 	 */
> > 	struct ib_uverbs_uptr resp;
> 
> This is associating the kernel ABI with a specific implementation of user space components.

I don't see any specific in the proposed ABI.

> 
> Ioctl implementations I looked at treated the ioctl buffer as input-output (except for read only or write only ioctl's).  Maybe there's examples of redirecting the response to separate buffer(s)?
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]