Re: [PATCH] riscv: VMAP_STACK overflow detection thread-safe

[Deepak Gupta]

On Thu, Nov 24, 2022 at 08:59:32AM +0000, Conor Dooley wrote:
> On Wed, Nov 23, 2022 at 11:10:22PM -0800, Deepak Gupta wrote:
> > On Thu, Nov 24, 2022 at 02:31:25PM +0800, Guo Ren wrote:
> > > On Thu, Nov 24, 2022 at 1:57 PM Deepak Gupta <debug@xxxxxxxxxxxx> wrote:
>
> > > > On Wed, Nov 23, 2022 at 5:28 PM Guo Ren <guoren@xxxxxxxxxx> wrote:
>
> > > > > On Thu, Nov 24, 2022 at 8:50 AM Deepak Gupta <debug@xxxxxxxxxxxx> wrote:
>
> > > > >> Fixes: 31da94c25aea835ceac00575a9fd206c5a833fed
> > > > >
> > > > > The patch gives more significant change than the Fixes, and Fixes would expand to the previous stable versions. Please don't set it as a Fixes, but an improved OVERSTACK dead path performance feature.
> > > > >
> > > >
> > > > Not a performance feature but more like correctness.
> > > > If kernel died and two CPUs raced to kernel stack overflow,
> > > > death post-mortem should be straightforward.
> > > We already have had a fixup, and your patch likes a feature with a
> > > significant change.
> > > https://lore.kernel.org/linux-riscv/20221030124517.2370-1-jszhang@xxxxxxxxxx/
> > > If it is for correctness, the simple lock is enough.
> >
> > Sure lock is enough. It's different way to solve the problem. But I don't
> > think it qualifies as significant change.
>
> Something to bear in mind is where in the cycle we are - there's likely
> just over a week left before v6.1.
> Since the lock is sufficient to fix the problem for v6.1, it's easy to
> view this patch as an optimisation or improvement that should go on top
> of that, smaller, patch.
> Especially when you have some questions yourself about the correctness
> for 32 bit!

Yes I'll have to revise the patch to accomodate 32bit.

> I've got no technical comment to make about the discussion here, but
> looking in from the "outside", that's the easy conclusion to jump to.
>
>
> > 	REG_S x31, TASK_TI_SPILL_REG(tp)
> > 	asm_per_cpu sp, overflow_stack, x31
> > 	li x31, OVERFLOW_STACK_SIZE
> > 	add sp, sp, x31
> > 	REG_L x31, TASK_TI_SPILL_REG(tp)
>
> btw, for this sort of thing, could you please use some whitespace to
> align the operands? Makes things significantly more readable.

Noted. I'll do that.

> Thanks,
> Conor.