public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor.dooley@microchip.com>
To: Deepak Gupta <debug@rivosinc.com>
Cc: Guo Ren <guoren@kernel.org>, <palmer@dabbelt.com>,
	<linux-kernel@vger.kernel.org>, <linux-riscv@lists.infradead.org>,
	<paul.walmsley@sifive.com>, Jisheng Zhang <jszhang@kernel.org>
Subject: Re: [PATCH] riscv: VMAP_STACK overflow detection thread-safe
Date: Thu, 24 Nov 2022 08:59:32 +0000	[thread overview]
Message-ID: <Y38ydMoqmqOPfBs0@wendy> (raw)
In-Reply-To: <20221124071022.GA1149630@debug.ba.rivosinc.com>

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!
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.

Thanks,
Conor.


  reply	other threads:[~2022-11-24  9:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-24  0:50 [PATCH] riscv: VMAP_STACK overflow detection thread-safe Deepak Gupta
2022-11-24  1:32 ` Guo Ren
2022-11-24  8:26   ` Deepak Gupta
2022-11-24  9:07     ` Deepak Gupta
     [not found] ` <CAJF2gTQ=Vr6neABtz9JSCei6oPEsyWTpb-Y=Rxt5jy6n1VEtGA@mail.gmail.com>
2022-11-24  5:56   ` Deepak Gupta
2022-11-24  6:31     ` Guo Ren
2022-11-24  7:10       ` Deepak Gupta
2022-11-24  8:59         ` Conor Dooley [this message]
2022-11-24  9:10           ` Deepak Gupta
2022-11-24  9:36         ` Guo Ren
2022-11-24  6:30 ` Conor Dooley
2022-11-24  7:23   ` Deepak Gupta
2022-11-24  7:39     ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y38ydMoqmqOPfBs0@wendy \
    --to=conor.dooley@microchip.com \
    --cc=debug@rivosinc.com \
    --cc=guoren@kernel.org \
    --cc=jszhang@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox