On 08/16/2017 09:00 PM, Tom Lane wrote:
Robert Haas <robertmh...@gmail.com> writes:
On Wed, Aug 16, 2017 at 1:44 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
I was feeling a bit uncomfortable with the BUFFERALIGN_DOWN() for a
different reason: if the caller has specified the exact amount of space it
needs, having shm_toc_create discard some could lead to an unexpected
failure.
Well, that's why Heikki also patched shm_toc_estimate. With the
patch, if the size being used in shm_toc_create comes from
shm_toc_estimate, it will always be aligned and nothing bad will
happen.
Sure. So the point is entirely about what should happen if someone
doesn't use shm_toc_estimate.
If the user invents another size out of whole cloth, then
they might get a few bytes less than they expect, but that's what you
get for not using shm_toc_estimate().
I don't buy that argument. A caller might think "Why do I need
shm_toc_estimate, when I can compute the *exact* size I need?".
And it would have worked, up till this proposed patch.
Well, no. The size of the shm_toc struct is subtracted from the size
that you give to shm_toc_create. As well as the sizes of the TOC
entries. And those sizes are private to shm_toc.c, so a caller has no
way to know what size it should pass to shm_toc_create(), in order to
have N bytes of space actually usable. You really need to use
shm_toc_estimate() if you want any guarantees on what will fit.
I've pushed the patch to fix this, with some extra comments and
reformatting shm_toc_estimate.
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.
Yeah. We are implicitly also relying on ShmemAlloc() to return
sufficiently-aligned memory. Palloc() too, although you probably
wouldn't use atomic ops on a palloc'd struct. I think we should
introduce a new ALIGNOF macro for that, and document that those
functions return memory with enough alignment. GENUINEMAX_ALIGNOF?
MAXSTRUCT_ALIGNOF?
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
