Hi Leon,
On Tue, Feb 16, 2021 at 8:50 AM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
> > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>
> > ---
> > 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,
Moving list_add down to the end was the initial code, but we noticed
the memory allocation could take quite
some time when system under memory pressure, hence we changed it in
d715ff8acbd5 ("RDMA/rtrs-srv: Don't guard the whole __alloc_srv with
srv_mutex")
Thanks for the comment.
