* [PATCH] RDMA: Fix an uninitialized variable
@ 2018-08-03 19:24 Dan Carpenter
2018-08-03 19:43 ` Jason Gunthorpe
2018-08-03 19:56 ` Bart Van Assche
0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2018-08-03 19:24 UTC (permalink / raw)
To: kernel-janitors
A couple of the callers assume that ib_post_send() initializes
*bad_send_wr. It doesn't look like we can rely on the ->post_send()
function to initialize it. For example in the i40iw_post_recv()
function and there are some error paths there which don't set it.
Here are the static checker warnings for reference:
net/sunrpc/xprtrdma/verbs.c:1564 rpcrdma_post_recvs() error: uninitialized symbol 'bad_wr'.
net/sunrpc/xprtrdma/svc_rdma_rw.c:350 svc_rdma_post_chunk_ctxt() error: uninitialized symbol 'bad_wr'.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 8caee90e6dee..0ebacbe5019e 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -3389,6 +3389,8 @@ static inline int ib_post_send(struct ib_qp *qp,
{
const struct ib_send_wr *dummy;
+ if (bad_send_wr)
+ *bad_send_wr = NULL;
return qp->device->post_send(qp, send_wr, bad_send_wr ? : &dummy);
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] RDMA: Fix an uninitialized variable
2018-08-03 19:24 [PATCH] RDMA: Fix an uninitialized variable Dan Carpenter
@ 2018-08-03 19:43 ` Jason Gunthorpe
2018-08-03 19:56 ` Bart Van Assche
1 sibling, 0 replies; 3+ messages in thread
From: Jason Gunthorpe @ 2018-08-03 19:43 UTC (permalink / raw)
To: kernel-janitors
On Fri, Aug 03, 2018 at 10:24:31PM +0300, Dan Carpenter wrote:
> A couple of the callers assume that ib_post_send() initializes
> *bad_send_wr. It doesn't look like we can rely on the ->post_send()
> function to initialize it. For example in the i40iw_post_recv()
> function and there are some error paths there which don't set it.
I think those are bugs in the drivers, if bad_wr is provided, and
post_send fails then it must be set to the wr that has a problem, left
unset/uninit is incorrect.
This is a high performance call path, so I'd prefer not to see
unnecessary babying of drivers..
Bart?
Thanks,
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] RDMA: Fix an uninitialized variable
2018-08-03 19:24 [PATCH] RDMA: Fix an uninitialized variable Dan Carpenter
2018-08-03 19:43 ` Jason Gunthorpe
@ 2018-08-03 19:56 ` Bart Van Assche
1 sibling, 0 replies; 3+ messages in thread
From: Bart Van Assche @ 2018-08-03 19:56 UTC (permalink / raw)
To: kernel-janitors
On Fri, 2018-08-03 at 13:43 -0600, Jason Gunthorpe wrote:
+AD4- On Fri, Aug 03, 2018 at 10:24:31PM +-0300, Dan Carpenter wrote:
+AD4- +AD4- A couple of the callers assume that ib+AF8-post+AF8-send() initializes
+AD4- +AD4- +ACo-bad+AF8-send+AF8-wr. It doesn't look like we can rely on the -+AD4-post+AF8-send()
+AD4- +AD4- function to initialize it. For example in the i40iw+AF8-post+AF8-recv()
+AD4- +AD4- function and there are some error paths there which don't set it.
+AD4-
+AD4- I think those are bugs in the drivers, if bad+AF8-wr is provided, and
+AD4- post+AF8-send fails then it must be set to the wr that has a problem, left
+AD4- unset/uninit is incorrect.
+AD4-
+AD4- This is a high performance call path, so I'd prefer not to see
+AD4- unnecessary babying of drivers..
+AD4-
+AD4- Bart?
Hello Dan and Jason,
The documentation of the bad+AF8-wr parameter for ib+AF8-post+AF8-send(), ib+AF8-post+AF8-recv()
and ib+AF8-post+AF8-srq+AF8-recv() is as follows:
+ACo- +AEA-bad+AF8-...wr: On an immediate failure, this parameter will reference
+ACo- the work request that failed to be posted on the QP.
In other words, nothing is guaranteed about the +ACo-bad+AF8-wr value if these
functions return 0. My proposal is to proceed as follows:
+ACo- In the ULPs that read +ACo-bad+AF8-wr if one of these functions returns 0, initialize
+ACo-bad+AF8-wr before calling one of these functions (I think all ULPs already do this).
+ACo- Make sure that all HW drivers set +ACo-bad+AF8-wr if the return value is not 0.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-03 19:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-03 19:24 [PATCH] RDMA: Fix an uninitialized variable Dan Carpenter
2018-08-03 19:43 ` Jason Gunthorpe
2018-08-03 19:56 ` Bart Van Assche
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.