Skip site navigation (1) Skip section navigation (2)

Re: [PATCH] Use CC atomic builtins if available [was: Re: TAS patch for building on armel/armhf thumb]

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: [PATCH] Use CC atomic builtins if available [was: Re: TAS patch for building on armel/armhf thumb]
Date: 2011-12-19 20:21:22
Message-ID: 4EEF9CC2.9080909@enterprisedb.com (view raw or flat)
Thread:
Lists: pgsql-bugs
On 19.12.2011 22:03, Noah Misch wrote:
> On Mon, Dec 19, 2011 at 05:09:11PM +0200, Heikki Linnakangas wrote:
>> Actually, I believe our Itanium (and possibly ARM, too) implementation
>> of S_UNLOCK() is wrong as it is. There is no platform-specific
>> S_UNLOCK() defined for Itanium, so we're using the generic
>> implementation:
>>
>> #if !defined(S_UNLOCK)
>> #define S_UNLOCK(lock)		(*((volatile slock_t *) (lock)) = 0)
>> #endif	 /* S_UNLOCK */
>>
>> That is not sufficient on platforms with a weak memory model, like Itanium.
>
> Other processors may observe the lock as held after its release, but there's no
> correctness problem.

I thought it would also be legal for a store to become visible to other 
processors, *after* the releasing of the lock. Which would be bad. For 
example, if you have:

volatile bool *shared = ...
SpinLockAcquire(lock);
shared->variable = true;
SpinLockRelease(lock);
more code

The macro-expanded code would look like:

<test and set> lock
shared->variable = true;
(*((volatile slock_t *) (lock)) = 0;
more code

I believe on an architecture with a weak memory model, like Itanium, 
there's no guarantee that the assignments will happen in that order. The 
lock might appear as released *before* the variable is set.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

In response to

pgsql-bugs by date

Next:From: Heikki LinnakangasDate: 2011-12-19 21:25:06
Subject: Re: Re: [PATCH] Use CC atomic builtins if available [was: Re: TAS patch for building on armel/armhf thumb]
Previous:From: Tom LaneDate: 2011-12-19 20:12:58
Subject: Re: Re: [PATCH] Use CC atomic builtins if available [was: Re: TAS patch for building on armel/armhf thumb]

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group