[v1,04/10] IB/uverbs: Provide implementation private memory for the uverbs_attr_bundle

Commit Message

Jason Gunthorpe Aug. 10, 2018, 2:14 a.m. UTC

From: Jason Gunthorpe <[e-mail]>

This already existed as the anonymous 'ctx' structure, but this was not
really a useful form. Hoist this struct into bundle_priv and rework the
internal things to use it instead.

Move a bunch of the processing internal state into the priv and reduce the
excessive use of function arguments.

Signed-off-by: Jason Gunthorpe <[e-mail]>
Reviewed-by: Leon Romanovsky <[e-mail]>
---
 drivers/infiniband/core/uverbs_ioctl.c | 112 +++++++++++++------------
 include/rdma/uverbs_ioctl.h            |   1 +
 2 files changed, 58 insertions(+), 55 deletions(-)

Patch

diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index 8fd64f2b147a56..74b0a40aaa36ea 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -35,6 +35,18 @@ 
 #include "rdma_core.h"
 #include "uverbs.h"
 
+struct bundle_priv {
+	struct ib_uverbs_attr __user *user_attrs;
+	struct ib_uverbs_attr *uattrs;
+	struct uverbs_obj_attr *destroy_attr;
+
+	/*
+	 * Must be last. bundle ends in a flex array which overlaps
+	 * internal_buffer.
+	 */
+	struct uverbs_attr_bundle bundle;
+};
+
 static bool uverbs_is_attr_cleared(const struct ib_uverbs_attr *uattr,
 				   u16 len)
 {
@@ -46,12 +58,11 @@  static bool uverbs_is_attr_cleared(const struct ib_uverbs_attr *uattr,
 			   0, uattr->len - len);
 }
 
-static int uverbs_process_attr(struct ib_uverbs_file *ufile,
+static int uverbs_process_attr(struct bundle_priv *pbundle,
 			       const struct ib_uverbs_attr *uattr,
 			       u16 attr_id,
 			       const struct uverbs_attr_spec_hash *attr_spec_bucket,
 			       struct uverbs_attr_bundle_hash *attr_bundle_h,
-			       struct uverbs_obj_attr **destroy_attr,
 			       struct ib_uverbs_attr __user *uattr_ptr)
 {
 	const struct uverbs_attr_spec *spec;
@@ -147,9 +158,9 @@  static int uverbs_process_attr(struct ib_uverbs_file *ufile,
 
 		/* specs are allowed to have only one destroy attribute */
 		WARN_ON(spec->u.obj.access == UVERBS_ACCESS_DESTROY &&
-			*destroy_attr);
+			pbundle->destroy_attr);
 		if (spec->u.obj.access == UVERBS_ACCESS_DESTROY)
-			*destroy_attr = o_attr;
+			pbundle->destroy_attr = o_attr;
 
 		/*
 		 * The type of uattr->data is u64 for UVERBS_ATTR_TYPE_IDR and
@@ -159,7 +170,7 @@  static int uverbs_process_attr(struct ib_uverbs_file *ufile,
 		 */
 		o_attr->uobject = uverbs_get_uobject_from_file(
 					spec->u.obj.obj_type,
-					ufile,
+					pbundle->bundle.ufile,
 					spec->u.obj.access,
 					uattr->data_s64);
 
@@ -187,10 +198,11 @@  static int uverbs_process_attr(struct ib_uverbs_file *ufile,
 	return 0;
 }
 
-static int uverbs_finalize_attrs(struct uverbs_attr_bundle *attrs_bundle,
+static int uverbs_finalize_attrs(struct bundle_priv *pbundle,
 				 struct uverbs_attr_spec_hash *const *spec_hash,
 				 size_t num, bool commit)
 {
+	struct uverbs_attr_bundle *attrs_bundle = &pbundle->bundle;
 	unsigned int i;
 	int ret = 0;
 
@@ -233,27 +245,25 @@  static int uverbs_finalize_attrs(struct uverbs_attr_bundle *attrs_bundle,
 	return ret;
 }
 
-static int uverbs_uattrs_process(struct ib_uverbs_file *ufile,
-				 const struct ib_uverbs_attr *uattrs,
-				 size_t num_uattrs,
+static int uverbs_uattrs_process(size_t num_uattrs,
 				 const struct uverbs_method_spec *method,
-				 struct uverbs_attr_bundle *attr_bundle,
-				 struct uverbs_obj_attr **destroy_attr,
-				 struct ib_uverbs_attr __user *uattr_ptr)
+				 struct bundle_priv *pbundle)
 {
+	struct uverbs_attr_bundle *attr_bundle = &pbundle->bundle;
+	struct ib_uverbs_attr __user *uattr_ptr = pbundle->user_attrs;
 	size_t i;
 	int ret = 0;
 	int num_given_buckets = 0;
 
 	for (i = 0; i < num_uattrs; i++) {
-		const struct ib_uverbs_attr *uattr = &uattrs[i];
+		const struct ib_uverbs_attr *uattr = &pbundle->uattrs[i];
 		u16 attr_id = uattr->attr_id;
 		struct uverbs_attr_spec_hash *attr_spec_bucket;
 
 		ret = uverbs_ns_idx(&attr_id, method->num_buckets);
 		if (ret < 0 || !method->attr_buckets[ret]) {
 			if (uattr->flags & UVERBS_ATTR_F_MANDATORY) {
-				uverbs_finalize_attrs(attr_bundle,
+				uverbs_finalize_attrs(pbundle,
 						      method->attr_buckets,
 						      num_given_buckets,
 						      false);
@@ -270,12 +280,13 @@  static int uverbs_uattrs_process(struct ib_uverbs_file *ufile,
 			num_given_buckets = ret + 1;
 
 		attr_spec_bucket = method->attr_buckets[ret];
-		ret = uverbs_process_attr(ufile, uattr, attr_id,
+		ret = uverbs_process_attr(pbundle,
+					  uattr, attr_id,
 					  attr_spec_bucket,
-					  &attr_bundle->hash[ret], destroy_attr,
+					  &attr_bundle->hash[ret],
 					  uattr_ptr++);
 		if (ret) {
-			uverbs_finalize_attrs(attr_bundle,
+			uverbs_finalize_attrs(pbundle,
 					      method->attr_buckets,
 					      num_given_buckets,
 					      false);
@@ -287,8 +298,9 @@  static int uverbs_uattrs_process(struct ib_uverbs_file *ufile,
 }
 
 static int uverbs_validate_kernel_mandatory(const struct uverbs_method_spec *method_spec,
-					    struct uverbs_attr_bundle *attr_bundle)
+					    struct bundle_priv *pbundle)
 {
+	struct uverbs_attr_bundle *attr_bundle = &pbundle->bundle;
 	unsigned int i;
 
 	for (i = 0; i < attr_bundle->num_buckets; i++) {
@@ -316,27 +328,22 @@  static int uverbs_validate_kernel_mandatory(const struct uverbs_method_spec *met
 	return 0;
 }
 
-static int uverbs_handle_method(struct ib_uverbs_attr __user *uattr_ptr,
-				const struct ib_uverbs_attr *uattrs,
-				size_t num_uattrs,
-				struct ib_device *ibdev,
-				struct ib_uverbs_file *ufile,
+static int uverbs_handle_method(size_t num_uattrs,
 				const struct uverbs_method_spec *method_spec,
-				struct uverbs_attr_bundle *attr_bundle)
+				struct bundle_priv *pbundle)
 {
+	struct uverbs_attr_bundle *attr_bundle = &pbundle->bundle;
 	int ret;
 	int finalize_ret;
 	int num_given_buckets;
-	struct uverbs_obj_attr *destroy_attr = NULL;
 
 	num_given_buckets =
-		uverbs_uattrs_process(ufile, uattrs, num_uattrs, method_spec,
-				      attr_bundle, &destroy_attr, uattr_ptr);
+		uverbs_uattrs_process(num_uattrs, method_spec, pbundle);
 	if (num_given_buckets <= 0)
 		return -EINVAL;
 
 	attr_bundle->num_buckets = num_given_buckets;
-	ret = uverbs_validate_kernel_mandatory(method_spec, attr_bundle);
+	ret = uverbs_validate_kernel_mandatory(method_spec, pbundle);
 	if (ret)
 		goto cleanup;
 
@@ -344,21 +351,21 @@  static int uverbs_handle_method(struct ib_uverbs_attr __user *uattr_ptr,
 	 * We destroy the HW object before invoking the handler, handlers do
 	 * not get to manipulate the HW objects.
 	 */
-	if (destroy_attr) {
-		ret = uobj_destroy(destroy_attr->uobject);
+	if (pbundle->destroy_attr) {
+		ret = uobj_destroy(pbundle->destroy_attr->uobject);
 		if (ret)
 			goto cleanup;
 	}
 
-	ret = method_spec->handler(ufile, attr_bundle);
+	ret = method_spec->handler(pbundle->bundle.ufile, attr_bundle);
 
-	if (destroy_attr) {
-		uobj_put_destroy(destroy_attr->uobject);
-		destroy_attr->uobject = NULL;
+	if (pbundle->destroy_attr) {
+		uobj_put_destroy(pbundle->destroy_attr->uobject);
+		pbundle->destroy_attr->uobject = NULL;
 	}
 
 cleanup:
-	finalize_ret = uverbs_finalize_attrs(attr_bundle,
+	finalize_ret = uverbs_finalize_attrs(pbundle,
 					     method_spec->attr_buckets,
 					     attr_bundle->num_buckets,
 					     !ret);
@@ -370,16 +377,13 @@  static int uverbs_handle_method(struct ib_uverbs_attr __user *uattr_ptr,
 static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 				struct ib_uverbs_file *file,
 				struct ib_uverbs_ioctl_hdr *hdr,
-				void __user *buf)
+				struct ib_uverbs_attr __user *user_attrs)
 {
 	const struct uverbs_object_spec *object_spec;
 	const struct uverbs_method_spec *method_spec;
 	long err = 0;
 	unsigned int i;
-	struct {
-		struct ib_uverbs_attr		*uattrs;
-		struct uverbs_attr_bundle	*uverbs_attr_bundle;
-	} *ctx = NULL;
+	struct bundle_priv *ctx;
 	struct uverbs_attr *curr_attr;
 	unsigned long *curr_bitmap;
 	size_t ctx_size;
@@ -397,12 +401,11 @@  static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 		return -EPROTONOSUPPORT;
 
 	ctx_size = sizeof(*ctx) +
-		   sizeof(struct uverbs_attr_bundle) +
 		   sizeof(struct uverbs_attr_bundle_hash) * method_spec->num_buckets +
 		   sizeof(*ctx->uattrs) * hdr->num_attrs +
-		   sizeof(*ctx->uverbs_attr_bundle->hash[0].attrs) *
+		   sizeof(*ctx->bundle.hash[0].attrs) *
 		   method_spec->num_child_attrs +
-		   sizeof(*ctx->uverbs_attr_bundle->hash[0].valid_bitmap) *
+		   sizeof(*ctx->bundle.hash[0].valid_bitmap) *
 			(method_spec->num_child_attrs / BITS_PER_LONG +
 			 method_spec->num_buckets);
 
@@ -413,10 +416,8 @@  static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->uverbs_attr_bundle = (void *)ctx + sizeof(*ctx);
-	ctx->uattrs = (void *)(ctx->uverbs_attr_bundle + 1) +
-			      (sizeof(ctx->uverbs_attr_bundle->hash[0]) *
-			       method_spec->num_buckets);
+	ctx->uattrs = (void *)(ctx + 1) +
+		      (sizeof(ctx->bundle.hash[0]) * method_spec->num_buckets);
 	curr_attr = (void *)(ctx->uattrs + hdr->num_attrs);
 	curr_bitmap = (void *)(curr_attr + method_spec->num_child_attrs);
 
@@ -432,23 +433,25 @@  static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 
 		curr_num_attrs = method_spec->attr_buckets[i]->num_attrs;
 
-		ctx->uverbs_attr_bundle->hash[i].attrs = curr_attr;
+		ctx->bundle.hash[i].attrs = curr_attr;
 		curr_attr += curr_num_attrs;
-		ctx->uverbs_attr_bundle->hash[i].num_attrs = curr_num_attrs;
-		ctx->uverbs_attr_bundle->hash[i].valid_bitmap = curr_bitmap;
+		ctx->bundle.hash[i].num_attrs = curr_num_attrs;
+		ctx->bundle.hash[i].valid_bitmap = curr_bitmap;
 		bitmap_zero(curr_bitmap, curr_num_attrs);
 		curr_bitmap += BITS_TO_LONGS(curr_num_attrs);
 	}
 
-	err = copy_from_user(ctx->uattrs, buf,
+	err = copy_from_user(ctx->uattrs, user_attrs,
 			     sizeof(*ctx->uattrs) * hdr->num_attrs);
 	if (err) {
 		err = -EFAULT;
 		goto out;
 	}
 
-	err = uverbs_handle_method(buf, ctx->uattrs, hdr->num_attrs, ib_dev,
-				   file, method_spec, ctx->uverbs_attr_bundle);
+	ctx->destroy_attr = NULL;
+	ctx->bundle.ufile = file;
+	ctx->user_attrs = user_attrs;
+	err = uverbs_handle_method(hdr->num_attrs, method_spec, ctx);
 
 	/*
 	 * EPROTONOSUPPORT is ONLY to be returned if the ioctl framework can
@@ -499,8 +502,7 @@  long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			goto out;
 		}
 
-		err = ib_uverbs_cmd_verbs(ib_dev, file, &hdr,
-					  (__user void *)arg + sizeof(hdr));
+		err = ib_uverbs_cmd_verbs(ib_dev, file, &hdr, user_hdr->attrs);
 	} else {
 		err = -ENOIOCTLCMD;
 	}
diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h
index 339996e80c1687..3b497d9ed39592 100644
--- a/include/rdma/uverbs_ioctl.h
+++ b/include/rdma/uverbs_ioctl.h
@@ -494,6 +494,7 @@  struct uverbs_attr_bundle_hash {
 };
 
 struct uverbs_attr_bundle {
+	struct ib_uverbs_file *ufile;
 	size_t				num_buckets;
 	struct uverbs_attr_bundle_hash  hash[];
 };