On Wed, Dec 13, 2023, at 02:13, Ian Kent wrote:
> On 13/12/23 05:48, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@xxxxxxxx>
>>
>> prepare_kstatmount() constructs a copy of 'struct kstatmount' on the stack
>> and copies it into the local variable on the stack of its caller. Because
>> of the size of this structure, this ends up overflowing the limit for
>> a single function's stack frame when prepare_kstatmount() gets inlined
>> and both copies are on the same frame without the compiler being able
>> to collapse them into one:
>>
>> fs/namespace.c:4995:1: error: stack frame size (1536) exceeds limit (1024) in '__se_sys_statmount' [-Werror,-Wframe-larger-than]
>> 4995 | SYSCALL_DEFINE4(statmount, const struct mnt_id_req __user *, req,
>>
>> Mark the inner function as noinline_for_stack so the second copy is
>> freed before calling do_statmount() enters filesystem specific code.
>> The extra copy of the structure is a bit inefficient, but this
>> system call should not be performance critical.
>
> Are you sure this is not performance sensitive, or is the performance
> critical comment not related to the system call being called many times?
>
>
> It's going to be a while (if ever) before callers change there ways.
>
> Consider what happens when a bunch of mounts are being mounted.
>
>
> First there are a lot of events and making the getting of mount info.
> more efficient means more of those events get processed (itself an issue
> that's going to need notification sub-system improvement) resulting in
> the system call being called even more.
Ok, I'll send a v2 that is more efficent. I expected it to look uglier,
but I don't think it's actually that bad:
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4957,15 +4957,12 @@ static int prepare_kstatmount(struct kstatmount *ks, struct mnt_id_req *kreq,
if (!access_ok(buf, bufsize))
return -EFAULT;
- *ks = (struct kstatmount){
- .mask = kreq->param,
- .buf = buf,
- .bufsize = bufsize,
- .seq = {
- .size = seq_size,
- .buf = kvmalloc(seq_size, GFP_KERNEL_ACCOUNT),
- },
- };
+ memset(ks, 0, sizeof(*ks));
+ ks->mask = kreq->param,
+ ks->buf = buf,
+ ks->bufsize = bufsize,
+ ks->seq.size = seq_size,
+ ks->seq.buf = kvmalloc(seq_size, GFP_KERNEL_ACCOUNT),
if (!ks->seq.buf)
return -ENOMEM;
return 0;
