Re: Atomics for heap_parallelscan_nextpage()
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
David Rowley <david.rowley@2ndquadrant.com>,
PostgreSQL-development <pgsql-hackers@postgresql.org>,
Amit Khandekar <amitdkhan.pg@gmail.com>,
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>,
Robert Haas <robertmhaas@gmail.com>
Date: 2017-08-16T18:09:08Z
Lists: pgsql-hackers
Andres Freund <andres@anarazel.de> writes: > On 2017-08-16 13:40:09 -0400, Tom Lane wrote: >> I was wondering why the shm_toc code was using BUFFERALIGN and not >> MAXALIGN, and I now suspect that the answer is "it's an entirely >> undocumented kluge to make the atomics code not crash on 32-bit >> machines, so long as nobody puts a pg_atomic_uint64 anywhere except in >> a shm_toc". > I don't think there were any atomics in affected code until earlier > today... And given it didn't work for shm_toc anyway, I'm not quite > following. Right, Robert pointed out that it's pre-existing code. My point should be read as "it's just blind luck that shm_toc is using bigger than MAXALIGN alignment, or this would never work on 32-bit machines". >> I'm not sure that that's good enough, and I'm damn sure that it >> shouldn't be undocumented. > 8 byte alignment would be good enough, so BUFFERALIGN ought to be > sufficient. But it'd be nicer to have a separate more descriptive knob. What I meant by possibly not good enough is that pg_atomic_uint64 used in other places isn't going to be very safe. We might be effectively all right as long as we have a coding rule that pg_atomic_uint64 can only be placed in memory handed out by ShmemAlloc or shm_toc_allocate, which both have bigger-than-MAXALIGN alignment practices. But this needs to be documented. regards, tom lane
Commits
-
Fix pg_atomic_u64 initialization.
- dcd052c8d20c 11.0 landed
-
Fix shm_toc.c to always return buffer-aligned memory.
- ac883ac453e9 11.0 landed
-
Use atomic ops to hand out pages to scan in parallel scan.
- 3cda10f41bfe 11.0 landed
-
Improve 64bit atomics support.
- e8fdbd58fe56 10.0 cited