Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

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

On 02/10/2020 09:15, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 30/09/2020 10:12, Lukasz Majewski wrote:
>>>>> diff --git a/nptl/pthread_rwlock_rdlock.c
>>>>> b/nptl/pthread_rwlock_rdlock.c index 04cecad395..2b8509a49c 100644
>>>>> --- a/nptl/pthread_rwlock_rdlock.c
>>>>> +++ b/nptl/pthread_rwlock_rdlock.c
>>>>> @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t
>>>>> *rwlock) {
>>>>>    LIBC_PROBE (rdlock_entry, 1, rwlock);    
>>>>
>>>> We need to move the systemtap probe to the 64-bit variant, so the
>>>> 64-bit time will still trigger it.  
>>>
>>> Those are syscalls, which don't require timespec time.  
>>
>> These are in fact systemtap markers [1] and they are not related to 
>> syscalls.
> 
> Ok. Thanks for the info (and link).
> 
>>
>>>
>>> The LIBC_PROBE() macros were there before, so I left them as is and
>>> only changed __pthread_rwlock_rdlock_full() to
>>> __pthread_rwlock_rdlock_full64().
>>>
>>>
>>> On the other hand - for example:
>>> __pthread_rwlock_clockwrlock() and __pthread_rwlock_clockwrlock64()
>>> are only calling __pthread_rwlock_wrlock_full64(), and there were no
>>> LIBC_PROBE() macros before.
>>>
>>> Do I understand correctly that you propose to add LIBC_PROBE()
>>> macros to e.g. __pthread_rwlock_clockwrlock64() ?  
>>
>> The idea of the LIBC_PROBE is to trigger an systemtap even when the
>> function is called so systemtap can act accordingly (and the markers
>> are enable only if --enable-systemtap is used on configure).
>>
> 
> Ok.
> 
>> Without moving the LIBC_PROBE, 64-bit time architectures won't see
>> the probes anymore since they will call the 64-bit time version
>> instead.
> 
> Please correct me if I'm wrong, but it seems to me that you are
> concerned if 
> 
> - __pthread_rwlock_rdlock_full64 
> - __pthread_rwlock_wrlock_full64
> 
> would be called as aliased (for e.g. x86-64) and redirected (for e.g.
> arm)?
> 
> This is not the case as above functions are local (i.e. helpers) ones
> defined in ./nptl/pthread_rwlock_common.c (and are not exported).
> 
> For them, the only change was to add '64' suffix.
> 
> The LIBC_PROBE() calls have been left untouched, surrounding them, so I
> think that we don't need to move them.
> 
> 
> Or maybe I do miss something important here? Thanks in advance for your
> help and explanation.

In fact this is a wrong assumption from my part, there is no LIBC_PROBE
on the timed variant rwlocks. The patch looks ok in this regard (sorry
for the noise).