From 2b35af67efc41f298a8c05f513f34be2660b8883 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 | 52 ++++++++++++++++++++++++++++++----- sapi/fpm/fpm/fpm_scoreboard.h | 4 +++ 3 files changed, 68 insertions(+), 7 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..da3b81ca5158f 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); + /* wait until no readers are active or lock is acquired */ + while (1) { + if (fpm_spinlock_with_max_retries(&scoreboard->reader_count, FPM_SCOREBOARD_SPINLOCK_MAX_RETRIES)) { + break; + } + + if (atomic_cmp_set(&scoreboard->reader_count, 0, 0) == 0) { + /* 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."); + } + + sched_yield(); + } + + /* atomically set the writer active flag once the lock is acquired */ + atomic_cmp_set(&scoreboard->writer_active, 0, 1); } /* }}} */ @@ -170,7 +204,7 @@ void fpm_scoreboard_update_commit( scoreboard->active_max = scoreboard->active; } - fpm_unlock(scoreboard->lock); + atomic_cmp_set(&scoreboard->writer_active, 1, 0); } /* }}} */ @@ -234,16 +268,20 @@ 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)) { + fpm_scoreboard_readers_increment(s); + + /* wait until no writer is active */ + if (!fpm_spinlock_with_max_retries(&s->writer_active, FPM_SCOREBOARD_SPINLOCK_MAX_RETRIES)) { + zlog(ZLOG_WARNING, "Reader waited too long for writer to release lock."); + fpm_scoreboard_readers_increment(s); return NULL; } + return s; } /* }}} */ @@ -253,7 +291,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;