Re: [patch 1/3] mutex subsystem: trylock

[Ingo Molnar <mingo@elte.hu>]

* Nicolas Pitre <nico@cam.org> wrote:

> > > > + * 1) if the exclusive store fails we fail, and
> > > > + *
> > > > + * 2) if the decremented value is not zero we don't even attempt the store.
> > > 
> > > 
> > > btw I really think that 1) is wrong. trylock should do everything it 
> > > can to get the semaphore short of sleeping. Just because some 
> > > cacheline got written to (which might even be shared!) in the middle 
> > > of the atomic op is not a good enough reason to fail the trylock imho. 
> > > Going into the slowpath.. fine. But here it's a quality of 
> > > implementation issue; you COULD get the semaphore without sleeping (at 
> > > least probably, you'd have to retry to know for sure) but because 
> > > something wrote to the same cacheline as the lock... no. that's just 
> > > not good enough.. sorry.
> > 
> > point. I solved this in my tree by calling the generic trylock <fn> if 
> > there's an __ex_flag failure in the ARMv6 case. Should be rare (and thus 
> > the call is under unlikely()), and should thus still enable the fast 
> > implementation.
> 
> I'd solve it like this instead (on top of your latest patches):

thanks, applied.

> +		"1: ldrex	%0, [%3]	\n"
> +		"subs		%1, %0, #1	\n"
> +		"strexeq	%2, %1, [%3]	\n"
> +		"movlt		%0, #0		\n"
> +		"cmpeq		%2, #0		\n"
> +		"bgt		1b		\n"

so we are back to what is in essence a cmpxchg implementation?

	Ingo