lkml.org 
[lkml]   [2015]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] [PATCH] sched: Add smp_rmb() in task rq locking cycles
On 04/28/2015 02:24 PM, Peter Zijlstra wrote:
> A few questions:
> On Tue, Apr 28, 2015 at 02:00:48PM -0400, Chris Metcalf wrote:
>
>>> static inline void arch_spin_lock(arch_spinlock_t *lock)
>>> {
>>> unsigned short head, tail;
>>>
>>> ___tns_lock(&lock->lock); /* XXX does the TNS imply a ___sync? */
> Does it? Something needs to provide the ACQUIRE semantics.

Yes, __insn_xxx() is a compiler barrier on tile.

Tile architectures do not need any hardware-level "acquire" semantics
since normally control dependency is sufficient for lock acquisition.
Loads and stores are issued in-order into the mesh network, but issued
loads don't block further instruction issue until a register read dependency
requires it. There is no speculative execution.


>>> head = lock->head;
>>> lock->head++;
>>> ___tns_unlock(&lock->lock);
>>>
>>> while (READ_ONCE(lock->tail) != head)
>>> cpu_relax();
>>> }
>>>
>>> static inline void arch_spin_unlock(arch_spinlock_t *lock)
>>> {
>>> /*
>>> * can do with regular load/store because the lock owner
>>> * is the only one going to do stores to the tail
>>> */
>>> unsigned short tail = READ_ONCE(lock->tail);
>>> smp_mb(); /* MB is stronger than RELEASE */
> Note that your code uses wmb(), wmb is strictly speaking not correct,
> as its weaker than RELEASE.
>
> _However_ it doesn't make any practical difference since all three
> barriers end up emitting __sync() so its not a bug per se.

Yes, this code dates back to 2.6.18 or so; today I would use
smp_store_release(). I like the trend in the kernel to move more
towards the C11 memory order model; I think it will help a lot.

--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com



\
 
 \ /
  Last update: 2015-04-28 21:21    [W:0.087 / U:0.656 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site