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 -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
