[RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[Jisheng Zhang]

After irq stack is supported, it's possible to use small THREAD_SIZE.
In fact, I tested this patch on a Lichee RV board, looks good so far.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 arch/riscv/include/asm/thread_info.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 67387a8bcb34..fdbf3890a1ab 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -12,11 +12,7 @@
 #include <linux/const.h>
 
 /* thread information allocation */
-#ifdef CONFIG_64BIT
-#define THREAD_SIZE_ORDER	(2)
-#else
 #define THREAD_SIZE_ORDER	(1)
-#endif
 #define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)
 
 #define IRQ_STACK_SIZE		THREAD_SIZE
-- 
2.34.1

RE: [RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[David Laight]

From: Jisheng Zhang
> Sent: 06 February 2022 17:44
> 
> After irq stack is supported, it's possible to use small THREAD_SIZE.
> In fact, I tested this patch on a Lichee RV board, looks good so far.

Is riscv using vmalloc with a guard page?

You won't find the deepest kernel stack use with trivial tests.
I'd hazard a guess that it is inside printk() in some error path.
Debugging stack overflow is horrid.

With no alloca() no recursion and limited indirect calls it is
technically possible to statically calculate maximum stack use.
But I don't think anyone has tried to do it for the linux kernel.
I did do it for an embedded system (that had almost no indirect calls)
and found we didn't have enough memory for the required stacks!

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[Arnd Bergmann]

On Sun, Feb 6, 2022 at 6:43 PM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> After irq stack is supported, it's possible to use small THREAD_SIZE.
> In fact, I tested this patch on a Lichee RV board, looks good so far.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>

I can definitely see the value in this, in particular when you get hardware with
small RAM configurations that would run a 32-bit kernel on other architectures.

However, it's worth pointing out that all other 64-bit architectures use 16KB or
more, so it is rather dangerous to be the first architecture to try this out in
a long time. Some on-stack structures have a lot of pointers and 'unsigned long'
members, so they need twice the space, while other structures are the
same size either way.

IRQ stacks obviously help here, but I don't think the 8KB size can be made
the default without a lot of testing of some of the more rarely used code paths.

Here are a few things that would be worth doing on the way to a smaller
kernel stack:

- do more compile-time testing with a lower CONFIG_FRAME_WARN value.
  Historically, this defaults to 2048 bytes on 64-bit architectures. This is
  much higher than we really want, but it takes work to find and eliminate
  the outliers. I previously had a series to reduce the limit to 1280 bytes on
  arm64 and still build all 'randconfig' configurations.

- Use a much lower limit during regression testing. There is already a config
   option to randomize the start of the thread stack, but you can also try
   adding a configurable offset to see how far you can push it for a given
   workload before you run into the guard page.

- With vmap stacks, using 12KB may be an option as well, giving you
   three pages for the stack plus one guard page, instead of 4 pages
   stack plus four guard pages.

- If you can make a convincing case for using a lower THREAD_SIZE,
  make this a compile-time option across all 64-bit architectures that
  support both IRQ stacks and VMAP stacks. The actual frame size
  does depend on the ISA a bit, and we know that parisc and ia64 are
  particularly, possibly s390 as well, but I would expect risc-v to be
  not much different from arm64 and x86 here.

       Arnd

Re: [RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[David Abdurachmanov]

On Sun, Feb 6, 2022 at 7:53 PM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> After irq stack is supported, it's possible to use small THREAD_SIZE.
> In fact, I tested this patch on a Lichee RV board, looks good so far.

We went from 8K to 16K somewhere in mid-2020 on riscv64 because we
were seeing some random crashes in various distributions (Debian,
Fedora, OpenSUSE). Thus we matched what other popular arches do, i.e.
16K.

Thus I would be very careful going back to 8K.

david

>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  arch/riscv/include/asm/thread_info.h | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index 67387a8bcb34..fdbf3890a1ab 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -12,11 +12,7 @@
>  #include <linux/const.h>
>
>  /* thread information allocation */
> -#ifdef CONFIG_64BIT
> -#define THREAD_SIZE_ORDER      (2)
> -#else
>  #define THREAD_SIZE_ORDER      (1)
> -#endif
>  #define THREAD_SIZE            (PAGE_SIZE << THREAD_SIZE_ORDER)
>
>  #define IRQ_STACK_SIZE         THREAD_SIZE
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[Andreas Schwab]

On Feb 07 2022, David Abdurachmanov wrote:

> On Sun, Feb 6, 2022 at 7:53 PM Jisheng Zhang <jszhang@kernel.org> wrote:
>>
>> After irq stack is supported, it's possible to use small THREAD_SIZE.
>> In fact, I tested this patch on a Lichee RV board, looks good so far.
>
> We went from 8K to 16K somewhere in mid-2020 on riscv64 because we
> were seeing some random crashes in various distributions (Debian,
> Fedora, OpenSUSE). Thus we matched what other popular arches do, i.e.
> 16K.

I think NFS is one of the worst offenders.  I'm running an Unleashed
with NFS root, which probably amplifies this.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[Arnd Bergmann]

On Mon, Feb 7, 2022 at 9:38 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Feb 07 2022, David Abdurachmanov wrote:
>
> > On Sun, Feb 6, 2022 at 7:53 PM Jisheng Zhang <jszhang@kernel.org> wrote:
> >>
> >> After irq stack is supported, it's possible to use small THREAD_SIZE.
> >> In fact, I tested this patch on a Lichee RV board, looks good so far.
> >
> > We went from 8K to 16K somewhere in mid-2020 on riscv64 because we
> > were seeing some random crashes in various distributions (Debian,
> > Fedora, OpenSUSE). Thus we matched what other popular arches do, i.e.
> > 16K.

It sounds like 2020 predates both HAVE_ARCH_VMAP_STACK and
IRQ stacks, so I would say that it was necessary back then because the
crashes were too hard to debug, but it's worth trying again now at least
as a compile-time option under CONFIG_EXPERT.

You need VMAP stacks to reliably get a backtrace out, and you need
IRQ stacks to make overflows happen reproducibly (and less often).

> I think NFS is one of the worst offenders.  I'm running an Unleashed
> with NFS root, which probably amplifies this.

I think there are a few others that can be equally bad, and some of them
might stack on top of others, such as swap over ecryptfs over nfs over
an encrypted network tunnel over infiniband. In the end, you just need
one badly written driver, or a compile bug to trigger it, and we have
plenty of both.

      Arnd

Re: [RFC PATCH] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64

[Jisheng Zhang]

On Mon, Feb 07, 2022 at 08:35:54AM +0100, Arnd Bergmann wrote:
> On Sun, Feb 6, 2022 at 6:43 PM Jisheng Zhang <jszhang@kernel.org> wrote:
> >
> > After irq stack is supported, it's possible to use small THREAD_SIZE.
> > In fact, I tested this patch on a Lichee RV board, looks good so far.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> 
> I can definitely see the value in this, in particular when you get hardware with
> small RAM configurations that would run a 32-bit kernel on other architectures.
> 
> However, it's worth pointing out that all other 64-bit architectures use 16KB or
> more, so it is rather dangerous to be the first architecture to try this out in
> a long time. Some on-stack structures have a lot of pointers and 'unsigned long'
> members, so they need twice the space, while other structures are the
> same size either way.
> 
> IRQ stacks obviously help here, but I don't think the 8KB size can be made
> the default without a lot of testing of some of the more rarely used code paths.
> 
> Here are a few things that would be worth doing on the way to a smaller
> kernel stack:
> 
> - do more compile-time testing with a lower CONFIG_FRAME_WARN value.
>   Historically, this defaults to 2048 bytes on 64-bit architectures. This is
>   much higher than we really want, but it takes work to find and eliminate
>   the outliers. I previously had a series to reduce the limit to 1280 bytes on
>   arm64 and still build all 'randconfig' configurations.
> 
> - Use a much lower limit during regression testing. There is already a config
>    option to randomize the start of the thread stack, but you can also try
>    adding a configurable offset to see how far you can push it for a given
>    workload before you run into the guard page.
> 
> - With vmap stacks, using 12KB may be an option as well, giving you
>    three pages for the stack plus one guard page, instead of 4 pages
>    stack plus four guard pages.
> 
> - If you can make a convincing case for using a lower THREAD_SIZE,
>   make this a compile-time option across all 64-bit architectures that
>   support both IRQ stacks and VMAP stacks. The actual frame size
>   does depend on the ISA a bit, and we know that parisc and ia64 are
>   particularly, possibly s390 as well, but I would expect risc-v to be
>   not much different from arm64 and x86 here.
> 

Hi Arnd

Thanks so much for all the suggestions. Item3 and Item4 look more
interesting to me.

Thanks a lot