Re: [COMMITTERS] pgsql: Do all accesses to shared buffer

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: "Jim C(dot) Nasby" <jnasby(at)pervasive(dot)com>, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Do all accesses to shared buffer
Date: 2005-10-21 22:09:00
Message-ID: 26241.1129932540@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Neil Conway <neilc(at)samurai(dot)com> writes:
> LWLocks certainly do protect shared data, and if the compiler rearranged
> loads and stores around LWLocks acquire/release, it would result in
> corruption. Tom was arguing it is unlikely the compiler will actually do
> this (because LWLockAcquire is an out-of-line function call that might
> invoke a system call, unlike SpinLockAcquire).

Actually it seems the sore spot is more likely to be SpinLockRelease,
which on many architectures expands to nothing more than an inlined

*(lockptr) = 0;

The lock pointer is declared as pointer to volatile, which should
discourage the compiler from removing the store altogether, but as
we've seen the compiler may be willing to rearrange the store with
respect to adjacent loads/stores that aren't marked volatile.

SpinLockAcquire contains an out-of-line function call, so although
the compiler could theoretically misoptimize things in the fast path,
it's probably much less risky than SpinLockRelease.

BTW, we may be perfectly safe on architectures like PPC, where
S_UNLOCK includes an __asm__ __volatile__ section for a hardware-level
optimization fence instruction. I wonder though if it'd be a good idea
to be marking those fence instructions with the "clobbers memory"
qualifier to ensure this?

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Martijn van Oosterhout 2005-10-22 09:50:32 Re: [COMMITTERS] pgsql: Do all accesses to shared buffer
Previous Message James William Pye 2005-10-21 21:55:15 python - be: Implement the new type system.

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2005-10-21 22:21:06 Re: [PATCHES] Win32 CHECK_FOR_INTERRUPTS() performance
Previous Message Tom Lane 2005-10-21 21:46:47 Re: [PATCHES] Win32 CHECK_FOR_INTERRUPTS() performance tweak