Date: Tue, 5 May 2026 11:08:38 -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: <afoWBgsygtkCNCRQ@nathan>
References: <afkUeI7UhacZ5ZFm@nathan>
 <369933.1777933007@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="UnKLvPhhsIm7C55J"
Content-Disposition: inline
In-Reply-To: <369933.1777933007@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/afoWBgsygtkCNCRQ%40nathan>
Precedence: bulk


--UnKLvPhhsIm7C55J
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, May 04, 2026 at 06:16:47PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> I noticed that s_lock.h points to a default implementation of tas() in
>> tas.s or s_lock.c, but AFAICT there hasn't been a tas() implementation in
>> s_lock.c since commit 718aa43a4e, and commit 25f36066dd seems to have
>> removed the last remaining tas.s files.  So, I think this is dead code.
> 
> It is, but I think the 0001 patch should be more like
> 
>  #if !defined(TAS)
> -extern int	tas(volatile slock_t *lock);		/* in port/.../tas.s, or
> -							 * s_lock.c */
> -
> -#define TAS(lock)		tas(lock)
> +#error "must provide a spinlock implementation"
>  #endif	 /* TAS */
> 
> Perhaps this could be merged with the earlier bit about erroring
> if not HAS_TEST_AND_SET.
> 
>> I also noticed that HAS_TEST_AND_SET just means that TAS is defined, so I
>> wrote a 0002 that removes it in favor of checking TAS directly.
> 
> I'm pretty much -1 on that; HAS_TEST_AND_SET is clearer than TAS, and
> removing it seems quite likely to break someone's code.  We could
> perhaps collect all the separate instances into this end location:
> 
> #if defined(TAS)
> #define HAS_TEST_AND_SET
> #else
> #error "must provide a spinlock implementation"
> #endif	 /* TAS */

Okay, here's a new version of the patch that I believe addresses both
points.

-- 
nathan

--UnKLvPhhsIm7C55J
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v2-0001-fix-up-TAS-in-s_lock.h.patch