Horton

Re: [HACKERS] Deadlock in XLogInsert at AIX

Noah Misch <noah@leadboat.com>

View thread View on postgr.es
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael.paquier@gmail.com>, Andres Freund <andres@anarazel.de>, Heikki Linnakangas <hlinnaka@iki.fi>, Konstantin Knizhnik <k.knizhnik@postgrespro.ru>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Bernd Helmle <mailings@oopsware.de>
Date: 2019-10-05T17:34:00Z
Lists: pgsql-hackers

Attachments

On Sat, Aug 31, 2019 at 03:30:26PM -0700, Noah Misch wrote:
> On Sat, Aug 31, 2019 at 02:27:55PM -0400, Tom Lane wrote:
> > Noah Misch <noah@leadboat.com> writes:
> > > Done.  fetch-add-variable-test-v1.patch just adds tests for non-constant
> > > addends and 16-bit edge cases.  Today's implementation handles those,
> > > PostgreSQL doesn't use them, and I might easily have broken them.
> > > fetch-add-xlc-asm-v1.patch moves xlc builds from the __fetch_and_add()
> > > intrinsic to inline asm.  fetch-add-gcc-xlc-unify-v1.patch moves fetch_add to
> > > inline asm for all other ppc compilers.  gcc-7.2.0 generates equivalent code
> > > before and after.  I plan to keep the third patch HEAD-only, back-patching the
> > > other two.  I tested with xlc v12 and v13.
> > 
> > Hm, no objection to the first two patches, but I don't understand
> > why the third patch goes to so much effort just to use "addi" rather
> > than (one assumes) "li" then "add"?  It doesn't seem likely that
> > that's buying much.
> 
> Changing an addi to li+add may not show up on benchmarks, but I can't claim
> it's immaterial.  I shouldn't unify the code if that makes the compiled code
> materially worse than what the gcc intrinsics produce today, hence the
> nontrivial (~50 line) bits to match the intrinsics' capabilities.

The first two patches have worked so far, but fetch-add-gcc-xlc-unify-v1.patch
broke older gcc: https://postgr.es/m/flat/7517.1568470247@sss.pgh.pa.us.  That
side thread settled on putting pg_atomic_compare_exchange_u{32,64}_impl
implementations in arch-ppc.h.  Attached fetch-add-gcc-xlc-unify-v2.patch does
so.  For compare_exchange, the generated code doesn't match gcc intrinsics
exactly; code comments discuss this.  Compared to v1, its other change is to
extract common __asm__ templates into macros instead of repeating them four
times (32/64bit, variable/const).

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.