Re: [HACKERS] Re: [COMMITTERS] pgsql: Allow Pin/UnpinBuffer to operate in a lockfree manner.
On 2016-04-11 23:59:21 -0400, Tom Lane wrote: > Andres Freund writes: > > Will fix (both initialization and use of pg_atomic_fetch_or_u32), and > > expand the documentation on why only atomic read/write are supposed to > > be used. > > FWIW, I'd vote against adding a SpinLockInit there. Well, it'd not be a SpinLockInit, but a pg_atomic_init_u32(), but ... > What it would mostly > do is prevent noticing future mistakes of the same ilk. It would be > better no doubt if we didn't have to rely on a nearly-dead platform > to detect this; but having such detection of a performance bug is better > than having no detection. Ok, works for me as well. I guess it'd be useful to add a "modern" animal that disables spinlocks & atomics... - Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Allow Pin/UnpinBuffer to operate in a lockfree manner.
Andres Freund writes: >>> The issue is likely that either Alexander or I somehow made >>> MarkLocalBufferDirty() use pg_atomic_fetch_or_u32(), instead of the >>> proper pg_atomic_read_u32()/pg_atomic_write_u32(). > Ok, so the theory above fits. Yah, especially in view of localbuf.c:297 ;-) > Will fix (both initialization and use of pg_atomic_fetch_or_u32), and > expand the documentation on why only atomic read/write are supposed to > be used. FWIW, I'd vote against adding a SpinLockInit there. What it would mostly do is prevent noticing future mistakes of the same ilk. It would be better no doubt if we didn't have to rely on a nearly-dead platform to detect this; but having such detection of a performance bug is better than having no detection. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Allow Pin/UnpinBuffer to operate in a lockfree manner.
On 2016-04-11 23:41:10 -0400, Tom Lane wrote: > Andres Freund writes: > > The issue is likely that either Alexander or I somehow made > > MarkLocalBufferDirty() use pg_atomic_fetch_or_u32(), instead of the > > proper pg_atomic_read_u32()/pg_atomic_write_u32(). > > The stack trace I'm seeing is > > #5 0x7279cc in elog_finish (elevel=6, > fmt=0x40057cf8 '\177' ...) at elog.c:1378 > #6 0x5cecd8 in s_lock_stuck (p=0x402995b8, file=0x21bae0 "s_lock.c", line=92) > at s_lock.c:81 > #7 0x5cedd4 in perform_spin_delay (status=0x7b03b8c8) at s_lock.c:130 > #8 0x5ced40 in s_lock (lock=0x6, file=0x20 , > line=6) at s_lock.c:96 > #9 0x53a4b0 in pg_atomic_compare_exchange_u32_impl (ptr=0x402995b8, > expected=0x5c, newval=58982400) at atomics.c:122 > #10 0x5a280c in MarkLocalBufferDirty (buffer=6) > at ../../../../src/include/port/atomics/generic.h:224 Ok, so the theory above fits. Will fix (both initialization and use of pg_atomic_fetch_or_u32), and expand the documentation on why only atomic read/write are supposed to be used. Thanks, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Allow Pin/UnpinBuffer to operate in a lockfree manner.
Andres Freund writes: > The issue is likely that either Alexander or I somehow made > MarkLocalBufferDirty() use pg_atomic_fetch_or_u32(), instead of the > proper pg_atomic_read_u32()/pg_atomic_write_u32(). The stack trace I'm seeing is #5 0x7279cc in elog_finish (elevel=6, fmt=0x40057cf8 '\177' ...) at elog.c:1378 #6 0x5cecd8 in s_lock_stuck (p=0x402995b8, file=0x21bae0 "s_lock.c", line=92) at s_lock.c:81 #7 0x5cedd4 in perform_spin_delay (status=0x7b03b8c8) at s_lock.c:130 #8 0x5ced40 in s_lock (lock=0x6, file=0x20 , line=6) at s_lock.c:96 #9 0x53a4b0 in pg_atomic_compare_exchange_u32_impl (ptr=0x402995b8, expected=0x5c, newval=58982400) at atomics.c:122 #10 0x5a280c in MarkLocalBufferDirty (buffer=6) at ../../../../src/include/port/atomics/generic.h:224 #11 0x59bba0 in MarkBufferDirty (buffer=6) at bufmgr.c:1489 #12 0x2c9cd0 in heap_multi_insert (relation=0x401c41d0, tuples=0x40201888, ntuples=561, cid=0, options=0, bistate=0x40153128) at heapam.c:2760 regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Allow Pin/UnpinBuffer to operate in a lockfree manner.
On 2016-04-11 23:24:36 -0400, Tom Lane wrote: > Andres Freund writes: > > Allow Pin/UnpinBuffer to operate in a lockfree manner. > > This commit has broken buildfarm member gaur, and no doubt pademelon > will be equally unhappy once it catches up to HEAD. The reason is that > you've caused localbuf.c to perform a whole bunch of atomic operations > on its buffer headers; and on machines that don't have native atomic > ops, there's a spinlock underlying those; and you did not bother to > ensure that appropriate SpinLockInit operations happen for local-buffer > headers. (HPPA, possibly alone among supported platforms, does not > think that SpinLockInit is equivalent to memset-to-zeroes.) That's obviously borked, and need to be fixed. > While we could fix this by performing suitable SpinLockInit's on > local-buffer headers, I have to think that that's fundamentally the > wrong direction. The *entire* *point* of having local buffers is > that they are not subject to concurrency overhead. So IMO, sticking > atomic-ops calls into localbuf.c is broken on its face. Note that localbuf.c tries to be careful to only use pg_atomic_read_u32/pg_atomic_write_u32 - which don't have a concurrency overhead as they don't utilize atomic ops. The issue is likely that either Alexander or I somehow made MarkLocalBufferDirty() use pg_atomic_fetch_or_u32(), instead of the proper pg_atomic_read_u32()/pg_atomic_write_u32(). Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
