Re: [PATCH] a new lock algorithm with lock value and CS counts in the same atomic int

[Rich Felker <dalias@libc.org>]

On Sun, Jun 18, 2017 at 02:49:54PM +0300, Alexander Monakov wrote:
> On Sun, 18 Jun 2017, Jens Gustedt wrote:
> > > This looks wrong in single-threaded case, __lock doesn't touch the
> > > lock, but __unlock modifies it unconditionally.
> > 
> > Right, probably there should be the same test as for the lock case.
> 
> Checking libc.threads_minus_1 on the unlock path won't work: as threads
> exit, it may become zero even though it weren't when acquiring the lock.
> 
> > Or we should drop that test all along. I don't think that it still serves
> > much purpose here. This is just trading one memory access against another.
> 
> It trades a simple read access against an atomic modification, though.

Indeed. This will be the difference between 1 cycle and 25-100 cycles
on many archs, and much worse on old mips where ll/sc work by
trap-and-emulate.

> I think the fastpath in __unlock can check the value of the lock against 0,
> exiting immediately if equal.

Do you mean that would indicate that __lock was a nop because
libc.threads_minus_1 was 0 at lock time? That's how it works in the
existing implementation so unless I missed some difference in the
logic I think it's safe.

Note that this assumes lock/unlock pairs always take place within a
single non-AS-safe libc function call, or if in an AS-safe function,
in a critical section where all signals are blocked. Otherwise it's
possible for the process to become MT between lock and unlock time.
Note that it's always possible to go the other direction (MT to
non-MT) while the lock is held, which is why you have to test
something that reflects the status at lock time, not at unlock time.

Rich