Messages in this thread | | | Date | Wed, 12 Jul 2017 14:49:14 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 10/17] RISC-V: Atomic and Locking Code |
| |
On Wed, Jul 12, 2017 at 08:40:49PM +0800, Boqun Feng wrote: > > +/** > > + * set_bit - Atomically set a bit in memory > > + * @nr: the bit to set > > + * @addr: the address to start counting from > > + * > > + * This function is atomic and may not be reordered. See __set_bit() > > This is incorrect, {set,change,clear}_bit() can be reordered, see > Documentation/memory-barriers.txt, they are just relaxed atomics. But I > think you just copy this from x86 code, so maybe x86 code needs help > too, at least claim that's only x86-specific guarantee.
Yeah, I suspect that's an x86 special (all our atomics are fully ordered).
> > +/** > > + * test_and_set_bit_lock - Set a bit and return its old value, for lock > > + * @nr: Bit to set > > + * @addr: Address to count from > > + * > > + * This operation is atomic and provides acquire barrier semantics. > > + * It can be used to implement bit locks. > > + */ > > +static inline int test_and_set_bit_lock( > > + unsigned long nr, volatile unsigned long *addr) > > +{ > > + return test_and_set_bit(nr, addr); > > If you want, you can open code an "amoor.aq" here, because > test_and_set_bit_lock() only needs an acquire barrier. > > > +} > > + > > +/** > > + * clear_bit_unlock - Clear a bit in memory, for unlock > > + * @nr: the bit to set > > + * @addr: the address to start counting from > > + * > > + * This operation is atomic and provides release barrier semantics. > > + */ > > +static inline void clear_bit_unlock( > > + unsigned long nr, volatile unsigned long *addr) > > +{ > > You need a smp_mb__before_atomic() here, because clear_bit() is only > relaxed atomic. And clear_bit_unlock() is a release.
alternatively you can do "amoand.rl".
| |