[bug report] block/rnbd-clt: Support polling mode for IO latency optimization

[bug report] block/rnbd-clt: Support polling mode for IO latency optimization

[Dan Carpenter]

Hello Gioh Kim,

The patch fa607fcb87f6: "block/rnbd-clt: Support polling mode for IO
latency optimization" from Apr 19, 2021, leads to the following
static checker warning:

	drivers/infiniband/ulp/rtrs/rtrs-clt.c:2998 rtrs_clt_rdma_cq_direct()
	error: uninitialized symbol 'cnt'.

drivers/infiniband/ulp/rtrs/rtrs-clt.c
  2977  int rtrs_clt_rdma_cq_direct(struct rtrs_clt *clt, unsigned int index)
  2978  {
  2979          int cnt;

What about if no sessions are connected?

  2980          struct rtrs_con *con;
  2981          struct rtrs_clt_sess *sess;
  2982          struct path_it it;
  2983  
  2984          rcu_read_lock();
  2985          for (path_it_init(&it, clt);
  2986               (sess = it.next_path(&it)) && it.i < it.clt->paths_num; it.i++) {
  2987                  if (READ_ONCE(sess->state) != RTRS_CLT_CONNECTED)
  2988                          continue;
  2989  
  2990                  con = sess->s.con[index + 1];
  2991                  cnt = ib_process_cq_direct(con->cq, -1);
  2992                  if (cnt)
  2993                          break;

I don't know any of this code but I would have expect use to add up
cnt for the whole loop...  Something like:

			ret = ib_process_cq_direct(con->cq, -1);
			if (ret < 0)
				break;
			cnt += ret;

  2994          }
  2995          path_it_deinit(&it);
  2996          rcu_read_unlock();
  2997  
  2998          return cnt;

		if (ret < 0)
			return ret;

		return cnt;

  2999  }

regards,
dan carpenter

Re: [bug report] block/rnbd-clt: Support polling mode for IO latency optimization

[Jack Wang]

Hi Dan,

Dan Carpenter <dan.carpenter@oracle.com> 于2021年4月21日周三 上午11:09写道：
>
> Hello Gioh Kim,
>
> The patch fa607fcb87f6: "block/rnbd-clt: Support polling mode for IO
> latency optimization" from Apr 19, 2021, leads to the following
> static checker warning:
>
>         drivers/infiniband/ulp/rtrs/rtrs-clt.c:2998 rtrs_clt_rdma_cq_direct()
>         error: uninitialized symbol 'cnt'.
>
> drivers/infiniband/ulp/rtrs/rtrs-clt.c
>   2977  int rtrs_clt_rdma_cq_direct(struct rtrs_clt *clt, unsigned int index)
>   2978  {
>   2979          int cnt;
>
> What about if no sessions are connected?
This indeed a bug, will fix it soon. cnt should be init to 0.
>
>   2980          struct rtrs_con *con;
>   2981          struct rtrs_clt_sess *sess;
>   2982          struct path_it it;
>   2983
>   2984          rcu_read_lock();
>   2985          for (path_it_init(&it, clt);
>   2986               (sess = it.next_path(&it)) && it.i < it.clt->paths_num; it.i++) {
>   2987                  if (READ_ONCE(sess->state) != RTRS_CLT_CONNECTED)
>   2988                          continue;
>   2989
>   2990                  con = sess->s.con[index + 1];
>   2991                  cnt = ib_process_cq_direct(con->cq, -1);
>   2992                  if (cnt)
>   2993                          break;
>
> I don't know any of this code but I would have expect use to add up
> cnt for the whole loop...  Something like:
>
>                         ret = ib_process_cq_direct(con->cq, -1);
ib_process_cq_direct does not return negative value AFAIK.
The idea is as we give -1 which means ib_process_cq_direct will poll
all pending cq.
when it returns positive value, we are done.
>                         if (ret < 0)
>                                 break;
>                         cnt += ret;
>
>   2994          }
>   2995          path_it_deinit(&it);
>   2996          rcu_read_unlock();
>   2997
>   2998          return cnt;
>
>                 if (ret < 0)
>                         return ret;
>
>                 return cnt;
>
>   2999  }
>
> regards,
> dan carpenter
Thanks for the reporting.

Regards!