Re: better atomics - v0.6

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Oskari Saarenmaa <os(at)ohmu(dot)fi>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)heroku(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Ants Aasma <ants(at)cybertec(dot)at>
Subject: Re: better atomics - v0.6
Date: 2014-09-23 22:11:48
Message-ID: 20140923221148.GH2521@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-09-24 00:27:25 +0300, Oskari Saarenmaa wrote:
> 23.09.2014, 15:18, Andres Freund kirjoitti:
> >On 2014-09-23 13:50:28 +0300, Oskari Saarenmaa wrote:
> >>23.09.2014, 00:01, Andres Freund kirjoitti:
> >>>The patches:
> >>>0001: The actual atomics API
> >>
> >>I tried building PG on Solaris 10/Sparc using GCC 4.9.0 (buildfarm animal
> >>dingo) with this patch but regression tests failed due to:
> >
> >Btw, if you could try sun studio it'd be great. I wrote the support for
> >it blindly, and I'd be surprised if I got it right on the first try.
>
> I just installed Solaris Studio 12.3 and tried compiling this:

Cool.

> "../../../../src/include/port/atomics/generic-sunpro.h", line 54: return
> value type mismatch
> "../../../../src/include/port/atomics/generic-sunpro.h", line 77: return
> value type mismatch
> "../../../../src/include/port/atomics/generic-sunpro.h", line 79: #if-less
> #endif
> "../../../../src/include/port/atomics/generic-sunpro.h", line 81: #if-less
> #endif
>
> atomic_add_64 and atomic_add_32 don't return anything (the atomic_add_*_nv
> variants return the new value) and there were a few extra #endifs.
> Regression tests pass after applying the attached patch which defines
> PG_HAS_ATOMIC_ADD_FETCH_U32.

Thanks for the fixes!

Hm. I think then it's better to simply not implement addition and rely
on cmpxchg.

> Also, it's not possible to compile PG with FORCE_ATOMICS_BASED_SPINLOCKS
> with these patches:
>
> "../../../../src/include/storage/s_lock.h", line 868: #error: PostgreSQL
> does not have native spinlock support on this platform....
>
> atomics/generic.h would implement atomic flags using operations exposed by
> atomics/generic-sunpro.h, but atomics/fallback.h is included before it and
> it defines functions for flag operations which s_lock.h doesn't want to use.

Right. It should check not just for flag support, but also for 32bit
atomics. Easily fixable.

I'll send a new version with your fixes incorporated tomorrow.

Thanks for looking into this! Very helpful.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2014-09-23 22:54:16 Re: Scaling shared buffer eviction
Previous Message Gregory Smith 2014-09-23 22:02:57 Re: Scaling shared buffer eviction