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
This commit is contained in:
Andres Freund 2016-04-13 16:42:01 -07:00
parent 6cead413bb
commit 80abbeba23
6 changed files with 29 additions and 27 deletions

View File

@ -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);

View File

@ -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)
{

View File

@ -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;

View File

@ -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
* ----------------------------------------------------------------

View File

@ -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);

View File

@ -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,