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

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index f402e0dd4b4b5621a0c848c28534a55059e38e7c..47644ea528b3d95300454afddeec9d1e90e2fce2 100644 (file)
--- 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 bb8b8bb29f621712e8cd04b70c72d6903d48b8b1..ddb653a06d795e708d335f1ae2adc56602d4d436 100644 (file)
--- 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 4a6ffb4f89038a06e21b3a3d0461c2598c231386..3902cbf2d962816e2ff1eff878494ab303204a6f 100644 (file)
--- 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 7c57430c81d8019233144d4adbced33d82592aec..4ab3f8027a56362f5c2ce3c4408f6e271edb40c1 100644 (file)
--- 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 11410e221b291afbbbad5375d55b3a898ad27c0b..50ea5c0eaf793a700b2f2af2a5614c87f733db55 100644 (file)
--- 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 7471dadd4b662b8ac6729d42d992ca7da283bfe9..c43e5b894362e9745bf19538b7993591abe34738 100644 (file)
--- 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,