Re: [PATCH] RDMA/rtrs-srv: suppress warnings passing zero to 'PTR_ERR'

From: Leon Romanovsky <leon@kernel.org>
To: Jack Wang <jinpu.wang@cloud.ionos.com>
Cc: linux-rdma@vger.kernel.org, bvanassche@acm.org,
	dledford@redhat.com, jgg@ziepe.ca, danil.kipnis@cloud.ionos.com,
	kernel test robot <lkp@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH] RDMA/rtrs-srv: suppress warnings passing zero to 'PTR_ERR'
Date: Tue, 16 Feb 2021 09:50:14 +0200	[thread overview]
Message-ID: <YCt5Nv+czQtYqQL9@unreal> (raw)
In-Reply-To: <20210216073958.13957-1-jinpu.wang@cloud.ionos.com>

On Tue, Feb 16, 2021 at 08:39:58AM +0100, Jack Wang wrote:
> smatch warnings:
> drivers/infiniband/ulp/rtrs/rtrs-srv.c:1805 rtrs_rdma_connect() warn: passing zero to 'PTR_ERR'
>
> Smatch seems confused by the refcount_read condition, so just
> treat it seperately.
>
> Fixes: f0751419d3a1 ("RDMA/rtrs: Only allow addition of path to an already established session")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index eb17c3a08810..2f6d665bea90 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1799,12 +1799,16 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
>  	}
>  	recon_cnt = le16_to_cpu(msg->recon_cnt);
>  	srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> +	if (IS_ERR(srv)) {
> +		err = PTR_ERR(srv);
> +		goto reject_w_err;
> +	}
>  	/*
>  	 * "refcount == 0" happens if a previous thread calls get_or_create_srv
>  	 * allocate srv, but chunks of srv are not allocated yet.
>  	 */
> -	if (IS_ERR(srv) || refcount_read(&srv->refcount) == 0) {
> -		err = PTR_ERR(srv);
> +	if (refcount_read(&srv->refcount) == 0) {

I would say that "list_add(&srv->ctx_list, &ctx->srv_list);" line in the
get_or_create_srv() is performed too early, relying on zero in the refcount
doesn't look great.

Thanks