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

[Leon Romanovsky <leon@kernel.org>]

On Fri, Oct 02, 2020 at 09:55:35AM -0300, Jason Gunthorpe wrote:
> 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?

Both NULL and RDMA_RESTRACK_CM_ID are not needed, this is how I
discovered the need to clean the cma.c, but left it here.

Thanks

>
> Jason