On Thu, May 23, 2019 at 10:30:13AM +0200, Andrea Parri wrote:
Hi Mark,
Hi Andrea,
On Wed, May 22, 2019 at 02:22:32PM +0100, Mark Rutland wrote:
Currently architectures return inconsistent types for atomic64 ops. Some return long (e..g. powerpc), some return long long (e.g. arc), and some return s64 (e.g. x86).
(only partially related, but probably worth asking:)
While reading the series, I realized that the following expression:
atomic64_t v; ... typeof(v.counter) my_val = atomic64_set(&v, VAL);
is a valid expression on some architectures (in part., on architectures which #define atomic64_set() to WRITE_ONCE()) but is invalid on others. (This is due to the fact that WRITE_ONCE() can be used as an rvalue in the above assignment; TBH, I ignore the reasons for having such rvalue?)
IIUC, similar considerations hold for atomic_set().
The question is whether this is a known/"expected" inconsistency in the implementation of atomic64_set() or if this would also need to be fixed /addressed (say in a different patchset)?
In either case, I don't think the intent is that they should be used that way, and from a quick scan, I can only fine a single relevant instance today:
[mark@lakrids:~/src/linux]% git grep '(return|=)\s+atomic(64)?_set' include/linux/vmw_vmci_defs.h: return atomic_set((atomic_t *)var, (u32)new_val); include/linux/vmw_vmci_defs.h: return atomic64_set(var, new_val);
[mark@lakrids:~/src/linux]% git grep '=\s+atomic_set' | wc -l 0 [mark@lakrids:~/src/linux]% git grep '=\s+atomic64_set' | wc -l 0
Any architectures implementing arch_atomic_* will have both of these functions returning void. Currently that's x86 and arm64, but (time permitting) I intend to migrate other architectures, so I guess we'll have to fix the above up as required.
I think it's best to avoid the construct above.
Thanks, Mark.