Re: [PATCH RFC 05/12] RDMA/rtrs-srv: Correct the checking of ib_map_mr_sg

Jinpu Wang
2022-11-16T05:44:39Z

On Wed, Nov 16, 2022 at 2:13 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
>
>
> On 11/15/22 7:46 PM, Jinpu Wang wrote:
> > On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >> We should check with nr_sgt, also the only successful case is that
> >> all sg elements are mapped, so make it explict.
> >>
> >> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> >> ---
> >> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> >> index 88eae0dcf87f..f3bf5bbb4377 100644
> >> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> >> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> >> @@ -622,8 +622,8 @@ static int map_cont_bufs(struct rtrs_srv_path *srv_path)
> >> }
> >> nr = ib_map_mr_sg(mr, sgt->sgl, nr_sgt,
> >> NULL, max_chunk_size);
> >> - if (nr < 0 || nr < sgt->nents) {
> >> - err = nr < 0 ? nr : -EINVAL;
> >> + if (nr != nr_sgt) {
> >> + err = -EINVAL;
> > but with this, the initial errno are lost, we only return EINVAL
>
> OK, assume you mean 'nr' here, I can change it back as iser_memory did.
> But seems the only negative value returned from ib_map_mr_sg is also
> "-EINVAL" from ib_sg_to_pages after go through hw drivers.
Yes, I meant nr.
there is also cases
"
if (unlikely(!mr->device->map_mr_sg))
return -ENOSYS;
"
>
> BTW, I looked all call sites of ib_map_mr_sg, seems they have different
> kind of checking.
>
> 1. if (ret < 0 || ret < nents)
>
> 2. if (ret < nents) or if (ret < 0)
>
> 3. if (ret != nents)
Yes, I also checked them. :)
>
> Thanks,
> Guoqing