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

[Guo Ren]

On Mon, Sep 19, 2022 at 4:35 PM Guo Ren <guoren@xxxxxxxxxx> wrote:
>
> On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> >
> > On Sat, Sep 10, 2022, at 2:52 PM, Guo Ren wrote:
> > > 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>
> > >> - 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.
> >
> > That sounds like a really bad idea, why would you want to risk
> > using such a small stack without CONFIG_VMAP_STACK?
> >
> > Are you worried about increased memory usage or something else?
> >
> > >  /* 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)
> >
> > This doesn't actually allow additional THREAD_SIZE values, as you
> > still round up to the nearest power of two.
> >
> > I think all the non-arch code can deal with non-power-of-2
> > sizes, so you'd just need
> >
> > #define THREAD_SIZE round_up(CONFIG_THREAD_SIZE, PAGE_SIZE)
> >
> > and fix up the risc-v specific code to do the right thing
> > as well. I now see that THREAD_SIZE_ORDER is not actually
> > used anywhere with CONFIG_VMAP_STACK, so I suppose that
> > definition can be skipped, but you still need a THREAD_ALIGN
> > definition that is a power of two and at least a page larger
> > than THREAD_SIZE.
> Sorry, I missed this part. I would RESEND v5

Hi Arnd,

How about this one: (only THREAD_SIZE, no THREAD_ORDER&SHIFT.)

commit 447cddede7898c35a9a3b8ab3d7bdb7b0de0714d (HEAD)
Author: Guo Ren <guoren@xxxxxxxxxx>
Date:   Mon Sep 5 22:53:06 2022 -0400

    riscv: Add config of thread stack size

    0cac21b02ba5 ("riscv: 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>

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index dfe600f3526c..8def456f328c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -442,6 +442,16 @@ 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.
+
 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..181fdfbd5e99 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -11,24 +11,12 @@
 #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            (PAGE_SIZE << THREAD_SIZE_ORDER)
+#define THREAD_SIZE            CONFIG_THREAD_SIZE

 /*
  * By aligning VMAP'd stacks to 2 * THREAD_SIZE, we can detect overflow by
- * checking sp & (1 << THREAD_SHIFT), which we can do cheaply in the entry
- * assembly.
+ * checking sp & THREAD_SIZE, which we can do cheaply in the entry assembly.
  */
 #ifdef CONFIG_VMAP_STACK
 #define THREAD_ALIGN            (2 * THREAD_SIZE)
@@ -36,7 +24,6 @@
 #define THREAD_ALIGN            THREAD_SIZE
 #endif

-#define THREAD_SHIFT            (PAGE_SHIFT + THREAD_SIZE_ORDER)
 #define OVERFLOW_STACK_SIZE     SZ_4K
 #define SHADOW_OVERFLOW_STACK_SIZE (1024)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 426529b84db0..1e35fb3bdae5 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -29,8 +29,8 @@ _restore_kernel_tpsp:

 #ifdef CONFIG_VMAP_STACK
        addi sp, sp, -(PT_SIZE_ON_STACK)
-       srli sp, sp, THREAD_SHIFT
-       andi sp, sp, 0x1
+       srli sp, sp, PAGE_SHIFT
+       andi sp, sp, (THREAD_SIZE >> PAGE_SHIFT)
        bnez sp, handle_kernel_stack_overflow
        REG_L sp, TASK_TI_KERNEL_SP(tp)
 #endif


>
> >
> >      Arnd
>
>
>
> --
> Best Regards
>  Guo Ren



-- 
Best Regards
 Guo Ren