Age | Commit message (Collapse) | Author |
|
Backpatch-through: 13
|
|
Previously we had a fallback implementation that made a harmless system
call, based on the assumption that system calls must contain a memory
barrier. That shouldn't be reached on any current system, and it seems
highly likely that we can easily find out how to request explicit memory
barriers, if we've already had to find out how to do atomics on a
hypothetical new system.
Removed comments and a function name referred to a spinlock used for
fallback memory barriers, but that changed in 1b468a13, which left some
misleading words behind in a few places.
Reviewed-by: Heikki Linnakangas <[email protected]>
Suggested-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/721bf39a-ed8a-44b0-8b8e-be3bd81db748%40technowledgy.de
Discussion: https://postgr.es/m/3351991.1697728588%40sss.pgh.pa.us
|
|
Previously we had a fallback implementation of pg_compiler_barrier()
that called an empty function across a translation unit boundary so the
compiler couldn't see what it did. That shouldn't be needed on any
current systems, and might not even work with a link time optimizer.
Since we now require compiler-specific knowledge of how to implement
atomics, we should also know how to implement compiler barriers on a
hypothetical new system.
Reviewed-by: Heikki Linnakangas <[email protected]>
Suggested-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/721bf39a-ed8a-44b0-8b8e-be3bd81db748%40technowledgy.de
Discussion: https://postgr.es/m/3351991.1697728588%40sss.pgh.pa.us
|
|
Modern versions of all relevant architectures and tool chains have
atomics support. Since edadeb07, there is no remaining reason to carry
code that simulates atomic flags and uint32 imperfectly with spinlocks.
64 bit atomics are still emulated with spinlocks, if needed, for now.
Any modern compiler capable of implementing C11 <stdatomic.h> must have
the underlying operations we need, though we don't require C11 yet. We
detect certain compilers and architectures, so hypothetical new systems
might need adjustments here.
Reviewed-by: Heikki Linnakangas <[email protected]>
Reviewed-by: Tom Lane <[email protected]> (concept, not the patch)
Reviewed-by: Andres Freund <[email protected]> (concept, not the patch)
Discussion: https://postgr.es/m/3351991.1697728588%40sss.pgh.pa.us
|
|
A later change will require atomic support, so it wouldn't make sense
for a hypothetical new system not to be able to implement spinlocks.
Reviewed-by: Heikki Linnakangas <[email protected]>
Reviewed-by: Tom Lane <[email protected]> (concept, not the patch)
Reviewed-by: Andres Freund <[email protected]> (concept, not the patch)
Discussion: https://postgr.es/m/3351991.1697728588%40sss.pgh.pa.us
|
|
This code wanted to ensure that the 'exchange' variable passed to
pg_atomic_compare_exchange_u64 has correct alignment, but apparently
platforms don't actually require anything that doesn't come naturally.
While messing with pg_atomic_monotonic_advance_u64: instead of using
Max() to determine the value to return, just use
pg_atomic_compare_exchange_u64()'s return value to decide; also, use
pg_atomic_compare_exchange_u64 instead of the _impl version; also remove
the unnecessary underscore at the end of variable name "target".
Backpatch to 17, where this code was introduced by commit bf3ff7bf83bc.
Reported-by: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/[email protected]
|
|
This old CPU architecture hasn't been produced in decades, and
whatever instances might still survive are surely too underpowered
for anyone to consider running Postgres on in production. We'd
nonetheless continued to carry code support for it (largely at my
insistence), because its unique implementation of spinlocks seemed
like a good edge case for our spinlock infrastructure. However,
our last buildfarm animal of this type was retired last year, and
it seems quite unlikely that another will emerge. Without the ability
to run tests, the argument that this is useful test code fails to
hold water. Furthermore, carrying code support for an untestable
architecture has costs not to be ignored. So, remove HPPA-specific
code, in the same vein as commits 718aa43a4 and 92d70b77e.
Discussion: https://postgr.es/m/[email protected]
|
|
Writing correct code using atomic variables is often difficult due
to the memory barrier semantics (or lack thereof) of the underlying
operations. This commit introduces atomic read/write functions
with full barrier semantics to ease this cognitive load. For
example, some spinlocks protect a single value, and these new
functions make it easy to convert the value to an atomic variable
(thus eliminating the need for the spinlock) without modifying the
barrier semantics previously provided by the spinlock. Since these
functions may be less performant than the other atomic reads and
writes, they are not suitable for every use-case. However, using a
single atomic operation with full barrier semantics may be more
performant in cases where a separate explicit barrier would
otherwise be required.
The base implementations for these new functions are atomic
exchanges (for writes) and atomic fetch/adds with 0 (for reads).
These implementations can be overwritten with better architecture-
specific versions as they are discovered.
This commit leaves converting existing code to use these new
functions as a future exercise.
Reviewed-by: Andres Freund, Yong Li, Jeff Davis
Discussion: https://postgr.es/m/20231110205128.GB1315705%40nathanxps13
|
|
Reported-by: Michael Paquier
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 12
|
|
Presently, all platforms implement atomic exchanges by performing
an atomic compare-and-swap in a loop until it succeeds. This can
be especially expensive when there is contention on the atomic
variable. This commit optimizes atomic exchanges on many platforms
by using compiler intrinsics, which should compile into something
much less expensive than a compare-and-swap loop. Since these
intrinsics have been available for some time, the inline assembly
implementations are omitted.
Suggested-by: Andres Freund
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20231129212905.GA1258737%40nathanxps13
|
|
The fallback implementation of pg_atomic_test_set_flag() that uses
atomic-exchange gives pg_atomic_exchange_u32_impl() an extra
argument. This issue has been present since the introduction of
the atomics API in commit b64d92f1a5.
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20231114035439.GA1809032%40nathanxps13
Backpatch-through: 12
|
|
Backpatch-through: 11
|
|
The use in regexec.c could remain, since we only try to keep headers C++
clean. But there really doesn't seem to be a good reason to use register in
that spot.
Discussion: https://postgr.es/m/[email protected]
|
|
Visual Studio 2015+ has support for a macro to control the alignement of
structures as of __declspec(align(#)), and this commit adds a definition
of pg_attribute_aligned() based on that. It happens that this was
already used in the implementation of atomics for MSVC. Note that there
is still no definition fo pg_attribute_packed(), so this does not impact
itemptr.h.
Author: James Coleman
Discussion: https://postgr.es/m/CAAaqYe-HbtZvR3msoMtk+hYW2S0e0OapzMW8icSMYTMA+mN8Aw@mail.gmail.com
|
|
This was added as a possible variant of __aarch64__ back when 64-bit
ARM was vaporware. It hasn't shown up in the wild since then, so remove.
Nathan Bossart
Discussion: https://www.postgresql.org/message-id/CAFBsxsEN5nW3uRh%3Djrs-QexDrC1btu0ZfriD3FFfb%3D3J6tAngg%40mail.gmail.com
|
|
Assume that we can use LWARX hint flags and the LWSYNC instruction
on any PPC machine. The check on the assembler's behavior was only
needed for Apple's old assembler, which is no longer of interest
now that we've de-supported all PPC-era versions of macOS (thanks
to them not having clock_gettime()). Also, given an up-to-date
assembler these instructions work even on Apple's old hardware.
It seems quite unlikely that anyone would be interested in running
current Postgres on PPC hardware that's so old as to not have
these instructions.
Hence, rip out associated configure test and manual configuration
options, and just use the modernized instructions all the time.
Also, update atomics/arch-ppc.h to use these instructions as well.
(It was already using LWSYNC unconditionally in another place,
providing further proof that nobody is using PG on hardware old
enough to have a problem with that.)
Discussion: https://postgr.es/m/[email protected]
|
|
This CPU architecture has been discontinued. We already removed HP-UX
support, we never supported Windows/Itanium, and the open source
operating systems that a vintage hardware owner might hope to run have
all either ended Itanium support or never fully released support (NetBSD
may eventually). The extra code we carry for this rare ISA is now
untested. It seems like a good time to remove it.
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/1415825.1656893299%40sss.pgh.pa.us
|
|
HP-UX hardware is no longer produced, build farm coverage recently
ended, and there are no known active maintainers targeting this OS.
Since there is a major rewrite of the build system in the pipeline for
PostgreSQL 16, and that requires development, testing and maintainance
for each OS and tool chain, it seems like a good time to drop support
for:
* HP-UX, the operating system.
* HP aCC, the HP-UX native compiler.
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/1415825.1656893299%40sss.pgh.pa.us
|
|
Architecture Reference Manual for ARMv8 B2.2.1 [1]
For explicit memory effects generated from an Exception level the
following rules apply:
- A read that is generated by a load instruction that loads a single
general-purpose register and is aligned to the size of the read in the
instruction is single-copy atomic.
- A write that is generated by a store instruction that stores a single
general-purpose register and is aligned to the size of the write in the
instruction is single-copy atomic.
[1] https://documentation-service.arm.com/static/61fbe8f4fa8173727a1b734e
https://developer.arm.com/documentation/ddi0487/latest
Author: Yura Sokolov <[email protected]>
Reviewed-by: Thomas Munro <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Discussion: https://postgr.es/m/f7f3f0febe27862711f924a7b0f39e065e547f4b.camel%40postgrespro.ru
Discussion: https://postgr.es/m/CA%2BhUKGKyJf7kwYkqDgzTE26Ra1m9nvM%3Deds2RSSu7WSL-r2wKw%40mail.gmail.com
|
|
Backpatch-through: 10
|
|
Backpatch-through: 9.5
|
|
Mostly already done, but there were some stragglers.
|
|
The implementation uses smaller code when the "expected" operand is a
small constant, but the implementation needlessly defined the set of
acceptable constants more narrowly than the ABI does. Core PostgreSQL
and PGXN don't use the constant path at all, so this is future-proofing.
Back-patch to v13, where commit 30ee5d17c20dbb282a9952b3048d6ad52d56c371
introduced this code.
Reviewed by Tom Lane. Reported by Christoph Berg.
Discussion: https://postgr.es/m/[email protected]
|
|
While xlc defines __64BIT__, gcc does not. Due to this oversight in
commit 30ee5d17c20dbb282a9952b3048d6ad52d56c371, gcc builds continued
implementing 64-bit atomics by way of intrinsics. Back-patch to v13,
where that commit first appeared.
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/[email protected]
|
|
Previously we used pg_atomic_write_64_impl inside
pg_atomic_init_u64. That works correctly, but on platforms without
64bit single copy atomicity it could trigger spurious valgrind errors
about uninitialized memory, because we use compare_and_swap for atomic
writes on such platforms.
I previously suppressed one instance of this problem (6c878edc1df),
but as Tom reports that wasn't enough. As the atomic variable cannot
yet be concurrently accessible during initialization, it seems better
to have pg_atomic_init_64_impl set the value directly.
Change pg_atomic_init_u32_impl for symmetry.
Reported-By: Tom Lane
Author: Andres Freund
Discussion: https://postgr.es/m/[email protected]
Backpatch: 9.5-
|
|
Backpatch-through: update all files in master, backpatch legal files through 9.4
|
|
This is more like how we handle s_lock.h and arch-x86.h.
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/[email protected]
|
|
This reverts commit e7ff59686eacf5021fb84be921116986c3828d8a. It
defined pg_atomic_fetch_add_u32_impl() without defining
pg_atomic_compare_exchange_u32_impl(), which is incompatible with
src/include/port/atomics/fallback.h. Per buildfarm member prairiedog.
Discussion: https://postgr.es/m/[email protected]
|
|
This is more like how we handle s_lock.h and arch-x86.h. This does not
materially affect code generation for gcc 7.2.0 or xlc 13.1.3.
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/[email protected]
|
|
PostgreSQL has been unusable when built with xlc 13 and newer, which are
incompatible with our use of __fetch_and_add(). Back-patch to 9.5,
which introduced pg_atomic_fetch_add_u32().
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/[email protected]
|
|
This addresses a couple of issues in the code:
- Typos and inconsistencies in comments and function declarations.
- Removal of unreferenced function declarations.
- Removal of unnecessary compile flags.
- A cleanup error in regressplans.sh.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/[email protected]
|
|
Backpatch-through: certain files through 9.4
|
|
The atomics fallback implementation for pg_atomic_flag was broken,
returning the inverted value from pg_atomic_test_set_flag(). This was
unnoticed because a) atomic flags were unused until recently b) the
test code wasn't run when the fallback implementation was in
use (because it didn't allow to test for some edge cases).
Fix the bug, and improve the fallback so it has the same behaviour as
the non-fallback implementation in the problematic edge cases. That
breaks ABI compatibility in the back branches when fallbacks are in
use, but given they were broken until now...
Author: Andres Freund
Reported-by: Daniel Gustafsson
Discussion:
https://postgr.es/m/[email protected]
https://postgr.es/m/[email protected]
Backpatch: 9.5-, where the atomics abstraction was introduced.
|
|
Backpatch-through: certain files through 9.3
|
|
In the generic atomic ops that rely on a loop around a CAS primitive,
there's no need to force the initial read of the "old" value to be atomic.
In the typically-rare case that we get a torn value, that simply means
that the first CAS attempt will fail; but it will update "old" to the
atomically-read value, so the next attempt has a chance of succeeding.
It was already being done that way in pg_atomic_exchange_u64_impl(),
but let's duplicate the approach in the rest.
(Given the current coding of the pg_atomic_read functions, this change
is a no-op anyway on popular platforms; it only makes a difference where
pg_atomic_read_u64_impl() is implemented as a CAS.)
In passing, also remove unnecessary take-a-pointer-and-dereference-it
coding in the pg_atomic_read functions. That seems to have been based
on a misunderstanding of what the C standard requires. What actually
matters is that the pointer be declared as pointing to volatile, which
it is.
I don't believe this will change the assembly code at all on x86
platforms (even ignoring the likelihood that these implementations
get overridden by others); but it may help on less-mainstream CPUs.
Discussion: https://postgr.es/m/[email protected]
|
|
In addition to __sync_fetch_and_add, gcc offers __sync_fetch_and_sub,
__sync_fetch_and_and, and __sync_fetch_and_or, which correspond directly
to primitive atomic ops that we want. Testing shows that in some cases
they generate better code than our generic implementations, so use them.
We've assumed that configure's test for __sync_val_compare_and_swap is
sufficient to allow assuming that __sync_fetch_and_add is available, so
make the same assumption for these functions. Should that prove to be
wrong, we can add more configure tests.
Yura Sokolov, reviewed by Jesper Pedersen and myself
Discussion: https://postgr.es/m/[email protected]
|
|
The pg_atomic_compare_exchange_xxx functions are defined to update
*expected to whatever they read from the target variable. Therefore,
there's no need to do additional explicit reads after we've initialized
the "old" variable. The actual benefit of this is somewhat debatable,
but it seems fairly unlikely to hurt anything, especially since we
will override the generic implementations in most performance-sensitive
cases.
Yura Sokolov, reviewed by Jesper Pedersen and myself
Discussion: https://postgr.es/m/[email protected]
|
|
This used to mean "Visual C++ except in those parts where Borland C++
was supported where it meant one of those". Now that we don't support
Borland C++ anymore, simplify by using _MSC_VER which is the normal way
to detect Visual C++.
|
|
This removes the support for building just libpq using Borland C++ or
Visual C++. This has not worked properly for years, and given the number
of complaints it's clearly not worth the maintenance burden.
Building libpq using the standard MSVC build system is of course still
supported, along with mingw.
|
|
When the 64bit atomics simulation is in use, we can't necessarily
guarantee the correct alignment of the atomics due to lack of compiler
support for doing so- that's fine from a safety perspective, because
everything is protected by a lock, but we asserted the alignment in
all cases. Weaken them. Per complaint from Alvaro Herrera.
My #ifdefery for PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY wasn't
sufficient. Fix that. Per complaint from Alexander Korotkov.
|
|
When adding atomics back in b64d92f1a, I added 64bit support as
optional; there wasn't yet a direct user in sight. That turned out to
be a bit short-sighted, it'd already have been useful a number of times.
Add a fallback implementation of 64bit atomics, just like the one we
have for 32bit atomics.
Additionally optimize reads/writes to 64bit on a number of platforms
where aligned writes of that size are atomic. This can now be tested
with PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY.
Author: Andres Freund
Reviewed-By: Amit Kapila
Discussion: https://postgr.es/m/[email protected]
|
|
Previously they were disabled due to performance concerns on 32bit
arm, where 64bit atomics are often implemented via kernel traps.
Author: Roman Shaposhnik
Discussion: http://postgr.es/m/CA+ULb+uErkFuXUCCXWHYvnV5KnAyjGUzzRcPA-M0cgO+Hm4RSA@mail.gmail.com
|
|
c.h #includes a number of core libc header files, such as <stdio.h>.
There's no point in re-including these after having read postgres.h,
postgres_fe.h, or c.h; so remove code that did so.
While at it, also fix some places that were ignoring our standard pattern
of "include postgres[_fe].h, then system header files, then other Postgres
header files". While there's not any great magic in doing it that way
rather than system headers last, it's silly to have just a few files
deviating from the general pattern. (But I didn't attempt to enforce this
globally, only in files I was touching anyway.)
I'd be the first to say that this is mostly compulsive neatnik-ism,
but over time it might save enough compile cycles to be useful.
|
|
configure can only probe the existence of gcc intrinsics, not how well
they're implemented, and unfortunately the answer is sometimes "badly".
In particular we've found that multiple compilers fail to implement
char-width __sync_lock_test_and_set() correctly on PPC; and even a correct
implementation would necessarily be pretty inefficient, since that hardware
has only a word-wide primitive to work with.
Given the knowledge we've accumulated in s_lock.h, it appears that it's
best to rely on int-width TAS operations on most non-Intel architectures.
Hence, pick int not char when both are nominally available to us in
generic-gcc.h (note that that code is not used for x86[_64]).
Back-patch to fix regression test failures on FreeBSD/PPC. Ordinarily
back-patching a change like this would be verboten because of ABI breakage.
But since pg_atomic_flag is not yet used in any Postgres data structure,
there's no ABI to break. It seems safer to back-patch to avoid possible
gotchas, if someday we do back-patch something that uses pg_atomic_flag.
Discussion: https://postgr.es/m/[email protected]
|
|
|
|
I somehow had assumed that in the spinlock (in turn possibly using
semaphores) based fallback atomics implementation 32 bit writes could be
done without a lock. As far as the write goes that's correct, since
postgres supports only platforms with single-copy atomicity for aligned
32bit writes. But writing without holding the spinlock breaks
read-modify-write operations like pg_atomic_compare_exchange_u32(),
since they'll potentially "miss" a concurrent write, which can't happen
in actual hardware implementations.
In 9.6+ when using the fallback atomics implementation this could lead
to buffer header locks not being properly marked as released, and
potentially some related state corruption. I don't see a related danger
in 9.5 (earliest release with the API), because pg_atomic_write_u32()
wasn't used in a concurrent manner there.
The state variable of local buffers, before this change, were
manipulated using pg_atomic_write_u32(), to avoid unnecessary
synchronization overhead. As that'd not be the case anymore, introduce
and use pg_atomic_unlocked_write_u32(), which does not correctly
interact with RMW operations.
This bug only caused issues when postgres is compiled on platforms
without atomics support (i.e. no common new platform), or when compiled
with --disable-atomics, which explains why this wasn't noticed in
testing.
Reported-By: Tom Lane
Discussion: <[email protected]>
Backpatch: 9.5-, where the atomic operations API was introduced.
|
|
pg_atomic_compare_exchange_*_impl() were providing only the semantics of
an acquire barrier. Buildfarm members hornet and mandrill revealed this
deficit beginning with commit 008608b9d51061b1f598c197477b3dc7be9c4a64.
While we have no report of symptoms in 9.5, we can't rule out the
possibility of certain compilers, hardware, or extension code relying on
these functions' specified barrier semantics. Back-patch to 9.5, where
commit b64d92f1a5602c55ee8b27a7ac474f03b7aee340 introduced atomics.
Reviewed by Andres Freund.
|
|
Architecture reference material specifies this order, and s_lock.h
inline assembly agrees. The former order failed to provide mutual
exclusion to lwlock.c and perhaps to other clients. The two xlc
buildfarm members, hornet and mandrill, have failed sixteen times with
duplicate key errors involving pg_class_oid_index or pg_type_oid_index.
Back-patch to 9.5, where commit b64d92f1a5602c55ee8b27a7ac474f03b7aee340
introduced atomics.
Reviewed by Andres Freund and Tom Lane.
|
|
|
|
Tatsuro Yamada
|