Re: [PATCH 1/3] RISC-V: implement xchg_small and cmpxchg_small for char and short

[Michael Clark <michaeljclark@mac.com>]

On 12/02/19 8:17 PM, Christoph Hellwig wrote:
>>   
>> +extern unsigned long __xchg_relaxed_small(volatile void *ptr, unsigned long new,
>> +					  unsigned int size);
>> +extern unsigned long __xchg_acquire_small(volatile void *ptr, unsigned long new,
> 
> No need for any of the externs on function declarations.
> 
>> +++ b/arch/riscv/kernel/cmpxchg.c
>> @@ -0,0 +1,118 @@
>> +/*
>> + * Copyright (C) 2017 Imagination Technologies
>> + * Author: Paul Burton <paul.burton@mips.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>> + * option) any later version.
>> + */
> 
> Please use an SPDX tag instead of the GPL boilerplate.

Okay.

> Also this code seems to be entirely generic and very similar to the
> mips code.  Did you consider making it a generic library routine
> in lib/ instead?

Indeed. After your feedback, I agree. There is no reason why small word 
atomic support cannot be generalized, as it is, it is written in terms 
of the underlying primitives and is not MIPS specific. The addition, is 
the addition of; relaxed, acquire, release; and they can be made 
optional if this code is made generic. We have to think how the arch 
instantiates the templates, in the C template idiom and that likely 
requires some Kconfig magic...

In the mean time, I've absorbed feedback regarding amoadd, and it may be 
the case that we end up with an arch specific spinlock for RISC-V. I 
have not research rwlocks yet. Below is the sample asm snipped from 
another email. Note: this code has not been tested yet...

spin_lock:
     lui     a2,0x10
     amoadd.w a5,a5,(a0)
     srliw   a4,a5,0x10
2:  slliw   a5,a5,0x10
     srliw   a5,a5,0x10
     bne     a5,a4,3f
     ret
3:  lw      a5,0(a0)
     fence   r,rw
     j       2b

spin_trylock:
     lui     a5,0x10
     lr.w.aq a4,(a0)
     addw    a5,a5,a4
     slliw   a3,a5,0x10
     srliw   a3,a3,0x10
     srliw   a4,a5,0x10
     beq     a3,a4,2f
1:  li      a0,0
     ret
2:  sc.w.rl a4,a5,(a0)
     bnez    a4,1b
     fence   r,rw
     li      a0,1
     ret

spin_unlock:
     fence   rw,w
1:  lr.w.aq a4,(a0)
     srliw   a5,a4,0x10
     addiw   a4,a4,1
     slliw   a4,a4,0x10
     srliw   a4,a4,0x10
     slliw   a5,a5,0x10
     or      a5,a5,a4
     sc.w.rl a4,a5,(a0)
     bnez    a5,1b
     ret

Here is the thread where I happened to be thinking at the time about (a 
bit of a tangent, but as we know, acos and asin can be formed with atan)

- https://patchwork.kernel.org/patch/10805093/

Reading the per-cpu amoadd thread made me consider a ticket spinlock 
that uses amoadd for lock/acquire and LR/SC for trylock and 
unlock/release. The idea is a compromise between fairness, code size and 
lock structure size. In this case we have a 32-bit spinlock and amoadd 
0x0001_0000 is used for head increment/acquire, while release is 
slightly more costly, but it balances out the code size quite nicely, 
compared to the previous 32-bit ticket spinlock code that I have been 
experimenting with (independently from Linux asm-generic qspinlock).

I am interested in fair locks, and would also like to try them in an OoO 
simulator that explores the constraints on the bounds of the RISC-V 
memory model, such as testing the correctness for the extent of what can 
be executed out of order. There is quite a bit of work to get one's head 
around this, because I would like to see possible alternate executions. 
We can't really do this with QEMU nor spike. Work such as this has 
previously been done with black-boxes, however I would like OoO 
simulation to be part of general simulation infrastructure for RISC-V

(although I don't currently have funding for that as there is a 
reasonably significant effort involved; small steps; like actually 
getting different lock variants to test...).

Michael.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv