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 ...
