| Message ID | 20220206174359.2986-1-jszhang@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] riscv: reduce THREAD_SIZE from 16KB to 8KB for RV64 | expand |
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)
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
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
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.
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
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
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
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(-)