On 10/29/23 20:41, wuqiang.matt wrote:
arch_cmpxchg_relaxed:
...
switch(sizeof((_p_))) {
case 4:
....
arch_cmpxchg:
...
BUILD_BUG_ON(sizeof(_p_) != 4);
...
_p is the address pointer, so I'm thinking it's a typo but I couldn't
yet confirm. There is not much about arc processors in the web :(
Hmm, indeed. This seems like a bug but it depends on the 'llock %0,
[%1]'
can take a 32bit address or 32bit data register. Usually it should
check the size of data, but need to check with ISA manual.
Vineet, can you check this suspicious bug?
ARCv2 is a 32-bit ISA and LLOCK/SCOND work on 32-bit data.
So the pointers will be 32-bit anyways. Is the issue that
pointer/cmpxchg operation could be on a smaller data type ?
For ARCv2 with CONFIG_ARC_HAS_LLSC, better add the data size checking and
only permit 32bit data size. Even for 32-bit system, data should can be
64bit 'long long'.
Right, makes sense. Can you send a patch ?
And In the case that CONFIG_ARC_HAS_LLSC is undefined, in
arch_cmpxchg: the
pointer size checking is unnecessary, since it's using spinlock
internally:
Correct, I had the same thought.
https://elixir.bootlin.com/linux/v6.6-rc7/source/arch/arc/include/asm/cmpxchg.h#L60:
BUILD_BUG_ON(sizeof(_p_) != 4); \
\
/* \
* spin lock/unlock provide the needed smp_mb() before/after \
*/ \
atomic_ops_lock(__flags); \
_prev_ = *_p_; \
if (_prev_ == _o_) \
*_p_ = _n_; \
atomic_ops_unlock(__flags);
Can you do that fix in same patch as well ?
Another question about the naming: arch_cmpxchg_relaxed() implemented if
CONFIG_ARC_HAS_LLSC is configured and arch_cmpxchg() defined for the
rest.
Are there any reasons for difference names ?
Yes the atomics API consists of _relaxed, _acquire, _release and unadorned.
_relaxed is the most efficient, with rest having some extra barriers.
If arch provides _relaxed, generic code can create the rest - assuming
arch hardware can support the relaxed ones.
That is true for LLSC.
However for !LLSC, spinlock versions already have full barriers due to
spinlock, so relaxed variant doesn't make sense.
As I checked, Synopsys has released 64bit ARC processors (HS66/HS68), but
I don't know the status of Linux kernel support.
Yes there's an internal ARC64 port running but it is not ready for
upstreaming yet.
-Vineet
