Re: linux-next: build failure after merge of the xarray tree

[Jason Gunthorpe]

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