This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][BZ 18743] PowerPC: Fix a race condition when eliding a lock

Torvald Riegel <> writes:

> On Thu, 2015-07-30 at 23:06 -0300, Tulio Magno Quites Machado Filho
> wrote:
>> Adhemerval Zanella <> writes:
>> > So if the transaction fails it will just jump to else block.
>> >
>> >> +    else								\
>> >> +      for (int i = __elision_aconf.try_tbegin; i > 0; i--)		\
>> >> +	{								\
>> >> +	  asm volatile("" ::: "memory");				\
>> >
>> > I can't really understand the requirement of this compiler barrier here.  If
>> > compiler is moving around the 'is_lock_free' *before* __builtin_tbegin IMHO
>> > this is a compiler bug.
>> This is the second problem and I agree with you again.  IMHO,  __builtin_tbegin
>> starts a critical path and the compiler should not be moving a memory access out
>> of the critical path or into the critical path.
>> However, as the current implementations of GCC have this "issue" (some people
>> may not agree with us), I believe we could carry this compiler barrier at least
>> until all GCC versions supporting __builtin_tbegin are fixed.
> I agree that a compiler should treat transaction begin / commit as
> something like a lock acquisition / release, assuming that the
> transaction semantics are equivalence to a single-global-lock semantics.

This has been reported to GCC as

> As far as glibc is concerned, I believe it's better to work around the
> compiler issue by creating a glibc-internal wrapper for __builtin_tbegin
> that enforces the semantics we believe it should have (eg, by adding the
> compiler barrier).

Agreed.  In fact, that's the right way of solving this issue.
The way I wrote my patch left some room for an instruction to be placed
between the compiler barrier and the begin of the transaction.

Anyway, I believe that's another problem and should not be confused with the
previous one.  So, I'll split this patch in 2: one that fixes BZ #18743 and
another one that fixes the compiler barriers on powerpc.  I'll synchronize
with GCC so that we use the same semantics (or as close as we can).

Tulio Magno

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]