From 80abbeba23d466b6541cf95082a9e1f36704424e Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 13 Apr 2016 16:42:01 -0700 Subject: [PATCH] Make init_spin_delay() C89 compliant and change stuck spinlock reporting. The current definition of init_spin_delay (introduced recently in 48354581a) wasn't C89 compliant. It's not legal to refer to refer to non-constant expressions, and the ptr argument was one. This, as reported by Tom, lead to a failure on buildfarm animal pademelon. The pointer, especially on system systems with ASLR, isn't super helpful anyway, though. So instead of making init_spin_delay into an inline function, make s_lock_stuck() report the function name in addition to file:line and change init_spin_delay() accordingly. While not a direct replacement, the function name is likely more useful anyway (line numbers are often hard to interpret in third party reports). This also fixes what file/line number is reported for waits via s_lock(). As PG_FUNCNAME_MACRO is now used outside of elog.h, move it to c.h. Reported-By: Tom Lane Discussion: 4369.1460435533@sss.pgh.pa.us --- src/backend/storage/buffer/bufmgr.c | 4 ++-- src/backend/storage/lmgr/lwlock.c | 2 +- src/backend/storage/lmgr/s_lock.c | 18 ++++++++++-------- src/include/c.h | 11 +++++++++++ src/include/storage/s_lock.h | 9 +++++---- src/include/utils/elog.h | 12 ------------ 6 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index f402e0dd4b..47644ea528 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4029,7 +4029,7 @@ rnode_comparator(const void *p1, const void *p2) uint32 LockBufHdr(BufferDesc *desc) { - SpinDelayStatus delayStatus = init_spin_delay(desc); + SpinDelayStatus delayStatus = init_local_spin_delay(); uint32 old_buf_state; while (true) @@ -4055,7 +4055,7 @@ LockBufHdr(BufferDesc *desc) static uint32 WaitBufHdrUnlocked(BufferDesc *buf) { - SpinDelayStatus delayStatus = init_spin_delay(buf); + SpinDelayStatus delayStatus = init_local_spin_delay(); uint32 buf_state; buf_state = pg_atomic_read_u32(&buf->state); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index bb8b8bb29f..ddb653a06d 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -870,7 +870,7 @@ LWLockWaitListLock(LWLock *lock) /* and then spin without atomic operations until lock is released */ { - SpinDelayStatus delayStatus = init_spin_delay(&lock->state); + SpinDelayStatus delayStatus = init_local_spin_delay(); while (old_state & LW_FLAG_LOCKED) { diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c index 4a6ffb4f89..3902cbf2d9 100644 --- a/src/backend/storage/lmgr/s_lock.c +++ b/src/backend/storage/lmgr/s_lock.c @@ -70,16 +70,18 @@ static int spins_per_delay = DEFAULT_SPINS_PER_DELAY; * s_lock_stuck() - complain about a stuck spinlock */ static void -s_lock_stuck(void *p, const char *file, int line) +s_lock_stuck(const char *file, int line, const char *func) { + if (!func) + func = "(unknown)"; #if defined(S_LOCK_TEST) fprintf(stderr, - "\nStuck spinlock (%p) detected at %s:%d.\n", - p, file, line); + "\nStuck spinlock detected at %s, %s:%d.\n", + func, file, line); exit(1); #else - elog(PANIC, "stuck spinlock (%p) detected at %s:%d", - p, file, line); + elog(PANIC, "stuck spinlock detected at %s, %s:%d", + func, file, line); #endif } @@ -87,9 +89,9 @@ s_lock_stuck(void *p, const char *file, int line) * s_lock(lock) - platform-independent portion of waiting for a spinlock. */ int -s_lock(volatile slock_t *lock, const char *file, int line) +s_lock(volatile slock_t *lock, const char *file, int line, const char *func) { - SpinDelayStatus delayStatus = init_spin_delay((void *) lock); + SpinDelayStatus delayStatus = init_spin_delay(file, line, func); while (TAS_SPIN(lock)) { @@ -127,7 +129,7 @@ perform_spin_delay(SpinDelayStatus *status) if (++(status->spins) >= spins_per_delay) { if (++(status->delays) > NUM_DELAYS) - s_lock_stuck(status->ptr, status->file, status->line); + s_lock_stuck(status->file, status->line, status->func); if (status->cur_delay == 0) /* first time to delay? */ status->cur_delay = MIN_DELAY_USEC; diff --git a/src/include/c.h b/src/include/c.h index 7c57430c81..4ab3f8027a 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -169,6 +169,17 @@ #define dummyret char #endif +/* Which __func__ symbol do we have, if any? */ +#ifdef HAVE_FUNCNAME__FUNC +#define PG_FUNCNAME_MACRO __func__ +#else +#ifdef HAVE_FUNCNAME__FUNCTION +#define PG_FUNCNAME_MACRO __FUNCTION__ +#else +#define PG_FUNCNAME_MACRO NULL +#endif +#endif + /* ---------------------------------------------------------------- * Section 2: bool, true, false, TRUE, FALSE, NULL * ---------------------------------------------------------------- diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h index 11410e221b..50ea5c0eaf 100644 --- a/src/include/storage/s_lock.h +++ b/src/include/storage/s_lock.h @@ -930,7 +930,7 @@ extern int tas_sema(volatile slock_t *lock); #if !defined(S_LOCK) #define S_LOCK(lock) \ - (TAS(lock) ? s_lock((lock), __FILE__, __LINE__) : 0) + (TAS(lock) ? s_lock((lock), __FILE__, __LINE__, PG_FUNCNAME_MACRO) : 0) #endif /* S_LOCK */ #if !defined(S_LOCK_FREE) @@ -983,7 +983,7 @@ extern slock_t dummy_spinlock; /* * Platform-independent out-of-line support routines */ -extern int s_lock(volatile slock_t *lock, const char *file, int line); +extern int s_lock(volatile slock_t *lock, const char *file, int line, const char *func); /* Support for dynamic adjustment of spins_per_delay */ #define DEFAULT_SPINS_PER_DELAY 100 @@ -1000,12 +1000,13 @@ typedef struct int spins; int delays; int cur_delay; - void *ptr; const char *file; int line; + const char *func; } SpinDelayStatus; -#define init_spin_delay(ptr) {0, 0, 0, (ptr), __FILE__, __LINE__} +#define init_spin_delay(file, line, func) {0, 0, 0, file, line, func} +#define init_local_spin_delay() init_spin_delay(__FILE__, __LINE__, PG_FUNCNAME_MACRO) void perform_spin_delay(SpinDelayStatus *status); void finish_spin_delay(SpinDelayStatus *status); diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index 7471dadd4b..c43e5b8943 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -71,18 +71,6 @@ #include "utils/errcodes.h" -/* Which __func__ symbol do we have, if any? */ -#ifdef HAVE_FUNCNAME__FUNC -#define PG_FUNCNAME_MACRO __func__ -#else -#ifdef HAVE_FUNCNAME__FUNCTION -#define PG_FUNCNAME_MACRO __FUNCTION__ -#else -#define PG_FUNCNAME_MACRO NULL -#endif -#endif - - /*---------- * New-style error reporting API: to be used in this way: * ereport(ERROR, -- 2.40.0