* [bug report] block/rnbd-clt: Support polling mode for IO latency optimization
@ 2021-04-21 9:07 Dan Carpenter
2021-04-22 9:58 ` Jack Wang
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2021-04-21 9:07 UTC (permalink / raw)
To: gi-oh.kim; +Cc: linux-rdma
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] block/rnbd-clt: Support polling mode for IO latency optimization
2021-04-21 9:07 [bug report] block/rnbd-clt: Support polling mode for IO latency optimization Dan Carpenter
@ 2021-04-22 9:58 ` Jack Wang
0 siblings, 0 replies; 2+ messages in thread
From: Jack Wang @ 2021-04-22 9:58 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Gi-Oh Kim, RDMA mailing list
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!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-04-22 9:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 9:07 [bug report] block/rnbd-clt: Support polling mode for IO latency optimization Dan Carpenter
2021-04-22 9:58 ` Jack Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).