public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nathan Bossart <[email protected]>
To: Tom Lane <[email protected]>
Cc: [email protected]
Subject: Re: small cleanup for s_lock.h
Date: Tue, 5 May 2026 12:57:10 -0500
Message-ID: <afovdi61nybDRbC3@nathan> (raw)
In-Reply-To: <[email protected]>
References: <afkUeI7UhacZ5ZFm@nathan>
	<[email protected]>
	<afoWBgsygtkCNCRQ@nathan>
	<[email protected]>

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

view thread (12+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: small cleanup for s_lock.h
  In-Reply-To: <afovdi61nybDRbC3@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox