linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Andrew Morton <akpm@linux-foundation.org>, Arnd Bergmann <arnd@arndb.de>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Nathan Chancellor <nathan@kernel.org>, Tom Rix <trix@redhat.com>,
	Xiaoming Ni <nixiaoming@huawei.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev, Christoph Hellwig <hch@lst.de>,
	Eric Dumazet <edumazet@google.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Nick Desaulniers <ndesaulniers@google.com>
Subject: [PATCH v2] fs/select: mark do_select noinline_for_stack
Date: Tue, 11 Oct 2022 13:55:47 -0700	[thread overview]
Message-ID: <20221011205547.14553-1-ndesaulniers@google.com> (raw)
In-Reply-To: <c8f87abd-9d66-40cf-bcea-e2b1388d3030@app.fastmail.com>

Effectively a revert of
commit ad312f95d41c ("fs/select: avoid clang stack usage warning")

Various configs can still push the stack useage of core_sys_select()
over the CONFIG_FRAME_WARN threshold (1024B on 32b targets).

  fs/select.c:619:5: error: stack frame size of 1048 bytes in function
  'core_sys_select' [-Werror,-Wframe-larger-than=]

core_sys_select() has a large stack allocation for `stack_fds` where it
tries to do something equivalent to "small string optimization" to
potentially avoid a kmalloc.

core_sys_select() calls do_select() which has another potentially large
stack allocation, `table`. Both of these values depend on
FRONTEND_STACK_ALLOC.

Mix those two large allocation with register spills which are
exacerbated by various configs and compiler versions and we can just
barely exceed the 1024B limit.

Rather than keep trying to find the right value of MAX_STACK_ALLOC or
FRONTEND_STACK_ALLOC, mark do_select() as noinline_for_stack.

The intent of FRONTEND_STACK_ALLOC is to help potentially avoid a
dynamic memory allocation. In that spirit, restore the previous
threshold but separate the stack frames.

Many tests of various configs for different architectures and various
versions of GCC were performed; do_select() was never inlined into
core_sys_select() or compat_core_sys_select(). The kernel is built with
the GCC specific flag `-fconserve-stack` which can limit inlining
depending on per-target thresholds of callee stack size, which helps
avoid the issue when using GCC. Clang is being more aggressive and not
considering the stack size when decided whether to inline or not. We may
consider using the clang-16+ flag `-finline-max-stacksize=` in the
future.

Link: https://lore.kernel.org/lkml/20221006222124.aabaemy7ofop7ccz@google.com/
Fixes: ad312f95d41c ("fs/select: avoid clang stack usage warning")
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes v1 -> v2:
* Drop the 32b specific guard, since I could reproduce the no-inlining
  w/ aarch64-linux-gnu-gcc-10 ARCH=arm64 defconfig, and per Arnd.
* Drop references to 32b in commit message.
* Add new paragraph in commit message at the end about -fconserve-stack
  and -finline-max-stacksize=.
* s/malloc/kmalloc/ in commit message.

 fs/select.c          | 1 +
 include/linux/poll.h | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/select.c b/fs/select.c
index 0ee55af1a55c..794e2a91b1fa 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -476,6 +476,7 @@ static inline void wait_key_set(poll_table *wait, unsigned long in,
 		wait->_key |= POLLOUT_SET;
 }
 
+noinline_for_stack
 static int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time)
 {
 	ktime_t expire, *to = NULL;
diff --git a/include/linux/poll.h b/include/linux/poll.h
index a9e0e1c2d1f2..d1ea4f3714a8 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -14,11 +14,7 @@
 
 /* ~832 bytes of stack space used max in sys_select/sys_poll before allocating
    additional memory. */
-#ifdef __clang__
-#define MAX_STACK_ALLOC 768
-#else
 #define MAX_STACK_ALLOC 832
-#endif
 #define FRONTEND_STACK_ALLOC	256
 #define SELECT_STACK_ALLOC	FRONTEND_STACK_ALLOC
 #define POLL_STACK_ALLOC	FRONTEND_STACK_ALLOC
-- 
2.38.0.rc2.412.g84df46c1b4-goog


  reply	other threads:[~2022-10-11 20:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-07  9:01 [PATCH] fs/select: avoid clang stack usage warning Arnd Bergmann
2019-03-07 16:19 ` Andi Kleen
2022-10-06 22:21 ` Nick Desaulniers
2022-10-07  8:28   ` Arnd Bergmann
2022-10-07 19:04     ` Nick Desaulniers
2022-10-07 20:11       ` [PATCH] fs/select: mark do_select noinline_for_stack for 32b Nick Desaulniers
2022-10-10  7:44         ` Christoph Hellwig
2022-10-10 16:42           ` Nick Desaulniers
2022-10-10 19:55         ` Arnd Bergmann
2022-10-13 18:52         ` kernel test robot
2022-10-07 21:42       ` [PATCH] fs/select: avoid clang stack usage warning Arnd Bergmann
2022-10-07 22:54         ` Nick Desaulniers
2022-10-10 18:40           ` Nick Desaulniers
2022-10-11 12:46             ` Arnd Bergmann
2022-10-11 20:55               ` Nick Desaulniers [this message]
2022-10-14 22:39                 ` [PATCH v2] fs/select: mark do_select noinline_for_stack Nathan Chancellor
2022-10-07 23:17         ` [PATCH] fs/select: avoid clang stack usage warning Kees Cook
2022-10-09 10:49       ` David Laight
2022-10-08  1:46   ` Andi Kleen
2022-10-11 20:50     ` Nick Desaulniers
2022-10-13 20:52       ` Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221011205547.14553-1-ndesaulniers@google.com \
    --to=ndesaulniers@google.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=darrick.wong@oracle.com \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mcgrof@kernel.org \
    --cc=nathan@kernel.org \
    --cc=nixiaoming@huawei.com \
    --cc=trix@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).