This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCHv3] PowerPC: Fix a race condition when eliding a lock
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: "Paul E. Murphy" <murphyp at linux dot vnet dot ibm dot com>, libc-alpha at sourceware dot org
- Date: Tue, 1 Sep 2015 18:58:11 -0300
- Subject: Re: [PATCHv3] PowerPC: Fix a race condition when eliding a lock
- Authentication-results: sourceware.org; auth=none
- References: <55D742D3 dot 9050600 at redhat dot com> <1440439895-11812-1-git-send-email-tuliom at linux dot vnet dot ibm dot com> <1441136302 dot 5089 dot 182 dot camel at otta> <55E60E88 dot 50104 at linaro dot org> <55E61799 dot 6010707 at linux dot vnet dot ibm dot com>
On 01-09-2015 18:24, Paul E. Murphy wrote:
>
>
> On 09/01/2015 03:46 PM, Adhemerval Zanella wrote:
>> Indeed the 'odd' comment does not make sense and we should just remove it
>> (I misread texasr definition). My initial idea was define some codes
>> that set the persistent failures and some that do not. I think I best
>> approach would be:
>>
>> /* tabort will set TEXASR(0:31) = ((_ABORT_LOCK_BUSY & 0xff) << 24) | 0x1
>> and the TEXASR persistent bit is bit 25 (32-7). Only the syscall
>> code means a persistent error that should trigger a default lock
>> acquisition. */
>> #define _ABORT_SYSCALL 0x1 /* Syscall issued. */
>> #define _ABORT_LOCK_BUSY 0x2 /* Lock already used. */
>> #define _ABORT_NESTED_TRYLOCK 0x4 /* Write operation in trylock. */
>
> The kernel defines several abort codes, we'll want to work with them, or
> recycle them as needed.
Yeap, but these are the one kernel itself generates (not GLIBC) and I think
it would be useful to use different code from abort generated from
GLIBC itself (so an external tool like perf can detect from where exactly
the abort came from).
And I used the number above just an example, checking the kernel code seems it
defines:
#define TM_CAUSE_PERSISTENT 0x01
#define TM_CAUSE_KVM_RESCHED 0xe0 /* From PAPR */
#define TM_CAUSE_KVM_FAC_UNAV 0xe2 /* From PAPR */
#define TM_CAUSE_RESCHED 0xde
#define TM_CAUSE_TLBI 0xdc
#define TM_CAUSE_FAC_UNAV 0xda
#define TM_CAUSE_SYSCALL 0xd8
#define TM_CAUSE_MISC 0xd6 /* future use */
#define TM_CAUSE_SIGNAL 0xd4
#define TM_CAUSE_ALIGNMENT 0xd2
#define TM_CAUSE_EMULATE 0xd0
So I think we might use 0xaX or something for the GLIBC ones.
>
> I'm not convinced any of the existing codes should be non-persistent:
>
> A pthread_mutex_trylock attempt within an elided pthread_mutex_lock is
> guaranteed to fail try_tbegin times if there is no contention on the lock.
> Aborts get increasingly expensive as you increase the amount of speculative
> execution.
>
> A busy lock likely indicates contention in the critical section which
> does not benefit from elision, I'd err on the side of a persistent
> failure.
>
I do not that much information to decide it, although I do see for pthread_mutex_t
at least the _ABORT_LOCK_BUSY is not persistent if the critical region does not
generate enough contention.