Horton

Re: [HACKERS] Deadlock in XLogInsert at AIX

Michael Paquier <michael.paquier@gmail.com>

View thread View on postgr.es
From: Michael Paquier <michael.paquier@gmail.com>
To: Noah Misch <noah@leadboat.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Konstantin Knizhnik <k.knizhnik@postgrespro.ru>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Bernd Helmle <mailings@oopsware.de>
Date: 2018-01-16T07:12:11Z
Lists: pgsql-hackers 
On Fri, Feb 03, 2017 at 12:26:50AM +0000, Noah Misch wrote:
> On Wed, Feb 01, 2017 at 02:39:25PM +0200, Heikki Linnakangas wrote:
>> @@ -73,11 +73,19 @@ pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
>>  static inline uint32
>>  pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
>>  {
>> +	uint32		ret;
>> +
>>  	/*
>> -	 * __fetch_and_add() emits a leading "sync" and trailing "isync", thereby
>> -	 * providing sequential consistency.  This is undocumented.
>> +	 * Use __sync() before and __isync() after, like in compare-exchange
>> +	 * above.
>>  	 */
>> -	return __fetch_and_add((volatile int *)&ptr->value, add_);
>> +	__sync();
>> +
>> +	ret = __fetch_and_add((volatile int *)&ptr->value, add_);
>> +
>> +	__isync();
>> +
>> +	return ret;
>>  }
> 
> Since this emits double syncs with older xlc, I recommend instead replacing
> the whole thing with inline asm.  As I opined in the last message of the
> thread you linked above, the intrinsics provide little value as abstractions
> if one checks the generated code to deduce how to use them.  Now that the
> generated code is xlc-version-dependent, the port is better off with
> compiler-independent asm like we have for ppc in s_lock.h.

Could it be cleaner to just use __xlc_ver__ to avoid double syncs on
past versions? I think that it would make the code more understandable
than just listing directly the instructions. As there have been other
bug reports from Tony Reix who has been working on AIX with XLC 13.1 and
that this thread got lost in the wild, I have added an entry in the next
CF:
https://commitfest.postgresql.org/17/1484/

As Heikki is not around these days, Noah, could you provide a new
version of the patch? This bug has been around for some time now, it
would be nice to move on.. I think I could have written patches myself,
but I don't have an AIX machine at hand. Of course not with XLC 13.1.
--
Michael

Commits

  1. For all ppc compilers, implement compare_exchange and fetch_add with asm.

  2. For PowerPC instruction "addi", use constraint "b".

  3. For all ppc compilers, implement pg_atomic_fetch_add_ with inline asm.

  4. Replace xlc __fetch_and_add() with inline asm.

  5. Test pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.