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

From: Guo Ren <guoren@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Palmer Dabbelt" <palmer@rivosinc.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Conor.Dooley" <conor.dooley@microchip.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Jisheng Zhang" <jszhang@kernel.org>,
	lazyparser@gmail.com, falcon@tinylab.org,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"Anup Patel" <apatel@ventanamicro.com>,
	"Atish Patra" <atishp@atishpatra.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	"Guo Ren" <guoren@linux.alibaba.com>,
	"Andreas Schwab" <schwab@suse.de>
Subject: Re: [PATCH V4 8/8] riscv: Add config of thread stack size
Date: Mon, 19 Sep 2022 16:38:02 +0800	[thread overview]
Message-ID: <CAJF2gTRzC9ES1EK3X4tHchTZMfbGJk2QLbD8VG=5x5bWD7Er0w@mail.gmail.com> (raw)
In-Reply-To: <2b06f28d-a13d-4c86-af04-39e383aaf07b@www.fastmail.com>

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