From cf83248600be8e163ebacc832f7ba1adbadca22d Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sat, 7 Sep 2024 23:11:16 +0100 Subject: [PATCH] Fix GH-13437: FPM: ERROR: scoreboard: failed to lock (already locked) --- sapi/fpm/fpm/fpm_atomic.h | 19 +++++++++++ sapi/fpm/fpm/fpm_scoreboard.c | 59 ++++++++++++++++++++++++++++++----- sapi/fpm/fpm/fpm_scoreboard.h | 4 +++ 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/sapi/fpm/fpm/fpm_atomic.h b/sapi/fpm/fpm/fpm_atomic.h index e3926e708c542..02009f6af9118 100644 --- a/sapi/fpm/fpm/fpm_atomic.h +++ b/sapi/fpm/fpm/fpm_atomic.h @@ -156,6 +156,25 @@ static inline int fpm_spinlock(atomic_t *lock, int try_once) /* {{{ */ } /* }}} */ +static inline int fpm_spinlock_with_max_retries(atomic_t *lock, unsigned int max_retries) +{ + unsigned int retries = 0; + + for (;;) { + if (atomic_cmp_set(lock, 0, 1)) { + return 1; + } + + sched_yield(); + + if (++retries > max_retries) { + return 0; + } + } + + return 1; +} + #define fpm_unlock(lock) lock = 0 #endif diff --git a/sapi/fpm/fpm/fpm_scoreboard.c b/sapi/fpm/fpm/fpm_scoreboard.c index 52d10a0416832..ba7d1ddb655f9 100644 --- a/sapi/fpm/fpm/fpm_scoreboard.c +++ b/sapi/fpm/fpm/fpm_scoreboard.c @@ -73,6 +73,24 @@ int fpm_scoreboard_init_main(void) return 0; } +static inline void fpm_scoreboard_readers_add(struct fpm_scoreboard_s *scoreboard, int num) +{ + /* this should be replaced with atomic_fetch_add but it is not currently supported on all platforms */ + fpm_spinlock(&scoreboard->lock, 1); + scoreboard->reader_count += num; + fpm_unlock(scoreboard->lock); +} + +static inline void fpm_scoreboard_readers_increment(struct fpm_scoreboard_s *scoreboard) +{ + fpm_scoreboard_readers_add(scoreboard, 1); +} + +static inline void fpm_scoreboard_readers_decrement(struct fpm_scoreboard_s *scoreboard) +{ + fpm_scoreboard_readers_add(scoreboard, -1); +} + static struct fpm_scoreboard_s *fpm_scoreboard_get_for_update(struct fpm_scoreboard_s *scoreboard) /* {{{ */ { if (!scoreboard) { @@ -93,7 +111,23 @@ void fpm_scoreboard_update_begin(struct fpm_scoreboard_s *scoreboard) /* {{{ */ return; } - fpm_spinlock(&scoreboard->lock, 0); + if (!fpm_spinlock_with_max_retries(&scoreboard->writer_active, FPM_SCOREBOARD_SPINLOCK_MAX_RETRIES)) { + /* in this case the writer might have crashed so just warn and continue as the lock was acquired */ + zlog(ZLOG_WARNING, "Writer waited too long for another writer to release lock."); + } + + /* wait until no readers are active or lock is acquired */ + int retries = 0; + while (scoreboard->reader_count > 0) { + if (++retries > FPM_SCOREBOARD_SPINLOCK_MAX_RETRIES) { + /* decrement reader count by 1 (assuming a killed or crashed reader) */ + fpm_scoreboard_readers_decrement(scoreboard); + zlog(ZLOG_WARNING, "Writer detected a potential crashed reader, decrementing reader count."); + retries = 0; + } + + sched_yield(); + } } /* }}} */ @@ -170,7 +204,7 @@ void fpm_scoreboard_update_commit( scoreboard->active_max = scoreboard->active; } - fpm_unlock(scoreboard->lock); + fpm_unlock(scoreboard->writer_active); } /* }}} */ @@ -234,16 +268,25 @@ struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get_from_child(struct fpm_chil struct fpm_scoreboard_s *fpm_scoreboard_acquire(struct fpm_scoreboard_s *scoreboard, int nohang) /* {{{ */ { - struct fpm_scoreboard_s *s; - - s = scoreboard ? scoreboard : fpm_scoreboard; + struct fpm_scoreboard_s *s = scoreboard ? scoreboard : fpm_scoreboard; if (!s) { return NULL; } - if (!fpm_spinlock(&s->lock, nohang)) { - return NULL; + fpm_scoreboard_readers_increment(s); + + /* wait until no writer is active */ + int retries = 0; + while (s->writer_active) { + sched_yield(); + + if (++retries > FPM_SCOREBOARD_SPINLOCK_MAX_RETRIES) { + zlog(ZLOG_WARNING, "Reader waited too long for writer to release lock."); + fpm_scoreboard_readers_decrement(s); + return NULL; + } } + return s; } /* }}} */ @@ -253,7 +296,7 @@ void fpm_scoreboard_release(struct fpm_scoreboard_s *scoreboard) { return; } - scoreboard->lock = 0; + fpm_scoreboard_readers_decrement(scoreboard); } struct fpm_scoreboard_s *fpm_scoreboard_copy(struct fpm_scoreboard_s *scoreboard, int copy_procs) diff --git a/sapi/fpm/fpm/fpm_scoreboard.h b/sapi/fpm/fpm/fpm_scoreboard.h index c488c64bfefc4..bd61eedd11767 100644 --- a/sapi/fpm/fpm/fpm_scoreboard.h +++ b/sapi/fpm/fpm/fpm_scoreboard.h @@ -18,6 +18,8 @@ #define FPM_SCOREBOARD_LOCK_HANG 0 #define FPM_SCOREBOARD_LOCK_NOHANG 1 +#define FPM_SCOREBOARD_SPINLOCK_MAX_RETRIES 50000 + struct fpm_scoreboard_proc_s { union { atomic_t lock; @@ -52,6 +54,8 @@ struct fpm_scoreboard_s { atomic_t lock; char dummy[16]; }; + atomic_t writer_active; + atomic_t reader_count; char pool[32]; int pm; time_t start_epoch;