Re: [PATCH V4 8/8] riscv: Add config of thread stack size

[Guo Ren]

On Thu, Sep 8, 2022 at 3:37 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Thu, Sep 8, 2022, at 4:25 AM, guoren@xxxxxxxxxx wrote:
> > From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> >
> > 0cac21b02ba5 ("risc v: use 16KB kernel stack on 64-bit") increase the
> > thread size mandatory, but some scenarios, such as D1 with a small
> > memory footprint, would suffer from that. After independent irq stack
> > support, let's give users a choice to determine their custom stack size.
> >
> > Signed-off-by: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Guo Ren <guoren@xxxxxxxxxx>
> > Cc: Andreas Schwab <schwab@xxxxxxx>
> > ---
> >  arch/riscv/Kconfig                   | 9 +++++++++
> >  arch/riscv/include/asm/thread_info.h | 4 ++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index da548ed7d107..e436b5793ab6 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -442,6 +442,15 @@ config IRQ_STACKS
> >         Add independent irq & softirq stacks for percpu to prevent kernel stack
> >         overflows. We may save some memory footprint by disabling IRQ_STACKS.
> >
> > +config THREAD_SIZE_ORDER
> > +     int "Pages of thread stack size (as a power of 2)"
> > +     range 1 4
> > +     default "1" if 32BIT
> > +     default "2" if 64BIT
> > +     help
> > +       Specify the Pages of thread stack size (from 8KB to 64KB), which also
> > +       affects irq stack size, which is equal to thread stack size.
>
> I would suggest hiding this under 'depends on EXPERT', no
> need to bother normal users with that question because the
> defaults are probably what everyone should use unless they are
> extremely limited.
>
> > #ifdef CONFIG_64BIT
> > -#define THREAD_SIZE_ORDER    (2 + KASAN_STACK_ORDER)
> > +#define THREAD_SIZE_ORDER    (CONFIG_THREAD_SIZE_ORDER + KASAN_STACK_ORDER)
> >  #else
> > -#define THREAD_SIZE_ORDER    (1 + KASAN_STACK_ORDER)
> > +#define THREAD_SIZE_ORDER    (CONFIG_THREAD_SIZE_ORDER + KASAN_STACK_ORDER)
> >  #endif
>
> The two sides of the #ifdef are now the same, so you no longer
> need both. You could also consider expressing the KASAN_STACK_ORDER
> bit in Kconfig logic for consistency, and put those into the
> defaults as well. Unless you actually use CONFIG_KASAN_STACK,
> the stack requirements of KASAN are not too bad, so that way one
> could decide to still use a smaller stack even with KASAN.
>
> If you want to make the setting really useful, you can add two
> more ideas:
>
> - When VMAP_STACK is set, make it possible to select non-power-of-two
>   stack sizes. Most importantly, 12KB should be a really interesting
>   choice as 8KB is probably still not enough for many 64-bit workloads,
>   but 16KB is often more than what you need. You probably don't
>   want to allow 64BIT/8KB without VMAP_STACK anyway since that just
>   makes it really hard to debug, so hiding the option when VMAP_STACK
>   is disabled may also be a good idea.
I don't want this config to depend on VMAP_STACK. Some D1 chips would
run with an 8K stack size and !VMAP_STACK.

Here is the new patch:

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index da548ed7d107..e7fcc3fbf48e 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -442,6 +442,24 @@ config IRQ_STACKS
          Add independent irq & softirq stacks for percpu to prevent
kernel stack
          overflows. We may save some memory footprint by disabling IRQ_STACKS.

+config THREAD_SIZE
+       int "Kernel stack size (in bytes)" if EXPERT
+       range 4096 65536
+       default 8192 if 32BIT && !KASAN
+       default 32768 if 64BIT && KASAN
+       default 16384
+       help
+         Specify the Pages of thread stack size (from 4KB to 64KB), which also
+         affects irq stack size, which is equal to thread stack size.
+
+config THREAD_SIZE_ORDER
+       int
+       default 0 if THREAD_SIZE = 4096
+       default 1 if THREAD_SIZE <= 8192
+       default 2 if THREAD_SIZE <= 16384
+       default 3 if THREAD_SIZE <= 32768
+       default 4
+
 endmenu # "Platform type"

 menu "Kernel features"
diff --git a/arch/riscv/include/asm/thread_info.h
b/arch/riscv/include/asm/thread_info.h
index 043da8ccc7e6..c970d41dc4c6 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -11,18 +11,8 @@
 #include <asm/page.h>
 #include <linux/const.h>

-#ifdef CONFIG_KASAN
-#define KASAN_STACK_ORDER 1
-#else
-#define KASAN_STACK_ORDER 0
-#endif
-
 /* thread information allocation */
-#ifdef CONFIG_64BIT
-#define THREAD_SIZE_ORDER      (2 + KASAN_STACK_ORDER)
-#else
-#define THREAD_SIZE_ORDER      (1 + KASAN_STACK_ORDER)
-#endif
+#define THREAD_SIZE_ORDER      CONFIG_THREAD_SIZE_ORDER
 #define THREAD_SIZE            (PAGE_SIZE << THREAD_SIZE_ORDER)


>
> - For testing purposes, you can even allow byte-exact stack sizes
>   that allow finding out what the actual minimum is by adding a
>   fixed offset during kernel entry. See add_random_kstack_offset()
>   for how to adjust the stack.
>
> With all those ideas added in, the Kconfig logic would be
> something like (assuming you can use
>
> config THREAD_SIZE
>        int "Kernel stack size (in bytes)" if VMAP_STACK && EXPERT
>        range 4096 65536
>        default 8192 if 32BIT && !KASAN
>        default 32768 if 64BIT && KASAN
>        default 16384
>
> config THREAD_SIZE_ORDER
>        int
>        default 0 if THREAD_SIZE = 4096
>        default 1 if THREAD_SIZE <= 8192
>        default 2 if THREAD_SIZE <= 16384
>        default 3 if THREAD_SIZE <= 32768
>        default 4
>
>       Arnd



--
Best Regards

 Guo Ren