Re: [PATCH] RISC-V: Break load reservations during switch_to

Palmer Dabbelt
2019-06-07T22:12:46Z

On Thu, 06 Jun 2019 12:32:01 PDT (-0700), schwab@linux-m68k.org wrote:
> On Jun 06 2019, Christoph Hellwig <hch@infradead.org> 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,
>
> Is it? Just because it is used before an open paren doesn't mean that
> the macro takes a parameter.

Yes, you're right -- the parens there aren't a macro parameter, they're the
assembly syntax for constant-offset loads. I guess I'd just assumed Christoph
was referring to some magic in how asm-offsets gets generated, as I've never
actually looked inside that stuff. I went ahead and just tested this

$ git diff | cat
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index 578bb5efc085..e3f06495dbf8 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -125,6 +125,7 @@ void asm_offsets(void)
DEFINE(TASK_THREAD_RA_RA,
offsetof(struct task_struct, thread.ra)
- offsetof(struct task_struct, thread.ra)
+ + 1
);
DEFINE(TASK_THREAD_SP_RA,
offsetof(struct task_struct, thread.sp)

and it causes the expected failure

$ make.cross ARCH=riscv -j1
make CROSS_COMPILE=/home/palmer/.local/opt/gcc-8.1.0-nolibc/riscv64-linux/bin/riscv64-linux- ARCH=riscv -j1
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
CHK include/generated/compile.h
AS arch/riscv/kernel/entry.o
arch/riscv/kernel/entry.S:344:3: error: #error "The offset between ra and ra is non-zero"
# error "The offset between ra and ra is non-zero"
^~~~~
make[1]: *** [scripts/Makefile.build:369: arch/riscv/kernel/entry.o] Error 1
make: *** [Makefile:1071: arch/riscv/kernel] Error 2

so I'm going to leave it alone. I'll submit a v2 with a better error message
and a cleaner sc.w/sc.d.