Date: Tue, 5 May 2026 12:57:10 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: small cleanup for s_lock.h
Message-ID: <afovdi61nybDRbC3@nathan>
References: <afkUeI7UhacZ5ZFm@nathan>
 <369933.1777933007@sss.pgh.pa.us>
 <afoWBgsygtkCNCRQ@nathan>
 <532705.1778000169@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <532705.1778000169@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/afovdi61nybDRbC3%40nathan>
Precedence: bulk

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