diff mbox series

[rdma-next,v1,15/16] RDMA/restrack: Implement software ID interface

Message ID 20190116115918.4125-16-leon@kernel.org (mailing list archive)
State Superseded
Headers show
Series Provide per-ID access to restrack objects | expand

Commit Message

Leon Romanovsky Jan. 16, 2019, 11:59 a.m. UTC 
From: Leon Romanovsky <leonro@mellanox.com>

Generate unique resource ID for SW and HW capable devices.
RES_VALID marker is introduced as a temporal measure till
all drivers are converted to use restrask IDs. After that,
it will be safe to remove RES_VISIBLE and expose all objects,
including internal ones.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/nldev.c    |   2 +-
 drivers/infiniband/core/restrack.c | 123 ++++++++++++++++++++++-------
 include/rdma/restrack.h            |  13 +++
 3 files changed, 109 insertions(+), 29 deletions(-)

Comments

Leon Romanovsky Jan. 17, 2019, 8:15 a.m. UTC | #1 
On Wed, Jan 16, 2019 at 01:59:17PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Generate unique resource ID for SW and HW capable devices.
> RES_VALID marker is introduced as a temporal measure till
> all drivers are converted to use restrask IDs. After that,
> it will be safe to remove RES_VISIBLE and expose all objects,
> including internal ones.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  drivers/infiniband/core/nldev.c    |   2 +-
>  drivers/infiniband/core/restrack.c | 123 ++++++++++++++++++++++-------
>  include/rdma/restrack.h            |  13 +++
>  3 files changed, 109 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index 7fa64bd43c40..de3e90d6fb5a 100644
> --- a/drivers/infiniband/core/nldev.c
> +++ b/drivers/infiniband/core/nldev.c
> @@ -1121,7 +1121,7 @@ static int res_get_common_dumpit(struct sk_buff *skb,
>
>  	xa = rdma_dev_to_xa(device, res_type);
>  	rdma_rt_read_lock(device, res_type);
> -	xa_for_each(xa, res, id, ULONG_MAX, XA_PRESENT) {
> +	xa_for_each(xa, res, id, ULONG_MAX, RES_VISIBLE) {
>  		if (idx < start)
>  			goto next;
>
> diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> index 08f282c91116..2e56f7e37132 100644
> --- a/drivers/infiniband/core/restrack.c
> +++ b/drivers/infiniband/core/restrack.c
> @@ -21,6 +21,30 @@ struct rt_xa_limit {
>  	u32 min;
>  	u32 max;
>  };
> +static int rt_xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
> +			      const struct rt_xa_limit *limit, u32 *next,
> +			      gfp_t gfp)
> +{
> +	int err;
> +
> +	if (*next < limit->max)
> +		*id = *next;
> +	else
> +		*id = limit->min;
> +
> +	xa_lock(xa);
> +	err = __xa_alloc(xa, id, limit->max, entry, gfp);
> +
> +	if (err && *next > limit->min) {
> +		*id = limit->min;
> +		err = __xa_alloc(xa, id, limit->max, entry, gfp);
> +	}
> +
> +	if (!err)
> +		*next = *id + 1;
> +	xa_unlock(xa);
> +	return err;
> +}
>
>  /**
>   * struct rdma_restrack_root - main resource tracking management
> @@ -40,6 +64,10 @@ struct rdma_restrack_root {
>  	 * @range: Allocation ID range between min and max
>  	 */
>  	struct rt_xa_limit range;
> +	/**
> +	 * @next_id: Next ID to support cyclic allocation
> +	 */
> +	u32 next_id;
>  };
>
>  /**
> @@ -187,7 +215,7 @@ int rdma_restrack_count(struct ib_device *dev, enum rdma_restrack_type type,
>  	u32 cnt = 0;
>
>  	rdma_rt_read_lock(dev, type);
> -	xa_for_each(xa, e, index, ULONG_MAX, XA_PRESENT) {
> +	xa_for_each(xa, e, index, ULONG_MAX, RES_VISIBLE) {
>  		if (ns == &init_pid_ns ||
>  		    (!rdma_is_kernel_res(e) &&
>  		     ns == task_active_pid_ns(e->task)))
> @@ -261,38 +289,57 @@ void rdma_restrack_set_task(struct rdma_restrack_entry *res,
>  }
>  EXPORT_SYMBOL(rdma_restrack_set_task);
>
> -static unsigned long res_to_id(struct rdma_restrack_entry *res)
> -{
> -	switch (res->type) {
> -	case RDMA_RESTRACK_PD:
> -	case RDMA_RESTRACK_MR:
> -	case RDMA_RESTRACK_CM_ID:
> -	case RDMA_RESTRACK_CTX:
> -	case RDMA_RESTRACK_CQ:
> -	case RDMA_RESTRACK_QP:
> -		return (unsigned long)res;
> -	default:
> -		WARN_ONCE(true, "Wrong resource tracking type %u\n", res->type);
> -		return 0;
> -	}
> -}
> -
> -static void rdma_restrack_add(struct rdma_restrack_entry *res)
> +/**
> + * rdma_restrack_add() - add new resoruce to DB and get ID in return
> + * @res: resoruce to add
> + *
> + * Return: 0 on success
> + */
> +int rdma_restrack_add(struct rdma_restrack_entry *res)
>  {
>  	struct ib_device *dev = res_to_dev(res);
>  	struct xarray *xa = rdma_dev_to_xa(dev, res->type);
> +	struct rdma_restrack_root *rt = dev->res;
>  	int ret;
>
> +	/*
> +	 * Once all drivers are converted, we can remove this check
> +	 * and remove call to rdma_restrack_add() from rdma_restrack_kadd()
> +	 * and rdma_restrack_uadd()
> +	 */
> +	if (xa_load(xa, res->id))
> +		return 0;

Duog, Jason,

There is a small glitch in this check which is visible once drivers will start
to use rdma_restrack_add() directly in my next series.

There is a fixup that is better to have, I will include it in case v2
will be required.

diff --git a/drivers/infiniband/core/restrack.c
b/drivers/infiniband/core/restrack.c
index 5d026fb67f6b..946869a48fda 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -91,6 +91,15 @@ int rdma_restrack_init(struct ib_device *dev)
                init_rwsem(&rt[i].rwsem);
                xa_init_flags(&rt[i].xa, XA_FLAGS_ALLOC);
                rt[i].range.max = U32_MAX;
+               /*
+                * This is supplementary part of xa_load() check in
+                * rdma_restrack_add(), and will be removed once all
+                * drivers converted. Those values are usable in SW
+                * allocation scheme only and will prevent assignment
+                * of 0 index.
+                */
+		rt[i].range.min = 1;
+		rt[i].next_id = 1;
        }
        return 0;

Thanks


> +
>  	kref_init(&res->kref);
>  	init_completion(&res->comp);
>  	res->valid = true;
>
> -	ret = xa_insert(xa, res_to_id(res), res, GFP_KERNEL);
> -	WARN_ONCE(ret == -EEXIST, "Tried to add non-unique type %d entry\n",
> -		  res->type);
> +	if (rt[res->type].range.max) {
> +		/* Not HW-capable device */
> +		ret = rt_xa_alloc_cyclic(xa, &res->id, res,
> +					 &rt[res->type].range,
> +					 &rt[res->type].next_id, GFP_KERNEL);
> +	} else {
> +		ret = xa_insert(xa, res->id, res, GFP_KERNEL);
> +	}
> +
> +	/*
> +	 * WARNs below indicates an error in driver code.
> +	 * The check of -EEXIST is never occuried and added here
> +	 * to allow simple removal of xa_load above.
> +	 */
> +	WARN_ONCE(ret == -EEXIST, "Tried to add non-unique %s entry %u\n",
> +		  type2str(res->type), res->id);
> +	WARN_ONCE(ret == -ENOSPC,
> +		  "There are no more free indexes for type %s entry %u\n",
> +		  type2str(res->type), res->id);
> +
>  	if (ret)
>  		res->valid = false;
> +
> +	return ret;
>  }
> +EXPORT_SYMBOL(rdma_restrack_add);
>
>  /**
>   * rdma_restrack_kadd() - add kernel object to the reource tracking database
> @@ -300,10 +347,20 @@ static void rdma_restrack_add(struct rdma_restrack_entry *res)
>   */
>  void rdma_restrack_kadd(struct rdma_restrack_entry *res)
>  {
> +	struct ib_device *dev = res_to_dev(res);
> +	struct xarray *xa = rdma_dev_to_xa(dev, res->type);
> +
>  	res->task = NULL;
>  	set_kern_name(res);
>  	res->user = false;
> -	rdma_restrack_add(res);
> +	/*
> +	 * Temporaly, we are not intested in return value,
> +	 * once conversion will be finished, it will be checked by drivers.
> +	 */
> +	if (rdma_restrack_add(res))
> +		return;
> +
> +	xa_set_mark(xa, res->id, RES_VISIBLE);
>  }
>  EXPORT_SYMBOL(rdma_restrack_kadd);
>
> @@ -313,6 +370,9 @@ EXPORT_SYMBOL(rdma_restrack_kadd);
>   */
>  void rdma_restrack_uadd(struct rdma_restrack_entry *res)
>  {
> +	struct ib_device *dev = res_to_dev(res);
> +	struct xarray *xa = rdma_dev_to_xa(dev, res->type);
> +
>  	if (res->type != RDMA_RESTRACK_CM_ID)
>  		res->task = NULL;
>
> @@ -321,7 +381,14 @@ void rdma_restrack_uadd(struct rdma_restrack_entry *res)
>  	res->kern_name = NULL;
>
>  	res->user = true;
> -	rdma_restrack_add(res);
> +	/*
> +	 * Temporaly, we are not intested in return value,
> +	 * once conversion will be finished, it will be checked by drivers.
> +	 */
> +	if (rdma_restrack_add(res))
> +		return;
> +
> +	xa_set_mark(xa, res->id, RES_VISIBLE);
>  }
>  EXPORT_SYMBOL(rdma_restrack_uadd);
>
> @@ -347,7 +414,8 @@ rdma_restrack_get_byid(struct ib_device *dev,
>  	struct rdma_restrack_entry *res;
>
>  	res = xa_load(xa, id);
> -	if (!res || xa_is_err(res) || !rdma_restrack_get(res))
> +	if (!res || !rdma_restrack_get(res) ||
> +	    !xa_get_mark(xa, res->id, RES_VISIBLE))
>  		return ERR_PTR(-ENOENT);
>  	return res;
>  }
> @@ -371,7 +439,6 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
>  {
>  	struct ib_device *dev = res_to_dev(res);
>  	struct xarray *xa;
> -	unsigned long id;
>
>  	if (!res->valid)
>  		goto out;
> @@ -393,8 +460,7 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
>  		return;
>
>  	xa = rdma_dev_to_xa(dev, res->type);
> -	id = res_to_id(res);
> -	if (!xa_load(xa, id))
> +	if (!xa_load(xa, res->id))
>  		goto out;
>
>  	rdma_restrack_put(res);
> @@ -402,7 +468,7 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
>  	wait_for_completion(&res->comp);
>
>  	down_write(&dev->res[res->type].rwsem);
> -	xa_erase(xa, id);
> +	xa_erase(xa, res->id);
>  	res->valid = false;
>  	up_write(&dev->res[res->type].rwsem);
>
> @@ -430,5 +496,6 @@ void rdma_rt_set_id_range(struct ib_device *dev, enum rdma_restrack_type type,
>
>  	rt[type].range.max = max;
>  	rt[type].range.min = reserved;
> +	rt[type].next_id = reserved;
>  }
>  EXPORT_SYMBOL(rdma_rt_set_id_range);
> diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> index 0706f51bb1f8..9d329098ddfc 100644
> --- a/include/rdma/restrack.h
> +++ b/include/rdma/restrack.h
> @@ -93,6 +93,11 @@ struct rdma_restrack_entry {
>  	 * @user: user resource
>  	 */
>  	bool			user;
> +	/*
> +	 * @id: unique to specific type identifier, for HW-capable devices,
> +	 * drivers are supposed to update it, because it is used as an index.
> +	 */
> +	u32 id;
>  };
>
>  int rdma_restrack_init(struct ib_device *dev);
> @@ -104,6 +109,14 @@ int rdma_restrack_count(struct ib_device *dev,
>  void rdma_restrack_kadd(struct rdma_restrack_entry *res);
>  void rdma_restrack_uadd(struct rdma_restrack_entry *res);
>
> +/**
> + * Addition of entry is performed in two steps approach:
> + * 1. Driver creates ID and allocates resource entry.
> + * 2. IB/core marks such entry as user/kernel and exports to nldev.c
> + */
> +#define RES_VISIBLE	XA_MARK_2
> +int rdma_restrack_add(struct rdma_restrack_entry *res);
> +
>  /**
>   * rdma_restrack_del() - delete object from the reource tracking database
>   * @res:  resource entry
> --
> 2.19.1
>
diff mbox series

Patch

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 7fa64bd43c40..de3e90d6fb5a 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -1121,7 +1121,7 @@  static int res_get_common_dumpit(struct sk_buff *skb,
 
 	xa = rdma_dev_to_xa(device, res_type);
 	rdma_rt_read_lock(device, res_type);
-	xa_for_each(xa, res, id, ULONG_MAX, XA_PRESENT) {
+	xa_for_each(xa, res, id, ULONG_MAX, RES_VISIBLE) {
 		if (idx < start)
 			goto next;
 
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 08f282c91116..2e56f7e37132 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -21,6 +21,30 @@  struct rt_xa_limit {
 	u32 min;
 	u32 max;
 };
+static int rt_xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
+			      const struct rt_xa_limit *limit, u32 *next,
+			      gfp_t gfp)
+{
+	int err;
+
+	if (*next < limit->max)
+		*id = *next;
+	else
+		*id = limit->min;
+
+	xa_lock(xa);
+	err = __xa_alloc(xa, id, limit->max, entry, gfp);
+
+	if (err && *next > limit->min) {
+		*id = limit->min;
+		err = __xa_alloc(xa, id, limit->max, entry, gfp);
+	}
+
+	if (!err)
+		*next = *id + 1;
+	xa_unlock(xa);
+	return err;
+}
 
 /**
  * struct rdma_restrack_root - main resource tracking management
@@ -40,6 +64,10 @@  struct rdma_restrack_root {
 	 * @range: Allocation ID range between min and max
 	 */
 	struct rt_xa_limit range;
+	/**
+	 * @next_id: Next ID to support cyclic allocation
+	 */
+	u32 next_id;
 };
 
 /**
@@ -187,7 +215,7 @@  int rdma_restrack_count(struct ib_device *dev, enum rdma_restrack_type type,
 	u32 cnt = 0;
 
 	rdma_rt_read_lock(dev, type);
-	xa_for_each(xa, e, index, ULONG_MAX, XA_PRESENT) {
+	xa_for_each(xa, e, index, ULONG_MAX, RES_VISIBLE) {
 		if (ns == &init_pid_ns ||
 		    (!rdma_is_kernel_res(e) &&
 		     ns == task_active_pid_ns(e->task)))
@@ -261,38 +289,57 @@  void rdma_restrack_set_task(struct rdma_restrack_entry *res,
 }
 EXPORT_SYMBOL(rdma_restrack_set_task);
 
-static unsigned long res_to_id(struct rdma_restrack_entry *res)
-{
-	switch (res->type) {
-	case RDMA_RESTRACK_PD:
-	case RDMA_RESTRACK_MR:
-	case RDMA_RESTRACK_CM_ID:
-	case RDMA_RESTRACK_CTX:
-	case RDMA_RESTRACK_CQ:
-	case RDMA_RESTRACK_QP:
-		return (unsigned long)res;
-	default:
-		WARN_ONCE(true, "Wrong resource tracking type %u\n", res->type);
-		return 0;
-	}
-}
-
-static void rdma_restrack_add(struct rdma_restrack_entry *res)
+/**
+ * rdma_restrack_add() - add new resoruce to DB and get ID in return
+ * @res: resoruce to add
+ *
+ * Return: 0 on success
+ */
+int rdma_restrack_add(struct rdma_restrack_entry *res)
 {
 	struct ib_device *dev = res_to_dev(res);
 	struct xarray *xa = rdma_dev_to_xa(dev, res->type);
+	struct rdma_restrack_root *rt = dev->res;
 	int ret;
 
+	/*
+	 * Once all drivers are converted, we can remove this check
+	 * and remove call to rdma_restrack_add() from rdma_restrack_kadd()
+	 * and rdma_restrack_uadd()
+	 */
+	if (xa_load(xa, res->id))
+		return 0;
+
 	kref_init(&res->kref);
 	init_completion(&res->comp);
 	res->valid = true;
 
-	ret = xa_insert(xa, res_to_id(res), res, GFP_KERNEL);
-	WARN_ONCE(ret == -EEXIST, "Tried to add non-unique type %d entry\n",
-		  res->type);
+	if (rt[res->type].range.max) {
+		/* Not HW-capable device */
+		ret = rt_xa_alloc_cyclic(xa, &res->id, res,
+					 &rt[res->type].range,
+					 &rt[res->type].next_id, GFP_KERNEL);
+	} else {
+		ret = xa_insert(xa, res->id, res, GFP_KERNEL);
+	}
+
+	/*
+	 * WARNs below indicates an error in driver code.
+	 * The check of -EEXIST is never occuried and added here
+	 * to allow simple removal of xa_load above.
+	 */
+	WARN_ONCE(ret == -EEXIST, "Tried to add non-unique %s entry %u\n",
+		  type2str(res->type), res->id);
+	WARN_ONCE(ret == -ENOSPC,
+		  "There are no more free indexes for type %s entry %u\n",
+		  type2str(res->type), res->id);
+
 	if (ret)
 		res->valid = false;
+
+	return ret;
 }
+EXPORT_SYMBOL(rdma_restrack_add);
 
 /**
  * rdma_restrack_kadd() - add kernel object to the reource tracking database
@@ -300,10 +347,20 @@  static void rdma_restrack_add(struct rdma_restrack_entry *res)
  */
 void rdma_restrack_kadd(struct rdma_restrack_entry *res)
 {
+	struct ib_device *dev = res_to_dev(res);
+	struct xarray *xa = rdma_dev_to_xa(dev, res->type);
+
 	res->task = NULL;
 	set_kern_name(res);
 	res->user = false;
-	rdma_restrack_add(res);
+	/*
+	 * Temporaly, we are not intested in return value,
+	 * once conversion will be finished, it will be checked by drivers.
+	 */
+	if (rdma_restrack_add(res))
+		return;
+
+	xa_set_mark(xa, res->id, RES_VISIBLE);
 }
 EXPORT_SYMBOL(rdma_restrack_kadd);
 
@@ -313,6 +370,9 @@  EXPORT_SYMBOL(rdma_restrack_kadd);
  */
 void rdma_restrack_uadd(struct rdma_restrack_entry *res)
 {
+	struct ib_device *dev = res_to_dev(res);
+	struct xarray *xa = rdma_dev_to_xa(dev, res->type);
+
 	if (res->type != RDMA_RESTRACK_CM_ID)
 		res->task = NULL;
 
@@ -321,7 +381,14 @@  void rdma_restrack_uadd(struct rdma_restrack_entry *res)
 	res->kern_name = NULL;
 
 	res->user = true;
-	rdma_restrack_add(res);
+	/*
+	 * Temporaly, we are not intested in return value,
+	 * once conversion will be finished, it will be checked by drivers.
+	 */
+	if (rdma_restrack_add(res))
+		return;
+
+	xa_set_mark(xa, res->id, RES_VISIBLE);
 }
 EXPORT_SYMBOL(rdma_restrack_uadd);
 
@@ -347,7 +414,8 @@  rdma_restrack_get_byid(struct ib_device *dev,
 	struct rdma_restrack_entry *res;
 
 	res = xa_load(xa, id);
-	if (!res || xa_is_err(res) || !rdma_restrack_get(res))
+	if (!res || !rdma_restrack_get(res) ||
+	    !xa_get_mark(xa, res->id, RES_VISIBLE))
 		return ERR_PTR(-ENOENT);
 	return res;
 }
@@ -371,7 +439,6 @@  void rdma_restrack_del(struct rdma_restrack_entry *res)
 {
 	struct ib_device *dev = res_to_dev(res);
 	struct xarray *xa;
-	unsigned long id;
 
 	if (!res->valid)
 		goto out;
@@ -393,8 +460,7 @@  void rdma_restrack_del(struct rdma_restrack_entry *res)
 		return;
 
 	xa = rdma_dev_to_xa(dev, res->type);
-	id = res_to_id(res);
-	if (!xa_load(xa, id))
+	if (!xa_load(xa, res->id))
 		goto out;
 
 	rdma_restrack_put(res);
@@ -402,7 +468,7 @@  void rdma_restrack_del(struct rdma_restrack_entry *res)
 	wait_for_completion(&res->comp);
 
 	down_write(&dev->res[res->type].rwsem);
-	xa_erase(xa, id);
+	xa_erase(xa, res->id);
 	res->valid = false;
 	up_write(&dev->res[res->type].rwsem);
 
@@ -430,5 +496,6 @@  void rdma_rt_set_id_range(struct ib_device *dev, enum rdma_restrack_type type,
 
 	rt[type].range.max = max;
 	rt[type].range.min = reserved;
+	rt[type].next_id = reserved;
 }
 EXPORT_SYMBOL(rdma_rt_set_id_range);
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index 0706f51bb1f8..9d329098ddfc 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -93,6 +93,11 @@  struct rdma_restrack_entry {
 	 * @user: user resource
 	 */
 	bool			user;
+	/*
+	 * @id: unique to specific type identifier, for HW-capable devices,
+	 * drivers are supposed to update it, because it is used as an index.
+	 */
+	u32 id;
 };
 
 int rdma_restrack_init(struct ib_device *dev);
@@ -104,6 +109,14 @@  int rdma_restrack_count(struct ib_device *dev,
 void rdma_restrack_kadd(struct rdma_restrack_entry *res);
 void rdma_restrack_uadd(struct rdma_restrack_entry *res);
 
+/**
+ * Addition of entry is performed in two steps approach:
+ * 1. Driver creates ID and allocates resource entry.
+ * 2. IB/core marks such entry as user/kernel and exports to nldev.c
+ */
+#define RES_VISIBLE	XA_MARK_2
+int rdma_restrack_add(struct rdma_restrack_entry *res);
+
 /**
  * rdma_restrack_del() - delete object from the reource tracking database
  * @res:  resource entry