On 2014-06-23 12:46:11 -0400, Robert Haas wrote:
> On Mon, Jun 23, 2014 at 12:29 PM, Andres Freund <and...@2ndquadrant.com> 
> wrote:
> >> > That we have support for platforms that we haven't even documented as
> >> > working (e.g. SuperH) or platforms that don't work in the realword
> >> > (m32r) means that that one has to think about and research so many more
> >> > edge cases that it's really hard to make progress. I don't know how
> >> > often I've now sequentially gone through s_lock.h, s_lock.c,
> >> > src/backend/port/tas to understand all the supported combinations.
> >>
> >> I agree that s_lock.h is the most painful part of this whole thing,
> >> mostly because I'd really like to get to the point where
> >> SpinLockAcquire() and SpinLockRelease() function as compiler barriers.
> >
> > s_lock.h is basically gone in my patch btw. Only old comments + defines
> > for TAS/TAS_SPIN/S_UNLOCK etc mapping the old calls onto atomic flags
> > remain.
> > The new code should, hopefully, all be barrier safe.
> 
> Urp.  I'm not sure that's at all a good idea.

I'm not sure either. But it got harder and harder to do it without that
because of currently interweaved dependencies. Barriers depend on
spinlocks, atomics depend on barriers, atomics depend on spinlocks. And
for a useful generic spinlock implementation spinlocks depend on
atomics.

And then there's the problem that the spinlock based atomics emulation
needs to influence the semaphore spinlock implementation because you
otherwise can get deadlocks (imagine a atomic var being manipulated
while a spinlock is held. If they map to the same semaphore...).

I guess it's possible to remove all traces of s_lock.h from barrier.h;
add a atomics using fallback to s_lock.h, forceable using a define; make
the the public part of the spinlock based fallback not require the
spinlock implementation somehow.
But even if, I think we should get rid of s_lock.h in a later commit for
9.5.

> I don't love s_lock.h,
> but if we make a wholesale change, we risk breaking things that work
> today in obscure ways that are hard to find and debug.  I thought we
> were talking about adding new facilities with their own fallbacks, not
> massively reengineering the facilities we've already got.

I think we can separate it if we want, but I doubt it'll reall make
stuff simpler.

> > I'll omit !gcc PA-RISC unless somebody complains. I don't think I'll be
> > able to modify hpux_hppa.s + build infrastructure correctly and there's
> > no buildfarm. That means it'll require --disable-spinlocks.
> 
> I dunno whether we should care in that particular case, but in general
> I don't think it's OK for lack of support for the *new* atomics to
> prevent us from using the *existing* TAS support.

I think I'll just need help from Tom for that case.

Greetings,

Andres Freund

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to