| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 30 Jul 2014 14:59:34 -0500
From: Steve Wise <swise@...ngridcomputing.com>
To: David Miller <davem@...emloft.net>, himangi774@...il.com
CC: trond.myklebust@...marydata.com, bfields@...ldses.org,
linux-nfs@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, julia.lawall@...6.fr
Subject: Re: [PATCH] svcrdma: delete double assignment
On 7/29/2014 5:50 PM, David Miller wrote:
> From: Himangi Saraogi <himangi774@...il.com>
> Date: Mon, 28 Jul 2014 20:59:38 +0530
>
>> Delete successive assignments to the same location.
>>
>> A simplified version of Coccinelle semantic match that finds this problem is as
>> follows:
>>
>> // <smpl>
>> @@
>> expression i;
>> @@
>>
>> *i = ...;
>> i = ...;
>> // </smpl>
>>
>> Signed-off-by: Himangi Saraogi <himangi774@...il.com>
> I am not so sure about this change either.
>
>> @@ -956,7 +956,6 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
>> dprintk("svcrdma: failed to create QP, ret=%d\n", ret);
>> goto errout;
>> }
>> - newxprt->sc_max_sge = qp_attr.cap.max_send_sge;
>> newxprt->sc_max_sge = qp_attr.cap.max_recv_sge;
>> newxprt->sc_sq_depth = qp_attr.cap.max_send_wr;
>> newxprt->sc_max_requests = qp_attr.cap.max_recv_wr;
> ->sc_max_sge is used to limit the number of segments used during sends,
> currently in this code. Grep for where it is used.
>
> Therefore, if anything, the correct thing to do would be to retain the
> first line rather than the second line.
>
> Someone who actually works on this code and understands it should
> really take a close look at this before anyone even thinks about
> applying this patch.
We should remove that whole block that tries to recover from a
rdma_create_qp() failure. I don't believe we would ever hit it. The
value initially stored in sc_max_sge is from the device attribute
max_sge returned from ib_query_device() just above the code we're
talking about. If rdma_create_qp() fails, it would not be because the
max_send_sge and max_recv_sge values passed in exceed the device's max...
Like this:
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c
b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index e7323fb..282a43b 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -942,23 +942,8 @@ static struct svc_xprt *svc_rdma_accept(struct
svc_xprt *xprt)
ret = rdma_create_qp(newxprt->sc_cm_id, newxprt->sc_pd, &qp_attr);
if (ret) {
- /*
- * XXX: This is a hack. We need a xx_request_qp interface
- * that will adjust the qp_attr's with a best-effort
- * number
- */
- qp_attr.cap.max_send_sge -= 2;
- qp_attr.cap.max_recv_sge -= 2;
- ret = rdma_create_qp(newxprt->sc_cm_id, newxprt->sc_pd,
- &qp_attr);
- if (ret) {
- dprintk("svcrdma: failed to create QP,
ret=%d\n", ret);
- goto errout;
- }
- newxprt->sc_max_sge = qp_attr.cap.max_send_sge;
- newxprt->sc_max_sge = qp_attr.cap.max_recv_sge;
- newxprt->sc_sq_depth = qp_attr.cap.max_send_wr;
- newxprt->sc_max_requests = qp_attr.cap.max_recv_wr;
+ dprintk("svcrdma: failed to create QP, ret=%d\n", ret);
+ goto errout;
}
newxprt->sc_qp = newxprt->sc_cm_id->qp;
Steve.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists