On Wed, Jan 16, 2019 at 07:37:06AM +0200, Leon Romanovsky wrote: > On Tue, Jan 15, 2019 at 02:42:50PM -0700, Jason Gunthorpe wrote: > > On Tue, Jan 15, 2019 at 01:22:43PM -0800, Matthew Wilcox wrote: > > > On Tue, Jan 15, 2019 at 02:15:41PM -0700, Jason Gunthorpe wrote: > > > > Matt, down the road can we get an xa_alloc_cyclic helper that handles > > > > this and the required locking? I have another need in the works as > > > > well.. > > > > > > http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray-conv > > > > > > all current users of idr_alloc_cyclic are converted to xa_alloc_cyclic() > > > in that tree. > > > > Maybe this is what your commit comment was talking about: > > > > int __xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry, > > const struct xa_limit *limit, u32 *next, gfp_t gfp) > > { > > XA_LIMIT(tmp, *next, limit->max); > > > > I would have expected to be: > > > > XA_LIMIT(tmp, min(limit->min, *next), limit->max); > > > > I think it is a very surprising API design that the limit is not > > respected in all cases. > > > > Since going below the limit also disables the 'cyclic' part of this > > function, why shouldn't callers have to use straight xa_alloc (or > > __xa_alloc if they need atomic next updates)?? > > > > ... Leon, Matt's version is shorter than yours maybe just include it > > as a static in your module and we can replace it when Matt's is > > merged? > > I don't like his double call to xa_alloc(), but it is not > super-important in our case, I will add this function to make > conversion easy. I used the following code to mimic xa_alloc_cycle(), for two reasons: 1. __xa_alloc() is different between Matthew tree and official. 2. I don't want to do double call to xa_alloc in case we are already at the end. diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c index 58a60730e835..c787b7f7d5f8 100644 --- a/drivers/infiniband/core/restrack.c +++ b/drivers/infiniband/core/restrack.c @@ -10,9 +10,44 @@ #include <linux/sched/task.h> #include <linux/pid_namespace.h> #include <linux/rwsem.h> #include "cma_priv.h" +/** + * FIXME: Replace with official xa_array_cyclic(), once it is merged + * http://git.infradead.org/users/willy/linux-dax.git/commit/8bcca93a53949d20877e82c2aaae0fd7c385b90d + */ +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 * entity, per-device @@ -28,13 +63,13 @@ struct rdma_restrack_root { */ struct xarray xa; /** - * @reserved: Keep aside this number of indexes + * @range: Allocation ID range between min and max */ - u32 reserved; + struct rt_xa_limit range; /** - * @max: Maximal supported number of indexes + * @next: Next ID to support cyclic allocation */ - u32 max; + u32 next_id; }; /** @@ -57,7 +92,7 @@ int rdma_restrack_init(struct ib_device *dev) for (i = 0 ; i < RDMA_RESTRACK_MAX; i++) { init_rwsem(&rt[i].rwsem); xa_init_flags(&rt[i].xa, XA_FLAGS_ALLOC); - rt[i].max = U32_MAX; + rt[i].range.max = U32_MAX; } return 0; @@ -281,14 +316,15 @@ int rdma_restrack_add(struct rdma_restrack_entry *res) init_completion(&res->comp); res->valid = true; - if (rt[res->type].max) { + if (rt[res->type].range.max) { /* Not HW-capable device */ - res->id = rt[res->type].reserved; - ret = xa_alloc(xa, &res->id, rt[res->type].max, res, - GFP_KERNEL); + 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 @@ -443,8 +479,9 @@ void rdma_rt_set_id_range(struct ib_device *dev, enum rdma_restrack_type type, { struct rdma_restrack_root *rt = dev->res; - rt[type].max = max; - rt[type].reserved = reserved; + rt[type].range.max = max; + rt[type].range.min = reserved; + rt[type].next_id = reserved; } EXPORT_SYMBOL(rdma_rt_set_id_range); Thanks > > Thanks > > > > > Jason
Attachment:
signature.asc
Description: PGP signature
