From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org, Daniel Gustafsson <daniel(at)yesql(dot)se> |
Subject: | Re: Bring atomic flag fallback up to snuff |
Date: | 2018-04-07 18:15:02 |
Message-ID: | 20180407181502.qphz4mllqgomgr7m@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2018-04-07 14:07:05 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > As Daniel pointed out in:
> > https://postgr.es/m/FB948276-7B32-4B77-83E6-D00167F8EEB4@yesql.se the
> > pg_atomic_flag fallback implementation is broken. That has gone
> > unnoticed because the fallback implementation wasn't testable until now:
> > ...
> > The attached fixes the bug and removes the edge-cases by storing a value
> > separate from the semaphore. I should have done that from the start.
> > This is an ABI break, but given the fallback didn't work at all, I don't
> > think that's a problem for backporting.
>
> > Fix attached. Comments?
>
> pademelon says it's wrong.
>
> 2018-04-07 13:39:34.982 EDT [1197:89] pg_regress/lock LOG: statement: SELECT test_atomic_ops();
> TRAP: UnalignedPointer("(((uintptr_t) ((uintptr_t)(ptr)) + ((sizeof(*ptr)) - 1)) & ~((uintptr_t) ((sizeof(*ptr)) - 1))) != (uintptr_t)(ptr)", File: "../../../src/include/port/atomics.h", Line: 177)
Yea, I just saw that.
Afaict it's "just" an over-eager / wrong assert. I can't for the heck of
it think why I wrote (9.5 timeframe)
AssertPointerAlignment(ptr, sizeof(*ptr));
where the bigger ones all have asserts alignment to their own size. I
assume I did because some platforms want to do atomics bigger than a
single int - but then I should've used sizeof(ptr->value). So far
pademelon is the only animal affected afaict - let me think about it for
a bit and come up with a patch, ok?
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2018-04-07 18:23:38 | Re: Bring atomic flag fallback up to snuff |
Previous Message | Andreas Seltenreich | 2018-04-07 18:08:57 | [sqlsmith] Failed assertion in create_gather_path |