diff mbox series

[01/10] IB/uverbs: Have the core code create the uverbs_root_spec

Message ID 20180803193143.11100-2-jgg@ziepe.ca (mailing list archive)
State Changes Requested
Headers show
Series Allow dissociation after destroy for the ioctl methods | expand

Commit Message

Jason Gunthorpe Aug. 3, 2018, 7:31 p.m. UTC
From: Jason Gunthorpe <jgg@mellanox.com>

There is no reason for drivers to do this, the core code should take of
everything. The drivers will provide their information from rodata to
describe their modifications to the core's base uapi specification.

The core uses this to build up the runtime uapi for each device.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 drivers/infiniband/core/uverbs_ioctl_merge.c |  2 -
 drivers/infiniband/core/uverbs_main.c        | 50 +++++++++++++-------
 drivers/infiniband/core/uverbs_std_types.c   |  1 -
 drivers/infiniband/hw/mlx5/main.c            | 45 +++++++-----------
 drivers/infiniband/hw/mlx5/mlx5_ib.h         |  1 +
 include/rdma/ib_verbs.h                      |  2 +-
 6 files changed, 51 insertions(+), 50 deletions(-)

Comments

Ruhl, Michael J Aug. 3, 2018, 8:44 p.m. UTC | #1
>-----Original Message-----
>From: Jason Gunthorpe [mailto:jgg@ziepe.ca]
>Sent: Friday, August 3, 2018 3:32 PM
>To: linux-rdma@vger.kernel.org; Leon Romanovsky <leonro@mellanox.com>;
>Guy Levi(SW) <guyle@mellanox.com>; Yishai Hadas
><yishaih@mellanox.com>; Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: Jason Gunthorpe <jgg@mellanox.com>
>Subject: [PATCH 01/10] IB/uverbs: Have the core code create the
>uverbs_root_spec
>
>From: Jason Gunthorpe <jgg@mellanox.com>
>
>There is no reason for drivers to do this, the core code should take of
>everything. The drivers will provide their information from rodata to
>describe their modifications to the core's base uapi specification.
>
>The core uses this to build up the runtime uapi for each device.
>
>Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
>---
> drivers/infiniband/core/uverbs_ioctl_merge.c |  2 -
> drivers/infiniband/core/uverbs_main.c        | 50 +++++++++++++-------
> drivers/infiniband/core/uverbs_std_types.c   |  1 -
> drivers/infiniband/hw/mlx5/main.c            | 45 +++++++-----------
> drivers/infiniband/hw/mlx5/mlx5_ib.h         |  1 +
> include/rdma/ib_verbs.h                      |  2 +-
> 6 files changed, 51 insertions(+), 50 deletions(-)
>
>diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c
>b/drivers/infiniband/core/uverbs_ioctl_merge.c
>index f81aa888ce5c4c..16b57592991581 100644
>--- a/drivers/infiniband/core/uverbs_ioctl_merge.c
>+++ b/drivers/infiniband/core/uverbs_ioctl_merge.c
>@@ -556,7 +556,6 @@ void uverbs_free_spec_tree(struct uverbs_root_spec
>*root)
>
> 	kfree(root);
> }
>-EXPORT_SYMBOL(uverbs_free_spec_tree);
>
> struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
> 						const struct
>uverbs_object_tree_def **trees)
>@@ -661,4 +660,3 @@ struct uverbs_root_spec
>*uverbs_alloc_spec_tree(unsigned int num_trees,
> 	uverbs_free_spec_tree(root_spec);
> 	return ERR_PTR(res);
> }
>-EXPORT_SYMBOL(uverbs_alloc_spec_tree);
>diff --git a/drivers/infiniband/core/uverbs_main.c
>b/drivers/infiniband/core/uverbs_main.c
>index 6f62146e9738a0..20003594b5d6a7 100644
>--- a/drivers/infiniband/core/uverbs_main.c
>+++ b/drivers/infiniband/core/uverbs_main.c
>@@ -994,6 +994,36 @@ static DEVICE_ATTR(abi_version, S_IRUGO,
>show_dev_abi_version, NULL);
> static CLASS_ATTR_STRING(abi_version, S_IRUGO,
> 			 __stringify(IB_USER_VERBS_ABI_VERSION));
>
>+static int ib_uverbs_create_uapi(struct ib_device *device,
>+				 struct ib_uverbs_device *uverbs_dev)
>+{
>+	const struct uverbs_object_tree_def **specs;
>+	struct uverbs_root_spec *specs_root;
>+	unsigned int num_specs = 1;
>+	unsigned int i;
>+
>+	if (device->driver_specs)
>+		for (i = 0; device->driver_specs[i]; i++)
>+			num_specs++;
>+
>+	specs = kmalloc_array(num_specs, sizeof(*specs), GFP_KERNEL);
>+	if (!specs)
>+		return -ENOMEM;
>+
>+	specs[0] = uverbs_default_get_objects();
>+	if (device->driver_specs)
>+		for (i = 0; device->driver_specs[i]; i++)
>+			specs[i+1] = device->driver_specs[i];
>+
>+	specs_root = uverbs_alloc_spec_tree(num_specs, specs);
>+	kfree(specs);
>+	if (IS_ERR(specs_root))
>+		return PTR_ERR(specs_root);
>+
>+	uverbs_dev->specs_root = specs_root;
>+	return 0;
>+}
>+
> static void ib_uverbs_add_one(struct ib_device *device)
> {
> 	int devnum;
>@@ -1036,6 +1066,9 @@ static void ib_uverbs_add_one(struct ib_device
>*device)
> 	rcu_assign_pointer(uverbs_dev->ib_dev, device);
> 	uverbs_dev->num_comp_vectors = device->num_comp_vectors;
>
>+	if (ib_uverbs_create_uapi(device, uverbs_dev))
>+		goto err;
>+
> 	cdev_init(&uverbs_dev->cdev, NULL);
> 	uverbs_dev->cdev.owner = THIS_MODULE;
> 	uverbs_dev->cdev.ops = device->mmap ? &uverbs_mmap_fops :
>&uverbs_fops;
>@@ -1055,23 +1088,6 @@ static void ib_uverbs_add_one(struct ib_device
>*device)
> 	if (device_create_file(uverbs_dev->dev, &dev_attr_abi_version))
> 		goto err_class;
>
>-	if (!device->driver_specs_root) {
>-		const struct uverbs_object_tree_def *default_root[] = {
>-			uverbs_default_get_objects()};
>-
>-		uverbs_dev->specs_root = uverbs_alloc_spec_tree(1,
>-								default_root);
>-		if (IS_ERR(uverbs_dev->specs_root))
>-			goto err_class;
>-	} else {
>-		uverbs_dev->specs_root = device->driver_specs_root;
>-		/*
>-		 * Take responsibility to free the specs allocated by the
>-		 * driver.
>-		 */
>-		device->driver_specs_root = NULL;
>-	}
>-
> 	ib_set_client_data(device, &uverbs_client, uverbs_dev);
>
> 	return;
>diff --git a/drivers/infiniband/core/uverbs_std_types.c
>b/drivers/infiniband/core/uverbs_std_types.c
>index 3aa7c7deac749a..7f22b820a21ba0 100644
>--- a/drivers/infiniband/core/uverbs_std_types.c
>+++ b/drivers/infiniband/core/uverbs_std_types.c
>@@ -316,4 +316,3 @@ const struct uverbs_object_tree_def
>*uverbs_default_get_objects(void)
> {
> 	return &uverbs_default_objects;
> }
>-EXPORT_SYMBOL_GPL(uverbs_default_get_objects);

That is tremendously cleaner than the original path.

This comment in uverbs_alloc_spec_tree() that seems to imply that device
can create a parse tree that will disable this interface:

/*
 * Devices which don't want to support ib_uverbs, should just allocate
 * an empty parsing tree. Every user-space command won't hit any valid
 * entry in the parsing tree and thus will fail.
 */

It seems that this patch would break this ability.  Is this correct, or should
the comment be updated/removed?

If you would like it:

Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

>diff --git a/drivers/infiniband/hw/mlx5/main.c
>b/drivers/infiniband/hw/mlx5/main.c
>index 13744b4631b452..f86d831ee27c5d 100644
>--- a/drivers/infiniband/hw/mlx5/main.c
>+++ b/drivers/infiniband/hw/mlx5/main.c
>@@ -5523,37 +5523,29 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE(
>
>	UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_F
>LAGS,
> 			     enum mlx5_ib_uapi_flow_action_flags));
>
>-#define NUM_TREES	5
> static int populate_specs_root(struct mlx5_ib_dev *dev)
> {
>-	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1]
>= {
>-		uverbs_default_get_objects()};
>-	size_t num_trees = 1;
>+	const struct uverbs_object_tree_def **trees = dev->driver_trees;
>+	size_t num_trees = 0;
>
>-	if (mlx5_accel_ipsec_device_caps(dev->mdev) &
>MLX5_ACCEL_IPSEC_CAP_DEVICE &&
>-	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
>-		default_root[num_trees++] = &mlx5_ib_flow_action;
>+	if (mlx5_accel_ipsec_device_caps(dev->mdev) &
>+	    MLX5_ACCEL_IPSEC_CAP_DEVICE)
>+		trees[num_trees++] = &mlx5_ib_flow_action;
>
>-	if (MLX5_CAP_DEV_MEM(dev->mdev, memic) &&
>-	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
>-		default_root[num_trees++] = &mlx5_ib_dm;
>+	if (MLX5_CAP_DEV_MEM(dev->mdev, memic))
>+		trees[num_trees++] = &mlx5_ib_dm;
>
> 	if (MLX5_CAP_GEN_64(dev->mdev, general_obj_types) &
>-			    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX &&
>-	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
>-		default_root[num_trees++] = mlx5_ib_get_devx_tree();
>+	    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX)
>+		trees[num_trees++] = mlx5_ib_get_devx_tree();
>
>-	num_trees += mlx5_ib_get_flow_trees(default_root + num_trees);
>+	num_trees += mlx5_ib_get_flow_trees(trees + num_trees);
>
>-	dev->ib_dev.driver_specs_root =
>-		uverbs_alloc_spec_tree(num_trees, default_root);
>+	WARN_ON(num_trees >= ARRAY_SIZE(dev->driver_trees));
>+	trees[num_trees] = NULL;
>+	dev->ib_dev.driver_specs = trees;
>
>-	return PTR_ERR_OR_ZERO(dev->ib_dev.driver_specs_root);
>-}
>-
>-static void depopulate_specs_root(struct mlx5_ib_dev *dev)
>-{
>-	uverbs_free_spec_tree(dev->ib_dev.driver_specs_root);
>+	return 0;
> }
>
> static int mlx5_ib_read_counters(struct ib_counters *counters,
>@@ -6092,11 +6084,6 @@ int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev
>*dev)
> 	return ib_register_device(&dev->ib_dev, NULL);
> }
>
>-static void mlx5_ib_stage_depopulate_specs(struct mlx5_ib_dev *dev)
>-{
>-	depopulate_specs_root(dev);
>-}
>-
> void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev)
> {
> 	destroy_umrc_res(dev);
>@@ -6231,7 +6218,7 @@ static const struct mlx5_ib_profile pf_profile = {
> 		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
> 	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
> 		     mlx5_ib_stage_populate_specs,
>-		     mlx5_ib_stage_depopulate_specs),
>+		     NULL),
> 	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
> 		     mlx5_ib_stage_ib_reg_init,
> 		     mlx5_ib_stage_ib_reg_cleanup),
>@@ -6279,7 +6266,7 @@ static const struct mlx5_ib_profile nic_rep_profile = {
> 		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
> 	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
> 		     mlx5_ib_stage_populate_specs,
>-		     mlx5_ib_stage_depopulate_specs),
>+		     NULL),
> 	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
> 		     mlx5_ib_stage_ib_reg_init,
> 		     mlx5_ib_stage_ib_reg_cleanup),
>diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h
>b/drivers/infiniband/hw/mlx5/mlx5_ib.h
>index b75754efc66309..320d4dfe8c2f41 100644
>--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
>+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
>@@ -860,6 +860,7 @@ to_mcounters(struct ib_counters *ibcntrs)
>
> struct mlx5_ib_dev {
> 	struct ib_device		ib_dev;
>+	const struct uverbs_object_tree_def *driver_trees[6];
> 	struct mlx5_core_dev		*mdev;
> 	struct mlx5_roce		roce[MLX5_MAX_PORTS];
> 	int				num_ports;
>diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>index 4ffe3e11e8fb6c..3b07201b9a804e 100644
>--- a/include/rdma/ib_verbs.h
>+++ b/include/rdma/ib_verbs.h
>@@ -2580,7 +2580,7 @@ struct ib_device {
> 	const struct cpumask *(*get_vector_affinity)(struct ib_device *ibdev,
> 						     int comp_vector);
>
>-	struct uverbs_root_spec		*driver_specs_root;
>+	const struct uverbs_object_tree_def *const *driver_specs;
> 	enum rdma_driver_id		driver_id;
> };
>
>--
>2.18.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Aug. 3, 2018, 8:49 p.m. UTC | #2
On Fri, Aug 03, 2018 at 08:44:24PM +0000, Ruhl, Michael J wrote:
> >diff --git a/drivers/infiniband/core/uverbs_std_types.c
> >b/drivers/infiniband/core/uverbs_std_types.c
> >index 3aa7c7deac749a..7f22b820a21ba0 100644
> >+++ b/drivers/infiniband/core/uverbs_std_types.c
> >@@ -316,4 +316,3 @@ const struct uverbs_object_tree_def
> >*uverbs_default_get_objects(void)
> > {
> > 	return &uverbs_default_objects;
> > }
> >-EXPORT_SYMBOL_GPL(uverbs_default_get_objects);
> 
> That is tremendously cleaner than the original path.
> 
> This comment in uverbs_alloc_spec_tree() that seems to imply that device
> can create a parse tree that will disable this interface:
>
> /*
>  * Devices which don't want to support ib_uverbs, should just allocate
>  * an empty parsing tree. Every user-space command won't hit any valid
>  * entry in the parsing tree and thus will fail.
>  */

Yes, that was something that was supported, but I have removed in this
series.

Drivers creating a struct ib_device *MUST* support verbs, period.

The only exception is usnic, and that error will never be repeated.

That said, the general infrastructure in this series is still general,
and it would be easy to create a a 'struct uverbs_api' that does not
include the verbs methods - eg for rdma-cm or something. Some gentle
rework would be needed but it is quite doable.

This is the approach to take if this interface is desired without
verbs (and run it on a different cdev than /dev/uverbs)

> It seems that this patch would break this ability.  Is this correct, or should
> the comment be updated/removed?

Yes, it should be removed. A later patch in this series drops that
comment already..

> If you would like it:
> 
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

Thanks!

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Aug. 6, 2018, 4:26 a.m. UTC | #3
On Fri, Aug 03, 2018 at 01:31:34PM -0600, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> There is no reason for drivers to do this, the core code should take of
> everything. The drivers will provide their information from rodata to
> describe their modifications to the core's base uapi specification.
>
> The core uses this to build up the runtime uapi for each device.
>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
>  drivers/infiniband/core/uverbs_ioctl_merge.c |  2 -
>  drivers/infiniband/core/uverbs_main.c        | 50 +++++++++++++-------
>  drivers/infiniband/core/uverbs_std_types.c   |  1 -
>  drivers/infiniband/hw/mlx5/main.c            | 45 +++++++-----------
>  drivers/infiniband/hw/mlx5/mlx5_ib.h         |  1 +
>  include/rdma/ib_verbs.h                      |  2 +-
>  6 files changed, 51 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c b/drivers/infiniband/core/uverbs_ioctl_merge.c
> index f81aa888ce5c4c..16b57592991581 100644
> --- a/drivers/infiniband/core/uverbs_ioctl_merge.c
> +++ b/drivers/infiniband/core/uverbs_ioctl_merge.c
> @@ -556,7 +556,6 @@ void uverbs_free_spec_tree(struct uverbs_root_spec *root)
>
>  	kfree(root);
>  }
> -EXPORT_SYMBOL(uverbs_free_spec_tree);
>
>  struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
>  						const struct uverbs_object_tree_def **trees)
> @@ -661,4 +660,3 @@ struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
>  	uverbs_free_spec_tree(root_spec);
>  	return ERR_PTR(res);
>  }
> -EXPORT_SYMBOL(uverbs_alloc_spec_tree);
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 6f62146e9738a0..20003594b5d6a7 100644
> --- a/drivers/infiniband/core/uverbs_main.c
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -994,6 +994,36 @@ static DEVICE_ATTR(abi_version, S_IRUGO, show_dev_abi_version, NULL);
>  static CLASS_ATTR_STRING(abi_version, S_IRUGO,
>  			 __stringify(IB_USER_VERBS_ABI_VERSION));
>
> +static int ib_uverbs_create_uapi(struct ib_device *device,
> +				 struct ib_uverbs_device *uverbs_dev)
> +{
> +	const struct uverbs_object_tree_def **specs;
> +	struct uverbs_root_spec *specs_root;
> +	unsigned int num_specs = 1;
> +	unsigned int i;
> +
> +	if (device->driver_specs)
> +		for (i = 0; device->driver_specs[i]; i++)
> +			num_specs++;
> +
> +	specs = kmalloc_array(num_specs, sizeof(*specs), GFP_KERNEL);
> +	if (!specs)
> +		return -ENOMEM;
> +
> +	specs[0] = uverbs_default_get_objects();
> +	if (device->driver_specs)
> +		for (i = 0; device->driver_specs[i]; i++)
> +			specs[i+1] = device->driver_specs[i];
> +
> +	specs_root = uverbs_alloc_spec_tree(num_specs, specs);
> +	kfree(specs);
> +	if (IS_ERR(specs_root))
> +		return PTR_ERR(specs_root);
> +
> +	uverbs_dev->specs_root = specs_root;
> +	return 0;
> +}
> +
>  static void ib_uverbs_add_one(struct ib_device *device)
>  {
>  	int devnum;
> @@ -1036,6 +1066,9 @@ static void ib_uverbs_add_one(struct ib_device *device)
>  	rcu_assign_pointer(uverbs_dev->ib_dev, device);
>  	uverbs_dev->num_comp_vectors = device->num_comp_vectors;
>
> +	if (ib_uverbs_create_uapi(device, uverbs_dev))
> +		goto err;
> +
>  	cdev_init(&uverbs_dev->cdev, NULL);
>  	uverbs_dev->cdev.owner = THIS_MODULE;
>  	uverbs_dev->cdev.ops = device->mmap ? &uverbs_mmap_fops : &uverbs_fops;
> @@ -1055,23 +1088,6 @@ static void ib_uverbs_add_one(struct ib_device *device)
>  	if (device_create_file(uverbs_dev->dev, &dev_attr_abi_version))
>  		goto err_class;
>
> -	if (!device->driver_specs_root) {
> -		const struct uverbs_object_tree_def *default_root[] = {
> -			uverbs_default_get_objects()};
> -
> -		uverbs_dev->specs_root = uverbs_alloc_spec_tree(1,
> -								default_root);
> -		if (IS_ERR(uverbs_dev->specs_root))
> -			goto err_class;
> -	} else {
> -		uverbs_dev->specs_root = device->driver_specs_root;
> -		/*
> -		 * Take responsibility to free the specs allocated by the
> -		 * driver.
> -		 */
> -		device->driver_specs_root = NULL;
> -	}
> -
>  	ib_set_client_data(device, &uverbs_client, uverbs_dev);
>
>  	return;
> diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
> index 3aa7c7deac749a..7f22b820a21ba0 100644
> --- a/drivers/infiniband/core/uverbs_std_types.c
> +++ b/drivers/infiniband/core/uverbs_std_types.c
> @@ -316,4 +316,3 @@ const struct uverbs_object_tree_def *uverbs_default_get_objects(void)
>  {
>  	return &uverbs_default_objects;
>  }
> -EXPORT_SYMBOL_GPL(uverbs_default_get_objects);
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 13744b4631b452..f86d831ee27c5d 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -5523,37 +5523,29 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE(
>  	UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_FLAGS,
>  			     enum mlx5_ib_uapi_flow_action_flags));
>
> -#define NUM_TREES	5
>  static int populate_specs_root(struct mlx5_ib_dev *dev)
>  {
> -	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
> -		uverbs_default_get_objects()};
> -	size_t num_trees = 1;
> +	const struct uverbs_object_tree_def **trees = dev->driver_trees;
> +	size_t num_trees = 0;

<...>

> --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> @@ -860,6 +860,7 @@ to_mcounters(struct ib_counters *ibcntrs)
>
>  struct mlx5_ib_dev {
>  	struct ib_device		ib_dev;
> +	const struct uverbs_object_tree_def *driver_trees[6];

Isn't this need to be "5" and not "6"? You moved one tree entry to core.

Thanks
Jason Gunthorpe Aug. 6, 2018, 7:17 p.m. UTC | #4
On Mon, Aug 06, 2018 at 07:26:26AM +0300, Leon Romanovsky wrote:
> > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> > index 13744b4631b452..f86d831ee27c5d 100644
> > +++ b/drivers/infiniband/hw/mlx5/main.c
> > @@ -5523,37 +5523,29 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE(
> >  	UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_FLAGS,
> >  			     enum mlx5_ib_uapi_flow_action_flags));
> >
> > -#define NUM_TREES	5
> >  static int populate_specs_root(struct mlx5_ib_dev *dev)
> >  {
> > -	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
> > -		uverbs_default_get_objects()};
> > -	size_t num_trees = 1;
> > +	const struct uverbs_object_tree_def **trees = dev->driver_trees;
> > +	size_t num_trees = 0;
> 
> <...>
> 
> > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > @@ -860,6 +860,7 @@ to_mcounters(struct ib_counters *ibcntrs)
> >
> >  struct mlx5_ib_dev {
> >  	struct ib_device		ib_dev;
> > +	const struct uverbs_object_tree_def *driver_trees[6];
> 
> Isn't this need to be "5" and not "6"? You moved one tree entry to core.

Not quite, one entry moved to core, but also the list had to become
NULL terminated, so no change in count. The original code used this:

  const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {

So 6 seems to be the right number..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Aug. 7, 2018, 6:22 a.m. UTC | #5
On Mon, Aug 06, 2018 at 01:17:15PM -0600, Jason Gunthorpe wrote:
> On Mon, Aug 06, 2018 at 07:26:26AM +0300, Leon Romanovsky wrote:
> > > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> > > index 13744b4631b452..f86d831ee27c5d 100644
> > > +++ b/drivers/infiniband/hw/mlx5/main.c
> > > @@ -5523,37 +5523,29 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE(
> > >  	UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_FLAGS,
> > >  			     enum mlx5_ib_uapi_flow_action_flags));
> > >
> > > -#define NUM_TREES	5
> > >  static int populate_specs_root(struct mlx5_ib_dev *dev)
> > >  {
> > > -	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
> > > -		uverbs_default_get_objects()};
> > > -	size_t num_trees = 1;
> > > +	const struct uverbs_object_tree_def **trees = dev->driver_trees;
> > > +	size_t num_trees = 0;
> >
> > <...>
> >
> > > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > > @@ -860,6 +860,7 @@ to_mcounters(struct ib_counters *ibcntrs)
> > >
> > >  struct mlx5_ib_dev {
> > >  	struct ib_device		ib_dev;
> > > +	const struct uverbs_object_tree_def *driver_trees[6];
> >
> > Isn't this need to be "5" and not "6"? You moved one tree entry to core.
>
> Not quite, one entry moved to core, but also the list had to become
> NULL terminated, so no change in count. The original code used this:
>
>   const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
>
> So 6 seems to be the right number..

I see, thanks

>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Aug. 9, 2018, 7:30 a.m. UTC | #6
On Fri, Aug 03, 2018 at 01:31:34PM -0600, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> There is no reason for drivers to do this, the core code should take of
> everything. The drivers will provide their information from rodata to
> describe their modifications to the core's base uapi specification.
>
> The core uses this to build up the runtime uapi for each device.
>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
>  drivers/infiniband/core/uverbs_ioctl_merge.c |  2 -
>  drivers/infiniband/core/uverbs_main.c        | 50 +++++++++++++-------
>  drivers/infiniband/core/uverbs_std_types.c   |  1 -
>  drivers/infiniband/hw/mlx5/main.c            | 45 +++++++-----------
>  drivers/infiniband/hw/mlx5/mlx5_ib.h         |  1 +
>  include/rdma/ib_verbs.h                      |  2 +-
>  6 files changed, 51 insertions(+), 50 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Guy Levi(SW) Aug. 9, 2018, 2:46 p.m. UTC | #7
> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg@ziepe.ca]
> Sent: Friday, August 03, 2018 10:32 PM
> To: linux-rdma@vger.kernel.org; Leon Romanovsky <leonro@mellanox.com>; Guy Levi(SW) <guyle@mellanox.com>; Yishai Hadas
> <yishaih@mellanox.com>; Ruhl, Michael J <michael.j.ruhl@intel.com>
> Cc: Jason Gunthorpe <jgg@mellanox.com>
> Subject: [PATCH 01/10] IB/uverbs: Have the core code create the uverbs_root_spec
> 
> From: Jason Gunthorpe <jgg@mellanox.com>
> 
> There is no reason for drivers to do this, the core code should take of
> everything. The drivers will provide their information from rodata to
> describe their modifications to the core's base uapi specification.
> 
> The core uses this to build up the runtime uapi for each device.
> 
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
>  drivers/infiniband/core/uverbs_ioctl_merge.c |  2 -
>  drivers/infiniband/core/uverbs_main.c        | 50 +++++++++++++-------
>  drivers/infiniband/core/uverbs_std_types.c   |  1 -
>  drivers/infiniband/hw/mlx5/main.c            | 45 +++++++-----------
>  drivers/infiniband/hw/mlx5/mlx5_ib.h         |  1 +
>  include/rdma/ib_verbs.h                      |  2 +-
>  6 files changed, 51 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c b/drivers/infiniband/core/uverbs_ioctl_merge.c
> index f81aa888ce5c4c..16b57592991581 100644
> --- a/drivers/infiniband/core/uverbs_ioctl_merge.c
> +++ b/drivers/infiniband/core/uverbs_ioctl_merge.c
> @@ -556,7 +556,6 @@ void uverbs_free_spec_tree(struct uverbs_root_spec *root)
> 
>  	kfree(root);
>  }
> -EXPORT_SYMBOL(uverbs_free_spec_tree);

Shouldn't it be removed from the global header file?

> 
>  struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
>  						const struct uverbs_object_tree_def **trees)
> @@ -661,4 +660,3 @@ struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
>  	uverbs_free_spec_tree(root_spec);
>  	return ERR_PTR(res);
>  }
> -EXPORT_SYMBOL(uverbs_alloc_spec_tree);

Ditto.

> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 6f62146e9738a0..20003594b5d6a7 100644
> --- a/drivers/infiniband/core/uverbs_main.c
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -994,6 +994,36 @@ static DEVICE_ATTR(abi_version, S_IRUGO, show_dev_abi_version, NULL);
>  static CLASS_ATTR_STRING(abi_version, S_IRUGO,
>  			 __stringify(IB_USER_VERBS_ABI_VERSION));
> 
> +static int ib_uverbs_create_uapi(struct ib_device *device,
> +				 struct ib_uverbs_device *uverbs_dev)
> +{
> +	const struct uverbs_object_tree_def **specs;
> +	struct uverbs_root_spec *specs_root;
> +	unsigned int num_specs = 1;
> +	unsigned int i;
> +
> +	if (device->driver_specs)
> +		for (i = 0; device->driver_specs[i]; i++)
> +			num_specs++;
> +
> +	specs = kmalloc_array(num_specs, sizeof(*specs), GFP_KERNEL);
> +	if (!specs)
> +		return -ENOMEM;
> +
> +	specs[0] = uverbs_default_get_objects();
> +	if (device->driver_specs)
> +		for (i = 0; device->driver_specs[i]; i++)
> +			specs[i+1] = device->driver_specs[i];
> +
> +	specs_root = uverbs_alloc_spec_tree(num_specs, specs);
> +	kfree(specs);
> +	if (IS_ERR(specs_root))
> +		return PTR_ERR(specs_root);
> +
> +	uverbs_dev->specs_root = specs_root;
> +	return 0;
> +}
> +
>  static void ib_uverbs_add_one(struct ib_device *device)
>  {
>  	int devnum;
> @@ -1036,6 +1066,9 @@ static void ib_uverbs_add_one(struct ib_device *device)
>  	rcu_assign_pointer(uverbs_dev->ib_dev, device);
>  	uverbs_dev->num_comp_vectors = device->num_comp_vectors;
> 
> +	if (ib_uverbs_create_uapi(device, uverbs_dev))
> +		goto err;
> +
>  	cdev_init(&uverbs_dev->cdev, NULL);
>  	uverbs_dev->cdev.owner = THIS_MODULE;
>  	uverbs_dev->cdev.ops = device->mmap ? &uverbs_mmap_fops : &uverbs_fops;
> @@ -1055,23 +1088,6 @@ static void ib_uverbs_add_one(struct ib_device *device)
>  	if (device_create_file(uverbs_dev->dev, &dev_attr_abi_version))
>  		goto err_class;
> 
> -	if (!device->driver_specs_root) {
> -		const struct uverbs_object_tree_def *default_root[] = {
> -			uverbs_default_get_objects()};
> -
> -		uverbs_dev->specs_root = uverbs_alloc_spec_tree(1,
> -								default_root);
> -		if (IS_ERR(uverbs_dev->specs_root))
> -			goto err_class;
> -	} else {
> -		uverbs_dev->specs_root = device->driver_specs_root;
> -		/*
> -		 * Take responsibility to free the specs allocated by the
> -		 * driver.
> -		 */
> -		device->driver_specs_root = NULL;
> -	}
> -
>  	ib_set_client_data(device, &uverbs_client, uverbs_dev);
> 
>  	return;
> diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
> index 3aa7c7deac749a..7f22b820a21ba0 100644
> --- a/drivers/infiniband/core/uverbs_std_types.c
> +++ b/drivers/infiniband/core/uverbs_std_types.c
> @@ -316,4 +316,3 @@ const struct uverbs_object_tree_def *uverbs_default_get_objects(void)
>  {
>  	return &uverbs_default_objects;
>  }
> -EXPORT_SYMBOL_GPL(uverbs_default_get_objects);
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 13744b4631b452..f86d831ee27c5d 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -5523,37 +5523,29 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE(
>  	UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_FLAGS,
>  			     enum mlx5_ib_uapi_flow_action_flags));
> 
> -#define NUM_TREES	5
>  static int populate_specs_root(struct mlx5_ib_dev *dev)
>  {
> -	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
> -		uverbs_default_get_objects()};
> -	size_t num_trees = 1;
> +	const struct uverbs_object_tree_def **trees = dev->driver_trees;
> +	size_t num_trees = 0;
> 
> -	if (mlx5_accel_ipsec_device_caps(dev->mdev) & MLX5_ACCEL_IPSEC_CAP_DEVICE &&
> -	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
> -		default_root[num_trees++] = &mlx5_ib_flow_action;
> +	if (mlx5_accel_ipsec_device_caps(dev->mdev) &
> +	    MLX5_ACCEL_IPSEC_CAP_DEVICE)
> +		trees[num_trees++] = &mlx5_ib_flow_action;
> 
> -	if (MLX5_CAP_DEV_MEM(dev->mdev, memic) &&
> -	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
> -		default_root[num_trees++] = &mlx5_ib_dm;
> +	if (MLX5_CAP_DEV_MEM(dev->mdev, memic))
> +		trees[num_trees++] = &mlx5_ib_dm;
> 
>  	if (MLX5_CAP_GEN_64(dev->mdev, general_obj_types) &
> -			    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX &&
> -	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
> -		default_root[num_trees++] = mlx5_ib_get_devx_tree();
> +	    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX)
> +		trees[num_trees++] = mlx5_ib_get_devx_tree();
> 
> -	num_trees += mlx5_ib_get_flow_trees(default_root + num_trees);
> +	num_trees += mlx5_ib_get_flow_trees(trees + num_trees);
> 
> -	dev->ib_dev.driver_specs_root =
> -		uverbs_alloc_spec_tree(num_trees, default_root);
> +	WARN_ON(num_trees >= ARRAY_SIZE(dev->driver_trees));
> +	trees[num_trees] = NULL;
> +	dev->ib_dev.driver_specs = trees;
> 
> -	return PTR_ERR_OR_ZERO(dev->ib_dev.driver_specs_root);
> -}
> -
> -static void depopulate_specs_root(struct mlx5_ib_dev *dev)
> -{
> -	uverbs_free_spec_tree(dev->ib_dev.driver_specs_root);
> +	return 0;
>  }
> 
>  static int mlx5_ib_read_counters(struct ib_counters *counters,
> @@ -6092,11 +6084,6 @@ int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev *dev)
>  	return ib_register_device(&dev->ib_dev, NULL);
>  }
> 
> -static void mlx5_ib_stage_depopulate_specs(struct mlx5_ib_dev *dev)
> -{
> -	depopulate_specs_root(dev);
> -}
> -
>  void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev)
>  {
>  	destroy_umrc_res(dev);
> @@ -6231,7 +6218,7 @@ static const struct mlx5_ib_profile pf_profile = {
>  		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
>  	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
>  		     mlx5_ib_stage_populate_specs,
> -		     mlx5_ib_stage_depopulate_specs),
> +		     NULL),
>  	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
>  		     mlx5_ib_stage_ib_reg_init,
>  		     mlx5_ib_stage_ib_reg_cleanup),
> @@ -6279,7 +6266,7 @@ static const struct mlx5_ib_profile nic_rep_profile = {
>  		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
>  	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
>  		     mlx5_ib_stage_populate_specs,
> -		     mlx5_ib_stage_depopulate_specs),
> +		     NULL),
>  	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
>  		     mlx5_ib_stage_ib_reg_init,
>  		     mlx5_ib_stage_ib_reg_cleanup),
> diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> index b75754efc66309..320d4dfe8c2f41 100644
> --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> @@ -860,6 +860,7 @@ to_mcounters(struct ib_counters *ibcntrs)
> 
>  struct mlx5_ib_dev {
>  	struct ib_device		ib_dev;
> +	const struct uverbs_object_tree_def *driver_trees[6];
>  	struct mlx5_core_dev		*mdev;
>  	struct mlx5_roce		roce[MLX5_MAX_PORTS];
>  	int				num_ports;
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 4ffe3e11e8fb6c..3b07201b9a804e 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2580,7 +2580,7 @@ struct ib_device {
>  	const struct cpumask *(*get_vector_affinity)(struct ib_device *ibdev,
>  						     int comp_vector);
> 
> -	struct uverbs_root_spec		*driver_specs_root;
> +	const struct uverbs_object_tree_def *const *driver_specs;
>  	enum rdma_driver_id		driver_id;
>  };
> 
> --
> 2.18.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Aug. 9, 2018, 3:31 p.m. UTC | #8
On Thu, Aug 09, 2018 at 05:46:46PM +0300, Guy Levi(SW) wrote:
>
> > -----Original Message-----
> > From: Jason Gunthorpe [mailto:jgg@ziepe.ca]
> > Sent: Friday, August 03, 2018 10:32 PM
> > To: linux-rdma@vger.kernel.org; Leon Romanovsky <leonro@mellanox.com>; Guy Levi(SW) <guyle@mellanox.com>; Yishai Hadas
> > <yishaih@mellanox.com>; Ruhl, Michael J <michael.j.ruhl@intel.com>
> > Cc: Jason Gunthorpe <jgg@mellanox.com>
> > Subject: [PATCH 01/10] IB/uverbs: Have the core code create the uverbs_root_spec
> >
> > From: Jason Gunthorpe <jgg@mellanox.com>
> >
> > There is no reason for drivers to do this, the core code should take of
> > everything. The drivers will provide their information from rodata to
> > describe their modifications to the core's base uapi specification.
> >
> > The core uses this to build up the runtime uapi for each device.
> >
> > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> > ---
> >  drivers/infiniband/core/uverbs_ioctl_merge.c |  2 -
> >  drivers/infiniband/core/uverbs_main.c        | 50 +++++++++++++-------
> >  drivers/infiniband/core/uverbs_std_types.c   |  1 -
> >  drivers/infiniband/hw/mlx5/main.c            | 45 +++++++-----------
> >  drivers/infiniband/hw/mlx5/mlx5_ib.h         |  1 +
> >  include/rdma/ib_verbs.h                      |  2 +-
> >  6 files changed, 51 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c b/drivers/infiniband/core/uverbs_ioctl_merge.c
> > index f81aa888ce5c4c..16b57592991581 100644
> > --- a/drivers/infiniband/core/uverbs_ioctl_merge.c
> > +++ b/drivers/infiniband/core/uverbs_ioctl_merge.c
> > @@ -556,7 +556,6 @@ void uverbs_free_spec_tree(struct uverbs_root_spec *root)
> >
> >  	kfree(root);
> >  }
> > -EXPORT_SYMBOL(uverbs_free_spec_tree);
>
> Shouldn't it be removed from the global header file?

It is not important, this function is removed in the following patch:
"IB/uverbs: Remove struct uverbs_root_spec and all supporting code"


>
> >
> >  struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
> >  						const struct uverbs_object_tree_def **trees)
> > @@ -661,4 +660,3 @@ struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
> >  	uverbs_free_spec_tree(root_spec);
> >  	return ERR_PTR(res);
> >  }
> > -EXPORT_SYMBOL(uverbs_alloc_spec_tree);
>
> Ditto.
>
> > diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> > index 6f62146e9738a0..20003594b5d6a7 100644
> > --- a/drivers/infiniband/core/uverbs_main.c
> > +++ b/drivers/infiniband/core/uverbs_main.c
> > @@ -994,6 +994,36 @@ static DEVICE_ATTR(abi_version, S_IRUGO, show_dev_abi_version, NULL);
> >  static CLASS_ATTR_STRING(abi_version, S_IRUGO,
> >  			 __stringify(IB_USER_VERBS_ABI_VERSION));
> >
> > +static int ib_uverbs_create_uapi(struct ib_device *device,
> > +				 struct ib_uverbs_device *uverbs_dev)
> > +{
> > +	const struct uverbs_object_tree_def **specs;
> > +	struct uverbs_root_spec *specs_root;
> > +	unsigned int num_specs = 1;
> > +	unsigned int i;
> > +
> > +	if (device->driver_specs)
> > +		for (i = 0; device->driver_specs[i]; i++)
> > +			num_specs++;
> > +
> > +	specs = kmalloc_array(num_specs, sizeof(*specs), GFP_KERNEL);
> > +	if (!specs)
> > +		return -ENOMEM;
> > +
> > +	specs[0] = uverbs_default_get_objects();
> > +	if (device->driver_specs)
> > +		for (i = 0; device->driver_specs[i]; i++)
> > +			specs[i+1] = device->driver_specs[i];
> > +
> > +	specs_root = uverbs_alloc_spec_tree(num_specs, specs);
> > +	kfree(specs);
> > +	if (IS_ERR(specs_root))
> > +		return PTR_ERR(specs_root);
> > +
> > +	uverbs_dev->specs_root = specs_root;
> > +	return 0;
> > +}
> > +
> >  static void ib_uverbs_add_one(struct ib_device *device)
> >  {
> >  	int devnum;
> > @@ -1036,6 +1066,9 @@ static void ib_uverbs_add_one(struct ib_device *device)
> >  	rcu_assign_pointer(uverbs_dev->ib_dev, device);
> >  	uverbs_dev->num_comp_vectors = device->num_comp_vectors;
> >
> > +	if (ib_uverbs_create_uapi(device, uverbs_dev))
> > +		goto err;
> > +
> >  	cdev_init(&uverbs_dev->cdev, NULL);
> >  	uverbs_dev->cdev.owner = THIS_MODULE;
> >  	uverbs_dev->cdev.ops = device->mmap ? &uverbs_mmap_fops : &uverbs_fops;
> > @@ -1055,23 +1088,6 @@ static void ib_uverbs_add_one(struct ib_device *device)
> >  	if (device_create_file(uverbs_dev->dev, &dev_attr_abi_version))
> >  		goto err_class;
> >
> > -	if (!device->driver_specs_root) {
> > -		const struct uverbs_object_tree_def *default_root[] = {
> > -			uverbs_default_get_objects()};
> > -
> > -		uverbs_dev->specs_root = uverbs_alloc_spec_tree(1,
> > -								default_root);
> > -		if (IS_ERR(uverbs_dev->specs_root))
> > -			goto err_class;
> > -	} else {
> > -		uverbs_dev->specs_root = device->driver_specs_root;
> > -		/*
> > -		 * Take responsibility to free the specs allocated by the
> > -		 * driver.
> > -		 */
> > -		device->driver_specs_root = NULL;
> > -	}
> > -
> >  	ib_set_client_data(device, &uverbs_client, uverbs_dev);
> >
> >  	return;
> > diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
> > index 3aa7c7deac749a..7f22b820a21ba0 100644
> > --- a/drivers/infiniband/core/uverbs_std_types.c
> > +++ b/drivers/infiniband/core/uverbs_std_types.c
> > @@ -316,4 +316,3 @@ const struct uverbs_object_tree_def *uverbs_default_get_objects(void)
> >  {
> >  	return &uverbs_default_objects;
> >  }
> > -EXPORT_SYMBOL_GPL(uverbs_default_get_objects);
> > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> > index 13744b4631b452..f86d831ee27c5d 100644
> > --- a/drivers/infiniband/hw/mlx5/main.c
> > +++ b/drivers/infiniband/hw/mlx5/main.c
> > @@ -5523,37 +5523,29 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE(
> >  	UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_FLAGS,
> >  			     enum mlx5_ib_uapi_flow_action_flags));
> >
> > -#define NUM_TREES	5
> >  static int populate_specs_root(struct mlx5_ib_dev *dev)
> >  {
> > -	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
> > -		uverbs_default_get_objects()};
> > -	size_t num_trees = 1;
> > +	const struct uverbs_object_tree_def **trees = dev->driver_trees;
> > +	size_t num_trees = 0;
> >
> > -	if (mlx5_accel_ipsec_device_caps(dev->mdev) & MLX5_ACCEL_IPSEC_CAP_DEVICE &&
> > -	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
> > -		default_root[num_trees++] = &mlx5_ib_flow_action;
> > +	if (mlx5_accel_ipsec_device_caps(dev->mdev) &
> > +	    MLX5_ACCEL_IPSEC_CAP_DEVICE)
> > +		trees[num_trees++] = &mlx5_ib_flow_action;
> >
> > -	if (MLX5_CAP_DEV_MEM(dev->mdev, memic) &&
> > -	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
> > -		default_root[num_trees++] = &mlx5_ib_dm;
> > +	if (MLX5_CAP_DEV_MEM(dev->mdev, memic))
> > +		trees[num_trees++] = &mlx5_ib_dm;
> >
> >  	if (MLX5_CAP_GEN_64(dev->mdev, general_obj_types) &
> > -			    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX &&
> > -	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
> > -		default_root[num_trees++] = mlx5_ib_get_devx_tree();
> > +	    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX)
> > +		trees[num_trees++] = mlx5_ib_get_devx_tree();
> >
> > -	num_trees += mlx5_ib_get_flow_trees(default_root + num_trees);
> > +	num_trees += mlx5_ib_get_flow_trees(trees + num_trees);
> >
> > -	dev->ib_dev.driver_specs_root =
> > -		uverbs_alloc_spec_tree(num_trees, default_root);
> > +	WARN_ON(num_trees >= ARRAY_SIZE(dev->driver_trees));
> > +	trees[num_trees] = NULL;
> > +	dev->ib_dev.driver_specs = trees;
> >
> > -	return PTR_ERR_OR_ZERO(dev->ib_dev.driver_specs_root);
> > -}
> > -
> > -static void depopulate_specs_root(struct mlx5_ib_dev *dev)
> > -{
> > -	uverbs_free_spec_tree(dev->ib_dev.driver_specs_root);
> > +	return 0;
> >  }
> >
> >  static int mlx5_ib_read_counters(struct ib_counters *counters,
> > @@ -6092,11 +6084,6 @@ int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev *dev)
> >  	return ib_register_device(&dev->ib_dev, NULL);
> >  }
> >
> > -static void mlx5_ib_stage_depopulate_specs(struct mlx5_ib_dev *dev)
> > -{
> > -	depopulate_specs_root(dev);
> > -}
> > -
> >  void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev)
> >  {
> >  	destroy_umrc_res(dev);
> > @@ -6231,7 +6218,7 @@ static const struct mlx5_ib_profile pf_profile = {
> >  		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
> >  	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
> >  		     mlx5_ib_stage_populate_specs,
> > -		     mlx5_ib_stage_depopulate_specs),
> > +		     NULL),
> >  	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
> >  		     mlx5_ib_stage_ib_reg_init,
> >  		     mlx5_ib_stage_ib_reg_cleanup),
> > @@ -6279,7 +6266,7 @@ static const struct mlx5_ib_profile nic_rep_profile = {
> >  		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
> >  	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
> >  		     mlx5_ib_stage_populate_specs,
> > -		     mlx5_ib_stage_depopulate_specs),
> > +		     NULL),
> >  	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
> >  		     mlx5_ib_stage_ib_reg_init,
> >  		     mlx5_ib_stage_ib_reg_cleanup),
> > diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > index b75754efc66309..320d4dfe8c2f41 100644
> > --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > @@ -860,6 +860,7 @@ to_mcounters(struct ib_counters *ibcntrs)
> >
> >  struct mlx5_ib_dev {
> >  	struct ib_device		ib_dev;
> > +	const struct uverbs_object_tree_def *driver_trees[6];
> >  	struct mlx5_core_dev		*mdev;
> >  	struct mlx5_roce		roce[MLX5_MAX_PORTS];
> >  	int				num_ports;
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 4ffe3e11e8fb6c..3b07201b9a804e 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -2580,7 +2580,7 @@ struct ib_device {
> >  	const struct cpumask *(*get_vector_affinity)(struct ib_device *ibdev,
> >  						     int comp_vector);
> >
> > -	struct uverbs_root_spec		*driver_specs_root;
> > +	const struct uverbs_object_tree_def *const *driver_specs;
> >  	enum rdma_driver_id		driver_id;
> >  };
> >
> > --
> > 2.18.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c b/drivers/infiniband/core/uverbs_ioctl_merge.c
index f81aa888ce5c4c..16b57592991581 100644
--- a/drivers/infiniband/core/uverbs_ioctl_merge.c
+++ b/drivers/infiniband/core/uverbs_ioctl_merge.c
@@ -556,7 +556,6 @@  void uverbs_free_spec_tree(struct uverbs_root_spec *root)
 
 	kfree(root);
 }
-EXPORT_SYMBOL(uverbs_free_spec_tree);
 
 struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
 						const struct uverbs_object_tree_def **trees)
@@ -661,4 +660,3 @@  struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees,
 	uverbs_free_spec_tree(root_spec);
 	return ERR_PTR(res);
 }
-EXPORT_SYMBOL(uverbs_alloc_spec_tree);
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 6f62146e9738a0..20003594b5d6a7 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -994,6 +994,36 @@  static DEVICE_ATTR(abi_version, S_IRUGO, show_dev_abi_version, NULL);
 static CLASS_ATTR_STRING(abi_version, S_IRUGO,
 			 __stringify(IB_USER_VERBS_ABI_VERSION));
 
+static int ib_uverbs_create_uapi(struct ib_device *device,
+				 struct ib_uverbs_device *uverbs_dev)
+{
+	const struct uverbs_object_tree_def **specs;
+	struct uverbs_root_spec *specs_root;
+	unsigned int num_specs = 1;
+	unsigned int i;
+
+	if (device->driver_specs)
+		for (i = 0; device->driver_specs[i]; i++)
+			num_specs++;
+
+	specs = kmalloc_array(num_specs, sizeof(*specs), GFP_KERNEL);
+	if (!specs)
+		return -ENOMEM;
+
+	specs[0] = uverbs_default_get_objects();
+	if (device->driver_specs)
+		for (i = 0; device->driver_specs[i]; i++)
+			specs[i+1] = device->driver_specs[i];
+
+	specs_root = uverbs_alloc_spec_tree(num_specs, specs);
+	kfree(specs);
+	if (IS_ERR(specs_root))
+		return PTR_ERR(specs_root);
+
+	uverbs_dev->specs_root = specs_root;
+	return 0;
+}
+
 static void ib_uverbs_add_one(struct ib_device *device)
 {
 	int devnum;
@@ -1036,6 +1066,9 @@  static void ib_uverbs_add_one(struct ib_device *device)
 	rcu_assign_pointer(uverbs_dev->ib_dev, device);
 	uverbs_dev->num_comp_vectors = device->num_comp_vectors;
 
+	if (ib_uverbs_create_uapi(device, uverbs_dev))
+		goto err;
+
 	cdev_init(&uverbs_dev->cdev, NULL);
 	uverbs_dev->cdev.owner = THIS_MODULE;
 	uverbs_dev->cdev.ops = device->mmap ? &uverbs_mmap_fops : &uverbs_fops;
@@ -1055,23 +1088,6 @@  static void ib_uverbs_add_one(struct ib_device *device)
 	if (device_create_file(uverbs_dev->dev, &dev_attr_abi_version))
 		goto err_class;
 
-	if (!device->driver_specs_root) {
-		const struct uverbs_object_tree_def *default_root[] = {
-			uverbs_default_get_objects()};
-
-		uverbs_dev->specs_root = uverbs_alloc_spec_tree(1,
-								default_root);
-		if (IS_ERR(uverbs_dev->specs_root))
-			goto err_class;
-	} else {
-		uverbs_dev->specs_root = device->driver_specs_root;
-		/*
-		 * Take responsibility to free the specs allocated by the
-		 * driver.
-		 */
-		device->driver_specs_root = NULL;
-	}
-
 	ib_set_client_data(device, &uverbs_client, uverbs_dev);
 
 	return;
diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index 3aa7c7deac749a..7f22b820a21ba0 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -316,4 +316,3 @@  const struct uverbs_object_tree_def *uverbs_default_get_objects(void)
 {
 	return &uverbs_default_objects;
 }
-EXPORT_SYMBOL_GPL(uverbs_default_get_objects);
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 13744b4631b452..f86d831ee27c5d 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -5523,37 +5523,29 @@  ADD_UVERBS_ATTRIBUTES_SIMPLE(
 	UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_FLAGS,
 			     enum mlx5_ib_uapi_flow_action_flags));
 
-#define NUM_TREES	5
 static int populate_specs_root(struct mlx5_ib_dev *dev)
 {
-	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
-		uverbs_default_get_objects()};
-	size_t num_trees = 1;
+	const struct uverbs_object_tree_def **trees = dev->driver_trees;
+	size_t num_trees = 0;
 
-	if (mlx5_accel_ipsec_device_caps(dev->mdev) & MLX5_ACCEL_IPSEC_CAP_DEVICE &&
-	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
-		default_root[num_trees++] = &mlx5_ib_flow_action;
+	if (mlx5_accel_ipsec_device_caps(dev->mdev) &
+	    MLX5_ACCEL_IPSEC_CAP_DEVICE)
+		trees[num_trees++] = &mlx5_ib_flow_action;
 
-	if (MLX5_CAP_DEV_MEM(dev->mdev, memic) &&
-	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
-		default_root[num_trees++] = &mlx5_ib_dm;
+	if (MLX5_CAP_DEV_MEM(dev->mdev, memic))
+		trees[num_trees++] = &mlx5_ib_dm;
 
 	if (MLX5_CAP_GEN_64(dev->mdev, general_obj_types) &
-			    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX &&
-	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
-		default_root[num_trees++] = mlx5_ib_get_devx_tree();
+	    MLX5_GENERAL_OBJ_TYPES_CAP_UCTX)
+		trees[num_trees++] = mlx5_ib_get_devx_tree();
 
-	num_trees += mlx5_ib_get_flow_trees(default_root + num_trees);
+	num_trees += mlx5_ib_get_flow_trees(trees + num_trees);
 
-	dev->ib_dev.driver_specs_root =
-		uverbs_alloc_spec_tree(num_trees, default_root);
+	WARN_ON(num_trees >= ARRAY_SIZE(dev->driver_trees));
+	trees[num_trees] = NULL;
+	dev->ib_dev.driver_specs = trees;
 
-	return PTR_ERR_OR_ZERO(dev->ib_dev.driver_specs_root);
-}
-
-static void depopulate_specs_root(struct mlx5_ib_dev *dev)
-{
-	uverbs_free_spec_tree(dev->ib_dev.driver_specs_root);
+	return 0;
 }
 
 static int mlx5_ib_read_counters(struct ib_counters *counters,
@@ -6092,11 +6084,6 @@  int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev *dev)
 	return ib_register_device(&dev->ib_dev, NULL);
 }
 
-static void mlx5_ib_stage_depopulate_specs(struct mlx5_ib_dev *dev)
-{
-	depopulate_specs_root(dev);
-}
-
 void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev)
 {
 	destroy_umrc_res(dev);
@@ -6231,7 +6218,7 @@  static const struct mlx5_ib_profile pf_profile = {
 		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
 	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
 		     mlx5_ib_stage_populate_specs,
-		     mlx5_ib_stage_depopulate_specs),
+		     NULL),
 	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
 		     mlx5_ib_stage_ib_reg_init,
 		     mlx5_ib_stage_ib_reg_cleanup),
@@ -6279,7 +6266,7 @@  static const struct mlx5_ib_profile nic_rep_profile = {
 		     mlx5_ib_stage_pre_ib_reg_umr_cleanup),
 	STAGE_CREATE(MLX5_IB_STAGE_SPECS,
 		     mlx5_ib_stage_populate_specs,
-		     mlx5_ib_stage_depopulate_specs),
+		     NULL),
 	STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
 		     mlx5_ib_stage_ib_reg_init,
 		     mlx5_ib_stage_ib_reg_cleanup),
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index b75754efc66309..320d4dfe8c2f41 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -860,6 +860,7 @@  to_mcounters(struct ib_counters *ibcntrs)
 
 struct mlx5_ib_dev {
 	struct ib_device		ib_dev;
+	const struct uverbs_object_tree_def *driver_trees[6];
 	struct mlx5_core_dev		*mdev;
 	struct mlx5_roce		roce[MLX5_MAX_PORTS];
 	int				num_ports;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 4ffe3e11e8fb6c..3b07201b9a804e 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2580,7 +2580,7 @@  struct ib_device {
 	const struct cpumask *(*get_vector_affinity)(struct ib_device *ibdev,
 						     int comp_vector);
 
-	struct uverbs_root_spec		*driver_specs_root;
+	const struct uverbs_object_tree_def *const *driver_specs;
 	enum rdma_driver_id		driver_id;
 };