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

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

On 28/04/2022 13:56, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>>>>  	(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 ?
> 
> No, that optimization follows our documented guidance, namely:
> 
> | Most applications should perform the same actions whether or not
> | @code{__libc_single_threaded} is true, except with less
> | synchronization.  If this rule is followed, a process that
> | subsequently becomes multi-threaded is already in a consistent state.
> 

So I wonder if

/* Lock the recursive named lock variable.  */
#define __libc_lock_lock_recursive(NAME) \
  do {                                                                        \
    void *self = THREAD_SELF;                                                 \
    if ((NAME).owner != self)                                                 \
      {									      \
	if (SINGLE_THREAD_P)                                                  \
	  (NAME).lock = 1;						      \
        else								      \
           lll_lock ((NAME).lock, LLL_PRIVATE);                               \
        (NAME).owner = self;                                                  \
      }                                                                       \
    ++(NAME).cnt;                                                             \
  } while (0)

/* Unlock the recursive named lock variable.  */
/* We do no error checking here.  */
#define __libc_lock_unlock_recursive(NAME) \
  do {                                                                        \
    if (--(NAME).cnt == 0)                                                    \
      {                                                                       \
        (NAME).owner = NULL;                                                  \
        if (SINGLE_THREAD_P)						      \
          (NAME).lock = 0;  						      \
        else                                                                  \
          lll_unlock ((NAME).lock, LLL_PRIVATE);                              \
      }                                                                       \
  } while (0)

Or if we are bounded to keep the current practice to check for single-thread and
skip locks internally.  It would be good to consolidate all the internal lock
usage and have the single-thread lock optimizations on all locks, not only on
pthread_mutex_lock.