From: Mark Brown broonie@linaro.org
struct rds_sock is rather large ausing the following warning in an ARM allmodconfig:
net/rds/iw_rdma.c:200:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
Fix this by dynamically allocating struct rds_sock in rds_iw_update_cm_id instead of allocating it on the stack.
Signed-off-by: Mark Brown broonie@linaro.org --- net/rds/iw_rdma.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/rds/iw_rdma.c b/net/rds/iw_rdma.c index a817705..cee5daa 100644 --- a/net/rds/iw_rdma.c +++ b/net/rds/iw_rdma.c @@ -180,22 +180,28 @@ int rds_iw_update_cm_id(struct rds_iw_device *rds_iwdev, struct rdma_cm_id *cm_i { struct sockaddr_in *src_addr, *dst_addr; struct rds_iw_device *rds_iwdev_old; - struct rds_sock rs; + struct rds_sock *rs; struct rdma_cm_id *pcm_id; int rc;
+ rs = kzalloc(sizeof(*rs), GFP_KERNEL); + if (!rs) + return -ENOMEM; + src_addr = (struct sockaddr_in *)&cm_id->route.addr.src_addr; dst_addr = (struct sockaddr_in *)&cm_id->route.addr.dst_addr;
- rs.rs_bound_addr = src_addr->sin_addr.s_addr; - rs.rs_bound_port = src_addr->sin_port; - rs.rs_conn_addr = dst_addr->sin_addr.s_addr; - rs.rs_conn_port = dst_addr->sin_port; + rs->rs_bound_addr = src_addr->sin_addr.s_addr; + rs->rs_bound_port = src_addr->sin_port; + rs->rs_conn_addr = dst_addr->sin_addr.s_addr; + rs->rs_conn_port = dst_addr->sin_port;
- rc = rds_iw_get_device(&rs, &rds_iwdev_old, &pcm_id); + rc = rds_iw_get_device(rs, &rds_iwdev_old, &pcm_id); if (rc) rds_iw_remove_cm_id(rds_iwdev, cm_id);
+ kfree(rs); + return rds_iw_add_cm_id(rds_iwdev, cm_id); }
On Sunday 24 August 2014 19:32:35 Mark Brown wrote:
From: Mark Brown broonie@linaro.org
struct rds_sock is rather large ausing the following warning in an ARM allmodconfig:
net/rds/iw_rdma.c:200:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
Fix this by dynamically allocating struct rds_sock in rds_iw_update_cm_id instead of allocating it on the stack.
Signed-off-by: Mark Brown broonie@linaro.org
Acked-by: Arnd Bergmann arnd@arndb.de
From: Mark Brown broonie@kernel.org Date: Sun, 24 Aug 2014 19:32:35 -0500
From: Mark Brown broonie@linaro.org
struct rds_sock is rather large ausing the following warning in an ARM allmodconfig:
net/rds/iw_rdma.c:200:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
Fix this by dynamically allocating struct rds_sock in rds_iw_update_cm_id instead of allocating it on the stack.
Signed-off-by: Mark Brown broonie@linaro.org
I'd like you to fix this differently. Creating pseudo instances of objects, and partially initializing it, just to satisfy an interface is always a really bad sign.
Create a key structure argument for rds_iw_get_device() and initialize that and pass it in instead, update the other caller similarly.
Thanks.
On Mon, Aug 25, 2014 at 12:57:40PM -0700, David Miller wrote:
From: Mark Brown broonie@kernel.org
From: Mark Brown broonie@linaro.org
struct rds_sock is rather large ausing the following warning in an ARM allmodconfig:
net/rds/iw_rdma.c:200:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
Fix this by dynamically allocating struct rds_sock in rds_iw_update_cm_id instead of allocating it on the stack.
Signed-off-by: Mark Brown broonie@linaro.org
I'd like you to fix this differently. Creating pseudo instances of objects, and partially initializing it, just to satisfy an interface is always a really bad sign.
Create a key structure argument for rds_iw_get_device() and initialize that and pass it in instead, update the other caller similarly.
I agree that the existing code looks like it could be improved even more but please bear in mind that I'm just looking for a clean build (we've got less than 20 warnings in allmodconfig including staging at the minute) rather than actively working on this code in particular - I've no ability to do more than build testing here.
From: Mark Brown broonie@kernel.org Date: Tue, 26 Aug 2014 07:54:09 +0100
On Mon, Aug 25, 2014 at 12:57:40PM -0700, David Miller wrote:
From: Mark Brown broonie@kernel.org
From: Mark Brown broonie@linaro.org
struct rds_sock is rather large ausing the following warning in an ARM allmodconfig:
net/rds/iw_rdma.c:200:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
Fix this by dynamically allocating struct rds_sock in rds_iw_update_cm_id instead of allocating it on the stack.
Signed-off-by: Mark Brown broonie@linaro.org
I'd like you to fix this differently. Creating pseudo instances of objects, and partially initializing it, just to satisfy an interface is always a really bad sign.
Create a key structure argument for rds_iw_get_device() and initialize that and pass it in instead, update the other caller similarly.
I agree that the existing code looks like it could be improved even more but please bear in mind that I'm just looking for a clean build (we've got less than 20 warnings in allmodconfig including staging at the minute) rather than actively working on this code in particular - I've no ability to do more than build testing here.
I understand that, but please fix this bug properly.
On Tue, Aug 26, 2014 at 08:29:06AM -0700, David Miller wrote:
From: Mark Brown broonie@kernel.org
I agree that the existing code looks like it could be improved even more but please bear in mind that I'm just looking for a clean build (we've got less than 20 warnings in allmodconfig including staging at the minute) rather than actively working on this code in particular - I've no ability to do more than build testing here.
I understand that, but please fix this bug properly.
Please set the expectation among the networking developers that their code should compile cleanly on all architectures; currently the networking code (mostly drivers rather than the core) is the single biggest source of warnings outside of staging and if we are requring things like this then that presents a substantial barrier to others contributing to addressing the warnings.
linaro-kernel@lists.linaro.org