lkml.org 
[lkml]   [2013]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 4/4] MCS Lock: Barrier corrections
On 11/11/2013 04:17 PM, Tim Chen wrote:
>> You could then augment that with [cmp]xchg_{acquire,release} as
>> appropriate.
>>
>>> +/*
>>> * In order to acquire the lock, the caller should declare a local node and
>>> * pass a reference of the node to this function in addition to the lock.
>>> * If the lock has already been acquired, then this will proceed to spin
>>> @@ -37,15 +62,19 @@ void mcs_spin_lock(struct mcs_spinlock **lock, struct mcs_spinlock *node)
>>> node->locked = 0;
>>> node->next = NULL;
>>>
>>> - prev = xchg(lock, node);
>>> + /* xchg() provides a memory barrier */
>>> + prev = xchg_acquire(lock, node);
>>> if (likely(prev == NULL)) {
>>> /* Lock acquired */
>>> return;
>>> }
>>> ACCESS_ONCE(prev->next) = node;
>>> - smp_wmb();
>>> - /* Wait until the lock holder passes the lock down */
>>> - while (!ACCESS_ONCE(node->locked))
>>> + /*
>>> + * Wait until the lock holder passes the lock down.
>>> + * Using smp_load_acquire() provides a memory barrier that
>>> + * ensures subsequent operations happen after the lock is acquired.
>>> + */
>>> + while (!(smp_load_acquire(&node->locked)))
>>> arch_mutex_cpu_relax();
> An alternate implementation is
> while (!ACCESS_ONCE(node->locked))
> arch_mutex_cpu_relax();
> smp_load_acquire(&node->locked);
>
> Leaving the smp_load_acquire at the end to provide appropriate barrier.
> Will that be acceptable?
>
> Tim

I second Tim's opinion. It will be help to have a smp_mb_load_acquire()
function that provide a memory barrier with load-acquire semantic. I
don't think we need one for store-release as that will not be in a loop.

Peter, what do you think about adding that to your patch?

-Longman


\
 
 \ /
  Last update: 2013-11-12 18:41    [W:0.080 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site