Re: [PATCH v2] Use CC atomic builtins as a fallback

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Martin Pitt <mpitt(at)debian(dot)org>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: [PATCH v2] Use CC atomic builtins as a fallback
Date: 2011-12-21 15:55:16
Message-ID: 5642.1324482916@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Martin Pitt <mpitt(at)debian(dot)org> writes:
> Tom Lane [2011-12-20 21:14 -0500]:
>> Another thing that is bothering me is that according to the gcc
>> manual, eg here,
>> http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html
>> __sync_lock_test_and_set is nominally provided for datatypes 1, 2,
>> 4, or 8 bytes in length, but the underlying hardware doesn't
>> necessarily support all those widths natively. If you pick the
>> wrong width then you don't get an inline operation at all, but a
>> call to some possibly inefficient library subroutine.

> It's even worse. Our original version of the patch used a char, and
> while that worked fine on the older Babbage board, it completely broke
> at runtime on e. g. a Panda board. It wasn't just slow, it caused
> hangs and test failures all over the place. With int it works
> flawlessly.

Yeah, that was another thing I found worrisome while googling: there
were a disturbingly large number of claims that __sync_lock_test_and_set
and/or __sync_lock_release were flat-out broken on various combinations
of gcc version and platform. After reading that, there is no way at all
that I'd accept your original patch to use these functions everywhere.

For the moment I'm inclined to consider using these functions *only* on
ARM, so as to limit our exposure to such bugs. That would also limit
the risk of using an inappropriate choice of lock width.

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2011-12-21 16:00:34 Re: R: R: R: R: R: BUG #6342: libpq blocks forever in "poll" function
Previous Message cmjnov92 2011-12-21 15:43:26 BUG #6349: Cannot install on 32 bit platform