This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 0/2] Port of lock elision to System/z v2
- From: Andreas Jaeger <aj at suse dot com>
- To: libc-alpha at sourceware dot org
- Date: Mon, 23 Sep 2013 09:44:57 +0200
- Subject: Re: [PATCH 0/2] Port of lock elision to System/z v2
- Authentication-results: sourceware.org; auth=none
- References: <20130920105925 dot GA374 at linux dot vnet dot ibm dot com> <523C2BC1 dot 2050707 at suse dot com> <20130920111704 dot GA3400 at linux dot vnet dot ibm dot com> <523C3693 dot 3000902 at suse dot com> <20130923073945 dot GA4231 at linux dot vnet dot ibm dot com>
On 09/23/2013 09:39 AM, Dominik Vogt wrote:
> On Fri, Sep 20, 2013 at 01:50:43PM +0200, Andreas Jaeger wrote:
>> I see:
>>
>> 2013-07-02 Andi Kleen <ak@linux.intel.com>
>> Hongjiu Lu <hongjiu.lu@intel.com>
>>
>> * pthread_mutex_lock.c
>> (__pthread_mutex_lock): Add lock elision support.
>> * pthread_mutex_timedlock.c (pthread_mutex_timedlock): Likewise.
>>
>>
>> So, it names not only the file but also the function changed - nothing
>> like this is seen in your change.
>
> Well, did you _look_ at my patch? Why do you claim that _my_
> patch changes functions in these files like the Intel patch did,
> when it doesn't?
I claimed that based on what I read in the ChangeLog.
>> Also, the first entry contains a text, the rest "Likewise."
>
> Which is wrong for the s390 patch. Elision support is not added
> _individually_ in each file, but the complete set of _new_ files
> adds elision support. Do I understand that correctly that I am
> required to add additional comments that are "syntactically"
> correct, even if they are semantically wrong?
They are new files? Then state it! I'm not asking for wrong entries, I'm
asking for following our conventions. And your attitude does not help here.
> --
> 2013-09-23 Dominik Vogt <vogt@linux.vnet.ibm.com>
>
> * sysdeps/unix/sysv/linux/s390/elision-conf.c:
> Add lock elision support for s390.
Just write:
New file. Adds lock elision support for s390.
And I'm happy ;)
> * sysdeps/unix/sysv/linux/s390/elision-conf.h: Likewise.
> * sysdeps/unix/sysv/linux/s390/elision-lock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/elision-timed.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/elision-trylock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/elision-unlock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/force-elision.h: Likewise.
> * sysdeps/unix/sysv/linux/s390/pthread_mutex_cond_lock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/pthread_mutex_lock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/pthread_mutex_timedlock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/pthread_mutex_trylock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/pthread_mutex_unlock.c: Likewise.
> * sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h (pthread_mutex_t):
> Likewise.
> * sysdeps/unix/sysv/linux/s390/Makefile: Likewise.
This is not a new file, isn't it? So, name the entries you add.
> * sysdeps/unix/sysv/linux/s390/htm.h
> (__builtin_tbegin, __builtin_tend, __builtin_tabort)
> (__builtin_non_tx_store, __builtin_tx_nesting_depth)
> (__builtin_tx_nesting_depth, TBEGIN, TEND, TABORT, NTSTG, ETND):
> Add.
> * sysdeps/unix/sysv/linux/s390/lowlevellock.h:
> (__lll_timedlock_elision, __lll_lock_elision, __lll_unlock_elision)
> (__lll_trylock_elision, lll_timedlock_elision, lll_lock_elision)
> (lll_unlock_elision, lll_trylock_elision):
> Add.
> --
Andreas
--
Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126