xprtrdma: silence frame size warning

Building verbs.o on 32 bits x86, with CONFIG_FRAME_WARN set to 1024, its
default value, triggers this GCC warning:
net/sunrpc/xprtrdma/verbs.c: In function ‘rpcrdma_register_default_external’:
net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]

Silence this warning by allocating "ipb" dynamically.

Signed-off-by Paul Bolle <[email protected]>

---
0) Compile tested only (on 32 bits x86). I don't have access to Infiniband hardware.

1) Please note that this is not a new warning. The oldest build log I have still available on this machine is for a v3.8 rc, and it already showed this warning.

2) I do hope my choice for the GFP_KERNEL flag is correct here.

net/sunrpc/xprtrdma/verbs.c | 6 +++++-

1 file changed, 5 insertions(+), 1 deletion(-)

net/sunrpc/xprtrdma/verbs.c +4 -1

index 93726560..939ccc8 100644
@@ -1736,11 +1736,14 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
 	int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
 				  IB_ACCESS_REMOTE_READ);
 	struct rpcrdma_mr_seg *seg1 = seg;
-	struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
+	struct ib_phys_buf *ipb;
 	int len, i, rc = 0;
 
 	if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
 		*nsegs = RPCRDMA_MAX_DATA_SEGS;
+	ipb = kmalloc(sizeof(*ipb) * *nsegs, GFP_KERNEL);
+	if (ipb == NULL)
+		return -ENOMEM;
 	for (len = 0, i = 0; i < *nsegs;) {
 		rpcrdma_map_one(ia, seg, writing);
 		ipb[i].addr = seg->mr_dma;

Quoted 42 lines · Paul Bolle

Building verbs.o on 32 bits x86, with CONFIG_FRAME_WARN set to 1024, its
default value, triggers this GCC warning:
net/sunrpc/xprtrdma/verbs.c: In function ‘rpcrdma_register_default_external’:
net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]

Silence this warning by allocating "ipb" dynamically.

Signed-off-by Paul Bolle <[email protected]>

---
0) Compile tested only (on 32 bits x86). I don't have access to Infiniband hardware.

1) Please note that this is not a new warning. The oldest build log I have still available on this machine is for a v3.8 rc, and it already showed this warning.

2) I do hope my choice for the GFP_KERNEL flag is correct here.

net/sunrpc/xprtrdma/verbs.c | 6 +++++-

1 file changed, 5 insertions(+), 1 deletion(-)

net/sunrpc/xprtrdma/verbs.c +4 -1

index 93726560..939ccc8 100644
@@ -1736,11 +1736,14 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
 	int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
 				  IB_ACCESS_REMOTE_READ);
 	struct rpcrdma_mr_seg *seg1 = seg;
-	struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
+	struct ib_phys_buf *ipb;
 	int len, i, rc = 0;
 
 	if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
 		*nsegs = RPCRDMA_MAX_DATA_SEGS;
+	ipb = kmalloc(sizeof(*ipb) * *nsegs, GFP_KERNEL);
+	if (ipb == NULL)
+		return -ENOMEM;
 	for (len = 0, i = 0; i < *nsegs;) {
 		rpcrdma_map_one(ia, seg, writing);
 		ipb[i].addr = seg->mr_dma;

net/sunrpc/xprtrdma/verbs.c rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg, · line 1778

Quoted 6 lines

net/sunrpc/xprtrdma/verbs.c +1 -0

@@ -1770,6 +1773,7 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
 		seg1->mr_len = len;
 	}
 	*nsegs = i;
+	kfree(ipb);
 	return rc;
 }

Hi Paul,

Unfortunately, the above could be called from a file write back context, so we cannot use GFP_KERNEL allocations. In fact, I suspect we should only use GFP_ATOMIC, and then have the rpc_task delay and retry if the allocation fails.

The problem is that it looks to me as if xprt_rdma_send_request will currently fail, if we return an error from rpcrdma_register_default_external.

Chuck, will you be able to look into the above issue as part of your RDMA work?

I’m building a queue of NFS/RDMA work on bugzilla.kernel.org. Let’s create a defect report there to document this, and it will get prioritized with the rest. Paul, can you do that to start us off? Product “File system”, Component “NFS”.

I can’t say that a warning on 32-bit x86 is going to be an especially high priority.

However, the underlying issue of allocating arrays of data segments on the stack is something that needs extended attention, and is already in plan.