Atomics in localbuf.c
What's the reason to use pg_atomic...read_...() and pg_atomic...write_...()
functions in localbuf.c?
It looks like there was an intention not to use them
/messages/by-id/CAPpHfdtfr3Aj7xJonXaKR8iY2p8uXOQ+e4BMpMDAM_5R4OcaDA@mail.gmail.com
but the following discussion does not explain the decision to use them.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Hi
On March 5, 2020 9:21:55 AM PST, Antonin Houska <ah@cybertec.at> wrote:
What's the reason to use pg_atomic...read_...() and
pg_atomic...write_...()
functions in localbuf.c?It looks like there was an intention not to use them
/messages/by-id/CAPpHfdtfr3Aj7xJonXaKR8iY2p8uXOQ+e4BMpMDAM_5R4OcaDA@mail.gmail.com
but the following discussion does not explain the decision to use them.
Read/write don't trigger locked/atomic operations. They just guarantee that you're not gonna read/write a torn value. Or a cached one. Since local/shared buffers share the buffer header definition, we still have to use proper functions to access the atomic variables.
Regards,
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund <andres@anarazel.de> wrote:
On March 5, 2020 9:21:55 AM PST, Antonin Houska <ah@cybertec.at> wrote:
What's the reason to use pg_atomic...read_...() and
pg_atomic...write_...()
functions in localbuf.c?It looks like there was an intention not to use them
/messages/by-id/CAPpHfdtfr3Aj7xJonXaKR8iY2p8uXOQ+e4BMpMDAM_5R4OcaDA@mail.gmail.com
but the following discussion does not explain the decision to use them.
Read/write don't trigger locked/atomic operations. They just guarantee that
you're not gonna read/write a torn value. Or a cached one. Since
local/shared buffers share the buffer header definition, we still have to
use proper functions to access the atomic variables.
Sure, the atomic operations are necessary for shared buffers, but I still
don't understand why they are needed for *local* buffers - local buffers their
headers (BufferDesc) in process local memory, so there should be no concerns
about concurrent access.
Another thing that makes me confused is this comment in InitLocalBuffers():
/*
* Intentionally do not initialize the buffer's atomic variable
* (besides zeroing the underlying memory above). That way we get
* errors on platforms without atomics, if somebody (re-)introduces
* atomic operations for local buffers.
*/
That sounds like there was an intention not to use the atomic functions in
localbuf.c, but eventually they ended up there. Do I still miss something?
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Hi
On March 5, 2020 12:42:06 PM PST, Antonin Houska <ah@cybertec.at> wrote:
Andres Freund <andres@anarazel.de> wrote:
On March 5, 2020 9:21:55 AM PST, Antonin Houska <ah@cybertec.at>
wrote:
What's the reason to use pg_atomic...read_...() and
pg_atomic...write_...()
functions in localbuf.c?It looks like there was an intention not to use them
/messages/by-id/CAPpHfdtfr3Aj7xJonXaKR8iY2p8uXOQ+e4BMpMDAM_5R4OcaDA@mail.gmail.com
but the following discussion does not explain the decision to use
them.
Read/write don't trigger locked/atomic operations. They just
guarantee that
you're not gonna read/write a torn value. Or a cached one. Since
local/shared buffers share the buffer header definition, we stillhave to
use proper functions to access the atomic variables.
Sure, the atomic operations are necessary for shared buffers, but I
still
don't understand why they are needed for *local* buffers - local
buffers their
headers (BufferDesc) in process local memory, so there should be no
concerns
about concurrent access.Another thing that makes me confused is this comment in
InitLocalBuffers():/*
* Intentionally do not initialize the buffer's atomic variable
* (besides zeroing the underlying memory above). That way we get
* errors on platforms without atomics, if somebody (re-)introduces
* atomic operations for local buffers.
*/That sounds like there was an intention not to use the atomic functions
in
localbuf.c, but eventually they ended up there. Do I still miss
something?
Again, the read/write functions do not imply atomic instructions.
Ants
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund <andres@anarazel.de> wrote:
On March 5, 2020 12:42:06 PM PST, Antonin Houska <ah@cybertec.at> wrote:
Andres Freund <andres@anarazel.de> wrote:
On March 5, 2020 9:21:55 AM PST, Antonin Houska <ah@cybertec.at>
wrote:
What's the reason to use pg_atomic...read_...() and
pg_atomic...write_...()
functions in localbuf.c?It looks like there was an intention not to use them
/messages/by-id/CAPpHfdtfr3Aj7xJonXaKR8iY2p8uXOQ+e4BMpMDAM_5R4OcaDA@mail.gmail.com
but the following discussion does not explain the decision to use
them.
Read/write don't trigger locked/atomic operations. They just
guarantee that
you're not gonna read/write a torn value. Or a cached one. Since
local/shared buffers share the buffer header definition, we stillhave to
use proper functions to access the atomic variables.
Sure, the atomic operations are necessary for shared buffers, but I
still
don't understand why they are needed for *local* buffers - local
buffers their
headers (BufferDesc) in process local memory, so there should be no
concerns
about concurrent access.Another thing that makes me confused is this comment in
InitLocalBuffers():/*
* Intentionally do not initialize the buffer's atomic variable
* (besides zeroing the underlying memory above). That way we get
* errors on platforms without atomics, if somebody (re-)introduces
* atomic operations for local buffers.
*/That sounds like there was an intention not to use the atomic functions
in
localbuf.c, but eventually they ended up there. Do I still miss
something?Again, the read/write functions do not imply atomic instructions.
ok. What I missed is that BufferDesc.state is declared as pg_atomic_uint32
rather than plain int, so the pg_atomic_...() functions should be used
regardless the buffer is shared or local. Sorry for the noise.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
On Fri, Mar 6, 2020 at 2:04 AM Antonin Houska <ah@cybertec.at> wrote:
ok. What I missed is that BufferDesc.state is declared as pg_atomic_uint32
rather than plain int, so the pg_atomic_...() functions should be used
regardless the buffer is shared or local. Sorry for the noise.
Right. I thought, though, that your question was why we did it that
way instead of just declaring them as uint32. I'm not sure it's very
important, but I think that question hasn't really been answered.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
On 2020-03-06 11:26:41 -0500, Robert Haas wrote:
On Fri, Mar 6, 2020 at 2:04 AM Antonin Houska <ah@cybertec.at> wrote:
ok. What I missed is that BufferDesc.state is declared as pg_atomic_uint32
rather than plain int, so the pg_atomic_...() functions should be used
regardless the buffer is shared or local. Sorry for the noise.Right. I thought, though, that your question was why we did it that
way instead of just declaring them as uint32. I'm not sure it's very
important, but I think that question hasn't really been answered.
I tried, at least:
Since local/shared buffers share the buffer header definition, we still have to use proper functions to access
the atomic variables.
There's only one struct BufferDesc. We could separate them out /
introduce a union or such. But that'd add some complexity / potential
for mistakes too.
Greetings,
Andres Freund
On Fri, Mar 6, 2020 at 2:06 PM Andres Freund <andres@anarazel.de> wrote:
Since local/shared buffers share the buffer header definition, we still have to use proper functions to access
the atomic variables.There's only one struct BufferDesc. We could separate them out /
introduce a union or such. But that'd add some complexity / potential
for mistakes too.
OK, got it. Thanks.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company