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

[Palmer Dabbelt]

On Thu, 06 Jun 2019 12:32:01 PDT (-0700), schwab@xxxxxxxxxxxxxx wrote:
> On Jun 06 2019, Christoph Hellwig <hch@xxxxxxxxxxxxx> 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.