On Tue, 11 Jul 2017 06:55:28 PDT (-0700), h...@infradead.org wrote:
> On Tue, Jul 11, 2017 at 02:22:15PM +0100, Will Deacon wrote:
>> The problem is that by supporting these hypothetical designs that can't do
>> atomics, you hurt sensible designs that *can* do the atomics because you
>> force them to take an additional indirection that could otherwise be
>> avoided.
>
> Agreed. But the new patchset seems to remove it already, so I guess
> we're fine on the kernel side. Now we just need to make sure the
> glibc API doesn't use any indirections.
>
> Note that it might make sense to emit these for very low end nommu
> designs. Maybe even running Linux, but in that case they'll just need
> a special non-standard ABI for very limited use cases.
glibc has never used these calls on machines with the A extension. They're
only used in one specific header file to emulate cmpxchg, and they're guarded
by something like "#ifdef riscv_atomic". Here's the glibc code (from a
slightly older version, glibc is also in submission so everything's a bit of a
mess there too) for reference
/* If the A (atomic) extension is not present, we need help from the
kernel to do atomic accesses. Linux provides two system calls for
this purpose. RISCV_ATOMIC_CMPXCHG will perform an atomic compare
and exchange operation for a 32-bit value. RISCV_ATOMIC_CMPXCHG64
will do the same for a 64-bit value. */
#include <sys/syscall.h>
#include <sysdep.h>
#define __HAVE_64B_ATOMICS (__riscv_xlen >= 64)
#define USE_ATOMIC_COMPILER_BUILTINS 0
#define __arch_compare_and_exchange_val_8_acq(mem, newval, oldval) \
(abort (), (__typeof (*mem)) 0)
#define __arch_compare_and_exchange_val_16_acq(mem, newval, oldval) \
(abort (), (__typeof (*mem)) 0)
/* The only basic operation needed is compare and exchange. */
#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
({ \
INTERNAL_SYSCALL_DECL (__err); \
(__typeof (*mem)) INTERNAL_SYSCALL (sysriscv, __err, 4, \
RISCV_ATOMIC_CMPXCHG, mem, oldval, newval); \
})
#define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
({ \
INTERNAL_SYSCALL_DECL (__err); \
(__typeof (*mem)) INTERNAL_SYSCALL (sysriscv, __err, 4, \
RISCV_ATOMIC_CMPXCHG64, mem, oldval, newval); \
})
We originally had these as a special Kconfig option, but then it was pointed
out that user binaries built on non-A systems wouldn't run on A systems. That
seemed like a bad idea, so we just enabled it everywhere.
I think we should just table this discussion for now: we can always add the
system calls back in if people build non-A Linux systems. We'll mark our glibc
port as requiring the A extension and delete the dead code there so nothing
knows about the syscalls.
Thanks!
