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

[Palmer Dabbelt]

On Thu, 06 Jun 2019 12:32:01 PDT (-0700), sch...@linux-m68k.org wrote:

On Jun 06 2019, Christoph Hellwig <h...@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.