Re: [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 27/04/2022 13:32, Adhemerval Zanella wrote:
> 
> 
> On 27/04/2022 10:30, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>>
>>> diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h
>>> index 6c2d6acfd1..abd84e71b4 100644
>>> --- a/sysdeps/nptl/libc-lock.h
>>> +++ b/sysdeps/nptl/libc-lock.h
>>> @@ -86,7 +86,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t;
>>>  # define __libc_lock_lock_recursive(NAME) \
>>>    do {									      \
>>>      void *self = THREAD_SELF;						      \
>>> -    if ((NAME).owner != self)						      \
>>> +    if (!SINGLE_THREAD_P && (NAME).owner != self)						      \
>>>        {									      \
>>>  	lll_lock ((NAME).lock, LLL_PRIVATE);				      \
>>>  	(NAME).owner = self;						      \
>>> @@ -104,7 +104,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t;
>>>    ({									      \
>>>      int result = 0;							      \
>>>      void *self = THREAD_SELF;						      \
>>> -    if ((NAME).owner != self)						      \
>>> +    if (!SINGLE_THREAD_P && (NAME).owner != self)						      \
>>>        {									      \
>>>  	if (lll_trylock ((NAME).lock) == 0)				      \
>>>  	  {								      \
>>> @@ -131,7 +131,8 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t;
>>>      if (--(NAME).cnt == 0)						      \
>>>        {									      \
>>>  	(NAME).owner = NULL;						      \
>>> -	lll_unlock ((NAME).lock, LLL_PRIVATE);				      \
>>> +        if (!SINGLE_THREAD_P)                                                 \
>>> +	  lll_unlock ((NAME).lock, LLL_PRIVATE);			      \
>>>        }									      \
>>>    } while (0)
>>>  #else
>>
>> I don't think this is correct if threads are created in the lock region.
> 
> I was not sure about this one and I think we the main issue in fact there is
> we can't use the single-thread optimization on unlock.  Maybe a better option
> would to use a different scheme as proposed by 
> https://hal.inria.fr/hal-01236734/document, where we can embedded lock and
> cnt in only one variable (as the lll_lock already does).

Don't 99f841c441feeaa9a3d97fd91bb3d6ec8073c982 have the issue for pthread_mutex_lock ?