Bug 68661 - xprtrdma: allocates a 1024 bytes array on the stack
Summary: xprtrdma: allocates a 1024 bytes array on the stack
Status: RESOLVED CODE_FIX
Alias: None
Product: File System
Classification: Unclassified
Component: NFS (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Chuck Lever
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-13 19:21 UTC by Paul Bolle
Modified: 2014-06-16 15:34 UTC (History)
1 user (show)

See Also:
Kernel Version: v3.13-rc8
Subsystem:
Regression: No
Bisected commit-id:


Attachments
Paul Bolle's proposed fix (1.77 KB, patch)
2014-01-23 16:38 UTC, Chuck Lever
Details | Diff

Description Paul Bolle 2014-01-13 19:21:39 UTC
0) 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=]

That warning can be seen for (quite) a number of releases now (at leats since v3.8).

1) This warning should be silenced by allocating "ipb" dynamically, but a patch to do that apparently requires more than trivial understanding of NFS. Since I lack such understanding, my straightforward patch to allocate that array on the heap was insufficient (see https://lkml.org/lkml/2014/1/13/325).

2) Note that discussion of that patch also uncovered that xprt_rdma_send_request() will currently fail, if we return an error from rpcrdma_register_default_external().
Comment 1 Chuck Lever 2014-01-14 16:19:53 UTC
This issue will be addressed when we do a major overhaul to get large arrays off the stack.  It is in plan for this year.
Comment 2 Chuck Lever 2014-01-23 16:30:27 UTC
[cel@manet linux-2.6]$ ARCH=x86_64 make checkstack | grep xprtrdma
0x0000331d rpcrdma_register_default_external [xprtrdma]:1064
0x00003421 rpcrdma_register_default_external [xprtrdma]:1064
0x000029cd rpcrdma_register_fmr_external [xprtrdma]:	568
0x00002b07 rpcrdma_register_fmr_external [xprtrdma]:	568
0x000040e4 rpcrdma_conn_upcall [xprtrdma]:		304
0x00002ced rpcrdma_register_frmr_external [xprtrdma]:	264
0x00002f33 rpcrdma_register_frmr_external [xprtrdma]:	264
0x00003d34 rpcrdma_ep_connect [xprtrdma]:		240
0x00000954 xprt_setup_rdma [xprtrdma]:			208
0x00004564 rpcrdma_ep_create [xprtrdma]:		208
0x000049b4 rpcrdma_ia_open [xprtrdma]:			208
0x00005964 rpcrdma_ep_post [xprtrdma]:			144
0x00005bfd rpcrdma_cq_event_upcall [xprtrdma]:		136
0x00005d7d rpcrdma_cq_event_upcall [xprtrdma]:		136
0x00002c0b rpcrdma_deregister_frmr_external [xprtrdma]:	112
0x00002cb4 rpcrdma_deregister_frmr_external [xprtrdma]:	112
0x00001e2d rpcrdma_create_chunks [xprtrdma]:		104
0x00001f7e rpcrdma_create_chunks [xprtrdma]:		104
0x0000214a rpcrdma_create_chunks [xprtrdma]:		104
[cel@manet linux-2.6]$ head Makefile
VERSION = 3
PATCHLEVEL = 13
SUBLEVEL = 0
EXTRAVERSION =
NAME = One Giant Leap for Frogkind

# *DOCUMENTATION*
# To see a list of typical targets execute "make help"
# More info can be located in ./README
# Comments in this file are targeted only to the developer, do not
[cel@manet linux-2.6]$
Comment 3 Chuck Lever 2014-01-23 16:38:34 UTC
Created attachment 123111 [details]
Paul Bolle's proposed fix

Attaching Paul's original fix, for reference.
Comment 4 Paul Bolle 2014-06-16 14:09:13 UTC
(In reply to Paul Bolle from comment #0)
> 0) 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=]
> 
> That warning can be seen for (quite) a number of releases now (at leats
> since v3.8).
> 

v3.16-rc1 shipped with commit 0ac531c18323 ("xprtrdma: Remove REGISTER memory registration mode"). That commit removed rpcrdma_register_default_external(). That made this warning go away, obviously.

It didn't introduce another warning, so verbs.o now builds silent again. That is good. It is also all I care about, but there may be a reason to keep this bug open. That's up to Chuck to decide.

Note You need to log in before you can comment on or make changes to this bug.