Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 80abbeb

Browse files
committed
Make init_spin_delay() C89 compliant and change stuck spinlock reporting.
The current definition of init_spin_delay (introduced recently in 4835458) 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
1 parent 6cead41 commit 80abbeb

File tree

6 files changed

+29
-27
lines changed

6 files changed

+29
-27
lines changed

src/backend/storage/buffer/bufmgr.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -4029,7 +4029,7 @@ rnode_comparator(const void *p1, const void *p2)
40294029
uint32
40304030
LockBufHdr(BufferDesc *desc)
40314031
{
4032-
SpinDelayStatus delayStatus = init_spin_delay(desc);
4032+
SpinDelayStatus delayStatus = init_local_spin_delay();
40334033
uint32 old_buf_state;
40344034

40354035
while (true)
@@ -4055,7 +4055,7 @@ LockBufHdr(BufferDesc *desc)
40554055
static uint32
40564056
WaitBufHdrUnlocked(BufferDesc *buf)
40574057
{
4058-
SpinDelayStatus delayStatus = init_spin_delay(buf);
4058+
SpinDelayStatus delayStatus = init_local_spin_delay();
40594059
uint32 buf_state;
40604060

40614061
buf_state = pg_atomic_read_u32(&buf->state);

src/backend/storage/lmgr/lwlock.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ LWLockWaitListLock(LWLock *lock)
870870

871871
/* and then spin without atomic operations until lock is released */
872872
{
873-
SpinDelayStatus delayStatus = init_spin_delay(&lock->state);
873+
SpinDelayStatus delayStatus = init_local_spin_delay();
874874

875875
while (old_state & LW_FLAG_LOCKED)
876876
{

src/backend/storage/lmgr/s_lock.c

+10-8
Original file line numberDiff line numberDiff line change
@@ -70,26 +70,28 @@ static int spins_per_delay = DEFAULT_SPINS_PER_DELAY;
7070
* s_lock_stuck() - complain about a stuck spinlock
7171
*/
7272
static void
73-
s_lock_stuck(void *p, const char *file, int line)
73+
s_lock_stuck(const char *file, int line, const char *func)
7474
{
75+
if (!func)
76+
func = "(unknown)";
7577
#if defined(S_LOCK_TEST)
7678
fprintf(stderr,
77-
"\nStuck spinlock (%p) detected at %s:%d.\n",
78-
p, file, line);
79+
"\nStuck spinlock detected at %s, %s:%d.\n",
80+
func, file, line);
7981
exit(1);
8082
#else
81-
elog(PANIC, "stuck spinlock (%p) detected at %s:%d",
82-
p, file, line);
83+
elog(PANIC, "stuck spinlock detected at %s, %s:%d",
84+
func, file, line);
8385
#endif
8486
}
8587

8688
/*
8789
* s_lock(lock) - platform-independent portion of waiting for a spinlock.
8890
*/
8991
int
90-
s_lock(volatile slock_t *lock, const char *file, int line)
92+
s_lock(volatile slock_t *lock, const char *file, int line, const char *func)
9193
{
92-
SpinDelayStatus delayStatus = init_spin_delay((void *) lock);
94+
SpinDelayStatus delayStatus = init_spin_delay(file, line, func);
9395

9496
while (TAS_SPIN(lock))
9597
{
@@ -127,7 +129,7 @@ perform_spin_delay(SpinDelayStatus *status)
127129
if (++(status->spins) >= spins_per_delay)
128130
{
129131
if (++(status->delays) > NUM_DELAYS)
130-
s_lock_stuck(status->ptr, status->file, status->line);
132+
s_lock_stuck(status->file, status->line, status->func);
131133

132134
if (status->cur_delay == 0) /* first time to delay? */
133135
status->cur_delay = MIN_DELAY_USEC;

src/include/c.h

+11
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,17 @@
169169
#define dummyret char
170170
#endif
171171

172+
/* Which __func__ symbol do we have, if any? */
173+
#ifdef HAVE_FUNCNAME__FUNC
174+
#define PG_FUNCNAME_MACRO __func__
175+
#else
176+
#ifdef HAVE_FUNCNAME__FUNCTION
177+
#define PG_FUNCNAME_MACRO __FUNCTION__
178+
#else
179+
#define PG_FUNCNAME_MACRO NULL
180+
#endif
181+
#endif
182+
172183
/* ----------------------------------------------------------------
173184
* Section 2: bool, true, false, TRUE, FALSE, NULL
174185
* ----------------------------------------------------------------

src/include/storage/s_lock.h

+5-4
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ extern int tas_sema(volatile slock_t *lock);
930930

931931
#if !defined(S_LOCK)
932932
#define S_LOCK(lock) \
933-
(TAS(lock) ? s_lock((lock), __FILE__, __LINE__) : 0)
933+
(TAS(lock) ? s_lock((lock), __FILE__, __LINE__, PG_FUNCNAME_MACRO) : 0)
934934
#endif /* S_LOCK */
935935

936936
#if !defined(S_LOCK_FREE)
@@ -983,7 +983,7 @@ extern slock_t dummy_spinlock;
983983
/*
984984
* Platform-independent out-of-line support routines
985985
*/
986-
extern int s_lock(volatile slock_t *lock, const char *file, int line);
986+
extern int s_lock(volatile slock_t *lock, const char *file, int line, const char *func);
987987

988988
/* Support for dynamic adjustment of spins_per_delay */
989989
#define DEFAULT_SPINS_PER_DELAY 100
@@ -1000,12 +1000,13 @@ typedef struct
10001000
int spins;
10011001
int delays;
10021002
int cur_delay;
1003-
void *ptr;
10041003
const char *file;
10051004
int line;
1005+
const char *func;
10061006
} SpinDelayStatus;
10071007

1008-
#define init_spin_delay(ptr) {0, 0, 0, (ptr), __FILE__, __LINE__}
1008+
#define init_spin_delay(file, line, func) {0, 0, 0, file, line, func}
1009+
#define init_local_spin_delay() init_spin_delay(__FILE__, __LINE__, PG_FUNCNAME_MACRO)
10091010
void perform_spin_delay(SpinDelayStatus *status);
10101011
void finish_spin_delay(SpinDelayStatus *status);
10111012

src/include/utils/elog.h

-12
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,6 @@
7171
#include "utils/errcodes.h"
7272

7373

74-
/* Which __func__ symbol do we have, if any? */
75-
#ifdef HAVE_FUNCNAME__FUNC
76-
#define PG_FUNCNAME_MACRO __func__
77-
#else
78-
#ifdef HAVE_FUNCNAME__FUNCTION
79-
#define PG_FUNCNAME_MACRO __FUNCTION__
80-
#else
81-
#define PG_FUNCNAME_MACRO NULL
82-
#endif
83-
#endif
84-
85-
8674
/*----------
8775
* New-style error reporting API: to be used in this way:
8876
* ereport(ERROR,

0 commit comments

Comments
 (0)