Re: [PATCH rdma-next v3 9/9] RDMA/restrack: Drop valid restrack field as source of ambiguity

[Jason Gunthorpe <jgg@nvidia.com>]

On Sat, Sep 26, 2020 at 01:19:38PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> The valid field was needed to distinguish between supported/not
> supported QPs, after the create_qp was changed to support all types,
> that field can be dropped in a favor of no_track field.
> 
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  drivers/infiniband/core/restrack.c | 29 ++++++++---------------------
>  include/rdma/restrack.h            |  9 ---------
>  2 files changed, 8 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> index 593af32d86a0..6ca3e6f3adb5 100644
> +++ b/drivers/infiniband/core/restrack.c
> @@ -143,7 +143,7 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
>  		return container_of(res, struct rdma_counter, res)->device;
>  	default:
>  		WARN_ONCE(true, "Wrong resource tracking type %u\n", res->type);
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  }
>  
> @@ -223,7 +223,7 @@ int __must_check rdma_restrack_add(struct rdma_restrack_entry *res)
>  	struct rdma_restrack_root *rt;
>  	int ret = 0;
>  
> -	if (!dev)
> +	if (IS_ERR_OR_NULL(dev))
>  		return -ENODEV;

dev can't be NULL

Not sure why this was changed? The error code is always thrown away,
what was wrong with keeping it as NULL? 

Now that all callers check the return code this should be a WARN_ON as
calling restrack_add in a way that is guarenteed to fail us a ULP
error.

> +	WARN_ONCE(!dev && res->type != RDMA_RESTRACK_CM_ID,
> +		  "IB device should be set for restrack type %s",
> +		  type2str(res->type));
> +	if (res->no_track || IS_ERR_OR_NULL(dev))
>  		goto out;

dev is never NULL so that WARN_ONCE doesn't work

Why does this exclude CM_ID? I thought all the fixing in the cm was so
restrack_add and _del were prefectly paired and a device must be
present?

Jason