[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 06 Jun 2019 12:10:31 -0700 (PDT)
From: Palmer Dabbelt <palmer@...ive.com>
To: Christoph Hellwig <hch@...radead.org>
CC: linux-riscv@...ts.infradead.org,
Paul Walmsley <paul.walmsley@...ive.com>, marco@...red.org,
me@...losedp.com, joel@...g.id.au, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] RISC-V: Break load reservations during switch_to
On Thu, 06 Jun 2019 02:05:18 PDT (-0700), Christoph Hellwig wrote:
> On Wed, Jun 05, 2019 at 04:17:35PM -0700, Palmer Dabbelt wrote:
>> REG_S ra, TASK_THREAD_RA_RA(a3)
>> + /*
>> + * The Linux ABI allows programs to depend on load reservations being
>> + * broken on context switches, but the ISA doesn't require that the
>> + * hardware ever breaks a load reservation. The only way to break a
>> + * load reservation is with a store conditional, so we emit one here.
>> + * Since nothing ever takes a load reservation on TASK_THREAD_RA_RA we
>> + * know this will always fail, but just to be on the safe side this
>> + * writes the same value that was unconditionally written by the
>> + * previous instruction.
>> + */
>> +#if (TASK_THREAD_RA_RA != 0)
>
> I don't think this check works as intended. TASK_THREAD_RA_RA is a
> parameterized macro, thus the above would never evaluate to 0. The
> error message also is rather odd while we're at it.
OK, I didn't try it. The resulting number can never be non-zero, which is why
I couldn't come up with an error message that made sense. I'm not opposed to
dropping the check.
>> +#if (__riscv_xlen == 64)
>> + sc.d x0, ra, 0(a3)
>> +#else
>> + sc.w x0, ra, 0(a3)
>> +#endif
>
> I'd rather add an macro ala REG_S to asm.h and distinguish between the
> xlen variants there:
>
> #define REG_SC __REG_SEL(sc.d, sc.w)
Ya, I guess I was just being lazy. I'll put that in a v2.
Powered by blists - more mailing lists