| Message ID | 20220908022506.1275799-9-guoren@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Add GENERIC_ENTRY, irq stack support | expand |
On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > 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@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@kernel.org> > Cc: Andreas Schwab <schwab@suse.de> > --- > 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. - 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
On Thu, Sep 8, 2022 at 3:37 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > 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@linux.alibaba.com> > > Signed-off-by: Guo Ren <guoren@kernel.org> > > Cc: Andreas Schwab <schwab@suse.de> > > --- > > 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
On Sat, Sep 10, 2022, at 2:52 PM, Guo Ren wrote: > On Thu, Sep 8, 2022 at 3:37 PM Arnd Bergmann <arnd@arndb.de> wrote: >> On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: >> > From: Guo Ren <guoren@linux.alibaba.com> >> - 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. Arnd
On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@arndb.de> 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@arndb.de> wrote: > >> On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: > >> > From: Guo Ren <guoren@linux.alibaba.com> > >> - 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? The requirement is from [1], and I think disabling CONFIG_VMAP_STACK would be the last step after serious testing. [1] https://www.cnx-software.com/2021/10/25/allwinner-d1s-f133-risc-v-processor-64mb-ddr2/ > > > /* 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. > > Arnd
On Sun, Sep 11, 2022, at 1:35 AM, Guo Ren wrote: > On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@arndb.de> 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@arndb.de> wrote: >> >> On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: >> >> > From: Guo Ren <guoren@linux.alibaba.com> >> >> - 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? > The requirement is from [1], and I think disabling CONFIG_VMAP_STACK > would be the last step after serious testing. I still don't see why you need to turn off VMAP_STACK at all if it works. The only downside I can see with using VMAP_STACK on a 64-bit system is that it may expose bugs with device drivers that do DMA to stack data. Once you have tested the system successfully, you can also assume that you have no such drivers. Arnd
On Mon, Sep 12, 2022 at 2:40 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Sun, Sep 11, 2022, at 1:35 AM, Guo Ren wrote: > > On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@arndb.de> 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@arndb.de> wrote: > >> >> On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: > >> >> > From: Guo Ren <guoren@linux.alibaba.com> > >> >> - 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? > > The requirement is from [1], and I think disabling CONFIG_VMAP_STACK > > would be the last step after serious testing. > > I still don't see why you need to turn off VMAP_STACK at all > if it works. The only downside I can see with using VMAP_STACK > on a 64-bit system is that it may expose bugs with device > drivers that do DMA to stack data. Once you have tested the > system successfully, you can also assume that you have no such > drivers. 1st, VMAP_STACK could be enabled&disabled in arch/Kconfig. If we don't force users to enable VMAP_STACK, why couldn't user adjust THREAD_SIZE? 2nd, VMAP_STACK is not free, we still need 1KB shadow_stack. The EXPERT is enough for your concern. > > Arnd -- Best Regards Guo Ren
On Mon, Sep 12, 2022, at 6:14 AM, Guo Ren wrote: > On Mon, Sep 12, 2022 at 2:40 AM Arnd Bergmann <arnd@arndb.de> wrote: >> On Sun, Sep 11, 2022, at 1:35 AM, Guo Ren wrote: >> > On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@arndb.de> wrote: >> >> >> >> 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? >> > The requirement is from [1], and I think disabling CONFIG_VMAP_STACK >> > would be the last step after serious testing. >> >> I still don't see why you need to turn off VMAP_STACK at all >> if it works. The only downside I can see with using VMAP_STACK >> on a 64-bit system is that it may expose bugs with device >> drivers that do DMA to stack data. Once you have tested the >> system successfully, you can also assume that you have no such >> drivers. > 1st, VMAP_STACK could be enabled&disabled in arch/Kconfig. If we don't > force users to enable VMAP_STACK, why couldn't user adjust > THREAD_SIZE? Turning off VMAP_STACK is harmless and may help debug issues with VMAP_STACK itself. It's also required on architectures that don't have KASAN_VMALLOC or something else that conflicts with it. Changing THREAD_SIZE is also fine, as long as VMAP_STACK catches the inevitable overflows. I would not object to having an option that allows setting the stack size larger than the default without VMAP_STACK, as long as setting it lower requires using VMAP_STACK. That would however add a lot more complexity and probably doesn't do what you want either. > 2nd, VMAP_STACK is not free, we still need 1KB shadow_stack. > The EXPERT is enough for your concern. It's actually more than the 1KB: you need both 1KB of shadow stack and 4KB per CPU for the actual overflow_stack. If you are micro-optimizing at this level, then a possible option may be to change the handle_kernel_stack_overflow() function to not preserve the task stack and just panic() without showing the backtrace. That way you don't see which code caused the issue, but at least you avoid corrupting random data. Arnd
On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@arndb.de> 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@arndb.de> wrote: > >> On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: > >> > From: Guo Ren <guoren@linux.alibaba.com> > >> - 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 > > Arnd
On Mon, Sep 12, 2022 at 4:25 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Sep 12, 2022, at 6:14 AM, Guo Ren wrote: > > On Mon, Sep 12, 2022 at 2:40 AM Arnd Bergmann <arnd@arndb.de> wrote: > >> On Sun, Sep 11, 2022, at 1:35 AM, Guo Ren wrote: > >> > On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@arndb.de> wrote: > >> >> > >> >> 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? > >> > The requirement is from [1], and I think disabling CONFIG_VMAP_STACK > >> > would be the last step after serious testing. > >> > >> I still don't see why you need to turn off VMAP_STACK at all > >> if it works. The only downside I can see with using VMAP_STACK > >> on a 64-bit system is that it may expose bugs with device > >> drivers that do DMA to stack data. Once you have tested the > >> system successfully, you can also assume that you have no such > >> drivers. > > 1st, VMAP_STACK could be enabled&disabled in arch/Kconfig. If we don't > > force users to enable VMAP_STACK, why couldn't user adjust > > THREAD_SIZE? > > Turning off VMAP_STACK is harmless and may help debug issues > with VMAP_STACK itself. It's also required on architectures > that don't have KASAN_VMALLOC or something else that conflicts > with it. > > Changing THREAD_SIZE is also fine, as long as VMAP_STACK catches > the inevitable overflows. I would not object to having an > option that allows setting the stack size larger than the > default without VMAP_STACK, as long as setting it lower requires > using VMAP_STACK. That would however add a lot more complexity > and probably doesn't do what you want either. Thx for the detailed clarification, I agree with the point. I've put an EXPERT on config. > > > 2nd, VMAP_STACK is not free, we still need 1KB shadow_stack. > > The EXPERT is enough for your concern. > > It's actually more than the 1KB: you need both 1KB of shadow > stack and 4KB per CPU for the actual overflow_stack. If you > are micro-optimizing at this level, then a possible option > may be to change the handle_kernel_stack_overflow() function > to not preserve the task stack and just panic() without > showing the backtrace. That way you don't see which code > caused the issue, but at least you avoid corrupting random > data. Thx for the detailed explanation, the handle_kernel_stack_overflow() is a novel idea, which I will consider later. > > Arnd -- Best Regards Guo Ren
On Mon, Sep 19, 2022 at 4:35 PM Guo Ren <guoren@kernel.org> wrote: > > On Sun, Sep 11, 2022 at 12:07 AM Arnd Bergmann <arnd@arndb.de> 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@arndb.de> wrote: > > >> On Thu, Sep 8, 2022, at 4:25 AM, guoren@kernel.org wrote: > > >> > From: Guo Ren <guoren@linux.alibaba.com> > > >> - 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@kernel.org> 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@linux.alibaba.com> Signed-off-by: Guo Ren <guoren@kernel.org> Cc: Andreas Schwab <schwab@suse.de> 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
On Tue, Sep 20, 2022, at 2:46 AM, Guo Ren wrote: > > How about this one: (only THREAD_SIZE, no THREAD_ORDER&SHIFT.) > > - > /* 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 So far looks fine. > > /* > * 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 The THREAD_ALIGN does not, this only works for power-of-two numbers of THREAD_SIZE, > 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) I think this needs to use THREAD_ALIGN, not THREAD_SIZE. Arnd
On Tue, Sep 20, 2022, at 2:46 AM, Guo Ren wrote: > 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. I still think this should be guarded in a way that prevents setting the stack to smaller than default values unless VMAP_STACK is set as well. Arnd
On Tue, Sep 20, 2022 at 3:15 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Sep 20, 2022, at 2:46 AM, Guo Ren wrote: > > > > > How about this one: (only THREAD_SIZE, no THREAD_ORDER&SHIFT.) > > > > - > > /* 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 > > > So far looks fine. > > > > > /* > > * 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 > > The THREAD_ALIGN does not, this only works for power-of-two numbers of > THREAD_SIZE, We double THREAD_SIZE to simplify the detection. See the commit log: The overflow detect is performed before any attempt is made to access the stack and the principle of stack overflow detection: kernel stacks are aligned to double their size, enabling overflow to be detected with a single bit test. For example, a 16K stack is aligned to 32K, ensuring that bit 14 of the SP must be zero. On an overflow (or underflow), this bit is flipped. Thus, overflow (of less than the size of the stack) can be detected by testing whether this bit is set. I would try to optimize the size of VMAP_STACK in another patch. > > > 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) > > I think this needs to use THREAD_ALIGN, not THREAD_SIZE. No, it's BIT[14], when THREAD_SIZE = 16K. > > Arnd
On Tue, Sep 20, 2022 at 3:18 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Sep 20, 2022, at 2:46 AM, Guo Ren wrote: > > > 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. > > I still think this should be guarded in a way that prevents > setting the stack to smaller than default values unless VMAP_STACK > is set as well. Current VMAP_STACK would double THREAD_SIZE. Let me see how to reduce the VMAP_STACK. > > Arnd
On Wed, Sep 21, 2022 at 2:13 PM Guo Ren <guoren@kernel.org> wrote: > > On Tue, Sep 20, 2022 at 3:18 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Tue, Sep 20, 2022, at 2:46 AM, Guo Ren wrote: > > > > > 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. > > > > I still think this should be guarded in a way that prevents > > setting the stack to smaller than default values unless VMAP_STACK > > is set as well. > Current VMAP_STACK would double THREAD_SIZE. Let me see how to reduce > the VMAP_STACK. Sorry, for my miss understanding. I have no idea to reduce the VMAP_STACK's THREAD_ALIGN, THREAD_SIZE*2 is fine. Here is my new patch: diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 76bde12d9f8c..669ae57356a2 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -443,6 +443,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 VMAP_STACK && 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..e7ae3f13b879 100644 --- a/arch/riscv/include/asm/thread_info.h +++ b/arch/riscv/include/asm/thread_info.h @@ -11,32 +11,17 @@ #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) -#else -#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 2207cf44a3bc..71ea850ff0db 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_ALIGN >> PAGE_SHIFT >> 1) bnez sp, handle_kernel_stack_overflow REG_L sp, TASK_TI_KERNEL_SP(tp) #endif > > > > > Arnd > > > > -- > Best Regards > Guo Ren
On Wed, Sep 21, 2022 at 4:23 PM Guo Ren <guoren@kernel.org> wrote: > > On Wed, Sep 21, 2022 at 2:13 PM Guo Ren <guoren@kernel.org> wrote: > > > > On Tue, Sep 20, 2022 at 3:18 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > On Tue, Sep 20, 2022, at 2:46 AM, Guo Ren wrote: > > > > > > > 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. > > > > > > I still think this should be guarded in a way that prevents > > > setting the stack to smaller than default values unless VMAP_STACK > > > is set as well. > > Current VMAP_STACK would double THREAD_SIZE. Let me see how to reduce > > the VMAP_STACK. > Sorry, for my miss understanding. I have no idea to reduce the > VMAP_STACK's THREAD_ALIGN, THREAD_SIZE*2 is fine. Here is my new > patch: > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 76bde12d9f8c..669ae57356a2 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -443,6 +443,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 VMAP_STACK && 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..e7ae3f13b879 100644 > --- a/arch/riscv/include/asm/thread_info.h > +++ b/arch/riscv/include/asm/thread_info.h > @@ -11,32 +11,17 @@ > #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) > -#else > -#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 2207cf44a3bc..71ea850ff0db 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_ALIGN >> PAGE_SHIFT >> 1) > bnez sp, handle_kernel_stack_overflow > REG_L sp, TASK_TI_KERNEL_SP(tp) > #endif Sorry for the update again, fixup !VMAP_STACK compile error. diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 76bde12d9f8c..602e577c429c 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -443,6 +443,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_ORDER + int "Kernel stack size (in power-of-two numbers of page size)" if VMAP_STACK && EXPERT + range 0 4 + default 1 if 32BIT && !KASAN + default 3 if 64BIT && KASAN + default 2 + 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..3f382490d8ed 100644 --- a/arch/riscv/include/asm/thread_info.h +++ b/arch/riscv/include/asm/thread_info.h @@ -11,24 +11,13 @@ #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_ORDER CONFIG_THREAD_SIZE_ORDER +#define THREAD_SIZE (1 << PAGE_SHIFT << THREAD_SIZE_ORDER) /* * 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 +25,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 5cbd6684ef52..62e8f3a3c942 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_ALIGN >> PAGE_SHIFT >> 1) bnez sp, handle_kernel_stack_overflow REG_L sp, TASK_TI_KERNEL_SP(tp) #endif > > > > > > > > > > Arnd > > > > > > > > -- > > Best Regards > > Guo Ren > > > > -- > Best Regards > Guo Ren
Hi Arnd,
No more coding conventions for the patch, THREAD_ALIGN &
THREAD_SIZE_ORDER are used by vmlinux.lds.S and common code. So here
is my considerable version:
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 76bde12d9f8c..602e577c429c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -443,6 +443,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_ORDER
+ int "Kernel stack size (in power-of-two numbers of page size)"
if VMAP_STACK && EXPERT
+ range 0 4
+ default 1 if 32BIT && !KASAN
+ default 3 if 64BIT && KASAN
+ default 2
+ 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..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)
It's a little bit back to the original, but KASAN_STACK_ORDER has been
deleted. VMAP_STACK && EXPERT added.
On Wed, Sep 21, 2022 at 6:31 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Wed, Sep 21, 2022 at 4:23 PM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Wed, Sep 21, 2022 at 2:13 PM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Tue, Sep 20, 2022 at 3:18 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > On Tue, Sep 20, 2022, at 2:46 AM, Guo Ren wrote:
> > > >
> > > > > 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.
> > > >
> > > > I still think this should be guarded in a way that prevents
> > > > setting the stack to smaller than default values unless VMAP_STACK
> > > > is set as well.
> > > Current VMAP_STACK would double THREAD_SIZE. Let me see how to reduce
> > > the VMAP_STACK.
> > Sorry, for my miss understanding. I have no idea to reduce the
> > VMAP_STACK's THREAD_ALIGN, THREAD_SIZE*2 is fine. Here is my new
> > patch:
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 76bde12d9f8c..669ae57356a2 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -443,6 +443,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 VMAP_STACK && 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..e7ae3f13b879 100644
> > --- a/arch/riscv/include/asm/thread_info.h
> > +++ b/arch/riscv/include/asm/thread_info.h
> > @@ -11,32 +11,17 @@
> > #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)
> > -#else
> > -#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 2207cf44a3bc..71ea850ff0db 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_ALIGN >> PAGE_SHIFT >> 1)
> > bnez sp, handle_kernel_stack_overflow
> > REG_L sp, TASK_TI_KERNEL_SP(tp)
> > #endif
> Sorry for the update again, fixup !VMAP_STACK compile error.
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 76bde12d9f8c..602e577c429c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -443,6 +443,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_ORDER
> + int "Kernel stack size (in power-of-two numbers of page size)"
> if VMAP_STACK && EXPERT
> + range 0 4
> + default 1 if 32BIT && !KASAN
> + default 3 if 64BIT && KASAN
> + default 2
> + 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..3f382490d8ed 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -11,24 +11,13 @@
> #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_ORDER CONFIG_THREAD_SIZE_ORDER
> +#define THREAD_SIZE (1 << PAGE_SHIFT << THREAD_SIZE_ORDER)
>
> /*
> * 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 +25,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 5cbd6684ef52..62e8f3a3c942 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_ALIGN >> PAGE_SHIFT >> 1)
> bnez sp, handle_kernel_stack_overflow
> REG_L sp, TASK_TI_KERNEL_SP(tp)
> #endif
>
> >
> >
> > >
> > > >
> > > > Arnd
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
>
>
>
> --
> Best Regards
> Guo Ren
On Wed, Sep 21, 2022, at 12:46 PM, Guo Ren wrote: > Hi Arnd, > > No more coding conventions for the patch, THREAD_ALIGN & > THREAD_SIZE_ORDER are used by vmlinux.lds.S and common code. So here > is my considerable version: > That's fine with me, it should get the job done. I still think a way to use 12KB stacks with VMAP_STACK would be a good addition, but that can come later, and we may want to do this in an architecture-independent way when we get there. Arnd
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. + 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..c64d995df6e1 100644 --- a/arch/riscv/include/asm/thread_info.h +++ b/arch/riscv/include/asm/thread_info.h @@ -19,9 +19,9 @@ /* thread information allocation */ #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 #define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER)