| From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: small cleanup for s_lock.h |
| Date: | 2026-05-05 16:08:38 |
| Message-ID: | afoWBgsygtkCNCRQ@nathan |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, May 04, 2026 at 06:16:47PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart(at)gmail(dot)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
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-fix-up-TAS-in-s_lock.h.patch | text/plain | 3.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Vlad Lesin | 2026-05-05 16:31:36 | Re: [PATCH] Fix ProcKill lock-group vs procLatch recycle race |
| Previous Message | Tristan Partin | 2026-05-05 16:01:04 | Re: Cleanup: Replace sscanf with strtol/strtoul in snapmgr |