| 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 15:49:30 |
| Message-ID: | afoRiqyHl0HxhzCB@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'd like to rewrite the comment at the top of the file, too, but haven't
>> gotten to that yet. I find it a little misleading, especially because
>> we #error if TAS isn't defined.
>
> No objection in principle to improving that comment, but what did you
> have in mind exactly?
I think the way the comment presents the macros gives a potentially
misleading impression about what you typically need to do to get a new
platform working, and you basically need to read through the whole file to
make sense of what's going on. Some of the macros it mentions have a
default implementation that we use everywhere (e.g., S_INIT_LOCK), and if
you're using gcc, you may be able to just use the __sync_lock_test_and_set
versions. If you _did_ need to add a new section for a new platform, you'd
probably be more interested in defining slock_t, HAS_TEST_AND_TEST/TAS,
S_UNLOCK, SPIN_DELAY, and maybe TAS_SPIN. In fact, you _must_ ensure TAS
is defined or else we'll fail to compile.
Although as I write this e-mail and think about how exactly I'd rewrite the
comment, I grow less confident about doing so...
--
nathan
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tristan Partin | 2026-05-05 16:01:04 | Re: Cleanup: Replace sscanf with strtol/strtoul in snapmgr |
| Previous Message | Ayush Tiwari | 2026-05-05 15:21:22 | Re: Changing the state of data checksums in a running cluster |