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

[Deepak Gupta <debug@rivosinc.com>]

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@rivosinc.com> wrote:
>
>> > > On Wed, Nov 23, 2022 at 5:28 PM Guo Ren <guoren@kernel.org> wrote:
>
>> > > > On Thu, Nov 24, 2022 at 8:50 AM Deepak Gupta <debug@rivosinc.com> 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@kernel.org/
>> > 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.
>