[ here is the updated prologue rebased against the proper tree (26.3-rt3) ] -------------------------- seqlock: serialize against writers There are currently several problems in -rt w.r.t. seqlock objects. RT moves mainline seqlock_t to "raw_seqlock_t", and creates a new seqlock_t object that is fully preemptible. Being preemptible is a great step towards deterministic behavior, but there are a few areas that need additional measures to protect new vulnerabilities created by the preemptible code. For the purposes of demonstration, consider three tasks of different priority: A, B, and C. A is the logically highest, and C is the lowest. A is trying to acquire a seqlock read critical section, while C is involved in write locks. Problem 1) If A spins in seqbegin due to writer contention retries, it may prevent C from running even if C currently holds the write lock. This is a deadlock. Problem 2) B may preempt C, preventing it from releasing the write critical section. In this case, A becomes inverted behind B. Problem 3) Lower priority tasks such as C may continually acquire the write section which subsequently causes A to continually retry and thus fail to make forward progress. Since C is lower priority it ideally should not cause delays in A. E.g. C should block if A is in a read-lock and C is <= A. This patch addresses Problems 1 & 2, and leaves 3 for a later time. This patch changes the internal seqlock_t implementation to substitute a rwlock for the basic spinlock previously used, and forces the readers to serialize with the writers under contention. Blocking on the read_lock simultaneously sleeps A (preventing problem 1), while boosting C to A's priority (preventing problem 2). Non reader-to-writer contended acquisitions, which are the predominant mode, remain free of atomic operations. Therefore the fast path should not be perturbed by this change. This fixes a real-world deadlock discovered under testing where all high priority readers were hogging the cpus and preventing a writer from releasing the lock (i.e. problem 1). Signed-off-by: Gregory Haskins <ghaskins@xxxxxxxxxx> --- include/linux/seqlock.h | 51 +++++++++++++++++++++++------------------------ 1 files changed, 25 insertions(+), 26 deletions(-) diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h index 345d726..605fcdb 100644 --- a/include/linux/seqlock.h +++ b/include/linux/seqlock.h @@ -3,9 +3,11 @@ /* * Reader/writer consistent mechanism without starving writers. This type of * lock for data where the reader wants a consistent set of information - * and is willing to retry if the information changes. Readers never - * block but they may have to retry if a writer is in - * progress. Writers do not wait for readers. + * and is willing to retry if the information changes. Readers block + * on write contention (and where applicable, pi-boost the writer). + * Readers without contention on entry acquire the critical section + * without any atomic operations, but they may have to retry if a writer + * enters before the critical section ends. Writers do not wait for readers. * * This is not as cache friendly as brlock. Also, this will not work * for data that contains pointers, because any writer could @@ -24,6 +26,8 @@ * * Based on x86_64 vsyscall gettimeofday * by Keith Owens and Andrea Arcangeli + * + * Priority inheritance and live-lock avoidance by Gregory Haskins */ #include <linux/spinlock.h> @@ -31,7 +35,7 @@ typedef struct { unsigned sequence; - spinlock_t lock; + rwlock_t lock; } __seqlock_t; typedef struct { @@ -57,7 +61,7 @@ typedef __raw_seqlock_t raw_seqlock_t; { 0, RAW_SPIN_LOCK_UNLOCKED(lockname) } #ifdef CONFIG_PREEMPT_RT -# define __SEQLOCK_UNLOCKED(lockname) { 0, __SPIN_LOCK_UNLOCKED(lockname) } +# define __SEQLOCK_UNLOCKED(lockname) { 0, __RW_LOCK_UNLOCKED(lockname) } #else # define __SEQLOCK_UNLOCKED(lockname) __RAW_SEQLOCK_UNLOCKED(lockname) #endif @@ -69,7 +73,7 @@ typedef __raw_seqlock_t raw_seqlock_t; do { *(x) = (raw_seqlock_t) __RAW_SEQLOCK_UNLOCKED(x); spin_lock_init(&(x)->lock); } while (0) #define seqlock_init(x) \ - do { *(x) = (seqlock_t) __SEQLOCK_UNLOCKED(x); spin_lock_init(&(x)->lock); } while (0) + do { *(x) = (seqlock_t) __SEQLOCK_UNLOCKED(x); rwlock_init(&(x)->lock); } while (0) #define DEFINE_SEQLOCK(x) \ seqlock_t x = __SEQLOCK_UNLOCKED(x) @@ -85,7 +89,7 @@ typedef __raw_seqlock_t raw_seqlock_t; */ static inline void __write_seqlock(seqlock_t *sl) { - spin_lock(&sl->lock); + write_lock(&sl->lock); ++sl->sequence; smp_wmb(); } @@ -103,14 +107,14 @@ static inline void __write_sequnlock(seqlock_t *sl) { smp_wmb(); sl->sequence++; - spin_unlock(&sl->lock); + write_unlock(&sl->lock); } #define __write_sequnlock_irqrestore(sl, flags) __write_sequnlock(sl) static inline int __write_tryseqlock(seqlock_t *sl) { - int ret = spin_trylock(&sl->lock); + int ret = write_trylock(&sl->lock); if (ret) { ++sl->sequence; @@ -120,18 +124,25 @@ static inline int __write_tryseqlock(seqlock_t *sl) } /* Start of read calculation -- fetch last complete writer token */ -static __always_inline unsigned __read_seqbegin(const seqlock_t *sl) +static __always_inline unsigned __read_seqbegin(seqlock_t *sl) { unsigned ret; -repeat: ret = sl->sequence; smp_rmb(); if (unlikely(ret & 1)) { - cpu_relax(); - goto repeat; + /* + * Serialze with the writer which will ensure they are + * pi-boosted if necessary and prevent us from starving + * them. + */ + read_lock(&sl->lock); + ret = sl->sequence; + read_unlock(&sl->lock); } + BUG_ON(ret & 1); + return ret; } @@ -142,20 +153,8 @@ repeat: */ static inline int __read_seqretry(seqlock_t *sl, unsigned iv) { - int ret; - smp_rmb(); - ret = (sl->sequence != iv); - /* - * If invalid then serialize with the writer, to make sure we - * are not livelocking it: - */ - if (unlikely(ret)) { - unsigned long flags; - spin_lock_irqsave(&sl->lock, flags); - spin_unlock_irqrestore(&sl->lock, flags); - } - return ret; + return (sl->sequence != iv); } static __always_inline void __write_seqlock_raw(raw_seqlock_t *sl) -- To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html