| Message ID | 20231213090015.518044-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] statmount: reduce runtime stack usage | expand |
On Wed, Dec 13, 2023 at 10:00:03AM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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, > > Replace the assignment with an in-place memset() plus assignment that > should always be more efficient for both stack usage and runtime cost. > > Fixes: 49889374ab92 ("statmount: simplify string option retrieval") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- I folded this patch and placed a link here. Thanks!
On 13/12/23 17:00, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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, > > Replace the assignment with an in-place memset() plus assignment that > should always be more efficient for both stack usage and runtime cost. Cunning plan, to use the work efficient instead of inefficient, ;( But, TBH, the libc integration seems complex but I also feel there's no choice and this looks fine too. > > Fixes: 49889374ab92 ("statmount: simplify string option retrieval") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > fs/namespace.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index d036196f949c..159f1df379fc 100644 > --- 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; This looks much better than what it replaces IMHO. Reviewed-by: Ian Kent <raven@themaw.net> Ian
diff --git a/fs/namespace.c b/fs/namespace.c index d036196f949c..159f1df379fc 100644 --- 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;