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