On 5/5/20 12:02 PM, Jürgen Groß wrote: > On 05.05.20 17:01, Arnd Bergmann wrote: >> On Tue, May 5, 2020 at 4:34 PM Jürgen Groß <jgr...@suse.com> wrote: >>> On 05.05.20 16:15, Arnd Bergmann wrote: >>>> The __xenbus_map_ring() function has two large arrays, 'map' and >>>> 'unmap' on its stack. When clang decides to inline it into its caller, >>>> xenbus_map_ring_valloc_hvm(), the total stack usage exceeds the >>>> warning >>>> limit for stack size on 32-bit architectures. >>>> >>>> drivers/xen/xenbus/xenbus_client.c:592:12: error: stack frame size >>>> of 1104 bytes in function 'xenbus_map_ring_valloc_hvm' >>>> [-Werror,-Wframe-larger-than=] >>>> >>>> As far as I can tell, other compilers don't inline it here, so we get >>>> no warning, but the stack usage is actually the same. It is possible >>>> for both arrays to use the same location on the stack, but the >>>> compiler >>>> cannot prove that this is safe because they get passed to external >>>> functions that may end up using them until they go out of scope. >>>> >>>> Move the two arrays into separate basic blocks to limit the scope >>>> and force them to occupy less stack in total, regardless of the >>>> inlining decision. >>> >>> Why don't you put both arrays into a union? >> >> I considered that as well, and don't really mind either way. I think >> it does >> get a bit ugly whatever we do. If you prefer the union, I can respin the >> patch that way. > > Hmm, thinking more about it I think the real clean solution would be to > extend struct map_ring_valloc_hvm to cover the pv case, too, to add the > map and unmap arrays (possibly as a union) to it and to allocate it > dynamically instead of having it on the stack. > > Would you be fine doing this?
Another option might be to factor out/modify code from xenbus_unmap_ring() and call the resulting code from __xenbus_map_ring()'s fail path. -boris