Thread

Re: small cleanup for s_lock.h

Nathan Bossart <nathandbossart@gmail.com> — 2026-05-05T17:57:10Z

On Tue, May 05, 2026 at 12:56:09PM -0400, Tom Lane wrote:
> After thinking some more I realized that what's confusing us here
> is an API-level problem.  s_lock.h's header comment says
> 
>  *  Usually, S_LOCK() is implemented in terms of even lower-level macros
>  *  TAS() and TAS_SPIN():
> 
> As things stand, we have no platforms where that's not the case,
> and so we've lost sight of the fact that the contract shouldn't be
> "you must provide TAS()".  It should be "you must either provide
> S_LOCK(), or provide TAS() to base it on".
> 
> A rough cut as to the right way to do this is attached.  The
> main loose end here is that it's not very clear what s_lock.c's
> s_lock() should do if there's no TAS (and hence no TAS_SPIN).
> Maybe we should just not compile that function at all without
> TAS; if a platform provides a non-default S_LOCK that needs a
> helper function, it's on the platform to supply that helper.

That makes sense to me.

> Also, after noting that HAS_TEST_AND_SET is referenced nowhere
> outside s_lock.h, I'm coming around to your previous position
> that it's redundant and we should drop it.  This is mainly because
> it's not clear to me whether it should be set on a platform that
> provides S_LOCK but not TAS.  I didn't touch that here though.
> 
> Lastly, I definitely agree now that the file's header comment needs
> some work.  Maybe this insight helps you with that?  (One thing
> I noticed is that the ending comment about "Equivalent OS-supplied
> mutex routines could be used too" feels pretty obsolete.  Maybe
> instead, "Equivalent compiler intrinsics are another popular option".)

I think it does help, thanks.  I'll give it a whirl.

-- 
nathan