Hi all,
After merging the xarray tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:
In file included from include/linux/uio.h:12,
from include/linux/socket.h:8,
from include/rdma/rdma_cm.h:37,
from drivers/infiniband/core/restrack.c:6:
drivers/infiniband/core/restrack.c: In function 'rt_xa_alloc_cyclic':
include/linux/kernel.h:40:18: warning: passing argument 3 of '__xa_alloc' makes pointer from integer without a cast [-Wint-conversion]
#define U32_MAX ((u32)~0U)
^~~~~~~~~~
drivers/infiniband/core/restrack.c:26:27: note: in expansion of macro 'U32_MAX'
err = __xa_alloc(xa, id, U32_MAX, entry, GFP_KERNEL);
^~~~~~~
In file included from include/linux/radix-tree.h:31,
from include/linux/fs.h:15,
from include/linux/seq_file.h:11,
from arch/powerpc/include/asm/machdep.h:12,
from arch/powerpc/include/asm/archrandom.h:7,
from include/linux/random.h:166,
from include/linux/net.h:22,
from include/linux/skbuff.h:29,
from include/linux/if_arp.h:26,
from include/rdma/ib_addr.h:39,
from include/rdma/rdma_cm.h:39,
from drivers/infiniband/core/restrack.c:6:
include/linux/xarray.h:524:61: note: expected 'void *' but argument is of type 'unsigned int'
int __must_check __xa_alloc(struct xarray *, u32 *id, void *entry,
~~~~~~^~~~~
drivers/infiniband/core/restrack.c:26:36: error: incompatible type for argument 4 of '__xa_alloc'
err = __xa_alloc(xa, id, U32_MAX, entry, GFP_KERNEL);
^~~~~
In file included from include/linux/radix-tree.h:31,
from include/linux/fs.h:15,
from include/linux/seq_file.h:11,
from arch/powerpc/include/asm/machdep.h:12,
from arch/powerpc/include/asm/archrandom.h:7,
from include/linux/random.h:166,
from include/linux/net.h:22,
from include/linux/skbuff.h:29,
from include/linux/if_arp.h:26,
from include/rdma/ib_addr.h:39,
from include/rdma/rdma_cm.h:39,
from drivers/infiniband/core/restrack.c:6:
include/linux/xarray.h:525:3: note: expected 'struct xa_limit' but argument is of type 'void *'
struct xa_limit, gfp_t);
^~~~~~~~~~~~~~~
drivers/infiniband/core/restrack.c:29:28: warning: passing argument 3 of '__xa_alloc' makes pointer from integer without a cast [-Wint-conversion]
err = __xa_alloc(xa, id, *next, entry, GFP_KERNEL);
^~~~~
In file included from include/linux/radix-tree.h:31,
from include/linux/fs.h:15,
from include/linux/seq_file.h:11,
from arch/powerpc/include/asm/machdep.h:12,
from arch/powerpc/include/asm/archrandom.h:7,
from include/linux/random.h:166,
from include/linux/net.h:22,
from include/linux/skbuff.h:29,
from include/linux/if_arp.h:26,
from include/rdma/ib_addr.h:39,
from include/rdma/rdma_cm.h:39,
from drivers/infiniband/core/restrack.c:6:
include/linux/xarray.h:524:61: note: expected 'void *' but argument is of type 'u32' {aka 'unsigned int'}
int __must_check __xa_alloc(struct xarray *, u32 *id, void *entry,
~~~~~~^~~~~
drivers/infiniband/core/restrack.c:29:35: error: incompatible type for argument 4 of '__xa_alloc'
err = __xa_alloc(xa, id, *next, entry, GFP_KERNEL);
^~~~~
In file included from include/linux/radix-tree.h:31,
from include/linux/fs.h:15,
from include/linux/seq_file.h:11,
from arch/powerpc/include/asm/machdep.h:12,
from arch/powerpc/include/asm/archrandom.h:7,
from include/linux/random.h:166,
from include/linux/net.h:22,
from include/linux/skbuff.h:29,
from include/linux/if_arp.h:26,
from include/rdma/ib_addr.h:39,
from include/rdma/rdma_cm.h:39,
from drivers/infiniband/core/restrack.c:6:
include/linux/xarray.h:525:3: note: expected 'struct xa_limit' but argument is of type 'void *'
struct xa_limit, gfp_t);
^~~~~~~~~~~~~~~
Caused by commit
fd47c2f99f04 ("RDMA/restrack: Convert internal DB from hash to XArray")
from the rdma tree interacting with commit
a3e4d3f97ec8 ("XArray: Redesign xa_alloc API")
from the xarray tree.
I added the following merge fix patch:
From: Stephen Rothwell <[email protected]>
Date: Thu, 21 Feb 2019 17:07:22 +1100
Subject: [PATCH] RDMA/restrack: fix for __xa_alloc() API change
Signed-off-by: Stephen Rothwell <[email protected]>
---
drivers/infiniband/core/restrack.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index fa804093fafb..5cb381a986c1 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -23,10 +23,10 @@ static int rt_xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
*id = 0;
xa_lock(xa);
- err = __xa_alloc(xa, id, U32_MAX, entry, GFP_KERNEL);
+ err = __xa_alloc(xa, id, entry, XA_LIMIT(*id, U32_MAX), GFP_KERNEL);
if (err && *next != U32_MAX) {
*id = 0;
- err = __xa_alloc(xa, id, *next, entry, GFP_KERNEL);
+ err = __xa_alloc(xa, id, entry, XA_LIMIT(0, *next), GFP_KERNEL);
}
if (!err)
--
2.20.1
--
Cheers,
Stephen Rothwell
On Thu, Feb 21, 2019 at 05:13:32PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the xarray tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> In file included from include/linux/uio.h:12,
> from include/linux/socket.h:8,
> from include/rdma/rdma_cm.h:37,
> from drivers/infiniband/core/restrack.c:6:
> drivers/infiniband/core/restrack.c: In function 'rt_xa_alloc_cyclic':
> include/linux/kernel.h:40:18: warning: passing argument 3 of '__xa_alloc' makes pointer from integer without a cast [-Wint-conversion]
> #define U32_MAX ((u32)~0U)
> ^~~~~~~~~~
> drivers/infiniband/core/restrack.c:26:27: note: in expansion of macro 'U32_MAX'
> err = __xa_alloc(xa, id, U32_MAX, entry, GFP_KERNEL);
> ^~~~~~~
> In file included from include/linux/radix-tree.h:31,
> from include/linux/fs.h:15,
> from include/linux/seq_file.h:11,
> from arch/powerpc/include/asm/machdep.h:12,
> from arch/powerpc/include/asm/archrandom.h:7,
> from include/linux/random.h:166,
> from include/linux/net.h:22,
> from include/linux/skbuff.h:29,
> from include/linux/if_arp.h:26,
> from include/rdma/ib_addr.h:39,
> from include/rdma/rdma_cm.h:39,
> from drivers/infiniband/core/restrack.c:6:
> include/linux/xarray.h:524:61: note: expected 'void *' but argument is of type 'unsigned int'
> int __must_check __xa_alloc(struct xarray *, u32 *id, void *entry,
> ~~~~~~^~~~~
> drivers/infiniband/core/restrack.c:26:36: error: incompatible type for argument 4 of '__xa_alloc'
> err = __xa_alloc(xa, id, U32_MAX, entry, GFP_KERNEL);
> ^~~~~
> In file included from include/linux/radix-tree.h:31,
> from include/linux/fs.h:15,
> from include/linux/seq_file.h:11,
> from arch/powerpc/include/asm/machdep.h:12,
> from arch/powerpc/include/asm/archrandom.h:7,
> from include/linux/random.h:166,
> from include/linux/net.h:22,
> from include/linux/skbuff.h:29,
> from include/linux/if_arp.h:26,
> from include/rdma/ib_addr.h:39,
> from include/rdma/rdma_cm.h:39,
> from drivers/infiniband/core/restrack.c:6:
> include/linux/xarray.h:525:3: note: expected 'struct xa_limit' but argument is of type 'void *'
> struct xa_limit, gfp_t);
> ^~~~~~~~~~~~~~~
> drivers/infiniband/core/restrack.c:29:28: warning: passing argument 3 of '__xa_alloc' makes pointer from integer without a cast [-Wint-conversion]
> err = __xa_alloc(xa, id, *next, entry, GFP_KERNEL);
> ^~~~~
> In file included from include/linux/radix-tree.h:31,
> from include/linux/fs.h:15,
> from include/linux/seq_file.h:11,
> from arch/powerpc/include/asm/machdep.h:12,
> from arch/powerpc/include/asm/archrandom.h:7,
> from include/linux/random.h:166,
> from include/linux/net.h:22,
> from include/linux/skbuff.h:29,
> from include/linux/if_arp.h:26,
> from include/rdma/ib_addr.h:39,
> from include/rdma/rdma_cm.h:39,
> from drivers/infiniband/core/restrack.c:6:
> include/linux/xarray.h:524:61: note: expected 'void *' but argument is of type 'u32' {aka 'unsigned int'}
> int __must_check __xa_alloc(struct xarray *, u32 *id, void *entry,
> ~~~~~~^~~~~
> drivers/infiniband/core/restrack.c:29:35: error: incompatible type for argument 4 of '__xa_alloc'
> err = __xa_alloc(xa, id, *next, entry, GFP_KERNEL);
> ^~~~~
> In file included from include/linux/radix-tree.h:31,
> from include/linux/fs.h:15,
> from include/linux/seq_file.h:11,
> from arch/powerpc/include/asm/machdep.h:12,
> from arch/powerpc/include/asm/archrandom.h:7,
> from include/linux/random.h:166,
> from include/linux/net.h:22,
> from include/linux/skbuff.h:29,
> from include/linux/if_arp.h:26,
> from include/rdma/ib_addr.h:39,
> from include/rdma/rdma_cm.h:39,
> from drivers/infiniband/core/restrack.c:6:
> include/linux/xarray.h:525:3: note: expected 'struct xa_limit' but argument is of type 'void *'
> struct xa_limit, gfp_t);
> ^~~~~~~~~~~~~~~
>
> Caused by commit
>
> fd47c2f99f04 ("RDMA/restrack: Convert internal DB from hash to XArray")
>
> from the rdma tree interacting with commit
>
> a3e4d3f97ec8 ("XArray: Redesign xa_alloc API")
>
> from the xarray tree.
Thanks Stephen, your change looks good.
Matthew, really? change of API in -rc7? And it after you pushed us to
base our -next on -rc5 after another API change? What should we do now?
Can you please ensure that you are sending your pull request to Linus,
after RDMA pull request will be successfully merged?
Thanks
Hi Leon,
On Thu, 21 Feb 2019 12:34:42 +0000 Leon Romanovsky <[email protected]> wrote:
>
> Matthew, really? change of API in -rc7? And it after you pushed us to
> base our -next on -rc5 after another API change? What should we do now?
The xarray API changes (to xa_alloc and __xa_alloc) have been in
linux-next for about 2 weeks ...
> Can you please ensure that you are sending your pull request to Linus,
> after RDMA pull request will be successfully merged?
It doesn't matter which goes in first, but whoever goes in last should
inform Linus about these (now 2) API change fixups. In fact you should
probably both let him know as he sometimes batches up similar groups of
merge requests (like fs or net) and doesn't do them in the order they
are submitted.
--
Cheers,
Stephen Rothwell
On Thu, Feb 21, 2019 at 11:48:01PM +1100, Stephen Rothwell wrote:
> Hi Leon,
>
> On Thu, 21 Feb 2019 12:34:42 +0000 Leon Romanovsky <[email protected]> wrote:
> >
> > Matthew, really? change of API in -rc7? And it after you pushed us to
> > base our -next on -rc5 after another API change? What should we do now?
>
> The xarray API changes (to xa_alloc and __xa_alloc) have been in
> linux-next for about 2 weeks ...
I looked on the latest update of pulled branch
http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray
>
> > Can you please ensure that you are sending your pull request to Linus,
> > after RDMA pull request will be successfully merged?
>
> It doesn't matter which goes in first, but whoever goes in last should
> inform Linus about these (now 2) API change fixups. In fact you should
> probably both let him know as he sometimes batches up similar groups of
> merge requests (like fs or net) and doesn't do them in the order they
> are submitted.
OK, we will do.
Thanks
>
> --
> Cheers,
> Stephen Rothwell
Hi Leon,
On Thu, 21 Feb 2019 13:16:31 +0000 Leon Romanovsky <[email protected]> wrote:
>
> > The xarray API changes (to xa_alloc and __xa_alloc) have been in
> > linux-next for about 2 weeks ...
>
> I looked on the latest update of pulled branch
> http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray
Yeah, there were 3 commits added today, but the API changing commit is older.
> > It doesn't matter which goes in first, but whoever goes in last should
> > inform Linus about these (now 2) API change fixups. In fact you should
> > probably both let him know as he sometimes batches up similar groups of
> > merge requests (like fs or net) and doesn't do them in the order they
> > are submitted.
>
> OK, we will do.
Thanks.
--
Cheers,
Stephen Rothwell
On Thu, Feb 21, 2019 at 12:34:42PM +0000, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 05:13:32PM +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the xarray tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
[API change]
So I ended up being really busy last week and not having time to send
my merge request for XArray yet. I'm going to send a request this week;
Leon and Jason, how does this merge resolution look?
Compile tested only.
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index a9f29156e486..8a93c0c95953 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -668,19 +668,10 @@ static int assign_name(struct ib_device *device, const char *name)
}
strlcpy(device->name, dev_name(&device->dev), IB_DEVICE_NAME_MAX);
- /* Cyclically allocate a user visible ID for the device */
- device->index = last_id;
- ret = xa_alloc(&devices, &device->index, INT_MAX, device, GFP_KERNEL);
- if (ret == -ENOSPC) {
- device->index = 0;
- ret = xa_alloc(&devices, &device->index, INT_MAX, device,
- GFP_KERNEL);
- }
- if (ret)
- goto out;
- last_id = device->index + 1;
-
- ret = 0;
+ ret = xa_alloc_cyclic(&devices, &device->index, device, xa_limit_31b,
+ &last_id, GFP_KERNEL);
+ if (ret > 0)
+ ret = 0;
out:
up_write(&devices_rwsem);
@@ -1059,14 +1050,14 @@ static int assign_client_id(struct ib_client *client)
* to get the LIFO order. The extra linked list can go away if xarray
* learns to reverse iterate.
*/
- if (list_empty(&client_list))
+ if (list_empty(&client_list)) {
client->client_id = 0;
- else
- client->client_id =
- list_last_entry(&client_list, struct ib_client, list)
- ->client_id;
- ret = xa_alloc(&clients, &client->client_id, INT_MAX, client,
- GFP_KERNEL);
+ } else {
+ struct ib_client *last = list_last_entry(&client_list,
+ struct ib_client, list);
+ client->client_id = last->client_id + 1;
+ }
+ ret = xa_insert(&clients, client->client_id, client, GFP_KERNEL);
if (ret)
goto out;
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index fa804093fafb..3b5ff2f7b5f8 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -13,28 +13,6 @@
#include "cma_priv.h"
#include "restrack.h"
-static int rt_xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
- u32 *next)
-{
- int err;
-
- *id = *next;
- if (*next == U32_MAX)
- *id = 0;
-
- xa_lock(xa);
- err = __xa_alloc(xa, id, U32_MAX, entry, GFP_KERNEL);
- if (err && *next != U32_MAX) {
- *id = 0;
- err = __xa_alloc(xa, id, *next, entry, GFP_KERNEL);
- }
-
- if (!err)
- *next = *id + 1;
- xa_unlock(xa);
- return err;
-}
-
/**
* rdma_restrack_init() - initialize and allocate resource tracking
* @dev: IB device
@@ -226,7 +204,8 @@ static void rdma_restrack_add(struct rdma_restrack_entry *res)
kref_init(&res->kref);
init_completion(&res->comp);
if (res->type != RDMA_RESTRACK_QP)
- ret = rt_xa_alloc_cyclic(&rt->xa, &res->id, res, &rt->next_id);
+ ret = xa_alloc_cyclic(&rt->xa, &res->id, res, xa_limit_32b,
+ &rt->next_id, GFP_KERNEL);
else {
/* Special case to ensure that LQPN points to right QP */
struct ib_qp *qp = container_of(res, struct ib_qp, res);
On Sun, Mar 10, 2019 at 07:44:34PM -0700, Matthew Wilcox wrote:
> On Thu, Feb 21, 2019 at 12:34:42PM +0000, Leon Romanovsky wrote:
> > On Thu, Feb 21, 2019 at 05:13:32PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > After merging the xarray tree, today's linux-next build (powerpc
> > > ppc64_defconfig) failed like this:
>
> [API change]
>
> So I ended up being really busy last week and not having time to send
> my merge request for XArray yet. I'm going to send a request this week;
> Leon and Jason, how does this merge resolution look?
>
> Compile tested only.
>
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index a9f29156e486..8a93c0c95953 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -668,19 +668,10 @@ static int assign_name(struct ib_device *device, const char *name)
> }
> strlcpy(device->name, dev_name(&device->dev), IB_DEVICE_NAME_MAX);
>
> - /* Cyclically allocate a user visible ID for the device */
> - device->index = last_id;
> - ret = xa_alloc(&devices, &device->index, INT_MAX, device, GFP_KERNEL);
> - if (ret == -ENOSPC) {
> - device->index = 0;
> - ret = xa_alloc(&devices, &device->index, INT_MAX, device,
> - GFP_KERNEL);
> - }
> - if (ret)
> - goto out;
> - last_id = device->index + 1;
> -
> - ret = 0;
> + ret = xa_alloc_cyclic(&devices, &device->index, device, xa_limit_31b,
> + &last_id, GFP_KERNEL);
> + if (ret > 0)
> + ret = 0;
>
> out:
> up_write(&devices_rwsem);
> @@ -1059,14 +1050,14 @@ static int assign_client_id(struct ib_client *client)
> * to get the LIFO order. The extra linked list can go away if xarray
> * learns to reverse iterate.
> */
> - if (list_empty(&client_list))
> + if (list_empty(&client_list)) {
> client->client_id = 0;
> - else
> - client->client_id =
> - list_last_entry(&client_list, struct ib_client, list)
> - ->client_id;
> - ret = xa_alloc(&clients, &client->client_id, INT_MAX, client,
> - GFP_KERNEL);
> + } else {
> + struct ib_client *last = list_last_entry(&client_list,
> + struct ib_client, list);
> + client->client_id = last->client_id + 1;
blank line after locals, but other wise these all looks fine..
Should have started out with the xa_insert version above..
Thanks,
Jason
On Mon, Mar 11, 2019 at 12:13:54PM +0000, Jason Gunthorpe wrote:
> > @@ -1059,14 +1050,14 @@ static int assign_client_id(struct ib_client *client)
> > * to get the LIFO order. The extra linked list can go away if xarray
> > * learns to reverse iterate.
> > */
> > - if (list_empty(&client_list))
> > + if (list_empty(&client_list)) {
> > client->client_id = 0;
> > - else
> > - client->client_id =
> > - list_last_entry(&client_list, struct ib_client, list)
> > - ->client_id;
> > - ret = xa_alloc(&clients, &client->client_id, INT_MAX, client,
> > - GFP_KERNEL);
> > + } else {
> > + struct ib_client *last = list_last_entry(&client_list,
> > + struct ib_client, list);
> > + client->client_id = last->client_id + 1;
>
> blank line after locals, but other wise these all looks fine..
Would you rather see this rendered as:
if (list_empty(&client_list)) {
client->client_id = 0;
} else {
struct ib_client *last;
last = list_last_entry(&client_list, struct ib_client, list);
client->client_id = last->client_id + 1;
}
or move the declaration of 'last' up to the top of the function?
> Should have started out with the xa_insert version above..
I didn't spot it until last night either ...
On Sun, Mar 10, 2019 at 07:44:34PM -0700, Matthew Wilcox wrote:
> On Thu, Feb 21, 2019 at 12:34:42PM +0000, Leon Romanovsky wrote:
> > On Thu, Feb 21, 2019 at 05:13:32PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > After merging the xarray tree, today's linux-next build (powerpc
> > > ppc64_defconfig) failed like this:
>
> [API change]
>
> So I ended up being really busy last week and not having time to send
> my merge request for XArray yet. I'm going to send a request this week;
> Leon and Jason, how does this merge resolution look?
>
> Compile tested only.
>
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index a9f29156e486..8a93c0c95953 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -668,19 +668,10 @@ static int assign_name(struct ib_device *device, const char *name)
> }
> strlcpy(device->name, dev_name(&device->dev), IB_DEVICE_NAME_MAX);
>
> - /* Cyclically allocate a user visible ID for the device */
> - device->index = last_id;
> - ret = xa_alloc(&devices, &device->index, INT_MAX, device, GFP_KERNEL);
> - if (ret == -ENOSPC) {
> - device->index = 0;
> - ret = xa_alloc(&devices, &device->index, INT_MAX, device,
> - GFP_KERNEL);
> - }
> - if (ret)
> - goto out;
> - last_id = device->index + 1;
> -
> - ret = 0;
> + ret = xa_alloc_cyclic(&devices, &device->index, device, xa_limit_31b,
> + &last_id, GFP_KERNEL);
> + if (ret > 0)
> + ret = 0;
>
> out:
> up_write(&devices_rwsem);
> @@ -1059,14 +1050,14 @@ static int assign_client_id(struct ib_client *client)
> * to get the LIFO order. The extra linked list can go away if xarray
> * learns to reverse iterate.
> */
> - if (list_empty(&client_list))
> + if (list_empty(&client_list)) {
> client->client_id = 0;
> - else
> - client->client_id =
> - list_last_entry(&client_list, struct ib_client, list)
> - ->client_id;
> - ret = xa_alloc(&clients, &client->client_id, INT_MAX, client,
> - GFP_KERNEL);
> + } else {
> + struct ib_client *last = list_last_entry(&client_list,
> + struct ib_client, list);
> + client->client_id = last->client_id + 1;
> + }
> + ret = xa_insert(&clients, client->client_id, client, GFP_KERNEL);
> if (ret)
> goto out;
>
> diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> index fa804093fafb..3b5ff2f7b5f8 100644
> --- a/drivers/infiniband/core/restrack.c
> +++ b/drivers/infiniband/core/restrack.c
> @@ -13,28 +13,6 @@
> #include "cma_priv.h"
> #include "restrack.h"
>
> -static int rt_xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
> - u32 *next)
> -{
> - int err;
> -
> - *id = *next;
> - if (*next == U32_MAX)
> - *id = 0;
> -
> - xa_lock(xa);
> - err = __xa_alloc(xa, id, U32_MAX, entry, GFP_KERNEL);
> - if (err && *next != U32_MAX) {
> - *id = 0;
> - err = __xa_alloc(xa, id, *next, entry, GFP_KERNEL);
> - }
> -
> - if (!err)
> - *next = *id + 1;
> - xa_unlock(xa);
> - return err;
> -}
> -
> /**
> * rdma_restrack_init() - initialize and allocate resource tracking
> * @dev: IB device
> @@ -226,7 +204,8 @@ static void rdma_restrack_add(struct rdma_restrack_entry *res)
> kref_init(&res->kref);
> init_completion(&res->comp);
> if (res->type != RDMA_RESTRACK_QP)
> - ret = rt_xa_alloc_cyclic(&rt->xa, &res->id, res, &rt->next_id);
> + ret = xa_alloc_cyclic(&rt->xa, &res->id, res, xa_limit_32b,
> + &rt->next_id, GFP_KERNEL);
restrack.c part looks good.
Thanks
> else {
> /* Special case to ensure that LQPN points to right QP */
> struct ib_qp *qp = container_of(res, struct ib_qp, res);
On Mon, Mar 11, 2019 at 05:31:05AM -0700, Matthew Wilcox wrote:
> On Mon, Mar 11, 2019 at 12:13:54PM +0000, Jason Gunthorpe wrote:
> > > @@ -1059,14 +1050,14 @@ static int assign_client_id(struct ib_client *client)
> > > * to get the LIFO order. The extra linked list can go away if xarray
> > > * learns to reverse iterate.
> > > */
> > > - if (list_empty(&client_list))
> > > + if (list_empty(&client_list)) {
> > > client->client_id = 0;
> > > - else
> > > - client->client_id =
> > > - list_last_entry(&client_list, struct ib_client, list)
> > > - ->client_id;
> > > - ret = xa_alloc(&clients, &client->client_id, INT_MAX, client,
> > > - GFP_KERNEL);
> > > + } else {
> > > + struct ib_client *last = list_last_entry(&client_list,
> > > + struct ib_client, list);
> > > + client->client_id = last->client_id + 1;
> >
> > blank line after locals, but other wise these all looks fine..
>
> Would you rather see this rendered as:
>
> if (list_empty(&client_list)) {
> client->client_id = 0;
> } else {
> struct ib_client *last;
>
> last = list_last_entry(&client_list, struct ib_client, list);
> client->client_id = last->client_id + 1;
> }
Don't care much either way. Only that the Linux style guide is to
always have the blank line after variable declarations in any block
> or move the declaration of 'last' up to the top of the function?
This one I dislike, variables should be in their narrowest scope for
clarity
> > Should have started out with the xa_insert version above..
>
> I didn't spot it until last night either ...
It is a leftover weird thinking logic from an earlier attempt I had
that was trying to get the last ID out of the xarray without the
linked list..
Jason