Re: [PATCH v6 4/5] MCS Lock: Barrier corrections

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 28, 2013 at 07:50:40PM +0000, Paul E. McKenney wrote:
> On Thu, Nov 28, 2013 at 06:53:41PM +0000, Will Deacon wrote:
> > Ok, so that then means that:
> > 
> > 	mb__before_spinlock();
> > 	spin_lock();
> > 
> > on ARM64 expands to:
> > 
> > 	dmb	ish
> > 	ldaxr	...
> > 
> > so there's a redundant half-barrier there. If we want to get rid of that, we
> > need mb__before_spinlock() to set a flag, then we could conditionalise
> > ldaxr/ldxr but it's really horrible and you have to deal with interrupts
> > etc. so in reality we just end up having extra barriers.
> 
> Given that there was just a dmb, how much does the ish &c really hurt?
> Would the performance difference be measurable at the system level?

There's no definitive answer, as it depends heavily on a combination of the
microarchitecture and specific platform implementation. To get some sort of
idea, I tried adding a dmb to the start of spin_unlock on ARMv7 and I saw a
3% performance hit in hackbench on my dual-cluster board.

Whether or not that's a big deal, I'm not sure, especially given that this
should be rare.

> > Or we have separate a spin_lock_mb() function.
> 
> And mutex_lock_mb().  And spin_lock_irqsave_mb().  And spin_lock_irq_mb().
> And...

Ok, point taken.

> Admittedly this is not yet a problem given the current very low usage
> of smp_mb__before_spinlock(), but the potential for API explosion is
> non-trivial.
> 
> That said, if the effect on ARM64 is measurable at the system level, I
> won't stand in the way of the additional APIs.
> 
> > > o	mb_after_spinlock():
> > > 
> > > 	o	Must appear immediatly after a lock acquisition.
> > > 	o	Upgrades an unlock+lock pair to a full barrier.
> > > 	o	Emits a no-op on ARM64, as in "do { } while (0)".
> > > 	o	Might need a separate flavor for queued locks on
> > > 		some platforms, but no sign of that yet.
> > 
> > Ok, so mb__after_spinlock() doesn't imply a full barrier but
> > mb__before_spinlock() does? I think people will get that wrong :)
> 
> As I said earlier in the thread, I am open to better names.
> 
> How about smp_mb__after_spin_unlock_lock_pair()?  That said, I am sure that
> I could come up with something longer given enough time.  ;-)

Ha! Well, I think the principles are sound, but the naming is key to making
sure that this interface is used correctly.

Will

--
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>




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]