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

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

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.

> 
> [1] https://sourceware.org/systemtap/wiki/glibcMarkers




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]