On Tue, 2013-11-05 at 12:22 +0100, Peter Zijlstra wrote:
> On Tue, Nov 05, 2013 at 10:15:38AM +0000, Will Deacon wrote:
> > Hello,
> >
> > On Mon, Nov 04, 2013 at 11:37:13PM +0000, Tim Chen wrote:
> > > The following changes are made to enable mcs_spinlock.h file to be
> > > widely included in other files without causing problem:
> > >
> > > 1) Include a number of prerequisite header files and define
> > > arch_mutex_cpu_relax(), if not previously defined.
> > > 2) Separate out mcs_spin_lock() into a mcs_spinlock.c file.
> > > 3) Make mcs_spin_unlock() an inlined function.
> >
> > [...]
> >
> > > +void mcs_spin_lock(struct mcs_spinlock **lock, struct mcs_spinlock *node)
> > > +{
> > > + struct mcs_spinlock *prev;
> > > +
> > > + /* Init node */
> > > + node->locked = 0;
> > > + node->next = NULL;
> > > +
> > > + prev = xchg(lock, node);
> > > + if (likely(prev == NULL)) {
> > > + /* Lock acquired */
> > > + node->locked = 1;
> > > + return;
> > > + }
> > > + ACCESS_ONCE(prev->next) = node;
> > > + smp_wmb();
> > > + /* Wait until the lock holder passes the lock down */
> > > + while (!ACCESS_ONCE(node->locked))
> > > + arch_mutex_cpu_relax();
> > > +}
> >
> > You have the barrier in a different place than the version in the header
> > file; is this intentional?
> >
> > Also, why is an smp_wmb() sufficient (as opposed to a full smp_mb()?). Are
> > there restrictions on the types of access that can occur in the critical
> > section?
>
> Oh, good spot. I missed it because it doesn't actually remove the one in
> the header, why is that?
>
Good catch. I merged an older version of Waiman's patch. I'll correct
this and update with the patch series with the newer one that should
only call the mcs_spin_lock in the header file.
Tim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>