Since commit 1a728dff855a ("locking/rwsem: Enable reader optimistic lock stealing") a sleeping, queued writer whose handoff timeout hasn't expired can have its lock stolen by a read lock issued before a woken queued writer was able to acquire a lock. To avoid write lock starvation, this is only permitted if no other read locks are held. However, this behaviour defeats the purpose of a reader periodically dropping the lock to allow writers in, as doing so only works if the write lock is acquired first, another read lock is held, or a queued writer's handoff timeout has expired. Provide a means of indicating this intent via down_read_nosteal() and down_read_killable_nosteal(). Implement it by exposing a steal parameter to rwsem_down_read_slowpath(), update the starvation logic to modify this parameter and modify the steal branch to check it first. Also drop the duplicated RWSEM_WRITER_LOCKED check and remove the goto as the steal condition makes this redundant. Exposing this function is safe as it only makes the lock more pessimistic, so does not impact lock contention. Signed-off-by: Lorenzo Stoakes (ARM) --- include/linux/rwsem.h | 2 ++ kernel/locking/rwsem.c | 71 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h index 6a1a7bae5f81..66ab6fad5e24 100644 --- a/include/linux/rwsem.h +++ b/include/linux/rwsem.h @@ -227,8 +227,10 @@ static inline void rwsem_assert_held_write(const struct rw_semaphore *sem) * lock for reading */ extern void down_read(struct rw_semaphore *sem) __acquires_shared(sem); +void down_read_nosteal(struct rw_semaphore *sem) __acquires_shared(sem); extern int __must_check down_read_interruptible(struct rw_semaphore *sem) __cond_acquires_shared(0, sem); extern int __must_check down_read_killable(struct rw_semaphore *sem) __cond_acquires_shared(0, sem); +int __must_check down_read_killable_nosteal(struct rw_semaphore *sem) __cond_acquires_shared(0, sem); /* * trylock for reading -- returns 1 if successful, 0 if contention diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c index b9c180ac1eee..75fa9fcdee91 100644 --- a/kernel/locking/rwsem.c +++ b/kernel/locking/rwsem.c @@ -1014,7 +1014,8 @@ static inline void rwsem_cond_wake_waiter(struct rw_semaphore *sem, long count, * Wait for the read lock to be granted */ static struct rw_semaphore __sched * -rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int state) +rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int state, + bool steal) { long adjustment = -RWSEM_READER_BIAS; long rcnt = (count >> RWSEM_READER_SHIFT); @@ -1023,17 +1024,16 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat /* * To prevent a constant stream of readers from starving a sleeping - * writer, don't attempt optimistic lock stealing if the lock is - * very likely owned by readers. + * writer, only attempt optimistic lock stealing if the lock is + * unlikely to be owned by readers. */ - if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) && - (rcnt > 1) && !(count & RWSEM_WRITER_LOCKED)) - goto queue; + steal &= !(atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) || + rcnt <= 1; /* * Reader optimistic lock stealing. */ - if (!(count & (RWSEM_WRITER_LOCKED | RWSEM_FLAG_HANDOFF))) { + if (steal && !(count & (RWSEM_WRITER_LOCKED | RWSEM_FLAG_HANDOFF))) { rwsem_set_reader_owned(sem); lockevent_inc(rwsem_rlock_steal); @@ -1052,7 +1052,6 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat return sem; } -queue: waiter.task = current; waiter.type = RWSEM_WAITING_FOR_READ; waiter.timeout = jiffies + RWSEM_WAIT_TIMEOUT; @@ -1281,14 +1280,15 @@ static struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem) /* * lock for reading */ -static __always_inline int __down_read_common(struct rw_semaphore *sem, int state) +static __always_inline int __down_read_common(struct rw_semaphore *sem, int state, + bool steal) { int ret = 0; long count; preempt_disable(); if (!rwsem_read_trylock(sem, &count)) { - if (IS_ERR(rwsem_down_read_slowpath(sem, count, state))) { + if (IS_ERR(rwsem_down_read_slowpath(sem, count, state, steal))) { ret = -EINTR; goto out; } @@ -1301,17 +1301,27 @@ static __always_inline int __down_read_common(struct rw_semaphore *sem, int stat static __always_inline void __down_read(struct rw_semaphore *sem) { - __down_read_common(sem, TASK_UNINTERRUPTIBLE); + __down_read_common(sem, TASK_UNINTERRUPTIBLE, true); +} + +static __always_inline void __down_read_nosteal(struct rw_semaphore *sem) +{ + __down_read_common(sem, TASK_UNINTERRUPTIBLE, false); } static __always_inline int __down_read_interruptible(struct rw_semaphore *sem) { - return __down_read_common(sem, TASK_INTERRUPTIBLE); + return __down_read_common(sem, TASK_INTERRUPTIBLE, true); } static __always_inline int __down_read_killable(struct rw_semaphore *sem) { - return __down_read_common(sem, TASK_KILLABLE); + return __down_read_common(sem, TASK_KILLABLE, true); +} + +static __always_inline int __down_read_killable_nosteal(struct rw_semaphore *sem) +{ + return __down_read_common(sem, TASK_KILLABLE, false); } static inline int __down_read_trylock(struct rw_semaphore *sem) @@ -1502,6 +1512,11 @@ static inline void __down_read(struct rw_semaphore *sem) rwbase_read_lock(&sem->rwbase, TASK_UNINTERRUPTIBLE); } +static inline void __down_read_nosteal(struct rw_semaphore *sem) +{ + __down_read(sem); +} + static inline int __down_read_interruptible(struct rw_semaphore *sem) { return rwbase_read_lock(&sem->rwbase, TASK_INTERRUPTIBLE); @@ -1512,6 +1527,11 @@ static inline int __down_read_killable(struct rw_semaphore *sem) return rwbase_read_lock(&sem->rwbase, TASK_KILLABLE); } +static inline int __down_read_killable_nosteal(struct rw_semaphore *sem) +{ + return __down_read_killable(sem); +} + static inline int __down_read_trylock(struct rw_semaphore *sem) { return rwbase_read_trylock(&sem->rwbase); @@ -1577,6 +1597,16 @@ void __sched down_read(struct rw_semaphore *sem) } EXPORT_SYMBOL(down_read); +void __sched down_read_nosteal(struct rw_semaphore *sem) + __no_context_analysis +{ + might_sleep(); + rwsem_acquire_read(&sem->dep_map, 0, 0, _RET_IP_); + + LOCK_CONTENDED(sem, __down_read_trylock, __down_read_nosteal); +} +EXPORT_SYMBOL(down_read_nosteal); + int __sched down_read_interruptible(struct rw_semaphore *sem) __no_context_analysis { @@ -1607,6 +1637,21 @@ int __sched down_read_killable(struct rw_semaphore *sem) } EXPORT_SYMBOL(down_read_killable); +int __sched down_read_killable_nosteal(struct rw_semaphore *sem) + __no_context_analysis +{ + might_sleep(); + rwsem_acquire_read(&sem->dep_map, 0, 0, _RET_IP_); + + if (LOCK_CONTENDED_RETURN(sem, __down_read_trylock, __down_read_killable_nosteal)) { + rwsem_release(&sem->dep_map, _RET_IP_); + return -EINTR; + } + + return 0; +} +EXPORT_SYMBOL(down_read_killable_nosteal); + /* * trylock for reading -- returns 1 if successful, 0 if contention */ -- 2.56.0