| Message ID | 20230214103030.1051950-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] maple_tree: reduce stack usage with gcc-9 and earlier | expand |
On 14.02.23 11:30, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gcc-10 changed the way inlining works to be less aggressive, but > older versions run into an oversized stack frame warning whenever > CONFIG_KASAN_STACK is enabled, as that forces variables from > inlined callees to be non-overlapping: > > lib/maple_tree.c: In function 'mas_wr_bnode': > lib/maple_tree.c:4320:1: error: the frame size of 1424 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > Change the annotations on mas_store_b_node() and mas_commit_b_node() > to explicitly forbid inlining in this configuration, which is > the same behavior that newer versions already have. > > Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> > Cc: Alexander Potapenko <glider@google.com> > Cc: Andrey Konovalov <andreyknvl@gmail.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > Cc: kasan-dev@googlegroups.com > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- Reviewed-by: David Hildenbrand <david@redhat.com>
On Tue, 14 Feb 2023 11:30:24 +0100 Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gcc-10 changed the way inlining works to be less aggressive, but > older versions run into an oversized stack frame warning whenever > CONFIG_KASAN_STACK is enabled, as that forces variables from > inlined callees to be non-overlapping: > > lib/maple_tree.c: In function 'mas_wr_bnode': > lib/maple_tree.c:4320:1: error: the frame size of 1424 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > Change the annotations on mas_store_b_node() and mas_commit_b_node() > to explicitly forbid inlining in this configuration, which is > the same behavior that newer versions already have. > > ... > > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -146,6 +146,13 @@ struct maple_subtree_state { > struct maple_big_node *bn; > }; > > +#ifdef CONFIG_KASAN_STACK > +/* Prevent mas_wr_bnode() from exceeding the stack frame limit */ > +#define noinline_for_kasan noinline_for_stack > +#else > +#define noinline_for_kasan inline > +#endif Should noinline_for_kasan be defined in kasan.h? maple_tree.c is unlikely to be the only place in the kernel which could use this treatment? I suppose we can do that when the need arises.
* Arnd Bergmann <arnd@kernel.org> [230214 05:30]: > From: Arnd Bergmann <arnd@arndb.de> > > gcc-10 changed the way inlining works to be less aggressive, but > older versions run into an oversized stack frame warning whenever > CONFIG_KASAN_STACK is enabled, as that forces variables from > inlined callees to be non-overlapping: > > lib/maple_tree.c: In function 'mas_wr_bnode': > lib/maple_tree.c:4320:1: error: the frame size of 1424 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > Change the annotations on mas_store_b_node() and mas_commit_b_node() > to explicitly forbid inlining in this configuration, which is > the same behavior that newer versions already have. > > Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> > Cc: Alexander Potapenko <glider@google.com> > Cc: Andrey Konovalov <andreyknvl@gmail.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > Cc: kasan-dev@googlegroups.com > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > lib/maple_tree.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index 5e9703189259..646297cae5d1 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -146,6 +146,13 @@ struct maple_subtree_state { > struct maple_big_node *bn; > }; > > +#ifdef CONFIG_KASAN_STACK > +/* Prevent mas_wr_bnode() from exceeding the stack frame limit */ nit: Should there be more info in here? You did add it to two functions and it's a problem for frame-larger-than=1024 Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> > +#define noinline_for_kasan noinline_for_stack > +#else > +#define noinline_for_kasan inline > +#endif > + > /* Functions */ > static inline struct maple_node *mt_alloc_one(gfp_t gfp) > { > @@ -2107,7 +2114,7 @@ static inline void mas_bulk_rebalance(struct ma_state *mas, unsigned char end, > * > * Return: The actual end of the data stored in @b_node > */ > -static inline void mas_store_b_node(struct ma_wr_state *wr_mas, > +static noinline_for_kasan void mas_store_b_node(struct ma_wr_state *wr_mas, > struct maple_big_node *b_node, unsigned char offset_end) > { > unsigned char slot; > @@ -3579,7 +3586,7 @@ static inline bool mas_reuse_node(struct ma_wr_state *wr_mas, > * @b_node: The maple big node > * @end: The end of the data. > */ > -static inline int mas_commit_b_node(struct ma_wr_state *wr_mas, > +static noinline_for_kasan int mas_commit_b_node(struct ma_wr_state *wr_mas, > struct maple_big_node *b_node, unsigned char end) > { > struct maple_node *node; > -- > 2.39.1 >
On Tue, Feb 14, 2023 at 8:40 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Tue, 14 Feb 2023 11:30:24 +0100 Arnd Bergmann <arnd@kernel.org> wrote: > > > From: Arnd Bergmann <arnd@arndb.de> > > > > gcc-10 changed the way inlining works to be less aggressive, but > > older versions run into an oversized stack frame warning whenever > > CONFIG_KASAN_STACK is enabled, as that forces variables from > > inlined callees to be non-overlapping: > > > > lib/maple_tree.c: In function 'mas_wr_bnode': > > lib/maple_tree.c:4320:1: error: the frame size of 1424 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > > > Change the annotations on mas_store_b_node() and mas_commit_b_node() > > to explicitly forbid inlining in this configuration, which is > > the same behavior that newer versions already have. > > > > ... > > > > --- a/lib/maple_tree.c > > +++ b/lib/maple_tree.c > > @@ -146,6 +146,13 @@ struct maple_subtree_state { > > struct maple_big_node *bn; > > }; > > > > +#ifdef CONFIG_KASAN_STACK > > +/* Prevent mas_wr_bnode() from exceeding the stack frame limit */ > > +#define noinline_for_kasan noinline_for_stack > > +#else > > +#define noinline_for_kasan inline > > +#endif > > Should noinline_for_kasan be defined in kasan.h? maple_tree.c is > unlikely to be the only place in the kernel which could use this > treatment? We could also define it in include/linux/compiler_types.h along with other KASAN attributes.
diff --git a/lib/maple_tree.c b/lib/maple_tree.c index 5e9703189259..646297cae5d1 100644 --- a/lib/maple_tree.c +++ b/lib/maple_tree.c @@ -146,6 +146,13 @@ struct maple_subtree_state { struct maple_big_node *bn; }; +#ifdef CONFIG_KASAN_STACK +/* Prevent mas_wr_bnode() from exceeding the stack frame limit */ +#define noinline_for_kasan noinline_for_stack +#else +#define noinline_for_kasan inline +#endif + /* Functions */ static inline struct maple_node *mt_alloc_one(gfp_t gfp) { @@ -2107,7 +2114,7 @@ static inline void mas_bulk_rebalance(struct ma_state *mas, unsigned char end, * * Return: The actual end of the data stored in @b_node */ -static inline void mas_store_b_node(struct ma_wr_state *wr_mas, +static noinline_for_kasan void mas_store_b_node(struct ma_wr_state *wr_mas, struct maple_big_node *b_node, unsigned char offset_end) { unsigned char slot; @@ -3579,7 +3586,7 @@ static inline bool mas_reuse_node(struct ma_wr_state *wr_mas, * @b_node: The maple big node * @end: The end of the data. */ -static inline int mas_commit_b_node(struct ma_wr_state *wr_mas, +static noinline_for_kasan int mas_commit_b_node(struct ma_wr_state *wr_mas, struct maple_big_node *b_node, unsigned char end) { struct maple_node *node;