locking, sched: The PREEMPT-RT locking infrastructure
Subsystems (8)
cover locking, sched: The PREEMPT-RT locking infrastructure
Folks,
the following series is an update to V1 which can be found here:
https://lore.kernel.org/r/[email protected]
It contains the bulk of the PREEMPT-RT locking infrastructure. In PREEMPT-RT enabled kernels the following locking primitives are substituted by RT-Mutex based variants:
mutex, rw_semaphore, spinlock, rwlock
semaphores are not substituted because they do not provide strict owner semantics.
ww_mutexes are also not substituted because the usage sites are not really RT relevant and it would require a full reimplementation to make them work correctly based on rtmutex. That might change in the future, but for now utilizing the existing variant is considered a safe and sane choice.
Of course raw_spinlocks are not touched either as they protect low level operations in the scheduler, timers and hardware access.
The most interesting parts of the series which need a lot of eyeballs are:
- the scheduler bits which provide the infrastructure for spinlock and rwlock substitution to ensure that the task state is preserved when blocking on such a lock and a regular wakeup is handled correctly and not lost
- the rtmutex core implementation to handle lock contention on spinlocks and rwlocks correctly vs. the task state
- the rw_semaphore/rwlock substitutions which utilize the same implementation vs. the reader/writer handling
- the isolation of the ww_mutex code which allows to build it stand alone. The typedef based solution might look a bit odd on the first glance, but that turned out to be the least intrusive variant.
- the PI futex related bits to handle the interaction between blocking on the underlying rtmutex and contention on the hash bucket lock which is converted to a 'sleeping spinlock'.
The rest surely needs a thorough review as well, but those parts are pretty straight forward. Quite some code restructuring and the actual wrapper functions to replace the existing !RT implementations.
The series survived quite some internal testing in RT kernels and is part of the recent 5.14-rc3-rt2 release.
For !RT kernels there is no functional change.
The series is also available from git:
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git rtmutex
and fully integrated into the v5.14-rc3-rt2 release:
https://lore.kernel.org/r/[email protected]
Changes vs. V1:
- Simplify the scheduler state logic (Peter Zijlstra)
- Split out rt_mutex_base to avoid that wrapped locks carry an unused lockdep map, which in turn avoids extra interfaces. (Peter Zijlstra)
- Pick up Peter's initial version of an rtmutex based ww_mutex and make it work. This replaces the mutex.c split of V1
- Pick up the BUG -> lockdep_assert_held() patch from Peter
- Extend adaptive spinwait to all rtmutex based locks which makes hackbench less unhappy.
- Various review comments addressed
Thanks,
tglx
--- a/kernel/locking/mutex-debug.h | 29 b/include/linux/rbtree_types.h | 34 b/include/linux/rwbase_rt.h | 38 b/include/linux/rwlock_rt.h | 140 ++ b/include/linux/spinlock_rt.h | 151 +++ b/include/linux/spinlock_types_raw.h | 65 + b/kernel/locking/rtmutex_api.c | 590 +++++++++++ b/kernel/locking/rwbase_rt.c | 263 +++++ b/kernel/locking/spinlock_rt.c | 257 +++++ b/kernel/locking/ww_mutex.h | 569 +++++++++++ b/kernel/locking/ww_rt_mutex.c | 76 + drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 4 include/linux/debug_locks.h | 3 include/linux/mutex.h | 93 + include/linux/preempt.h | 4 include/linux/rbtree.h | 30 include/linux/rtmutex.h | 55 - include/linux/rwlock_types.h | 39 include/linux/rwsem.h | 58 + include/linux/sched.h | 77 + include/linux/sched/wake_q.h | 14 include/linux/spinlock.h | 15 include/linux/spinlock_api_smp.h | 3 include/linux/spinlock_types.h | 45 include/linux/ww_mutex.h | 50 - kernel/Kconfig.locks | 2 kernel/futex.c | 479 +++++++-- kernel/locking/Makefile | 3 kernel/locking/mutex-debug.c | 5 kernel/locking/mutex.c | 431 -------- kernel/locking/mutex.h | 33 kernel/locking/rtmutex.c | 1082 +++++++++------------- kernel/locking/rtmutex_common.h | 122 +- kernel/locking/rwsem.c | 109 ++ kernel/locking/spinlock.c | 7 kernel/locking/spinlock_debug.c | 5 kernel/rcu/tree_plugin.h | 6 kernel/sched/core.c | 113 +- lib/Kconfig.debug | 11 lib/test_lockup.c | 8 40 files changed, 3733 insertions(+), 1385 deletions(-)
PATCH 1/63 sched: Split out the wakeup state check
From: Thomas Gleixner <[email protected]>
RT kernels have a slightly more complicated handling of wakeups due to 'sleeping' spin/rwlocks. If a task is blocked on such a lock then the original state of the task is preserved over the blocking and any regular (non lock related) wakeup has to be targeted at the saved state to ensure that these wakeups are not lost. Once the task acquired the lock it restores the task state from the saved state.
To avoid cluttering try_to_wake_up() with that logic, split the wake up state check out into an inline helper and use it at both places where task::state is checked against the state argument of try_to_wake_up().
No functional change.
--- kernel/sched/core.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
---
kernel/sched/core.c +18 -6
@@ -3564,6 +3564,22 @@ static void ttwu_queue(struct task_struc }/* +* Invoked from try_to_wake_up() to check whether the task can be woken up. +* +* The caller holds p::pi_lock if p != current or has preemption +* disabled when p == current. +*/ +static __always_inline +bool ttwu_state_match(struct task_struct *p, unsigned int state, int *success) +{ +if (READ_ONCE (p->__state) & state) { + *success =1 ; +return true; + } +return false; +} + +/* * Notes on Program-Order guarantees on SMP systems. * * MIGRATION
@@ -3702,10 +3718,9 @@ try_to_wake_up(struct task_struct *p, un* - we're serialized against set_special_state() by virtue of * it disabling IRQs (this allows not taking ->pi_lock). */ -if (!(READ_ONCE (p->__state) & state)) +if (!ttwu_state_match (p, state, &success)) goto out; - success =1 ;trace_sched_waking (p);WRITE_ONCE (p->__state,TASK_RUNNING );trace_sched_wakeup (p);
@@ -3720,14 +3735,11 @@ try_to_wake_up(struct task_struct *p, un*/ raw_spin_lock_irqsave (&p->pi_lock, flags);smp_mb__after_spinlock (); -if (!(READ_ONCE (p->__state) & state)) +if (!ttwu_state_match (p, state, &success)) goto unlock;trace_sched_waking (p); -/* We're going to change ->state: */ - success =1 ; -/* * Ensure we load p->on_rq _after_ p->state, otherwise it would * be possible to, falsely, observe p->on_rq == 0 and get stuck
PATCH 2/63 sched: Introduce TASK_RTLOCK_WAIT
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
RT kernels have an extra quirk for try_to_wake_up() to handle task state preservation accross blocking on a 'sleeping' spin/rwlock.
For this to function correctly and under all circumstances try_to_wake_up() must be able to identify whether the wakeup is lock related or not and whether the task is waiting for a lock or not.
The original approach was to use a special wake_flag argument for try_to_wake_up() and just use TASK_UNINTERRUPTIBLE for the tasks wait state and the try_to_wake_up() state argument.
This works in principle, but due to the fact that try_to_wake_up() cannot determine whether the task is waiting for a RT lock wakeup or for a regular wakeup it's suboptimal.
RT kernels save the original task state when blocking on a RT lock and restore it when the lock has been acquired. Any non lock related wakeup is checked against the saved state and if it matches the saved state is set to running so that the wakeup is not lost when the state is restored.
While the necessary logic for the wake_flag based solution is trivial the downside is that any regular wakeup with TASK_UNINTERRUPTIBLE in the state argument set will wake the task despite the fact that it is still blocked on the lock. That's not a fatal problem as the lock wait has do deal with spurious wakeups anyway, but it introduces unneccesary latencies.
Introduce the TASK_RTLOCK_WAIT state bit which will be set when a task blocks on a RT lock.
The lock wakeup will use wake_up_state(TASK_RTLOCK_WAIT) so both the waiting state and the wakeup state are distinguishable, which avoids spurious wakeups and allows better analysis.
--- include/linux/sched.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
---
include/linux/sched.h +3 -1
@@ -95,7 +95,9 @@ struct task_group;#define TASK_WAKING 0x0200#define TASK_NOLOAD 0x0400#define TASK_NEW 0x0800 -#define TASK_STATE_MAX 0x1000 +/* RT specific auxilliary flag to mark RT lock waiters */ +#define TASK_RTLOCK_WAIT 0x1000 +#define TASK_STATE_MAX 0x2000/* Convenience macros for the sake of set_current_state: */ #define TASK_KILLABLE (TASK_WAKEKILL | TASK_UNINTERRUPTIBLE)
PATCH 3/63 sched: Prepare for RT sleeping spin/rwlocks
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Waiting for spinlocks and rwlocks on non RT enabled kernels is task::state preserving. Any wakeup which matches the state is valid.
RT enabled kernels substitutes them with 'sleeping' spinlocks. This creates an issue vs. task::state.
In order to block on the lock the task has to overwrite task::state and a consecutive wakeup issued by the unlocker sets the state back to TASK_RUNNING. As a consequence the task loses the state which was set before the lock acquire and also any regular wakeup targeted at the task while it is blocked on the lock.
To handle this gracefully add a 'saved_state' member to task_struct which is used in the following way:
1) When a task blocks on a 'sleeping' spinlock, the current state is saved in task::saved_state before it is set to TASK_RTLOCK_WAIT.
2) When the task unblocks and after acquiring the lock, it restores the saved state.
3) When a regular wakeup happens for a task while it is blocked then the state change of that wakeup is redirected to operate on task::saved_state.
This is also required when the task state is running because the task might have been woken up from the lock wait and has not yet restored the saved state.
To make it complete provide the necessary helpers to save and restore the saved state along with the necessary documentation how the RT lock blocking is supposed to work.
For non-RT kernels there is no functional change.
--- include/linux/sched.h | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ kernel/sched/core.c | 33 +++++++++++++++++++++++ 2 files changed, 103 insertions(+)
---
include/linux/sched.h +70 -0
@@ -155,6 +155,27 @@ struct task_group;WRITE_ONCE (current->__state, (state_value)); \raw_spin_unlock_irqrestore (¤t->pi_lock, flags); \ } while (0) + + +#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \ + current->saved_state = current->__state; \ + current->saved_state_change = current->task_state_change;\ + current->task_state_change = _THIS_IP_; \ + WRITE_ONCE(current->__state, TASK_RTLOCK_WAIT); \ + raw_spin_unlock(¤t->pi_lock); \ + } while (0); + +#define current_restore_rtlock_saved_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \ + current->task_state_change = current->saved_state_change;\ + WRITE_ONCE(current->__state, current->saved_state); \ + current->saved_state = TASK_RUNNING; \ + raw_spin_unlock(¤t->pi_lock); \ + } while (0); +#else /* * set_current_state() includes a barrier so that the write of current->state
@@ -213,6 +234,47 @@ struct task_group;raw_spin_unlock_irqrestore (¤t->pi_lock, flags); \ } while (0) +/* +* PREEMPT_RT specific variants for "sleeping" spin/rwlocks +* +* RT's spin/rwlock substitutions are state preserving. The state of the +* task when blocking on the lock is saved in task_struct::saved_state and +* restored after the lock has been acquired. These operations are +* serialized by task_struct::pi_lock against try_to_wake_up(). Any non RT +* lock related wakeups while the task is blocked on the lock are +* redirected to operate on task_struct::saved_state to ensure that these +* are not dropped. On restore task_struct::saved_state is set to +* TASK_RUNNING so any wakeup attempt redirected to saved_state will fail. +* +* The lock operation looks like this: +* +* current_save_and_set_rtlock_wait_state(); +* for (;;) { +* if (try_lock()) +* break; +* raw_spin_unlock_irq(&lock->wait_lock); +* schedule_rtlock(); +* raw_spin_lock_irq(&lock->wait_lock); +* set_current_state(TASK_RTLOCK_WAIT); +* } +* current_restore_rtlock_saved_state(); + */ +#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \ + current->saved_state = current->__state; \ + WRITE_ONCE(current->__state, TASK_RTLOCK_WAIT); \ + raw_spin_unlock(¤t->pi_lock); \ + } while (0); + +#define current_restore_rtlock_saved_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \ + WRITE_ONCE(current->__state, current->saved_state); \ + current->saved_state = TASK_RUNNING; \ + raw_spin_unlock(¤t->pi_lock); \ + } while (0); +#endif #define get_current_state () READ_ONCE(current->__state)
@@ -670,6 +732,11 @@ struct task_struct {#endif unsigned int __state; +#ifdef CONFIG_PREEMPT_RT +/* saved state for "spinlock sleepers" */ +unsigned int saved_state; +#endif +/* * This begins the randomizable portion of task_struct. Only * scheduling-critical items should be added above here.
@@ -1359,6 +1426,9 @@ struct task_struct {struct kmap_ctrl kmap_ctrl;#ifdef CONFIG_DEBUG_ATOMIC_SLEEP unsigned long task_state_change; +# ifdef CONFIG_PREEMPT_RT +unsigned long saved_state_change; +# endif #endif int pagefault_disabled;#ifdef CONFIG_MMU
kernel/sched/core.c +33 -0
@@ -3568,14 +3568,47 @@ static void ttwu_queue(struct task_struc* * The caller holds p::pi_lock if p != current or has preemption * disabled when p == current. +* +* The rules of PREEMPT_RT saved_state: +* +* The related locking code always holds p::pi_lock when updating +* p::saved_state, which means the code is fully serialized in both cases. +* +* The lock wait and lock wakeups happen via TASK_RTLOCK_WAIT. No other +* bits set. This allows to distinguish all wakeup scenarios. */ static __always_inline bool ttwu_state_match(struct task_struct *p, unsigned int state, int *success) { +if (IS_ENABLED (CONFIG_DEBUG_PREEMPT )) { +WARN_ON_ONCE ((state &TASK_RTLOCK_WAIT ) && + state !=TASK_RTLOCK_WAIT ); + } +if (READ_ONCE (p->__state) & state) { *success =1 ;return true; } + +#ifdef CONFIG_PREEMPT_RT +/* +* Saved state preserves the task state accross blocking on +* a RT lock. If the state matches, set p::saved_state to +* TASK_RUNNING, but do not wake the task because it waits +* for a lock wakeup. Also indicate success because from +* the regular waker's point of view this has succeeded. +* +* After acquiring the lock the task will restore p::state +* from p::saved_state which ensures that the regular +* wakeup is not lost. The restore will also set +* p::saved_state to TASK_RUNNING so any further tests will +* not result in false positives vs. @success +*/ +if (p->saved_state & state) { + p->saved_state =TASK_RUNNING ; + *success =1 ; + } +#endif return false; }
Re: [patch 03/63] sched: Prepare for RT sleeping spin/rwlocks
include/linux/sched.h struct task_group; · line 163
Quoted 13 lines · Thomas Gleixner
include/linux/sched.h +6
-0@@ -155,6 +155,27 @@ struct task_group; WRITE_ONCE(current->__state, (state_value)); \ raw_spin_unlock_irqrestore(¤t->pi_lock, flags); \ } while (0) + + +#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \ + current->saved_state = current->__state; \
^^should that be using READ_ONCE()?
include/linux/sched.h struct task_group; · line 163
Quoted 4 lines
include/linux/sched.h +4
-0+#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \ + current->saved_state = current->__state; \
^^ditto?
Peter Zijlstra
maintainerRe: [patch 03/63] sched: Prepare for RT sleeping spin/rwlocks
Quoted 13 lines · Mike Galbraith
include/linux/sched.h struct task_group; · line 162
Quoted 12 lines · Thomas Gleixner
include/linux/sched.h +5
-0@@ -155,6 +155,27 @@ struct task_group; WRITE_ONCE(current->__state, (state_value)); \ raw_spin_unlock_irqrestore(¤t->pi_lock, flags); \ } while (0) + + +#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \
That wants to be irqsafe methinks, I realize this is PREEMPT_RT only and there the _irqfoo crap is a no-op so this doesn't really matter one way or the other, but still, taking PI lock without IRQ disable makes my head go BUG-BUG-BUG :-)
Quoted 3 lines
include/linux/sched.h struct task_group; · line 163
Quoted 1 line
include/linux/sched.h +1
-0+ current->saved_state = current->__state; \^^should that be using
READ_ONCE()?
I think we're good here, per this being 'current' and serialized against wakeup by virtue of holding pi_lock, __state is actually stable.
Quoted 5 lines
Quoted 5 lines
include/linux/sched.h +5
-0+ current->saved_state_change = current->task_state_change;\ + current->task_state_change = _THIS_IP_; \ +WRITE_ONCE (current->__state,TASK_RTLOCK_WAIT ); \ +raw_spin_unlock (¤t->pi_lock); \ + }while (0 );
Re: [patch 03/63] sched: Prepare for RT sleeping spin/rwlocks
Quoted 19 lines · Peter Zijlstra
Quoted 13 lines · Mike Galbraith
include/linux/sched.h struct task_group; · line 162
Quoted 12 lines · Thomas Gleixner
include/linux/sched.h +5
-0@@ -155,6 +155,27 @@ struct task_group; WRITE_ONCE(current->__state, (state_value)); \ raw_spin_unlock_irqrestore(¤t->pi_lock, flags); \ } while (0) + + +#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \That wants to be irqsafe methinks, I realize this is
PREEMPT_RTonly and there the_irqfoocrap is a no-op so this doesn't really matter one way or the other, but still, taking PI lock without IRQ disable makes my head go BUG-BUG-BUG :-)
Actually the rule to lock PI lock irqsave still persists on RT, but this has to be called with interrupts disabled (rtmutex::wait_lock is held), so adding a lockdep_assert_irqs_disable() might be good enough to spare the extra save/restore. Hmm?
Thanks,
tglx
Peter Zijlstra
maintainerRe: [patch 03/63] sched: Prepare for RT sleeping spin/rwlocks
Quoted 22 lines · Thomas Gleixner
Quoted 19 lines · Peter Zijlstra
Quoted 13 lines · Mike Galbraith
include/linux/sched.h struct task_group; · line 162
Quoted 12 lines · Thomas Gleixner
include/linux/sched.h +5
-0@@ -155,6 +155,27 @@ struct task_group; WRITE_ONCE(current->__state, (state_value)); \ raw_spin_unlock_irqrestore(¤t->pi_lock, flags); \ } while (0) + + +#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \That wants to be irqsafe methinks, I realize this is
PREEMPT_RTonly and there the_irqfoocrap is a no-op so this doesn't really matter one way or the other, but still, taking PI lock without IRQ disable makes my head go BUG-BUG-BUG :-)
Quoted 4 lines
Actually the rule to lock PI lock irqsave still persists on RT, but this has to be called with interrupts disabled (
rtmutex::wait_lockis held), so adding alockdep_assert_irqs_disable()might be good enough to spare the extra save/restore. Hmm?
Works for me.. and yes, duh, raw_.
Re: [patch 03/63] sched: Prepare for RT sleeping spin/rwlocks
Quoted 23 lines · Peter Zijlstra
Quoted 22 lines · Thomas Gleixner
Quoted 19 lines · Peter Zijlstra
Quoted 13 lines · Mike Galbraith
include/linux/sched.h struct task_group; · line 162
Quoted 12 lines · Thomas Gleixner
include/linux/sched.h +5
-0@@ -155,6 +155,27 @@ struct task_group; WRITE_ONCE(current->__state, (state_value)); \ raw_spin_unlock_irqrestore(¤t->pi_lock, flags); \ } while (0) + + +#define current_save_and_set_rtlock_wait_state () \ + do { \ + raw_spin_lock(¤t->pi_lock); \That wants to be irqsafe methinks, I realize this is
PREEMPT_RTonly and there the_irqfoocrap is a no-op so this doesn't really matter one way or the other, but still, taking PI lock without IRQ disable makes my head go BUG-BUG-BUG :-)
Quoted 6 lines
Quoted 4 lines
Actually the rule to lock PI lock irqsave still persists on RT, but this has to be called with interrupts disabled (
rtmutex::wait_lockis held), so adding alockdep_assert_irqs_disable()might be good enough to spare the extra save/restore. Hmm?Works for me.. and yes, duh,
raw_.
And the lockdep_assert_irqs_disabled() is redundant as well because if raw_spin_lock(¤t->pi_lock) is invoked with interrupts enabled lockdep will yell too. But, yes it makes it more explicit.
Thanks,
tglx
PATCH 4/63 sched: Rework the __schedule() preempt argument
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
PREEMPT_RT needs to hand a special state into __schedule() when a task blocks on a 'sleeping' spin/rwlock. This is required to handle rcu_note_context_switch() correctly without having special casing in the RCU code. From an RCU point of view the blocking on the sleeping spinlock is equivalent to preemption because the task might be in a read side critical section.
schedule_debug() also has a check which would trigger with the !preempt case, but that could be handled differently.
To avoid adding another argument and extra checks which cannot be optimized out by the compiler the following solution has been chosen:
- Replace the boolean 'preempt' argument with an unsigned integer 'sched_mode' argument and define constants to hand in:
(0 == No preemption, 1 = preemption).
- Add two masks to apply on that mode one for the debug/rcu invocations and one for the actual scheduling decision.
For a non RT kernel these masks are UINT_MAX, i.e. all bits are set which allows the compiler to optimze the AND operation out because it is not masking out anything. IOW, it's not different from the boolean.
RT enabled kernels will define these masks seperately.
No functional change.
---
V2: Simplify the masking logic
--- kernel/sched/core.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-)
---
kernel/sched/core.c +23 -11
@@ -5822,6 +5822,18 @@ pick_next_task(struct rq *rq, struct tas#endif /* CONFIG_SCHED_CORE */ /* +* Constants for the sched_mode argument of __schedule(). +* +* The mode argument allows RT enabled kernels to differentiate a +* preemption from blocking on an 'sleeping' spin/rwlock. Note, that +* SM_MASK_PREEMPT for !RT has all bits set which allows the compiler to +* optimize the AND operation out and just check for zero. +*/ +#define SM_NONE 0x0 +#define SM_PREEMPT 0x1 +#define SM_MASK_PREEMPT (~0U) + +/* * __schedule() is the main scheduler function. * * The main means of driving the scheduler and thus entering this function are:
@@ -5860,7 +5872,7 @@ pick_next_task(struct rq *rq, struct tas* * WARNING: must be called with preemption disabled! */ -static void __sched notrace __schedule(bool preempt) +static void __sched notrace __schedule(unsigned int sched_mode) {struct task_struct *prev, *next;unsigned long *switch_count;
@@ -5873,13 +5885,13 @@ static void __sched notrace __schedule(b rq =cpu_rq (cpu); prev = rq->curr; -schedule_debug (prev, preempt); +schedule_debug (prev, !!sched_mode);if (sched_feat (HRTICK ) ||sched_feat (HRTICK_DL ))hrtick_clear (rq);local_irq_disable (); -rcu_note_context_switch (preempt); +rcu_note_context_switch (!!sched_mode);/* * Make sure that signal_pending_state()->signal_pending() below
@@ -5913,7 +5925,7 @@ static void __sched notrace __schedule(b* - ptrace_{,un}freeze_traced() can change ->state underneath us. */ prev_state =READ_ONCE (prev->__state); -if (!preempt && prev_state) { +if (!(sched_mode &SM_MASK_PREEMPT ) && prev_state) {if (signal_pending_state (prev_state, prev)) {WRITE_ONCE (prev->__state,TASK_RUNNING ); }else {
@@ -5979,7 +5991,7 @@ static void __sched notrace __schedule(bmigrate_disable_switch (rq, prev);psi_sched_switch (prev, next, !task_on_rq_queued (prev)); -trace_sched_switch (preempt, prev, next); +trace_sched_switch (sched_mode &SM_MASK_PREEMPT , prev, next);/* Also unlocks the rq: */ rq =context_switch (rq, prev, next, &rf);
@@ -6000,7 +6012,7 @@ void __noreturn do_task_dead(void)/* Tell freezer to ignore us: */ current->flags |=PF_NOFREEZE ; -__schedule (false); +__schedule (SM_NONE );BUG ();/* Avoid "noreturn function does return" - but don't continue if BUG() is a NOP: */
@@ -6061,7 +6073,7 @@ asmlinkage __visible void __sched schedusched_submit_work (tsk);do {preempt_disable (); -__schedule (false); +__schedule (SM_NONE );sched_preempt_enable_no_resched (); }while (need_resched ());sched_update_worker (tsk);
@@ -6089,7 +6101,7 @@ void __sched schedule_idle(void)*/ WARN_ON_ONCE (current->__state);do { -__schedule (false); +__schedule (SM_NONE ); }while (need_resched ()); }
@@ -6142,7 +6154,7 @@ static void __sched notrace preempt_sche*/ preempt_disable_notrace ();preempt_latency_start (1 ); -__schedule (true); +__schedule (SM_PREEMPT );preempt_latency_stop (1 );preempt_enable_no_resched_notrace ();
@@ -6221,7 +6233,7 @@ asmlinkage __visible void __sched notrac* an infinite recursion. */ prev_ctx =exception_enter (); -__schedule (true); +__schedule (SM_PREEMPT );exception_exit (prev_ctx);preempt_latency_stop (1 );
@@ -6370,7 +6382,7 @@ asmlinkage __visible void __sched preempdo {preempt_disable ();local_irq_enable (); -__schedule (true); +__schedule (SM_PREEMPT );local_irq_disable ();sched_preempt_enable_no_resched (); }while (need_resched ());
PATCH 5/63 sched: Provide schedule point for RT locks
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
RT enabled kernels substitute spin/rwlocks with 'sleeping' variants based on rtmutex. Blocking on such a lock is similar to preemption versus:
- I/O scheduling and worker handling because these functions might block on another substituted lock or come from a lock contention within these functions.
- RCU considers this like a preemption because the task might be in a read side critical section.
Add a seperate scheduling point for this and hand a new scheduling mode argument to __schedule() which allows along with seperate mode masks to handle this gracefully from within the scheduler without proliferating that to other subsystems like RCU.
---
V2: Adopt to the simplified mask logic
--- include/linux/sched.h | 3 +++ kernel/sched/core.c | 20 +++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
---
include/linux/sched.h +3 -0
@@ -294,6 +294,9 @@ extern long schedule_timeout_idle(long t asmlinkage void schedule(void);extern void schedule_preempt_disabled (void ); asmlinkage void preempt_schedule_irq(void); +#ifdef CONFIG_PREEMPT_RT +extern void schedule_rtlock (void ); +#endif extern int __must_check io_schedule_prepare (void );extern void io_schedule_finish (int token);
kernel/sched/core.c +19 -1
@@ -5831,7 +5831,13 @@ pick_next_task(struct rq *rq, struct tas*/ #define SM_NONE 0x0#define SM_PREEMPT 0x1 -#define SM_MASK_PREEMPT (~0U) +#define SM_RTLOCK_WAIT 0x2 + +#ifndef CONFIG_PREEMPT_RT +# define SM_MASK_PREEMPT (~0U) +#else +# define SM_MASK_PREEMPT SM_PREEMPT +#endif /* * __schedule() is the main scheduler function.
@@ -6136,6 +6142,18 @@ void __sched schedule_preempt_disabled(vpreempt_disable (); } +#ifdef CONFIG_PREEMPT_RT +void __sched notrace schedule_rtlock(void) +{ +do { +preempt_disable (); +__schedule (SM_RTLOCK_WAIT ); +sched_preempt_enable_no_resched (); + }while (need_resched ()); +} +NOKPROBE_SYMBOL (schedule_rtlock); +#endif + static void __sched notrace preempt_schedule_common(void) { do {
PATCH 6/63 sched/wake_q: Provide WAKE_Q_HEAD_INITIALIZER
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The RT specific spin/rwlock implementation requires special handling of the to be woken waiters. Provide a WAKE_Q_HEAD_INITIALIZER which can be used by the rtmutex code to implement a RT aware wake_q derivative.
--- include/linux/sched/wake_q.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
---
include/linux/sched/wake_q.h +5 -2
@@ -42,8 +42,11 @@ struct wake_q_head {#define WAKE_Q_TAIL ((struct wake_q_node *) 0x01) -#define DEFINE_WAKE_Q (name) \ - struct wake_q_head name = { WAKE_Q_TAIL, &name.first } +#define WAKE_Q_HEAD_INITIALIZER (name) \ + { WAKE_Q_TAIL, &name.first } + +#define DEFINE_WAKE_Q (name) \ + struct wake_q_head name = WAKE_Q_HEAD_INITIALIZER(name)static inline void wake_q_init (struct wake_q_head *head) {
PATCH 7/63 media/atomisp: Use lockdep instead of *mutex_is_locked()
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Subject: [patch 07/63] media/atomisp: Use lockdep instead of *mutex_is_locked()
The only user of rt_mutex_is_locked() is an anti-pattern, remove it.
---
--- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/media/atomisp/pci/atomisp_ioctl.c +2 -2
@@ -1904,8 +1904,8 @@ int __atomisp_streamoff(struct file *fildev_dbg (isp->dev,"Stop stream on pad %d for asd%d\n" ,atomisp_subdev_source_pad (vdev), asd->index); -BUG_ON (!rt_mutex_is_locked (&isp->mutex)); -BUG_ON (!mutex_is_locked (&isp->streamoff_mutex)); +lockdep_assert_held (&isp->mutex); +lockdep_assert_held (&isp->streamoff_mutex);if (type !=V4L2_BUF_TYPE_VIDEO_CAPTURE ) {dev_dbg (isp->dev,"unsupported v4l2 buf type\n" );
PATCH 8/63 rtmutex: Remove rt_mutex_is_locked()
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
No more users.
--- include/linux/rtmutex.h | 11 ----------- 1 file changed, 11 deletions(-)
include/linux/rtmutex.h +0 -11
@@ -67,17 +67,6 @@ do { \#define DEFINE_RT_MUTEX (mutexname) \ struct rt_mutex mutexname = __RT_MUTEX_INITIALIZER(mutexname) -/** -* rt_mutex_is_locked - is the mutex locked -* @lock: the mutex to be queried -* -* Returns 1 if the mutex is locked, 0 if unlocked. -*/ -static inline int rt_mutex_is_locked (struct rt_mutex *lock) -{ -return lock->owner !=NULL ; -} -extern void __rt_mutex_init (struct rt_mutex *lock,const char *name,struct lock_class_key *key);#ifdef CONFIG_DEBUG_LOCK_ALLOC
PATCH 9/63 rtmutex: Convert macros to inlines
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
Inlines are typesafe...
--- kernel/locking/rtmutex.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)
---
kernel/locking/rtmutex.c +27 -4
@@ -141,8 +141,19 @@ static __always_inline void fixup_rt_mut* set up. */ #ifndef CONFIG_DEBUG_RT_MUTEXES -# define rt_mutex_cmpxchg_acquire (l,c,n) (cmpxchg_acquire(&l->owner, c, n) == c) -# define rt_mutex_cmpxchg_release (l,c,n) (cmpxchg_release(&l->owner, c, n) == c) +static __always_inline bool rt_mutex_cmpxchg_acquire(struct rt_mutex *lock, +struct task_struct *old, +struct task_struct *new) +{ +return cmpxchg_acquire (&lock->owner, old, new) == old; +} + +static __always_inline bool rt_mutex_cmpxchg_release(struct rt_mutex *lock, +struct task_struct *old, +struct task_struct *new) +{ +return cmpxchg_release (&lock->owner, old, new) == old; +}/* * Callers must hold the ->wait_lock -- which is the whole purpose as we force
@@ -201,8 +212,20 @@ static __always_inline bool unlock_rt_mu }#else -# define rt_mutex_cmpxchg_acquire (l,c,n) (0) -# define rt_mutex_cmpxchg_release (l,c,n) (0) +static __always_inline bool rt_mutex_cmpxchg_acquire(struct rt_mutex *lock, +struct task_struct *old, +struct task_struct *new) +{ +return false; + +} + +static __always_inline bool rt_mutex_cmpxchg_release(struct rt_mutex *lock, +struct task_struct *old, +struct task_struct *new) +{ +return false; +} static __always_inline void mark_rt_mutex_waiters(struct rt_mutex *lock) {
PATCH 10/63 rtmutex: Switch to try_cmpxchg()
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Allows the compiler to generate better code depending on the architecture.
--- kernel/locking/rtmutex.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/locking/rtmutex.c +2 -2
@@ -145,14 +145,14 @@ static __always_inline bool rt_mutex_cmpstruct task_struct *old, struct task_struct *new) { - return cmpxchg_acquire(&lock->owner, old, new) == old; + return try_cmpxchg_acquire(&lock->owner, &old, new); } static __always_inline bool rt_mutex_cmpxchg_release(struct rt_mutex *lock,struct task_struct *old,struct task_struct *new) { -return cmpxchg_release (&lock->owner, old, new) == old; +return try_cmpxchg_release (&lock->owner, &old, new); }/*
PATCH 11/63 rtmutex: Split API and implementation
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Prepare for reusing the inner functions of rtmutex for RT lock substitutions.
--- kernel/locking/Makefile | 2 kernel/locking/rtmutex.c | 479 +--------------------------------------- kernel/locking/rtmutex_api.c | 453 +++++++++++++++++++++++++++++++++++++ kernel/locking/rtmutex_common.h | 78 +++--- 4 files changed, 514 insertions(+), 498 deletions(-) create mode 100644 kernel/locking/rtmutex_api.c
---
kernel/locking/Makefile +1 -1
@@ -24,7 +24,7 @@ obj-$(CONFIG_SMP) += spinlock.o obj-$(CONFIG_LOCK_SPIN_ON_OWNER) += osq_lock.o obj-$(CONFIG_PROVE_LOCKING) += spinlock.o obj-$(CONFIG_QUEUED_SPINLOCKS) += qspinlock.o -obj-$(CONFIG_RT_MUTEXES) += rtmutex.o +obj-$(CONFIG_RT_MUTEXES) += rtmutex_api.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock_debug.o obj-$(CONFIG_QUEUED_RWLOCKS) += qrwlock.o
kernel/locking/rtmutex.c +17 -462
527 lines not on this page · raw message
kernel/locking/rtmutex_api.c new file +453 -0
453 lines not on this page · raw message
kernel/locking/rtmutex_common.h +43 -35
100 lines not on this page · raw message
PATCH 12/63 rtmutex: Split out the inner parts of struct rtmutex
In reply to Thomas GleixnerRT builds substitutions for rwsem, mutex, spinlock and rwlock around rtmutexes. Split the inner working out so each lock substitution can use them with the appropiate lockdep annotations. This avoid having an extra unused lockdep map in the wrapped rtmutex.
No functional change.
---
V2: New patch
--- include/linux/rtmutex.h | 29 +++++++++++++----- kernel/futex.c | 4 +- kernel/locking/rtmutex.c | 64 ++++++++++++++++++++-------------------- kernel/locking/rtmutex_api.c | 39 +++++++++++++----------- kernel/locking/rtmutex_common.h | 38 +++++++++++------------ kernel/rcu/tree_plugin.h | 6 +-- 6 files changed, 100 insertions(+), 80 deletions(-)
include/linux/rtmutex.h +21 -8
@@ -19,6 +19,21 @@extern int max_lock_depth;/* for sysctl */ +struct rt_mutex_base { +raw_spinlock_t wait_lock; +struct rb_root_cached waiters; +struct task_struct *owner; +}; + +#define __RT_MUTEX_BASE_INITIALIZER (rtbasename) \ +{ \ + .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(rtbasename.wait_lock), \ + .waiters = RB_ROOT_CACHED, \ + .owner = NULL \ +} + +extern void rt_mutex_base_init (struct rt_mutex_base *rtb); +/** * The rt_mutex structure *
@@ -28,9 +43,7 @@ extern int max_lock_depth; /* for sysctl* @owner: the mutex owner */ struct rt_mutex { -raw_spinlock_t wait_lock; -struct rb_root_cached waiters; -struct task_struct *owner; +struct rt_mutex_base rtmutex;#ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map dep_map;#endif
@@ -58,11 +71,11 @@ do { \#define __DEP_MAP_RT_MUTEX_INITIALIZER (mutexname)#endif -#define __RT_MUTEX_INITIALIZER (mutexname) \ - { .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(mutexname.wait_lock) \ - , .waiters = RB_ROOT_CACHED \ - , .owner = NULL \ - __DEP_MAP_RT_MUTEX_INITIALIZER(mutexname)} +#define __RT_MUTEX_INITIALIZER (mutexname) \ +{ \ + .rtmutex = __RT_MUTEX_BASE_INITIALIZER(mutexname.rtmutex) \ + __DEP_MAP_RT_MUTEX_INITIALIZER(mutexname) \ +}#define DEFINE_RT_MUTEX (mutexname) \ struct rt_mutex mutexname = __RT_MUTEX_INITIALIZER(mutexname)
kernel/futex.c +2 -2
@@ -179,7 +179,7 @@ struct futex_pi_state {/* * The PI object: */ -struct rt_mutex pi_mutex; +struct rt_mutex_base pi_mutex;struct task_struct *owner;refcount_t refcount;
@@ -3254,7 +3254,7 @@ static int futex_wait_requeue_pi(u32 __u ret = ret <0 ? ret :0 ; } }else { -struct rt_mutex *pi_mutex; +struct rt_mutex_base *pi_mutex;/* * We have been woken up by futex_unlock_pi(), a timeout, or a
kernel/locking/rtmutex.c +33 -31
240 lines not on this page · raw message
kernel/locking/rtmutex_api.c +22 -17
@@ -26,12 +26,18 @@ static __always_inline int __rt_mutex_lomight_sleep ();mutex_acquire (&lock->dep_map, subclass,0 , _RET_IP_); - ret =__rt_mutex_lock (lock, state); + ret =__rt_mutex_lock (&lock->rtmutex, state);if (ret)mutex_release (&lock->dep_map, _RET_IP_);return ret; } +void rt_mutex_base_init (struct rt_mutex_base *rtb) +{ +__rt_mutex_base_init (rtb); +} +EXPORT_SYMBOL (rt_mutex_base_init); +#ifdef CONFIG_DEBUG_LOCK_ALLOC /** * rt_mutex_lock_nested - lock a rt_mutex
@@ -93,7 +99,7 @@ int __sched rt_mutex_trylock(struct rt_mif (IS_ENABLED (CONFIG_DEBUG_RT_MUTEXES ) &&WARN_ON_ONCE (!in_task ()))return 0 ; - ret =__rt_mutex_trylock (lock); + ret =__rt_mutex_trylock (&lock->rtmutex);if (ret)mutex_acquire (&lock->dep_map,0 ,1 , _RET_IP_);
@@ -109,19 +115,19 @@ EXPORT_SYMBOL_GPL(rt_mutex_trylock); void __sched rt_mutex_unlock(struct rt_mutex *lock) {mutex_release (&lock->dep_map, _RET_IP_); -__rt_mutex_unlock (lock); +__rt_mutex_unlock (&lock->rtmutex); }EXPORT_SYMBOL_GPL (rt_mutex_unlock);/* * Futex variants, must not use fastpath. */ -int __sched rt_mutex_futex_trylock(struct rt_mutex *lock) +int __sched rt_mutex_futex_trylock(struct rt_mutex_base *lock) {return rt_mutex_slowtrylock (lock); } -int __sched __rt_mutex_futex_trylock(struct rt_mutex *lock) +int __sched __rt_mutex_futex_trylock(struct rt_mutex_base *lock) {return __rt_mutex_slowtrylock (lock); }
@@ -133,7 +139,7 @@ int __sched __rt_mutex_futex_trylock(str* @lock: The rt_mutex to be unlocked * @wake_q: The wake queue head from which to get the next lock waiter */ -bool __sched __rt_mutex_futex_unlock(struct rt_mutex *lock, +bool __sched __rt_mutex_futex_unlock(struct rt_mutex_base *lock,struct wake_q_head *wake_q) {lockdep_assert_held (&lock->wait_lock);
@@ -156,7 +162,7 @@ bool __sched __rt_mutex_futex_unlock(strreturn true;/* call postunlock() */ } -void __sched rt_mutex_futex_unlock(struct rt_mutex *lock) +void __sched rt_mutex_futex_unlock(struct rt_mutex_base *lock) {DEFINE_WAKE_Q (wake_q);unsigned long flags;
@@ -185,9 +191,8 @@ void __sched __rt_mutex_init(struct rt_m struct lock_class_key *key) {debug_check_no_locks_freed ((void *)lock,sizeof (*lock)); +__rt_mutex_base_init (&lock->rtmutex);lockdep_init_map (&lock->dep_map, name, key,0 ); - -__rt_mutex_basic_init (lock); }EXPORT_SYMBOL_GPL (__rt_mutex_init);
@@ -205,10 +210,10 @@ EXPORT_SYMBOL_GPL(__rt_mutex_init);* possible at this point because the pi_state which contains the rtmutex * is not yet visible to other tasks. */ -void __sched rt_mutex_init_proxy_locked(struct rt_mutex *lock, +void __sched rt_mutex_init_proxy_locked(struct rt_mutex_base *lock,struct task_struct *proxy_owner) { -__rt_mutex_basic_init (lock); +__rt_mutex_base_init (lock);rt_mutex_set_owner (lock, proxy_owner); }
@@ -224,7 +229,7 @@ void __sched rt_mutex_init_proxy_locked(* possible because it belongs to the pi_state which is about to be freed * and it is not longer visible to other tasks. */ -void __sched rt_mutex_proxy_unlock(struct rt_mutex *lock) +void __sched rt_mutex_proxy_unlock(struct rt_mutex_base *lock) {debug_rt_mutex_proxy_unlock (lock);rt_mutex_set_owner (lock,NULL );
@@ -249,7 +254,7 @@ void __sched rt_mutex_proxy_unlock(struc* * Special API call for PI-futex support. */ -int __sched __rt_mutex_start_proxy_lock(struct rt_mutex *lock, +int __sched __rt_mutex_start_proxy_lock(struct rt_mutex_base *lock,struct rt_mutex_waiter *waiter,struct task_struct *task) {
@@ -296,7 +301,7 @@ int __sched __rt_mutex_start_proxy_lock(* * Special API call for PI-futex support. */ -int __sched rt_mutex_start_proxy_lock(struct rt_mutex *lock, +int __sched rt_mutex_start_proxy_lock(struct rt_mutex_base *lock,struct rt_mutex_waiter *waiter,struct task_struct *task) {
@@ -328,7 +333,7 @@ int __sched rt_mutex_start_proxy_lock(st* * Special API call for PI-futex support */ -int __sched rt_mutex_wait_proxy_lock(struct rt_mutex *lock, +int __sched rt_mutex_wait_proxy_lock(struct rt_mutex_base *lock,struct hrtimer_sleeper *to,struct rt_mutex_waiter *waiter) {
@@ -368,7 +373,7 @@ int __sched rt_mutex_wait_proxy_lock(str* * Special API call for PI-futex support */ -bool __sched rt_mutex_cleanup_proxy_lock(struct rt_mutex *lock, +bool __sched rt_mutex_cleanup_proxy_lock(struct rt_mutex_base *lock,struct rt_mutex_waiter *waiter) {bool cleanup = false;
@@ -413,7 +418,7 @@ bool __sched rt_mutex_cleanup_proxy_lock void __sched rt_mutex_adjust_pi(struct task_struct *task) {struct rt_mutex_waiter *waiter; -struct rt_mutex *next_lock; +struct rt_mutex_base *next_lock;unsigned long flags;raw_spin_lock_irqsave (&task->pi_lock, flags);
kernel/locking/rtmutex_common.h +19 -19
@@ -32,7 +32,7 @@ struct rt_mutex_waiter {struct rb_node tree_entry;struct rb_node pi_tree_entry;struct task_struct *task; -struct rt_mutex *lock; +struct rt_mutex_base *lock;int prio;u64 deadline; };
@@ -40,26 +40,26 @@ struct rt_mutex_waiter {/* * PI-futex support (proxy locking functions, etc.): */ -extern void rt_mutex_init_proxy_locked (struct rt_mutex *lock, +extern void rt_mutex_init_proxy_locked (struct rt_mutex_base *lock,struct task_struct *proxy_owner); -extern void rt_mutex_proxy_unlock (struct rt_mutex *lock); -extern int __rt_mutex_start_proxy_lock (struct rt_mutex *lock, +extern void rt_mutex_proxy_unlock (struct rt_mutex_base *lock); +extern int __rt_mutex_start_proxy_lock (struct rt_mutex_base *lock,struct rt_mutex_waiter *waiter,struct task_struct *task); -extern int rt_mutex_start_proxy_lock (struct rt_mutex *lock, +extern int rt_mutex_start_proxy_lock (struct rt_mutex_base *lock,struct rt_mutex_waiter *waiter,struct task_struct *task); -extern int rt_mutex_wait_proxy_lock (struct rt_mutex *lock, +extern int rt_mutex_wait_proxy_lock (struct rt_mutex_base *lock,struct hrtimer_sleeper *to,struct rt_mutex_waiter *waiter); -extern bool rt_mutex_cleanup_proxy_lock (struct rt_mutex *lock, +extern bool rt_mutex_cleanup_proxy_lock (struct rt_mutex_base *lock,struct rt_mutex_waiter *waiter); -extern int rt_mutex_futex_trylock (struct rt_mutex *l); -extern int __rt_mutex_futex_trylock (struct rt_mutex *l); +extern int rt_mutex_futex_trylock (struct rt_mutex_base *l); +extern int __rt_mutex_futex_trylock (struct rt_mutex_base *l); -extern void rt_mutex_futex_unlock (struct rt_mutex *lock); -extern bool __rt_mutex_futex_unlock (struct rt_mutex *lock, +extern void rt_mutex_futex_unlock (struct rt_mutex_base *lock); +extern bool __rt_mutex_futex_unlock (struct rt_mutex_base *lock,struct wake_q_head *wake_q);extern void rt_mutex_postunlock (struct wake_q_head *wake_q);
@@ -69,12 +69,12 @@ extern void rt_mutex_postunlock(struct w* unconditionally. */ #ifdef CONFIG_RT_MUTEXES -static inline int rt_mutex_has_waiters (struct rt_mutex *lock) +static inline int rt_mutex_has_waiters (struct rt_mutex_base *lock) {return !RB_EMPTY_ROOT (&lock->waiters.rb_root); } -static inline struct rt_mutex_waiter *rt_mutex_top_waiter (struct rt_mutex *lock) +static inline struct rt_mutex_waiter *rt_mutex_top_waiter (struct rt_mutex_base *lock) {struct rb_node *leftmost =rb_first_cached (&lock->waiters);struct rt_mutex_waiter *w =NULL ;
@@ -99,7 +99,7 @@ static inline struct rt_mutex_waiter *ta#define RT_MUTEX_HAS_WAITERS 1UL -static inline struct task_struct *rt_mutex_owner (struct rt_mutex *lock) +static inline struct task_struct *rt_mutex_owner (struct rt_mutex_base *lock) {unsigned long owner = (unsigned long )READ_ONCE (lock->owner);
@@ -121,21 +121,21 @@ enum rtmutex_chainwalk { RT_MUTEX_FULL_CHAINWALK, }; -static inline void __rt_mutex_basic_init (struct rt_mutex *lock) +static inline void __rt_mutex_base_init (struct rt_mutex_base *lock) { - lock->owner =NULL ;raw_spin_lock_init (&lock->wait_lock); lock->waiters =RB_ROOT_CACHED ; + lock->owner =NULL ; }/* Debug functions */ -static inline void debug_rt_mutex_unlock (struct rt_mutex *lock) +static inline void debug_rt_mutex_unlock (struct rt_mutex_base *lock) {if (IS_ENABLED (CONFIG_DEBUG_RT_MUTEXES ))DEBUG_LOCKS_WARN_ON (rt_mutex_owner (lock) != current); } -static inline void debug_rt_mutex_proxy_unlock (struct rt_mutex *lock) +static inline void debug_rt_mutex_proxy_unlock (struct rt_mutex_base *lock) {if (IS_ENABLED (CONFIG_DEBUG_RT_MUTEXES ))DEBUG_LOCKS_WARN_ON (!rt_mutex_owner (lock));
@@ -163,7 +163,7 @@ static inline void rt_mutex_init_waiter(#else /* CONFIG_RT_MUTEXES */ /* Used in rcu/tree_plugin.h */ -static inline struct task_struct *rt_mutex_owner (struct rt_mutex *lock) +static inline struct task_struct *rt_mutex_owner (struct rt_mutex_base *lock) {return NULL ; }
kernel/rcu/tree_plugin.h +3 -3
@@ -559,7 +559,7 @@ rcu_preempt_deferred_qs_irqrestore(strucWRITE_ONCE (rnp->exp_tasks, np);if (IS_ENABLED (CONFIG_RCU_BOOST )) {/* Snapshot ->boost_mtx ownership w/rnp->lock held. */ - drop_boost_mutex =rt_mutex_owner (&rnp->boost_mtx) == t; + drop_boost_mutex =rt_mutex_owner (&rnp->boost_mtx.rtmutex) == t;if (&t->rcu_node_entry == rnp->boost_tasks)WRITE_ONCE (rnp->boost_tasks, np); }
@@ -586,7 +586,7 @@ rcu_preempt_deferred_qs_irqrestore(struc/* Unboost if we were boosted. */ if (IS_ENABLED (CONFIG_RCU_BOOST ) && drop_boost_mutex) -rt_mutex_futex_unlock (&rnp->boost_mtx); +rt_mutex_futex_unlock (&rnp->boost_mtx.rtmutex);/* * If this was the last task on the expedited lists,
@@ -1083,7 +1083,7 @@ static int rcu_boost(struct rcu_node *rn* section. */ t = container_of(tb, struct task_struct, rcu_node_entry); -rt_mutex_init_proxy_locked (&rnp->boost_mtx, t); +rt_mutex_init_proxy_locked (&rnp->boost_mtx.rtmutex, t);raw_spin_unlock_irqrestore_rcu_node (rnp, flags);/* Lock only for side effect: boosts task t's priority. */ rt_mutex_lock (&rnp->boost_mtx);
PATCH 13/63 locking/rtmutex: Provide rt_mutex_slowlock_locked()
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Split the inner workings of rt_mutex_slowlock() out into a seperate function which can be reused by the upcoming RT lock substitutions, e.g. for rw_semaphores.
---
V2: Add the dropped debug_rt_mutex_free_waiter() - Valentin
--- kernel/locking/rtmutex.c | 100 ++++++++++++++++++++++++------------------- kernel/locking/rtmutex_api.c | 2 2 files changed, 59 insertions(+), 43 deletions(-)
---
kernel/locking/rtmutex.c +58 -42
@@ -1106,7 +1106,7 @@ static void __sched remove_waiter(struct }/** -* __rt_mutex_slowlock() - Perform the wait-wake-try-to-take loop +* rt_mutex_slowlock_block() - Perform the wait-wake-try-to-take loop * @lock: the rt_mutex to take * @state: the state the task should block in (TASK_INTERRUPTIBLE * or TASK_UNINTERRUPTIBLE)
@@ -1115,10 +1115,10 @@ static void __sched remove_waiter(struct* * Must be called with lock->wait_lock held and interrupts disabled */ -static int __sched __rt_mutex_slowlock(struct rt_mutex_base *lock, -unsigned int state, -struct hrtimer_sleeper *timeout, -struct rt_mutex_waiter *waiter) +static int __sched rt_mutex_slowlock_block(struct rt_mutex_base *lock, +unsigned int state, +struct hrtimer_sleeper *timeout, +struct rt_mutex_waiter *waiter) {int ret =0 ;
@@ -1168,52 +1168,37 @@ static void __sched rt_mutex_handle_dead } } -/* -* Slow path lock function: +/** +* __rt_mutex_slowlock - Locking slowpath invoked with lock::wait_lock held +* @lock: The rtmutex to block lock +* @state: The task state for sleeping +* @chwalk: Indicator whether full or partial chainwalk is requested +* @waiter: Initializer waiter for blocking */ -static int __sched rt_mutex_slowlock(struct rt_mutex_base *lock, -unsigned int state, -struct hrtimer_sleeper *timeout, -enum rtmutex_chainwalk chwalk) +static int __sched __rt_mutex_slowlock(struct rt_mutex_base *lock, +unsigned int state, +enum rtmutex_chainwalk chwalk, +struct rt_mutex_waiter *waiter) { -struct rt_mutex_waiter waiter; -unsigned long flags; -int ret =0 ; - -rt_mutex_init_waiter (&waiter); +int ret; -/* -* Technically we could use raw_spin_[un]lock_irq() here, but this can -* be called in early boot if the cmpxchg() fast path is disabled -* (debug, no architecture support). In this case we will acquire the -* rtmutex with lock->wait_lock held. But we cannot unconditionally -* enable interrupts in that early boot case. So we need to use the -* irqsave/restore variants. -*/ -raw_spin_lock_irqsave (&lock->wait_lock, flags); +lockdep_assert_held (&lock->wait_lock);/* Try to acquire the lock again: */ -if (try_to_take_rt_mutex (lock, current,NULL )) { -raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +if (try_to_take_rt_mutex (lock, current,NULL ))return 0 ; - }set_current_state (state); -/* Setup the timer, when timeout != NULL */ -if (unlikely (timeout)) -hrtimer_start_expires (&timeout->timer,HRTIMER_MODE_ABS ); - - ret =task_blocks_on_rt_mutex (lock, &waiter, current, chwalk); + ret =task_blocks_on_rt_mutex (lock, waiter, current, chwalk);if (likely (!ret)) -/* sleep on the mutex */ - ret =__rt_mutex_slowlock (lock, state, timeout, &waiter); + ret =rt_mutex_slowlock_block (lock, state,NULL , waiter);if (unlikely (ret)) {__set_current_state (TASK_RUNNING ); -remove_waiter (lock, &waiter); -rt_mutex_handle_deadlock (ret, chwalk, &waiter); +remove_waiter (lock, waiter); +rt_mutex_handle_deadlock (ret, chwalk, waiter); }/*
@@ -1221,14 +1206,45 @@ static int __sched rt_mutex_slowlock(str* unconditionally. We might have to fix that up. */ fixup_rt_mutex_waiters (lock); +return ret; +} -raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +static inline int __rt_mutex_slowlock_locked (struct rt_mutex_base *lock, +unsigned int state) +{ +struct rt_mutex_waiter waiter; +int ret; -/* Remove pending timer: */ -if (unlikely (timeout)) -hrtimer_cancel (&timeout->timer); +rt_mutex_init_waiter (&waiter); + + ret =__rt_mutex_slowlock (lock, state,RT_MUTEX_MIN_CHAINWALK , &waiter);debug_rt_mutex_free_waiter (&waiter); +return ret; +} + +/* +* rt_mutex_slowlock - Locking slowpath invoked when fast path fails +* @lock: The rtmutex to block lock +* @state: The task state for sleeping +*/ +static int __sched rt_mutex_slowlock(struct rt_mutex_base *lock, +unsigned int state) +{ +unsigned long flags; +int ret; + +/* +* Technically we could use raw_spin_[un]lock_irq() here, but this can +* be called in early boot if the cmpxchg() fast path is disabled +* (debug, no architecture support). In this case we will acquire the +* rtmutex with lock->wait_lock held. But we cannot unconditionally +* enable interrupts in that early boot case. So we need to use the +* irqsave/restore variants. +*/ +raw_spin_lock_irqsave (&lock->wait_lock, flags); + ret =__rt_mutex_slowlock_locked (lock, state); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags);return ret; }
@@ -1239,7 +1255,7 @@ static __always_inline int __rt_mutex_loif (likely (rt_mutex_cmpxchg_acquire (lock,NULL , current)))return 0 ; -return rt_mutex_slowlock (lock, state,NULL ,RT_MUTEX_MIN_CHAINWALK ); +return rt_mutex_slowlock (lock, state); } static int __sched __rt_mutex_slowtrylock(struct rt_mutex_base *lock)
kernel/locking/rtmutex_api.c +1 -1
@@ -342,7 +342,7 @@ int __sched rt_mutex_wait_proxy_lock(strraw_spin_lock_irq (&lock->wait_lock);/* sleep on the mutex */ set_current_state (TASK_INTERRUPTIBLE ); - ret =__rt_mutex_slowlock (lock,TASK_INTERRUPTIBLE , to, waiter); + ret =rt_mutex_slowlock_block (lock,TASK_INTERRUPTIBLE , to, waiter);/* * try_to_take_rt_mutex() sets the waiter bit unconditionally. We might * have to fix that up.
PATCH 14/63 rtmutex: Provide rt_mutex_base_is_locked()
In reply to Thomas GleixnerProvide rt_mutex_base_is_locked() which will be used for various wrapped locking primitives for RT.
--- include/linux/rtmutex.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
include/linux/rtmutex.h +11 -0
@@ -32,6 +32,17 @@ struct rt_mutex_base { .owner = NULL \ } +/** +* rt_mutex_base_is_locked - is the rtmutex locked +* @lock: the mutex to be queried +* +* Returns true if the mutex is locked, false if unlocked. +*/ +static inline bool rt_mutex_base_is_locked (struct rt_mutex_base *lock) +{ +return lock->owner !=NULL ; +} +extern void rt_mutex_base_init (struct rt_mutex_base *rtb);/**
PATCH 15/63 locking: Add base code for RT rw_semaphore and rwlock
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
On PREEMPT_RT rw_semaphores and rwlocks are substituted with a rtmutex and a reader count. The implementation is writer unfair as it is not feasible to do priority inheritance on multiple readers, but experience has shown that realtime workloads are not the typical workloads which are sensitive to writer starvation.
The inner workings of rw_semaphores and rwlocks on RT are almost indentical except for the task state and signal handling. rw_semaphores are not state preserving over a contention, they are expected to enter and leave with state == TASK_RUNNING. rwlocks have a mechanism to preserve the state of the task at entry and restore it after unblocking taking potential non-lock related wakeups into account. rw_semaphores can also be subject to signal handling interrupting a blocked state, while rwlocks ignore signals.
To avoid code duplication, provide a shared implementation which takes the small difference vs. state and signals into account. The code is included into the relevant rw_semaphore/rwlock base code and compiled for each use case seperately.
--- include/linux/rwbase_rt.h | 38 ++++++ kernel/locking/rwbase_rt.c | 263 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 301 insertions(+) create mode 100644 include/linux/rwbase_rt.h create mode 100644 kernel/locking/rwbase_rt.c
---
include/linux/rwbase_rt.h new file +38 -0
@@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0-only +#ifndef _LINUX_RW_BASE_RT_H +#define _LINUX_RW_BASE_RT_H + +#include <linux/rtmutex.h> +#include <linux/atomic.h> + +#define READER_BIAS (1U << 31) +#define WRITER_BIAS (1U << 30) + +struct rwbase_rt { +atomic_t readers; +struct rt_mutex_base rtmutex; +}; + +#define __RWBASE_INITIALIZER (name) \ +{ \ + .readers = ATOMIC_INIT(READER_BIAS), \ + .rtmutex = __RT_MUTEX_BASE_INITIALIZER(name.rtmutex), \ +} + +#define init_rwbase_rt (rwbase) \ + do { \ + rt_mutex_base_init(&(rwbase)->rtmutex); \ + atomic_set(&(rwbase)->readers, READER_BIAS); \ + } while (0) + + +static __always_inline bool rw_base_is_locked(struct rwbase_rt *rwb) +{ +return atomic_read (&rwb->readers) !=READER_BIAS ; +} + +static __always_inline bool rw_base_is_contended(struct rwbase_rt *rwb) +{ +return atomic_read (&rwb->readers) >0 ; +} +#endif
kernel/locking/rwbase_rt.c new file +263 -0
263 lines not on this page · raw message
Re: [patch 15/63] locking: Add base code for RT rw_semaphore and rwlock
kernel/locking/rwbase_rt.c line 206
Quoted 8 lines · Thomas Gleixner
kernel/locking/rwbase_rt.c new file +8
-0+static int __sched rwbase_write_lock(struct rwbase_rt *rwb, +unsigned int state) +{ +struct rt_mutex_base *rtm = &rwb->rtmutex; +unsigned long flags; + +/* Take the rtmutex as a first step */ +if (rwbase_rtmutex_lock_state (rtm, state))
This function is used here before it was defined in next patch (patch 16).
Cheers,
Longman
Re: [patch 15/63] locking: Add base code for RT rw_semaphore and rwlock
Quoted 11 lines · Waiman Long
kernel/locking/rwbase_rt.c line 206
Quoted 8 lines · Thomas Gleixner
kernel/locking/rwbase_rt.c new file +8
-0+static int __sched rwbase_write_lock(struct rwbase_rt *rwb, +unsigned int state) +{ +struct rt_mutex_base *rtm = &rwb->rtmutex; +unsigned long flags; + +/* Take the rtmutex as a first step */ +if (rwbase_rtmutex_lock_state (rtm, state))This function is used here before it was defined in next patch (patch 16).
Correct and there are more I think. But that's a hen and egg problem:
The base code cannot be compiled without the actual implementation for either rw_semaphore or rw_lock substitutions. The implementations cannot be compiled without the base code.
So I chose to split it up so the base logic is separate.
Thanks,
tglx
Re: [patch 15/63] locking: Add base code for RT rw_semaphore and rwlock
Quoted 14 lines · Thomas Gleixner
Quoted 10 lines · Waiman Long
kernel/locking/rwbase_rt.c line 206
Quoted 8 lines · Thomas Gleixner
kernel/locking/rwbase_rt.c new file +8
-0+static int __sched rwbase_write_lock(struct rwbase_rt *rwb, +unsigned int state) +{ +struct rt_mutex_base *rtm = &rwb->rtmutex; +unsigned long flags; + +/* Take the rtmutex as a first step */ +if (rwbase_rtmutex_lock_state (rtm, state))This function is used here before it was defined in next patch (patch 16).
Correct and there are more I think. But that's a hen and egg problem:
Quoted 8 lines
The base code cannot be compiled without the actual implementation for either
rw_semaphoreorrw_locksubstitutions. The implementations cannot be compiled without the base code.So I chose to split it up so the base logic is separate.
Thanks,
I realize that after the fact that the code introduced by this patch won't be compiled until the later patch is merged. So it is all OK.
Thanks,
Longman
PATCH 16/63 locking/rwsem: Add rtmutex based R/W semaphore implementation
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The RT specific R/W semaphore implementation used to restrict the number of readers to one because a writer cannot block on multiple readers and inherit its priority or budget.
The single reader restricting was painful in various ways:
- Performance bottleneck for multi-threaded applications in the page fault path (mmap sem)
- Progress blocker for drivers which are carefully crafted to avoid the potential reader/writer deadlock in mainline.
The analysis of the writer code paths shows, that properly written RT tasks should not take them. Syscalls like mmap(), file access which take mmap sem write locked have unbound latencies which are completely unrelated to mmap sem. Other R/W sem users like graphics drivers are not suitable for RT tasks either.
So there is little risk to hurt RT tasks when the RT rwsem implementation is done in the following way:
- Allow concurrent readers
- Make writers block until the last reader left the critical section. This blocking is not subject to priority/budget inheritance.
- Readers blocked on a writer inherit their priority/budget in the normal way.
There is a drawback with this scheme. R/W semaphores become writer unfair though the applications which have triggered writer starvation (mostly on mmap_sem) in the past are not really the typical workloads running on a RT system. So while it's unlikely to hit writer starvation, it's possible. If there are unexpected workloads on RT systems triggering it, the problem has to be revisited.
---
V2: Fix indent fail (Peter Z)
--- include/linux/rwsem.h | 58 ++++++++++++++++++++++++++ kernel/locking/rwsem.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+)
---
include/linux/rwsem.h +58 -0
@@ -16,6 +16,9 @@#include <linux/spinlock.h> #include <linux/atomic.h> #include <linux/err.h> + +#ifndef CONFIG_PREEMPT_RT +#ifdef CONFIG_RWSEM_SPIN_ON_OWNER #include <linux/osq_lock.h> #endif
@@ -119,6 +122,61 @@ static inline int rwsem_is_contended(strreturn !list_empty (&sem->wait_list); } +#else /* !CONFIG_PREEMPT_RT */ + +#include <linux/rwbase_rt.h> + +struct rw_semaphore { +struct rwbase_rt rwbase; +#ifdef CONFIG_DEBUG_LOCK_ALLOC +struct lockdep_map dep_map; +#endif +}; + +#define __RWSEM_INITIALIZER (name) \ + { \ + .rwbase = __RWBASE_INITIALIZER(name), \ + RW_DEP_MAP_INIT(name) \ + } + +#define DECLARE_RWSEM (lockname) \ + struct rw_semaphore lockname = __RWSEM_INITIALIZER(lockname) + +#ifdef CONFIG_DEBUG_LOCK_ALLOC +extern void __rwsem_init (struct rw_semaphore *rwsem,const char *name, +struct lock_class_key *key); +#else +static inline void __rwsem_init (struct rw_semaphore *rwsem,const char *name, +struct lock_class_key *key) +{ +} +#endif + +#define init_rwsem (sem) \ +do { \ + static struct lock_class_key __key; \ + \ + init_rwbase_rt(&(sem)->rwbase); \ + __rwsem_init((sem), #sem, &__key); \ +} while (0) + +static __always_inline int rwsem_is_locked(struct rw_semaphore *sem) +{ +return rw_base_is_locked (&sem->rwbase); +} + +static __always_inline int rwsem_is_contended(struct rw_semaphore *sem) +{ +return rw_base_is_contended (&sem->rwbase); +} + +#endif /* CONFIG_PREEMPT_RT */ + +/* +* The functions below are the same for all rwsem implementations including +* the RT specific variant. +*/ +/* * lock for reading */
kernel/locking/rwsem.c +108 -0
120 lines not on this page · raw message
PATCH 17/63 locking/rtmutex: Add wake_state to rt_mutex_waiter
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Regular sleeping locks like mutexes, rtmutexes and rw_semaphores are always entering and leaving a blocking section with task state == TASK_RUNNING.
On a non-RT kernel spinlocks and rwlocks never affect the task state, but on RT kernels these locks are converted to rtmutex based 'sleeping' locks.
So in case of contention the task goes to block which requires to carefully preserve the task state and restore it after acquiring the lock taking regular wakeups for the task into account which happened while the task was blocked. This state preserving is achieved by having a seperate task state for blocking on a RT spin/rwlock and a saved_state field in task_struct along with careful handling of these wakeup scenarios in try_to_wake_up().
To avoid conditionals in the rtmutex code, store the wake state which has to be used for waking a lock waiter in rt_mutex_waiter which allows to handle the regular and RT spin/rwlocks by handing it to wake_up_state().
---
V2: Use unsigned int for wake_state (Peter Z.)
--- kernel/locking/rtmutex.c | 2 +- kernel/locking/rtmutex_common.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
---
kernel/locking/rtmutex.c +1 -1
@@ -692,7 +692,7 @@ static int __sched rt_mutex_adjust_prio_* to get the lock. */ if (prerequeue_top_waiter !=rt_mutex_top_waiter (lock)) -wake_up_process (rt_mutex_top_waiter (lock)->task); +wake_up_state (waiter->task, waiter->wake_state);raw_spin_unlock_irq (&lock->wait_lock);return 0 ; }
kernel/locking/rtmutex_common.h +9 -0
@@ -25,6 +25,7 @@* @pi_tree_entry: pi node to enqueue into the mutex owner waiters tree * @task: task reference to the blocked task * @lock: Pointer to the rt_mutex on which the waiter blocks +* @wake_state: Wakeup state to use (TASK_NORMAL or TASK_RTLOCK_WAIT) * @prio: Priority of the waiter * @deadline: Deadline of the waiter if applicable */
@@ -33,6 +34,7 @@ struct rt_mutex_waiter {struct rb_node pi_tree_entry;struct task_struct *task;struct rt_mutex_base *lock; +unsigned int wake_state;int prio;u64 deadline; };
@@ -158,9 +160,16 @@ static inline void rt_mutex_init_waiter(debug_rt_mutex_init_waiter (waiter);RB_CLEAR_NODE (&waiter->pi_tree_entry);RB_CLEAR_NODE (&waiter->tree_entry); + waiter->wake_state =TASK_NORMAL ; waiter->task = NULL; } +static inline void rtlock_init_rtmutex_waiter (struct rt_mutex_waiter *waiter) +{ +rt_mutex_init_waiter (waiter); + waiter->wake_state =TASK_RTLOCK_WAIT ; +} +#else /* CONFIG_RT_MUTEXES */ /* Used in rcu/tree_plugin.h */ static inline struct task_struct *rt_mutex_owner (struct rt_mutex_base *lock)
PATCH 18/63 locking/rtmutex: Provide rt_wake_q and helpers
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
To handle the difference of wakeups for regular sleeping locks (mutex, rtmutex, rw_semaphore) and the wakeups for 'sleeping' spin/rwlocks on PREEMPT_RT enabled kernels correctly, it is required to provide a wake_q construct which allows to keep them seperate.
Provide a wrapper around wake_q and the required helpers, which will be extended with the state handling later.
No functional change.
---
V2: Rename according to PeterZ
--- kernel/locking/rtmutex.c | 15 +++++++++++++++ kernel/locking/rtmutex_common.h | 14 ++++++++++++++ 2 files changed, 29 insertions(+)
---
kernel/locking/rtmutex.c +15 -0
@@ -347,6 +347,21 @@ static __always_inline void rt_mutex_adjrt_mutex_setprio (p, pi_task); } +/* RT mutex specific wake_q wrappers */ +static __always_inline void rt_mutex_wake_q_add(struct rt_wake_q_head *wqh, +struct rt_mutex_waiter *w) +{ +wake_q_add (&wqh->head, w->task); +} + +static __always_inline void rt_mutex_wake_up_q(struct rt_wake_q_head *wqh) +{ +wake_up_q (&wqh->head); + +/* Pairs with preempt_disable() in mark_wakeup_next_waiter() */ +preempt_enable (); +} +/* * Deadlock detection is conditional: *
kernel/locking/rtmutex_common.h +14 -0
@@ -39,6 +39,20 @@ struct rt_mutex_waiter {u64 deadline; }; +/** +* rt_wake_q_head - Wrapper around regular wake_q_head to support +* "sleeping" spinlocks on RT +* @head: The regular wake_q_head for sleeping lock variants +*/ +struct rt_wake_q_head { +struct wake_q_head head; +}; + +#define DEFINE_RT_WAKE_Q (name) \ + struct rt_wake_q_head name = { \ + .head = WAKE_Q_HEAD_INITIALIZER(name.head), \ + } +/* * PI-futex support (proxy locking functions, etc.): */
PATCH 19/63 locking/rtmutex: Use rt_mutex_wake_q_head
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Prepare for the required state aware handling of waiter wakeups via wake_q and switch the rtmutex code over to the rtmutex specific wrapper.
No functional change.
---
V2: Adopt to rename
--- kernel/futex.c | 8 ++++---- kernel/locking/rtmutex.c | 12 ++++++------ kernel/locking/rtmutex_api.c | 19 ++++++++----------- kernel/locking/rtmutex_common.h | 4 ++-- 4 files changed, 20 insertions(+), 23 deletions(-)
---
kernel/futex.c +4 -4
@@ -1493,11 +1493,11 @@ static void mark_wake_futex(struct wake_*/ static int wake_futex_pi (u32 __user *uaddr,u32 uval,struct futex_pi_state *pi_state) { -u32 curval, newval;struct rt_mutex_waiter *top_waiter;struct task_struct *new_owner;bool postunlock = false; -DEFINE_WAKE_Q (wake_q); +DEFINE_RT_WAKE_Q (wqh); +u32 curval, newval;int ret =0 ; top_waiter =rt_mutex_top_waiter (&pi_state->pi_mutex);
@@ -1549,14 +1549,14 @@ static int wake_futex_pi(u32 __user *uad* not fail. */ pi_state_update_owner (pi_state, new_owner); - postunlock =__rt_mutex_futex_unlock (&pi_state->pi_mutex, &wake_q); + postunlock =__rt_mutex_futex_unlock (&pi_state->pi_mutex, &wqh); } out_unlock:raw_spin_unlock_irq (&pi_state->pi_mutex.wait_lock);if (postunlock) -rt_mutex_postunlock (&wake_q); +rt_mutex_postunlock (&wqh);return ret; }
kernel/locking/rtmutex.c +6 -6
@@ -1017,7 +1017,7 @@ static int __sched task_blocks_on_rt_mut* * Called with lock->wait_lock held and interrupts disabled. */ -static void __sched mark_wakeup_next_waiter(struct wake_q_head *wake_q, +static void __sched mark_wakeup_next_waiter(struct rt_wake_q_head *wqh,struct rt_mutex_base *lock) {struct rt_mutex_waiter *waiter;
@@ -1054,10 +1054,10 @@ static void __sched mark_wakeup_next_wai* deboost but before waking our donor task, hence the preempt_disable() * before unlock. * -* Pairs with preempt_enable() in rt_mutex_postunlock(); +* Pairs with preempt_enable() in rt_mutex_wake_up_q(); */ preempt_disable (); -wake_q_add (wake_q, waiter->task); +rt_mutex_wake_q_add (wqh, waiter);raw_spin_unlock (¤t->pi_lock); }
@@ -1328,7 +1328,7 @@ static __always_inline int __rt_mutex_tr*/ static void __sched rt_mutex_slowunlock(struct rt_mutex_base *lock) { -DEFINE_WAKE_Q (wake_q); +DEFINE_RT_WAKE_Q (wqh);unsigned long flags;/* irqsave required to support early boot calls */
@@ -1381,10 +1381,10 @@ static void __sched rt_mutex_slowunlock(* * Queue the next waiter for wakeup once we release the wait_lock. */ -mark_wakeup_next_waiter (&wake_q, lock); +mark_wakeup_next_waiter (&wqh, lock);raw_spin_unlock_irqrestore (&lock->wait_lock, flags); -rt_mutex_postunlock (&wake_q); +rt_mutex_wake_up_q (&wqh); } static __always_inline void __rt_mutex_unlock(struct rt_mutex_base *lock)
kernel/locking/rtmutex_api.c +8 -11
@@ -137,10 +137,10 @@ int __sched __rt_mutex_futex_trylock(str* do not use the fast-path, can be simple and will not need to retry. * * @lock: The rt_mutex to be unlocked -* @wake_q: The wake queue head from which to get the next lock waiter +* @wqh: The wake queue head from which to get the next lock waiter */ bool __sched __rt_mutex_futex_unlock(struct rt_mutex_base *lock, -struct wake_q_head *wake_q) +struct rt_wake_q_head *wqh) {lockdep_assert_held (&lock->wait_lock);
@@ -157,23 +157,23 @@ bool __sched __rt_mutex_futex_unlock(str* avoid inversion prior to the wakeup. preempt_disable() * therein pairs with rt_mutex_postunlock(). */ -mark_wakeup_next_waiter (wake_q, lock); +mark_wakeup_next_waiter (wqh, lock);return true;/* call postunlock() */ } void __sched rt_mutex_futex_unlock(struct rt_mutex_base *lock) { -DEFINE_WAKE_Q (wake_q); +DEFINE_RT_WAKE_Q (wqh);unsigned long flags;bool postunlock;raw_spin_lock_irqsave (&lock->wait_lock, flags); - postunlock =__rt_mutex_futex_unlock (lock, &wake_q); + postunlock =__rt_mutex_futex_unlock (lock, &wqh);raw_spin_unlock_irqrestore (&lock->wait_lock, flags);if (postunlock) -rt_mutex_postunlock (&wake_q); +rt_mutex_postunlock (&wqh); }/**
@@ -441,12 +441,9 @@ void __sched rt_mutex_adjust_pi(struct t/* * Performs the wakeup of the top-waiter and re-enables preemption. */ -void __sched rt_mutex_postunlock(struct wake_q_head *wake_q) +void __sched rt_mutex_postunlock(struct rt_wake_q_head *wqh) { -wake_up_q (wake_q); - -/* Pairs with preempt_disable() in mark_wakeup_next_waiter() */ -preempt_enable (); +rt_mutex_wake_up_q (wqh); }#ifdef CONFIG_DEBUG_RT_MUTEXES
kernel/locking/rtmutex_common.h +2 -2
@@ -76,9 +76,9 @@ extern int __rt_mutex_futex_trylock(struextern void rt_mutex_futex_unlock (struct rt_mutex_base *lock);extern bool __rt_mutex_futex_unlock (struct rt_mutex_base *lock, -struct wake_q_head *wake_q); +struct rt_wake_q_head *wqh); -extern void rt_mutex_postunlock (struct wake_q_head *wake_q); +extern void rt_mutex_postunlock (struct rt_wake_q_head *wqh);/* * Must be guarded because this header is included from rcu/tree_plugin.h
PATCH 20/63 locking/rtmutex: Prepare RT rt_mutex_wake_q for RT locks
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Add a rtlock_task pointer to rt_mutex_wake_q which allows to handle the RT specific wakeup for spin/rwlock waiters. The pointer is just consuming 4/8 bytes on stack so it is provided unconditionaly to avoid #ifdeffery all over the place.
No functional change for non-RT enabled kernels.
---
V2: Make it symmetric (PeterZ)
--- include/linux/sched/wake_q.h | 7 ++++++- kernel/locking/rtmutex.c | 12 ++++++++++-- kernel/locking/rtmutex_common.h | 3 +++ kernel/sched/core.c | 4 ++-- 4 files changed, 21 insertions(+), 5 deletions(-)
---
include/linux/sched/wake_q.h +6 -1
@@ -61,6 +61,11 @@ static inline bool wake_q_empty(struct wextern void wake_q_add (struct wake_q_head *head,struct task_struct *task);extern void wake_q_add_safe (struct wake_q_head *head,struct task_struct *task); -extern void wake_up_q (struct wake_q_head *head); +extern void __wake_up_q (struct wake_q_head *head,unsigned int state); + +static inline void wake_up_q (struct wake_q_head *head) +{ +__wake_up_q (head,TASK_NORMAL ); +}#endif /* _LINUX_SCHED_WAKE_Q_H */
kernel/locking/rtmutex.c +10 -2
@@ -351,12 +351,20 @@ static __always_inline void rt_mutex_adj static __always_inline void rt_mutex_wake_q_add(struct rt_wake_q_head *wqh,struct rt_mutex_waiter *w) { -wake_q_add (&wqh->head, w->task); +if (IS_ENABLED (CONFIG_PREEMPT_RT ) && w->wake_state !=TASK_NORMAL ) { +wake_q_add (&wqh->rt_head, w->task); + }else { +wake_q_add (&wqh->head, w->task); + } } static __always_inline void rt_mutex_wake_up_q(struct rt_wake_q_head *wqh) { -wake_up_q (&wqh->head); +if (IS_ENABLED (CONFIG_PREEMPT_RT ) && !wake_q_empty (&wqh->rt_head)) +__wake_up_q (&wqh->rt_head,TASK_RTLOCK_WAIT ); + +if (!wake_q_empty (&wqh->head)) +wake_up_q (&wqh->head);/* Pairs with preempt_disable() in mark_wakeup_next_waiter() */ preempt_enable ();
kernel/locking/rtmutex_common.h +3 -0
@@ -43,14 +43,17 @@ struct rt_mutex_waiter {* rt_wake_q_head - Wrapper around regular wake_q_head to support * "sleeping" spinlocks on RT * @head: The regular wake_q_head for sleeping lock variants +* @rt_head: The wake_q_head for RT lock (spin/rwlock) variants */ struct rt_wake_q_head {struct wake_q_head head; +struct wake_q_head rt_head; };#define DEFINE_RT_WAKE_Q (name) \ struct rt_wake_q_head name = { \ .head = WAKE_Q_HEAD_INITIALIZER(name.head), \ + .rt_head = WAKE_Q_HEAD_INITIALIZER(name.rt_head),\ }/*
kernel/sched/core.c +2 -2
@@ -916,7 +916,7 @@ void wake_q_add_safe(struct wake_q_headput_task_struct (task); } -void wake_up_q (struct wake_q_head *head) +void __wake_up_q (struct wake_q_head *head,unsigned int state) {struct wake_q_node *node = head->first;
@@ -932,7 +932,7 @@ void wake_up_q(struct wake_q_head *head)* wake_up_process() executes a full barrier, which pairs with * the queueing in wake_q_add() so as not to miss wakeups. */ -wake_up_process (task); +wake_up_state (task, state);put_task_struct (task); } }
PATCH 21/63 locking/rtmutex: Guard regular sleeping locks specific functions
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Guard the regular sleeping lock specific functionality which is used for rtmutex on non-RT enabled kernels and for mutex, rtmutex and semaphores on RT enabled kernels so the code can be reused for the RT specific implementation of spinlocks and rwlocks in a different compilation unit.
No functional change.
--- kernel/locking/rtmutex.c | 254 ++++++++++++++++++++++--------------------- kernel/locking/rtmutex_api.c | 1 kernel/locking/rwsem.c | 1 3 files changed, 133 insertions(+), 123 deletions(-)
---
kernel/locking/rtmutex.c +131 -123
@@ -1069,10 +1069,139 @@ static void __sched mark_wakeup_next_wairaw_spin_unlock (¤t->pi_lock); } +static int __sched __rt_mutex_slowtrylock(struct rt_mutex_base *lock) +{ +int ret =try_to_take_rt_mutex (lock, current,NULL ); + +/* +* try_to_take_rt_mutex() sets the lock waiters bit +* unconditionally. Clean this up. +*/ +fixup_rt_mutex_waiters (lock); + +return ret; +} + +/* +* Slow path try-lock function: +*/ +static int __sched rt_mutex_slowtrylock(struct rt_mutex_base *lock) +{ +unsigned long flags; +int ret; + +/* +* If the lock already has an owner we fail to get the lock. +* This can be done without taking the @lock->wait_lock as +* it is only being read, and this is a trylock anyway. +*/ +if (rt_mutex_owner (lock)) +return 0 ; + +/* +* The mutex has currently no owner. Lock the wait lock and try to +* acquire the lock. We use irqsave here to support early boot calls. +*/ +raw_spin_lock_irqsave (&lock->wait_lock, flags); + + ret =__rt_mutex_slowtrylock (lock); + +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); + +return ret; +} + +static __always_inline int __rt_mutex_trylock(struct rt_mutex_base *lock) +{ +if (likely (rt_mutex_cmpxchg_acquire (lock,NULL , current))) +return 1 ; + +return rt_mutex_slowtrylock (lock); +} + +/* +* Slow path to release a rt-mutex. +*/ +static void __sched rt_mutex_slowunlock(struct rt_mutex_base *lock) +{ +DEFINE_RT_WAKE_Q (wqh); +unsigned long flags; + +/* irqsave required to support early boot calls */ +raw_spin_lock_irqsave (&lock->wait_lock, flags); + +debug_rt_mutex_unlock (lock); + +/* +* We must be careful here if the fast path is enabled. If we +* have no waiters queued we cannot set owner to NULL here +* because of: +* +* foo->lock->owner = NULL; +* rtmutex_lock(foo->lock); <- fast path +* free = atomic_dec_and_test(foo->refcnt); +* rtmutex_unlock(foo->lock); <- fast path +* if (free) +* kfree(foo); +* raw_spin_unlock(foo->lock->wait_lock); +* +* So for the fastpath enabled kernel: +* +* Nothing can set the waiters bit as long as we hold +* lock->wait_lock. So we do the following sequence: +* +* owner = rt_mutex_owner(lock); +* clear_rt_mutex_waiters(lock); +* raw_spin_unlock(&lock->wait_lock); +* if (cmpxchg(&lock->owner, owner, 0) == owner) +* return; +* goto retry; +* +* The fastpath disabled variant is simple as all access to +* lock->owner is serialized by lock->wait_lock: +* +* lock->owner = NULL; +* raw_spin_unlock(&lock->wait_lock); +*/ +while (!rt_mutex_has_waiters (lock)) { +/* Drops lock->wait_lock ! */ +if (unlock_rt_mutex_safe (lock, flags) == true) +return ; +/* Relock the rtmutex and try again */ +raw_spin_lock_irqsave (&lock->wait_lock, flags); + } + +/* +* The wakeup next waiter path does not suffer from the above +* race. See the comments there. +* +* Queue the next waiter for wakeup once we release the wait_lock. +*/ +mark_wakeup_next_waiter (&wqh, lock); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); + +rt_mutex_wake_up_q (&wqh); +} + +static __always_inline void __rt_mutex_unlock(struct rt_mutex_base *lock) +{ +if (likely (rt_mutex_cmpxchg_release (lock, current,NULL ))) +return ; + +rt_mutex_slowunlock (lock); +} + +#ifdef RT_MUTEX_BUILD_MUTEX +/* +* Functions required for: +* - rtmutex, futex on all kernels +* - mutex and rwsem substitutions on RT kernels +*/ +/* * Remove a waiter from a lock and give up * -* Must be called with lock->wait_lock held and interrupts disabled. I must +* Must be called with lock->wait_lock held and interrupts disabled. It must * have just failed to try_to_take_rt_mutex(). */ static void __sched remove_waiter(struct rt_mutex_base *lock,
@@ -1280,125 +1409,4 @@ static __always_inline int __rt_mutex_lo return rt_mutex_slowlock(lock, state); } - -static int __sched __rt_mutex_slowtrylock(struct rt_mutex_base *lock) -{ -int ret =try_to_take_rt_mutex (lock, current,NULL ); - -/* -* try_to_take_rt_mutex() sets the lock waiters bit -* unconditionally. Clean this up. -*/ -fixup_rt_mutex_waiters (lock); - -return ret; -} - -/* -* Slow path try-lock function: -*/ -static int __sched rt_mutex_slowtrylock(struct rt_mutex_base *lock) -{ -unsigned long flags; -int ret; - -/* -* If the lock already has an owner we fail to get the lock. -* This can be done without taking the @lock->wait_lock as -* it is only being read, and this is a trylock anyway. -*/ -if (rt_mutex_owner (lock)) -return 0 ; - -/* -* The mutex has currently no owner. Lock the wait lock and try to -* acquire the lock. We use irqsave here to support early boot calls. -*/ -raw_spin_lock_irqsave (&lock->wait_lock, flags); - - ret =__rt_mutex_slowtrylock (lock); - -raw_spin_unlock_irqrestore (&lock->wait_lock, flags); - -return ret; -} - -static __always_inline int __rt_mutex_trylock(struct rt_mutex_base *lock) -{ -if (likely (rt_mutex_cmpxchg_acquire (lock,NULL , current))) -return 1 ; - -return rt_mutex_slowtrylock (lock); -} - -/* -* Slow path to release a rt-mutex. -*/ -static void __sched rt_mutex_slowunlock(struct rt_mutex_base *lock) -{ -DEFINE_RT_WAKE_Q (wqh); -unsigned long flags; - -/* irqsave required to support early boot calls */ -raw_spin_lock_irqsave (&lock->wait_lock, flags); - -debug_rt_mutex_unlock (lock); - -/* -* We must be careful here if the fast path is enabled. If we -* have no waiters queued we cannot set owner to NULL here -* because of: -* -* foo->lock->owner = NULL; -* rtmutex_lock(foo->lock); <- fast path -* free = atomic_dec_and_test(foo->refcnt); -* rtmutex_unlock(foo->lock); <- fast path -* if (free) -* kfree(foo); -* raw_spin_unlock(foo->lock->wait_lock); -* -* So for the fastpath enabled kernel: -* -* Nothing can set the waiters bit as long as we hold -* lock->wait_lock. So we do the following sequence: -* -* owner = rt_mutex_owner(lock); -* clear_rt_mutex_waiters(lock); -* raw_spin_unlock(&lock->wait_lock); -* if (cmpxchg(&lock->owner, owner, 0) == owner) -* return; -* goto retry; -* -* The fastpath disabled variant is simple as all access to -* lock->owner is serialized by lock->wait_lock: -* -* lock->owner = NULL; -* raw_spin_unlock(&lock->wait_lock); -*/ -while (!rt_mutex_has_waiters (lock)) { -/* Drops lock->wait_lock ! */ -if (unlock_rt_mutex_safe (lock, flags) == true) -return ; -/* Relock the rtmutex and try again */ -raw_spin_lock_irqsave (&lock->wait_lock, flags); - } - -/* -* The wakeup next waiter path does not suffer from the above -* race. See the comments there. -* -* Queue the next waiter for wakeup once we release the wait_lock. -*/ -mark_wakeup_next_waiter (&wqh, lock); -raw_spin_unlock_irqrestore (&lock->wait_lock, flags); - -rt_mutex_wake_up_q (&wqh); -} - -static __always_inline void __rt_mutex_unlock(struct rt_mutex_base *lock) -{ -if (likely (rt_mutex_cmpxchg_release (lock, current,NULL ))) -return ; - -rt_mutex_slowunlock (lock); -} +#endif /* RT_MUTEX_BUILD_MUTEX */
kernel/locking/rtmutex_api.c +1 -0
@@ -5,6 +5,7 @@#include <linux/spinlock.h> #include <linux/export.h> +#define RT_MUTEX_BUILD_MUTEX #include "rtmutex.c" /*
kernel/locking/rwsem.c +1 -0
@@ -1347,6 +1347,7 @@ static inline void __downgrade_write(str#else /* !CONFIG_PREEMPT_RT */ +#define RT_MUTEX_BUILD_MUTEX #include "rtmutex.c" #define rwbase_set_and_save_current_state (state) \
PATCH 22/63 locking/spinlock: Split the lock types header
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Move raw_spinlock into its own file. Prepare for RT 'sleeping spinlocks' to avoid header recursion as RT locks require rtmutex.h which in turn requires the raw spinlock types.
No functional change.
--- include/linux/rwlock_types.h | 4 ++ include/linux/spinlock.h | 4 ++ include/linux/spinlock_types.h | 19 ---------- include/linux/spinlock_types_raw.h | 65 +++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 include/linux/spinlock_types_raw.h
---
include/linux/rwlock_types.h +4 -0
@@ -1,6 +1,10 @@#ifndef __LINUX_RWLOCK_TYPES_H#define __LINUX_RWLOCK_TYPES_H +#if !defined(__LINUX_SPINLOCK_TYPES_H) +# error "Do not include directly, include spinlock_types.h" +#endif +/* * include/linux/rwlock_types.h - generic rwlock type definitions * and initializers
include/linux/spinlock.h +4 -0
@@ -12,6 +12,8 @@* asm/spinlock_types.h: contains the arch_spinlock_t/arch_rwlock_t and the * initializers * +* linux/spinlock_types_raw: +* The raw types and initializers * linux/spinlock_types.h: * defines the generic type and initializers *
@@ -31,6 +33,8 @@* contains the generic, simplified UP spinlock type. * (which is an empty structure on non-debug builds) * +* linux/spinlock_types_raw: +* The raw RT types and initializers * linux/spinlock_types.h: * defines the generic type and initializers *
include/linux/spinlock_types.h +1 -18
@@ -9,24 +9,7 @@* Released under the General Public License (GPL). */ -#if defined(CONFIG_SMP ) -# include <asm/spinlock_types.h> -#else -# include <linux/spinlock_types_up.h> -#endif - -#include <linux/lockdep_types.h> - -typedef struct raw_spinlock { -arch_spinlock_t raw_lock; -#ifdef CONFIG_DEBUG_SPINLOCK -unsigned int magic, owner_cpu; -void *owner; -#endif -#ifdef CONFIG_DEBUG_LOCK_ALLOC -struct lockdep_map dep_map; -#endif -}raw_spinlock_t ; +#include <linux/spinlock_types_raw.h> #define SPINLOCK_MAGIC 0xdead4ead
include/linux/spinlock_types_raw.h new file +65 -0
65 lines not on this page · raw message
Re: [patch 22/63] locking/spinlock: Split the lock types header
Quoted 68 lines · Thomas Gleixner
From: Thomas Gleixner <[email protected]>
Move
raw_spinlockinto its own file. Prepare for RT 'sleeping spinlocks' to avoid header recursion as RT locks requirertmutex.hwhich in turn requires the raw spinlock types.No functional change.
Signed-off-by Thomas Gleixner <[email protected]>--- include/linux/rwlock_types.h | 4 ++ include/linux/spinlock.h | 4 ++ include/linux/spinlock_types.h | 19 ---------- include/linux/spinlock_types_raw.h | 65 +++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 18 deletions(-)create mode 100644
include/linux/spinlock_types_raw.h
---include/linux/rwlock_types.h +4
-0@@ -1,6 +1,10 @@#ifndef __LINUX_RWLOCK_TYPES_H#define __LINUX_RWLOCK_TYPES_H +#if !defined(__LINUX_SPINLOCK_TYPES_H) +# error "Do not include directly, include spinlock_types.h" +#endif +/* * include/linux/rwlock_types.h - generic rwlock type definitions * and initializers include/linux/spinlock.h +4
-0@@ -12,6 +12,8 @@* asm/spinlock_types.h: contains the arch_spinlock_t/arch_rwlock_t and the * initializers * +* linux/spinlock_types_raw: +* The raw types and initializers * linux/spinlock_types.h: * defines the generic type and initializers * @@ -31,6 +33,8 @@* contains the generic, simplified UP spinlock type. * (which is an empty structure on non-debug builds) * +* linux/spinlock_types_raw: +* The raw RT types and initializers * linux/spinlock_types.h: * defines the generic type and initializers * include/linux/spinlock_types.h +0
-12@@ -9,24 +9,7 @@* Released under the General Public License (GPL). */ -#if defined(CONFIG_SMP ) -# include <asm/spinlock_types.h> -#else -# include <linux/spinlock_types_up.h> -#endif - -#include <linux/lockdep_types.h> - -typedef struct raw_spinlock { - arch_spinlock_t raw_lock; -#ifdef CONFIG_DEBUG_SPINLOCK - unsigned int magic, owner_cpu;
include/linux/spinlock_types.h line 15
Quoted 10 lines
include/linux/spinlock_types.h +1
-6-void *owner; -#endif -#ifdef CONFIG_DEBUG_LOCK_ALLOC -struct lockdep_map dep_map; -#endif -} raw_spinlock_t; +#include <linux/spinlock_types_raw.h> #define SPINLOCK_MAGIC 0xdead4ead
Most of the code in spinlock_types.h are moved into spinlock_types_raw.h. However, macros like SPINLOCK_MAGIC and those that followed are not removed from spinlock_types.h in this patch leading to the same set of macro definitions in two different files. Should we eliminate the duplicate macro definitions either from spinlock_types.h or from spinlock_types_raw.h?
Cheers,
Longman
Re: [patch 22/63] locking/spinlock: Split the lock types header
Quoted 10 lines · Waiman Long
Quoted 2 lines · Thomas Gleixner
From: Thomas Gleixner <[email protected]>
Most of the code in
spinlock_types.hare moved intospinlock_types_raw.h. However, macros likeSPINLOCK_MAGICand those that followed are not removed fromspinlock_types.hin this patch leading to the same set of macro definitions in two different files. Should we eliminate the duplicate macro definitions either fromspinlock_types.hor fromspinlock_types_raw.h?
Right you are.
PATCH 23/63 locking/rtmutex: Prevent future include recursion hell
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
rtmutex only needs raw_spinlock_t, but it includes spinlock_types.h which is not a problem on an non RT enabled kernel.
RT kernels substitute regular spinlocks with 'sleeping' spinlocks which are based on rtmutexes and therefore must be able to include rtmutex.h.
Include spinlock_types_raw.h instead.
--- include/linux/rtmutex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
---
include/linux/rtmutex.h +1 -1
@@ -15,7 +15,7 @@#include <linux/linkage.h> #include <linux/rbtree.h> -#include <linux/spinlock_types.h> +#include <linux/spinlock_types_raw.h> extern int max_lock_depth;/* for sysctl */
PATCH 24/63 locking/lockdep: Reduce includes in debug_locks.h
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
The inclusion of printk.h leads to a circular dependency if spinlock_t is based on rtmutexes on RT enabled kernels.
Include only atomic.h (xchg()) and cache.h (__read_mostly) which is all what debug_locks.h requires.
--- include/linux/debug_locks.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
---
include/linux/debug_locks.h +1 -2
@@ -3,8 +3,7 @@#define __LINUX_DEBUG_LOCKING_H#include <linux/atomic.h> -#include <linux/bug.h> -#include <linux/printk.h> +#include <linux/cache.h> struct task_struct ;
PATCH 25/63 rbtree: Split out the rbtree type definitions
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
rtmutex.h needs the definition of struct rb_root_cached. rbtree.h includes kernel.h which includes spinlock.h. That works nicely for non-RT enabled kernels, but on RT enabled kernels spinlocks are based on rtmutexes which creates another circular header dependency as spinlocks.h will require rtmutex.h.
Split out the type definitions and move them into their own header file so the rtmutex header can include just those.
--- include/linux/rbtree.h | 30 +----------------------------- include/linux/rbtree_types.h | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 29 deletions(-) create mode 100644 include/linux/rbtree_types.h
---
include/linux/rbtree.h +1 -29
@@ -19,22 +19,11 @@#include <linux/kernel.h> #include <linux/stddef.h> +#include <linux/rbtree_types.h> #include <linux/rcupdate.h> -struct rb_node { -unsigned long __rb_parent_color; -struct rb_node *rb_right; -struct rb_node *rb_left; -} __attribute__((aligned (sizeof (long )))); -/* The alignment might seem pointless, but allegedly CRIS needs it */ - -struct rb_root { -struct rb_node *rb_node; -}; -#define rb_parent (r) ((struct rb_node *)((r)->__rb_parent_color & ~3)) -#define RB_ROOT (struct rb_root) { NULL, }#define rb_entry (ptr, type, member) container_of(ptr, type, member)#define RB_EMPTY_ROOT (root) (READ_ONCE((root)->rb_node) == NULL)
@@ -112,23 +101,6 @@ static inline void rb_link_node_rcu(stru typeof(*pos), field); 1; }); \ pos = n) -/* -* Leftmost-cached rbtrees. -* -* We do not cache the rightmost node based on footprint -* size vs number of potential users that could benefit -* from O(1) rb_last(). Just not worth it, users that want -* this feature can always implement the logic explicitly. -* Furthermore, users that want to cache both pointers may -* find it a bit asymmetric, but that's ok. -*/ -struct rb_root_cached { -struct rb_root rb_root; -struct rb_node *rb_leftmost; -}; - -#define RB_ROOT_CACHED (struct rb_root_cached) { {NULL, }, NULL } -/* Same as rb_first(), but O(1) */ #define rb_first_cached (root) (root)->rb_leftmost
include/linux/rbtree_types.h new file +34 -0
@@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#ifndef _LINUX_RBTREE_TYPES_H +#define _LINUX_RBTREE_TYPES_H + +struct rb_node { +unsigned long __rb_parent_color; +struct rb_node *rb_right; +struct rb_node *rb_left; +} __attribute__((aligned (sizeof (long )))); +/* The alignment might seem pointless, but allegedly CRIS needs it */ + +struct rb_root { +struct rb_node *rb_node; +}; + +/* +* Leftmost-cached rbtrees. +* +* We do not cache the rightmost node based on footprint +* size vs number of potential users that could benefit +* from O(1) rb_last(). Just not worth it, users that want +* this feature can always implement the logic explicitly. +* Furthermore, users that want to cache both pointers may +* find it a bit asymmetric, but that's ok. +*/ +struct rb_root_cached { +struct rb_root rb_root; +struct rb_node *rb_leftmost; +}; + +#define RB_ROOT (struct rb_root) { NULL, } +#define RB_ROOT_CACHED (struct rb_root_cached) { {NULL, }, NULL } + +#endif
PATCH 26/63 locking/rtmutex: Include only rbtree types
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
rtmutex.h needs the definition of struct rb_root_cached. rbtree.h includes kernel.h which includes spinlock.h. That works nicely for non-RT enabled kernels, but on RT enabled kernels spinlocks are based on rtmutexes which creates another circular header dependency as spinlocks.h will require rtmutex.h.
Include rbtree_types.h instead.
--- include/linux/rtmutex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
---
include/linux/rtmutex.h +1 -1
@@ -14,7 +14,7 @@#define __LINUX_RT_MUTEX_H#include <linux/linkage.h> -#include <linux/rbtree.h> +#include <linux/rbtree_types.h> #include <linux/spinlock_types_raw.h> extern int max_lock_depth;/* for sysctl */
PATCH 27/63 locking/spinlock: Provide RT specific spinlock type
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
RT replaces spinlocks with a simple wrapper around a rtmutex which turns spinlocks on RT into 'sleeping' spinlocks. The actual implementation of the spinlock API differs from a regular rtmutex as it does neither handle timeouts nor signals and it is state preserving accross the lock operation.
--- include/linux/spinlock_types.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
---
include/linux/spinlock_types.h +26 -0
@@ -51,6 +51,9 @@#define DEFINE_RAW_SPINLOCK (x) raw_spinlock_t x = __RAW_SPIN_LOCK_UNLOCKED(x) +#ifndef CONFIG_PREEMPT_RT + +/* Non PREEMPT_RT kernels map spinlock to raw_spinlock */ typedef struct spinlock { union { struct raw_spinlock rlock;
@@ -79,6 +82,29 @@ typedef struct spinlock {#define DEFINE_SPINLOCK (x) spinlock_t x = __SPIN_LOCK_UNLOCKED(x) +#else /* !CONFIG_PREEMPT_RT */ + +/* PREEMPT_RT kernels map spinlock to rt_mutex */ +#include <linux/rtmutex.h> + +typedef struct spinlock { +struct rt_mutex_base lock; +#ifdef CONFIG_DEBUG_LOCK_ALLOC +struct lockdep_map dep_map; +#endif +}spinlock_t ; + +#define __SPIN_LOCK_UNLOCKED (name) \ + { \ + .lock = __RT_MUTEX_BASE_INITIALIZER(name.lock), \ + SPIN_DEP_MAP_INIT(name) \ + } + +#define DEFINE_SPINLOCK (name) \ + spinlock_t name = __SPIN_LOCK_UNLOCKED(name) + +#endif /* CONFIG_PREEMPT_RT */ +#include <linux/rwlock_types.h> #endif /* __LINUX_SPINLOCK_TYPES_H */
PATCH 28/63 locking/spinlock: Provide RT variant header
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Provide the necessary wrappers around the actual rtmutex based spinlock implementation.
--- include/linux/spinlock.h | 11 ++ include/linux/spinlock_api_smp.h | 3 include/linux/spinlock_rt.h | 149 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 include/linux/spinlock_rt.h
---
include/linux/spinlock.h +10 -1
@@ -312,8 +312,10 @@ static inline void do_raw_spin_unlock(ra 1 : ({ local_irq_restore(flags); 0; }); \ }) -/* Include rwlock functions */ +#ifndef CONFIG_PREEMPT_RT +/* Include rwlock functions for !RT */ #include <linux/rwlock.h> +#endif /* * Pull the _spin_*()/_read_*()/_write_*() functions/declarations:
@@ -324,6 +326,9 @@ static inline void do_raw_spin_unlock(ra# include <linux/spinlock_api_up.h> #endif +/* Non PREEMPT_RT kernel map to raw spinlocks */ +#ifndef CONFIG_PREEMPT_RT +/* * Map the spin_lock functions to the raw variants for PREEMPT_RT=n */
@@ -458,6 +463,10 @@ static __always_inline int spin_is_conte#define assert_spin_locked (lock) assert_raw_spin_locked(&(lock)->rlock) +#else /* !CONFIG_PREEMPT_RT */ +# include <linux/spinlock_rt.h> +#endif /* CONFIG_PREEMPT_RT */ +/* * Pull the atomic_t declaration: * (asm-mips/atomic.h needs above definitions)
include/linux/spinlock_api_smp.h +3 -0
@@ -187,6 +187,9 @@ static inline int __raw_spin_trylock_bh( return 0; } +/* PREEMPT_RT has it's own rwlock implementation */ +#ifndef CONFIG_PREEMPT_RT #include <linux/rwlock_api_smp.h> +#endif #endif /* __LINUX_SPINLOCK_API_SMP_H */
include/linux/spinlock_rt.h new file +149 -0
149 lines not on this page · raw message
PATCH 29/63 locking/rtmutex: Provide the spin/rwlock core lock function
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
A simplified version of the rtmutex slowlock function which neither handles signals nor timeouts and is careful about preserving the state of the blocked task accross the lock operation.
--- kernel/locking/rtmutex.c | 60 ++++++++++++++++++++++++++++++++++++++++ kernel/locking/rtmutex_common.h | 2 - 2 files changed, 61 insertions(+), 1 deletion(-)
---
kernel/locking/rtmutex.c +60 -0
@@ -1410,3 +1410,63 @@ static __always_inline int __rt_mutex_loreturn rt_mutex_slowlock (lock, state); }#endif /* RT_MUTEX_BUILD_MUTEX */ + +#ifdef RT_MUTEX_BUILD_SPINLOCKS +/* +* Functions required for spin/rw_lock substitution on RT kernels +*/ + +/** +* rtlock_slowlock_locked - Slow path lock acquisition for RT locks +* @lock: The underlying rt mutex +*/ +static void __sched rtlock_slowlock_locked(struct rt_mutex_base *lock) +{ +struct rt_mutex_waiter waiter; + +lockdep_assert_held (&lock->wait_lock); + +if (try_to_take_rt_mutex (lock, current,NULL )) +return ; + +rt_mutex_init_rtlock_waiter (&waiter); + +/* Save current state and set state to TASK_RTLOCK_WAIT */ +current_save_and_set_rtlock_wait_state (); + +task_blocks_on_rt_mutex (lock, &waiter, current,RT_MUTEX_MIN_CHAINWALK ); + +for (;;) { +/* Try to acquire the lock again. */ +if (try_to_take_rt_mutex (lock, current, &waiter)) +break ; + +raw_spin_unlock_irq (&lock->wait_lock); + +schedule_rtlock (); + +raw_spin_lock_irq (&lock->wait_lock); +set_current_state (TASK_RTLOCK_WAIT ); + } + +/* Restore the task state */ +current_restore_rtlock_saved_state (); + +/* +* try_to_take_rt_mutex() sets the waiter bit unconditionally. We +* might have to fix that up: +*/ +fixup_rt_mutex_waiters (lock); +debug_rt_mutex_free_waiter (&waiter); +} + +static __always_inline void __sched rtlock_slowlock(struct rt_mutex_base *lock) +{ +unsigned long flags; + +raw_spin_lock_irqsave (&lock->wait_lock, flags); +rtlock_slowlock_locked (lock); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +} + +#endif /* RT_MUTEX_BUILD_SPINLOCKS */
kernel/locking/rtmutex_common.h +1 -1
@@ -181,7 +181,7 @@ static inline void rt_mutex_init_waiter( waiter->task =NULL ; } -static inline void rtlock_init_rtmutex_waiter (struct rt_mutex_waiter *waiter) +static inline void rt_mutex_init_rtlock_waiter (struct rt_mutex_waiter *waiter) {rt_mutex_init_waiter (waiter); waiter->wake_state =TASK_RTLOCK_WAIT ;
PATCH 30/63 locking/spinlock: Provide RT variant
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Provide the actual locking functions which make use of the general and spinlock specific rtmutex code.
--- kernel/locking/Makefile | 1 kernel/locking/spinlock_rt.c | 128 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 kernel/locking/spinlock_rt.c
---
kernel/locking/Makefile +1 -0
@@ -25,6 +25,7 @@ obj-$(CONFIG_LOCK_SPIN_ON_OWNER) += osq_ obj-$(CONFIG_PROVE_LOCKING) += spinlock.o obj-$(CONFIG_QUEUED_SPINLOCKS) += qspinlock.o obj-$(CONFIG_RT_MUTEXES) += rtmutex_api.o +obj-$(CONFIG_PREEMPT_RT) += spinlock_rt.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock_debug.o obj-$(CONFIG_QUEUED_RWLOCKS) += qrwlock.o
kernel/locking/spinlock_rt.c new file +128 -0
128 lines not on this page · raw message
Re: [patch 30/63] locking/spinlock: Provide RT variant
Quoted 29 lines · Thomas Gleixner
From: Thomas Gleixner <[email protected]>
Provide the actual locking functions which make use of the general and spinlock specific rtmutex code.
Signed-off-by Thomas Gleixner <[email protected]>--- kernel/locking/Makefile | 1 kernel/locking/spinlock_rt.c | 128 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+)create mode 100644
kernel/locking/spinlock_rt.c
---kernel/locking/Makefile +1
-0@@ -25,6 +25,7 @@ obj-$(CONFIG_LOCK_SPIN_ON_OWNER) += osq_ obj-$(CONFIG_PROVE_LOCKING) += spinlock.o obj-$(CONFIG_QUEUED_SPINLOCKS) += qspinlock.o obj-$(CONFIG_RT_MUTEXES) += rtmutex_api.o +obj-$(CONFIG_PREEMPT_RT) += spinlock_rt.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock_debug.o obj-$(CONFIG_QUEUED_RWLOCKS) += qrwlock.okernel/locking/spinlock_rt.c new file +4
-0@@ -0,0 +1,128 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* +* PREEMPT_RT substitution for spin/rw_locks +*
kernel/locking/spinlock_rt.c line 14
Quoted 10 lines
kernel/locking/spinlock_rt.c new file +10
-0+* spin_lock and rw_lock on RT are based on rtmutex with a few twists to +* resemble the non RT semantics +* +* - Contrary to a plain rtmutex, spin_lock and rw_lock are state +* preserving. The task state is saved before blocking on the underlying +* rtmutex and restored when the lock has been acquired. Regular wakeups +* during that time are redirected to the saved state so no wake up is +* missed. +* +* - Non RT spin/rw_locks disable preemption and evtl. interrupts.
"evtl."?
Cheers,
Longman
Re: [patch 30/63] locking/spinlock: Provide RT variant
Quoted 7 lines · Waiman Long
Quoted 4 lines · Thomas Gleixner
From: Thomas Gleixner <[email protected]>
+ * missed.
+ *
+ * - Non RT spin/rw_locks disable preemption and evtl. interrupts."evtl."?
German abbreviation for eventually. Let me make that understandable for non .de folks.
Thanks,
tglx
PATCH 31/63 locking/rwlock: Provide RT variant
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Similar to rw_semaphores on RT the rwlock substitution is not writer fair because it's not feasible to have a writer inherit it's priority to multiple readers. Readers blocked on a writer follow the normal rules of priority inheritance. Like RT spinlocks RT rwlocks are state preserving accross the slow lock operations (contended case).
--- include/linux/rwlock_rt.h | 140 ++++++++++++++++++++++++++++++++++++++++ include/linux/rwlock_types.h | 35 +++++++++- include/linux/spinlock_rt.h | 2 kernel/Kconfig.locks | 2 kernel/locking/spinlock.c | 7 ++ kernel/locking/spinlock_debug.c | 5 + kernel/locking/spinlock_rt.c | 129 ++++++++++++++++++++++++++++++++++++ 7 files changed, 317 insertions(+), 3 deletions(-) create mode 100644 include/linux/rwlock_rt.h
---
include/linux/rwlock_rt.h new file +140 -0
@@ -0,0 +1,140 @@ +// SPDX-License-Identifier: GPL-2.0-only +#ifndef __LINUX_RWLOCK_RT_H +#define __LINUX_RWLOCK_RT_H + +#ifndef __LINUX_SPINLOCK_RT_H +#error Do not include directly. Use spinlock.h +#endif + +#ifdef CONFIG_DEBUG_LOCK_ALLOC +extern void __rt_rwlock_init (rwlock_t *rwlock,const char *name, +struct lock_class_key *key); +#else +static inline void __rt_rwlock_init (rwlock_t *rwlock,char *name, +struct lock_class_key *key) +{ +} +#endif + +#define rwlock_init (rwl) \ +do { \ + static struct lock_class_key __key; \ + \ + init_rwbase_rt(&(rwl)->rwbase); \ + __rt_rwlock_init(rwl, #rwl, &__key); \ +} while (0) + +extern void rt_read_lock (rwlock_t *rwlock); +extern int rt_read_trylock (rwlock_t *rwlock); +extern void rt_read_unlock (rwlock_t *rwlock); +extern void rt_write_lock (rwlock_t *rwlock); +extern int rt_write_trylock (rwlock_t *rwlock); +extern void rt_write_unlock (rwlock_t *rwlock); + +static __always_inline void read_lock(rwlock_t *rwlock) +{ +rt_read_lock (rwlock); +} + +static __always_inline void read_lock_bh(rwlock_t *rwlock) +{ +local_bh_disable (); +rt_read_lock (rwlock); +} + +static __always_inline void read_lock_irq(rwlock_t *rwlock) +{ +rt_read_lock (rwlock); +} + +#define read_lock_irqsave (lock, flags) \ + do { \ + typecheck(unsigned long, flags); \ + rt_read_lock(lock); \ + flags = 0; \ + } while (0) + +#define read_trylock (lock) __cond_lock(lock, rt_read_trylock(lock)) + +static __always_inline void read_unlock(rwlock_t *rwlock) +{ +rt_read_unlock (rwlock); +} + +static __always_inline void read_unlock_bh(rwlock_t *rwlock) +{ +rt_read_unlock (rwlock); +local_bh_enable (); +} + +static __always_inline void read_unlock_irq(rwlock_t *rwlock) +{ +rt_read_unlock (rwlock); +} + +static __always_inline void read_unlock_irqrestore(rwlock_t *rwlock, +unsigned long flags) +{ +rt_read_unlock (rwlock); +} + +static __always_inline void write_lock(rwlock_t *rwlock) +{ +rt_write_lock (rwlock); +} + +static __always_inline void write_lock_bh(rwlock_t *rwlock) +{ +local_bh_disable (); +rt_write_lock (rwlock); +} + +static __always_inline void write_lock_irq(rwlock_t *rwlock) +{ +rt_write_lock (rwlock); +} + +#define write_lock_irqsave (lock, flags) \ + do { \ + typecheck(unsigned long, flags); \ + rt_write_lock(lock); \ + flags = 0; \ + } while (0) + +#define write_trylock (lock) __cond_lock(lock, rt_write_trylock(lock)) + +#define write_trylock_irqsave (lock, flags) \ +({ \ + int __locked; \ + \ + typecheck(unsigned long, flags); \ + flags = 0; \ + __locked = write_trylock(lock); \ + __locked; \ +}) + +static __always_inline void write_unlock(rwlock_t *rwlock) +{ +rt_write_unlock (rwlock); +} + +static __always_inline void write_unlock_bh(rwlock_t *rwlock) +{ +rt_write_unlock (rwlock); +local_bh_enable (); +} + +static __always_inline void write_unlock_irq(rwlock_t *rwlock) +{ +rt_write_unlock (rwlock); +} + +static __always_inline void write_unlock_irqrestore(rwlock_t *rwlock, +unsigned long flags) +{ +rt_write_unlock (rwlock); +} + +#define rwlock_is_contended (lock) (((void)(lock), 0)) + +#endif
include/linux/rwlock_types.h +33 -2
@@ -5,9 +5,9 @@# error "Do not include directly, include spinlock_types.h"#endif +#ifndef CONFIG_PREEMPT_RT /* -* include/linux/rwlock_types.h - generic rwlock type definitions -* and initializers +* generic rwlock type definitions and initializers * * portions Copyright 2005, Red Hat, Inc., Ingo Molnar * Released under the General Public License (GPL).
@@ -50,4 +50,35 @@ typedef struct {#define DEFINE_RWLOCK (x) rwlock_t x = __RW_LOCK_UNLOCKED(x) +#else /* !CONFIG_PREEMPT_RT */ + +#include <linux/rwbase_rt.h> + +typedef struct { +struct rwbase_rt rwbase; +atomic_t readers; +#ifdef CONFIG_DEBUG_LOCK_ALLOC +struct lockdep_map dep_map; +#endif +}rwlock_t ; + +#ifdef CONFIG_DEBUG_LOCK_ALLOC +# define RW_DEP_MAP_INIT (lockname) .dep_map = { .name = #lockname } +#else +# define RW_DEP_MAP_INIT (lockname) +#endif + +#define __RW_LOCK_UNLOCKED (name) __RWLOCK_RT_INITIALIZER(name) + +#define DEFINE_RWLOCK (name) \ + rwlock_t name = __RW_LOCK_UNLOCKED(name) + +#define __RWLOCK_RT_INITIALIZER (name) \ +{ \ + .rwbase = __RWBASE_INITIALIZER(name), \ + RW_DEP_MAP_INIT(name) \ +} + +#endif /* CONFIG_PREEMPT_RT */ +#endif /* __LINUX_RWLOCK_TYPES_H */
include/linux/spinlock_rt.h +2 -0
@@ -146,4 +146,6 @@ static inline int spin_is_locked(spinloc#define assert_spin_locked (lock) BUG_ON(!spin_is_locked(lock)) +#include <linux/rwlock_rt.h> +#endif
kernel/Kconfig.locks +1 -1
@@ -251,7 +251,7 @@ config ARCH_USE_QUEUED_RWLOCKS config QUEUED_RWLOCKS def_bool y if ARCH_USE_QUEUED_RWLOCKS - depends on SMP + depends on SMP && !PREEMPT_RT config ARCH_HAS_MMIOWB bool
kernel/locking/spinlock.c +7 -0
@@ -124,8 +124,11 @@ void __lockfunc __raw_##op##_lock_bh(loc* __[spin|read|write]_lock_bh() */ BUILD_LOCK_OPS (spin, raw_spinlock); + +#ifndef CONFIG_PREEMPT_RT BUILD_LOCK_OPS (read, rwlock);BUILD_LOCK_OPS (write, rwlock); +#endif #endif
@@ -209,6 +212,8 @@ void __lockfunc _raw_spin_unlock_bh(raw_EXPORT_SYMBOL (_raw_spin_unlock_bh);#endif +#ifndef CONFIG_PREEMPT_RT + #ifndef CONFIG_INLINE_READ_TRYLOCK int __lockfunc _raw_read_trylock(rwlock_t *lock) {
@@ -353,6 +358,8 @@ void __lockfunc _raw_write_unlock_bh(rwlEXPORT_SYMBOL (_raw_write_unlock_bh);#endif +#endif /* !CONFIG_PREEMPT_RT */ +#ifdef CONFIG_DEBUG_LOCK_ALLOC void __lockfunc _raw_spin_lock_nested(raw_spinlock_t *lock, int subclass)
kernel/locking/spinlock_debug.c +5 -0
@@ -31,6 +31,7 @@ void __raw_spin_lock_init(raw_spinlock_tEXPORT_SYMBOL (__raw_spin_lock_init); +#ifndef CONFIG_PREEMPT_RT void __rwlock_init (rwlock_t *lock,const char *name,struct lock_class_key *key) {
@@ -48,6 +49,7 @@ void __rwlock_init(rwlock_t *lock, const }EXPORT_SYMBOL (__rwlock_init); +#endif static void spin_dump (raw_spinlock_t *lock,const char *msg) {
@@ -139,6 +141,7 @@ void do_raw_spin_unlock(raw_spinlock_t *arch_spin_unlock (&lock->raw_lock); } +#ifndef CONFIG_PREEMPT_RT static void rwlock_bug (rwlock_t *lock,const char *msg) {if (!debug_locks_off ())
@@ -228,3 +231,5 @@ void do_raw_write_unlock(rwlock_t *lock)debug_write_unlock (lock); arch_write_unlock(&lock->raw_lock); } + +#endif /* !CONFIG_PREEMPT_RT */
kernel/locking/spinlock_rt.c +129 -0
132 lines not on this page · raw message
PATCH 32/63 locking/mutex: Consolidate core headers
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Having two header files which contain just the non-debug and debug variants is mostly waste of disc space and has no real value. Stick the debug variants into the common mutex.h file as counterpart to the stubs for the non-debug case.
That allows to add helpers and defines to the common header for the upcoming handling of mutexes and ww_mutexes on PREEMPT_RT.
--- kernel/locking/mutex-debug.c | 4 +--- kernel/locking/mutex-debug.h | 29 ----------------------------- kernel/locking/mutex.c | 6 +----- kernel/locking/mutex.h | 20 +++++++++++++++++--- 4 files changed, 19 insertions(+), 40 deletions(-) delete mode 100644 kernel/locking/mutex-debug.h
---
kernel/locking/mutex-debug.c +1 -3
@@ -1,6 +1,4 @@/* -* kernel/mutex-debug.c -* * Debugging code for mutexes * * Started by Ingo Molnar:
@@ -22,7 +20,7 @@#include <linux/interrupt.h> #include <linux/debug_locks.h> -#include "mutex-debug.h" +#include "mutex.h" /* * Must be called with lock->wait_lock held.
kernel/locking/mutex-debug.h deleted +0 -29
@@ -1,29 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* -* Mutexes: blocking mutual exclusion locks -* -* started by Ingo Molnar: -* -* Copyright (C) 2004, 2005, 2006 Red Hat, Inc., Ingo Molnar <[email protected]> -* -* This file contains mutex debugging related internal declarations, -* prototypes and inline functions, for the CONFIG_DEBUG_MUTEXES case. -* More details are in kernel/mutex-debug.c. -*/ - -/* -* This must be called with lock->wait_lock held. -*/ -extern void debug_mutex_lock_common (struct mutex *lock, -struct mutex_waiter *waiter); -extern void debug_mutex_wake_waiter (struct mutex *lock, -struct mutex_waiter *waiter); -extern void debug_mutex_free_waiter (struct mutex_waiter *waiter); -extern void debug_mutex_add_waiter (struct mutex *lock, -struct mutex_waiter *waiter, -struct task_struct *task); -extern void debug_mutex_remove_waiter (struct mutex *lock,struct mutex_waiter *waiter, -struct task_struct *task); -extern void debug_mutex_unlock (struct mutex *lock); -extern void debug_mutex_init (struct mutex *lock,const char *name, -struct lock_class_key *key);
kernel/locking/mutex.c +1 -5
@@ -30,11 +30,7 @@#include <linux/debug_locks.h> #include <linux/osq_lock.h> -#ifdef CONFIG_DEBUG_MUTEXES -# include "mutex-debug.h" -#else -# include "mutex.h" -#endif +#include "mutex.h" void __mutex_init (struct mutex *lock,const char *name,struct lock_class_key *key)
kernel/locking/mutex.h +17 -3
@@ -5,11 +5,24 @@* started by Ingo Molnar: * * Copyright (C) 2004, 2005, 2006 Red Hat, Inc., Ingo Molnar <[email protected]> -* -* This file contains mutex debugging related internal prototypes, for the -* !CONFIG_DEBUG_MUTEXES case. Most of them are NOPs: */ +#ifdef CONFIG_DEBUG_MUTEXES +extern void debug_mutex_lock_common (struct mutex *lock, +struct mutex_waiter *waiter); +extern void debug_mutex_wake_waiter (struct mutex *lock, +struct mutex_waiter *waiter); +extern void debug_mutex_free_waiter (struct mutex_waiter *waiter); +extern void debug_mutex_add_waiter (struct mutex *lock, +struct mutex_waiter *waiter, +struct task_struct *task); +extern void debug_mutex_remove_waiter (struct mutex *lock,struct mutex_waiter *waiter, +struct task_struct *task); +extern void debug_mutex_unlock (struct mutex *lock); +extern void debug_mutex_init (struct mutex *lock,const char *name, +struct lock_class_key *key); +#else /* CONFIG_DEBUG_MUTEXES */ +#define debug_mutex_wake_waiter (lock, waiter) do { } while (0)#define debug_mutex_free_waiter (waiter) do { } while (0)#define debug_mutex_add_waiter (lock, waiter, ti) do { } while (0)
@@ -21,3 +34,4 @@ static inline void debug_mutex_lock_common(struct mutex *lock, struct mutex_waiter *waiter) { } +#endif /* !CONFIG_DEBUG_MUTEXES */
PATCH 33/63 locking/mutex: Move waiter to core header
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Move the mutex waiter declaration from the global to the core local header. There is no reason to expose it outside of the core code.
--- include/linux/mutex.h | 13 ------------- kernel/locking/mutex.h | 13 +++++++++++++ 2 files changed, 13 insertions(+), 13 deletions(-)
---
include/linux/mutex.h +0 -13
@@ -74,19 +74,6 @@ struct ww_mutex {#endif }; -/* -* This is the control structure for tasks blocked on mutex, -* which resides on the blocked task's kernel stack: -*/ -struct mutex_waiter { -struct list_head list; -struct task_struct *task; -struct ww_acquire_ctx *ww_ctx; -#ifdef CONFIG_DEBUG_MUTEXES -void *magic; -#endif -}; -#ifdef CONFIG_DEBUG_MUTEXES #define __DEBUG_MUTEX_INITIALIZER (lockname) \
kernel/locking/mutex.h +13 -0
@@ -7,6 +7,19 @@* Copyright (C) 2004, 2005, 2006 Red Hat, Inc., Ingo Molnar <[email protected]> */ +/* +* This is the control structure for tasks blocked on mutex, which resides +* on the blocked task's kernel stack: +*/ +struct mutex_waiter { +struct list_head list; +struct task_struct *task; +struct ww_acquire_ctx *ww_ctx; +#ifdef CONFIG_DEBUG_MUTEXES +void *magic; +#endif +}; +#ifdef CONFIG_DEBUG_MUTEXES extern void debug_mutex_lock_common (struct mutex *lock,struct mutex_waiter *waiter);
PATCH 34/63 locking/ww_mutex: Move ww_mutex declarations into ww_mutex.h
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Move the ww_mutex declarations into the ww_mutex specific header where they belong.
Preperatory change to allow compiling ww_mutex standalone.
--- include/linux/mutex.h | 11 ----------- include/linux/ww_mutex.h | 8 ++++++++ 2 files changed, 8 insertions(+), 11 deletions(-)
---
include/linux/mutex.h +0 -11
@@ -20,9 +20,6 @@#include <linux/osq_lock.h> #include <linux/debug_locks.h> -struct ww_class ; -struct ww_acquire_ctx ; -/* * Simple, straightforward mutexes with strict semantics: *
@@ -66,14 +63,6 @@ struct mutex {#endif }; -struct ww_mutex { -struct mutex base; -struct ww_acquire_ctx *ctx; -#ifdef CONFIG_DEBUG_MUTEXES -struct ww_class *ww_class; -#endif -}; -#ifdef CONFIG_DEBUG_MUTEXES #define __DEBUG_MUTEX_INITIALIZER (lockname) \
include/linux/ww_mutex.h +8 -0
@@ -28,6 +28,14 @@ struct ww_class {unsigned int is_wait_die; }; +struct ww_mutex { +struct mutex base; +struct ww_acquire_ctx *ctx; +#ifdef CONFIG_DEBUG_MUTEXES +struct ww_class *ww_class; +#endif +}; +struct ww_acquire_ctx {struct task_struct *task;unsigned long stamp;
PATCH 35/63 locking/mutex: Make mutex::wait_lock raw
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The wait_lock of mutex is really a low level lock. Convert it to a raw_spinlock like the wait_lock of rtmutex.
--- include/linux/mutex.h | 4 ++-- kernel/locking/mutex.c | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-)
---
include/linux/mutex.h +2 -2
@@ -50,7 +50,7 @@*/ struct mutex {atomic_long_t owner; -spinlock_t wait_lock; +raw_spinlock_t wait_lock;#ifdef CONFIG_MUTEX_SPIN_ON_OWNER struct optimistic_spin_queue osq;/* Spinner MCS lock */ #endif
@@ -105,7 +105,7 @@ do { \#define __MUTEX_INITIALIZER (lockname) \ { .owner = ATOMIC_LONG_INIT(0) \ - , .wait_lock = __SPIN_LOCK_UNLOCKED(lockname.wait_lock) \ + , .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(lockname.wait_lock) \ , .wait_list = LIST_HEAD_INIT(lockname.wait_list) \ __DEBUG_MUTEX_INITIALIZER(lockname) \ __DEP_MAP_MUTEX_INITIALIZER(lockname) }
kernel/locking/mutex.c +11 -11
@@ -36,7 +36,7 @@ void __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key) { atomic_long_set(&lock->owner, 0); -spin_lock_init (&lock->wait_lock); +raw_spin_lock_init (&lock->wait_lock);INIT_LIST_HEAD (&lock->wait_list);#ifdef CONFIG_MUTEX_SPIN_ON_OWNER osq_lock_init (&lock->osq);
@@ -487,9 +487,9 @@ ww_mutex_set_context_fastpath(struct ww_* Uh oh, we raced in fastpath, check if any of the waiters need to * die or wound us. */ -spin_lock (&lock->base.wait_lock); +raw_spin_lock (&lock->base.wait_lock);__ww_mutex_check_waiters (&lock->base, ctx); -spin_unlock (&lock->base.wait_lock); +raw_spin_unlock (&lock->base.wait_lock); }#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
@@ -964,7 +964,7 @@ static __always_inline int __schedreturn 0 ; } -spin_lock (&lock->wait_lock); +raw_spin_lock (&lock->wait_lock);/* * After waiting to acquire the wait_lock, try again. */
@@ -1028,7 +1028,7 @@ static __always_inline int __sched goto err; } -spin_unlock (&lock->wait_lock); +raw_spin_unlock (&lock->wait_lock);schedule_preempt_disabled ();/*
@@ -1051,9 +1051,9 @@ static __always_inline int __sched (first && mutex_optimistic_spin(lock, ww_ctx, &waiter))) break; -spin_lock (&lock->wait_lock); +raw_spin_lock (&lock->wait_lock); } -spin_lock (&lock->wait_lock); +raw_spin_lock (&lock->wait_lock); acquired:__set_current_state (TASK_RUNNING );
@@ -1078,7 +1078,7 @@ static __always_inline int __schedif (ww_ctx)ww_mutex_lock_acquired (ww, ww_ctx); -spin_unlock (&lock->wait_lock); +raw_spin_unlock (&lock->wait_lock);preempt_enable ();return 0 ;
@@ -1086,7 +1086,7 @@ static __always_inline int __sched__set_current_state (TASK_RUNNING );__mutex_remove_waiter (lock, &waiter); err_early_kill: -spin_unlock (&lock->wait_lock); +raw_spin_unlock (&lock->wait_lock);debug_mutex_free_waiter (&waiter);mutex_release (&lock->dep_map, ip);preempt_enable ();
@@ -1255,7 +1255,7 @@ static noinline void __sched __mutex_unl owner = old; } -spin_lock (&lock->wait_lock); +raw_spin_lock (&lock->wait_lock);debug_mutex_unlock (lock);if (!list_empty (&lock->wait_list)) {/* get the first entry from the wait-list: */
@@ -1272,7 +1272,7 @@ static noinline void __sched __mutex_unlif (owner &MUTEX_FLAG_HANDOFF )__mutex_handoff (lock, next); -spin_unlock (&lock->wait_lock); +raw_spin_unlock (&lock->wait_lock);wake_up_q (&wake_q); }
PATCH 36/63 locking/ww_mutex: Simplify lockdep annotation
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
No functional change.
--- kernel/locking/mutex.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
---
kernel/locking/mutex.c +10 -9
@@ -949,6 +949,10 @@ static __always_inline int __sched*/ if (ww_ctx->acquired ==0 ) ww_ctx->wounded =0 ; + +#ifdef CONFIG_DEBUG_LOCK_ALLOC + nest_lock = &ww_ctx->dep_map; +#endif }preempt_disable ();
@@ -1102,10 +1106,9 @@ static int __sched static int __sched__ww_mutex_lock (struct mutex *lock,unsigned int state,unsigned int subclass, -struct lockdep_map *nest_lock,unsigned long ip, -struct ww_acquire_ctx *ww_ctx) +unsigned long ip,struct ww_acquire_ctx *ww_ctx) { -return __mutex_lock_common (lock, state, subclass, nest_lock, ip, ww_ctx, true); +return __mutex_lock_common (lock, state, subclass,NULL , ip, ww_ctx, true); }#ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -1185,8 +1188,7 @@ ww_mutex_lock(struct ww_mutex *lock, strmight_sleep (); ret =__ww_mutex_lock (&lock->base,TASK_UNINTERRUPTIBLE , -0 , ctx ? &ctx->dep_map :NULL , _RET_IP_, - ctx); +0 , _RET_IP_, ctx);if (!ret && ctx && ctx->acquired >1 )return ww_mutex_deadlock_injection (lock, ctx);
@@ -1201,8 +1203,7 @@ ww_mutex_lock_interruptible(struct ww_mumight_sleep (); ret =__ww_mutex_lock (&lock->base,TASK_INTERRUPTIBLE , -0 , ctx ? &ctx->dep_map :NULL , _RET_IP_, - ctx); +0 , _RET_IP_, ctx);if (!ret && ctx && ctx->acquired >1 )return ww_mutex_deadlock_injection (lock, ctx);
@@ -1376,7 +1377,7 @@ static noinline int __sched static noinline int __sched__ww_mutex_lock_slowpath (struct ww_mutex *lock,struct ww_acquire_ctx *ctx) { -return __ww_mutex_lock (&lock->base,TASK_UNINTERRUPTIBLE ,0 ,NULL , +return __ww_mutex_lock (&lock->base,TASK_UNINTERRUPTIBLE ,0 , _RET_IP_, ctx); }
@@ -1384,7 +1385,7 @@ static noinline int __sched __ww_mutex_lock_interruptible_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { - return __ww_mutex_lock(&lock->base, TASK_INTERRUPTIBLE, 0, NULL, + return __ww_mutex_lock(&lock->base, TASK_INTERRUPTIBLE, 0, _RET_IP_, ctx); }
PATCH 37/63 locking/ww_mutex: Gather mutex_waiter initialization
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
--- kernel/locking/mutex-debug.c | 1 + kernel/locking/mutex.c | 12 +++--------- 2 files changed, 4 insertions(+), 9 deletions(-)
---
kernel/locking/mutex-debug.c +1 -0
@@ -30,6 +30,7 @@ void debug_mutex_lock_common(struct mutememset (waiter,MUTEX_DEBUG_INIT ,sizeof (*waiter)); waiter->magic = waiter;INIT_LIST_HEAD (&waiter->list); + waiter->ww_ctx =MUTEX_POISON_WW_CTX ; }void debug_mutex_wake_waiter (struct mutex *lock,struct mutex_waiter *waiter)
kernel/locking/mutex.c +3 -9
@@ -980,17 +980,15 @@ static __always_inline int __sched }debug_mutex_lock_common (lock, &waiter); + waiter.task = current; +if (ww_ctx) + waiter.ww_ctx = ww_ctx;lock_contended (&lock->dep_map, ip);if (!use_ww_ctx) {/* add waiting tasks to the end of the waitqueue (FIFO): */ __mutex_add_waiter (lock, &waiter, &lock->wait_list); - - -#ifdef CONFIG_DEBUG_MUTEXES - waiter.ww_ctx =MUTEX_POISON_WW_CTX ; -#endif }else {/* * Add in stamp order, waking up waiters that must kill
@@ -999,12 +997,8 @@ static __always_inline int __sched ret =__ww_mutex_add_waiter (&waiter, lock, ww_ctx);if (ret) goto err_early_kill; - - waiter.ww_ctx = ww_ctx; } - waiter.task = current; -set_current_state (state);for (;;) {/*
PATCH 38/63 locking/ww_mutex: Split up ww_mutex_unlock()
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Split the ww related part out into a helper function so it can be reused for a rtmutex based ww_mutex implementation.
--- kernel/locking/mutex.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
---
kernel/locking/mutex.c +13 -13
@@ -750,19 +750,7 @@ EXPORT_SYMBOL(mutex_unlock);*/ void __sched ww_mutex_unlock(struct ww_mutex *lock) { -/* -* The unlocking fastpath is the 0->1 transition from 'locked' -* into 'unlocked' state: -*/ -if (lock->ctx) { -#ifdef CONFIG_DEBUG_MUTEXES -DEBUG_LOCKS_WARN_ON (!lock->ctx->acquired); -#endif -if (lock->ctx->acquired >0 ) - lock->ctx->acquired--; - lock->ctx =NULL ; - } - +__ww_mutex_unlock (lock);mutex_unlock (&lock->base); }EXPORT_SYMBOL (ww_mutex_unlock);
@@ -915,6 +903,18 @@ static inline int __schedreturn 0 ; } +static void __ww_mutex_unlock (struct ww_mutex *lock) +{ +if (lock->ctx) { +#ifdef CONFIG_DEBUG_MUTEXES +DEBUG_LOCKS_WARN_ON (!lock->ctx->acquired); +#endif +if (lock->ctx->acquired >0 ) + lock->ctx->acquired--; + lock->ctx =NULL ; + } +} +/* * Lock a mutex (possibly interruptible), slowpath: */
PATCH 39/63 locking/ww_mutex: Split W/W implementation logic
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Split the W/W mutex helper functions out into a seperate header file so they can be shared with a rtmutex based variant later.
--- kernel/locking/mutex.c | 370 ---------------------------------------------- kernel/locking/ww_mutex.h | 369 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 370 insertions(+), 369 deletions(-) create mode 100644 kernel/locking/ww_mutex.h
---
kernel/locking/mutex.c +1 -369
@@ -282,215 +282,7 @@ void __sched mutex_lock(struct mutex *loEXPORT_SYMBOL (mutex_lock);#endif -/* -* Wait-Die: -* The newer transactions are killed when: -* It (the new transaction) makes a request for a lock being held -* by an older transaction. -* -* Wound-Wait: -* The newer transactions are wounded when: -* An older transaction makes a request for a lock being held by -* the newer transaction. -*/ - -/* -* Associate the ww_mutex @ww with the context @ww_ctx under which we acquired -* it. -*/ -static __always_inline void -ww_mutex_lock_acquired (struct ww_mutex *ww,struct ww_acquire_ctx *ww_ctx) -{ -#ifdef CONFIG_DEBUG_MUTEXES -/* -* If this WARN_ON triggers, you used ww_mutex_lock to acquire, -* but released with a normal mutex_unlock in this call. -* -* This should never happen, always use ww_mutex_unlock. -*/ -DEBUG_LOCKS_WARN_ON (ww->ctx); - -/* -* Not quite done after calling ww_acquire_done() ? -*/ -DEBUG_LOCKS_WARN_ON (ww_ctx->done_acquire); - -if (ww_ctx->contending_lock) { -/* -* After -EDEADLK you tried to -* acquire a different ww_mutex? Bad! -*/ -DEBUG_LOCKS_WARN_ON (ww_ctx->contending_lock != ww); - -/* -* You called ww_mutex_lock after receiving -EDEADLK, -* but 'forgot' to unlock everything else first? -*/ -DEBUG_LOCKS_WARN_ON (ww_ctx->acquired >0 ); - ww_ctx->contending_lock =NULL ; - } - -/* -* Naughty, using a different class will lead to undefined behavior! -*/ -DEBUG_LOCKS_WARN_ON (ww_ctx->ww_class != ww->ww_class); -#endif - ww_ctx->acquired++; - ww->ctx = ww_ctx; -} - -/* -* Determine if context @a is 'after' context @b. IOW, @a is a younger -* transaction than @b and depending on algorithm either needs to wait for -* @b or die. -*/ -static inline bool __sched -__ww_ctx_stamp_after (struct ww_acquire_ctx *a,struct ww_acquire_ctx *b) -{ - -return (signed long )(a->stamp - b->stamp) >0 ; -} - -/* -* Wait-Die; wake a younger waiter context (when locks held) such that it can -* die. -* -* Among waiters with context, only the first one can have other locks acquired -* already (ctx->acquired > 0), because __ww_mutex_add_waiter() and -* __ww_mutex_check_kill() wake any but the earliest context. -*/ -static bool __sched -__ww_mutex_die (struct mutex *lock,struct mutex_waiter *waiter, -struct ww_acquire_ctx *ww_ctx) -{ -if (!ww_ctx->is_wait_die) -return false; - -if (waiter->ww_ctx->acquired >0 && -__ww_ctx_stamp_after (waiter->ww_ctx, ww_ctx)) { -debug_mutex_wake_waiter (lock, waiter); -wake_up_process (waiter->task); - } - -return true; -} - -/* -* Wound-Wait; wound a younger @hold_ctx if it holds the lock. -* -* Wound the lock holder if there are waiters with older transactions than -* the lock holders. Even if multiple waiters may wound the lock holder, -* it's sufficient that only one does. -*/ -static bool __ww_mutex_wound (struct mutex *lock, -struct ww_acquire_ctx *ww_ctx, -struct ww_acquire_ctx *hold_ctx) -{ -struct task_struct *owner =__mutex_owner (lock); - -lockdep_assert_held (&lock->wait_lock); - -/* -* Possible through __ww_mutex_add_waiter() when we race with -* ww_mutex_set_context_fastpath(). In that case we'll get here again -* through __ww_mutex_check_waiters(). -*/ -if (!hold_ctx) -return false; - -/* -* Can have !owner because of __mutex_unlock_slowpath(), but if owner, -* it cannot go away because we'll have FLAG_WAITERS set and hold -* wait_lock. -*/ -if (!owner) -return false; - -if (ww_ctx->acquired >0 &&__ww_ctx_stamp_after (hold_ctx, ww_ctx)) { - hold_ctx->wounded =1 ; - -/* -* wake_up_process() paired with set_current_state() -* inserts sufficient barriers to make sure @owner either sees -* it's wounded in __ww_mutex_check_kill() or has a -* wakeup pending to re-read the wounded state. -*/ -if (owner != current) -wake_up_process (owner); - -return true; - } - -return false; -} - -/* -* We just acquired @lock under @ww_ctx, if there are later contexts waiting -* behind us on the wait-list, check if they need to die, or wound us. -* -* See __ww_mutex_add_waiter() for the list-order construction; basically the -* list is ordered by stamp, smallest (oldest) first. -* -* This relies on never mixing wait-die/wound-wait on the same wait-list; -* which is currently ensured by that being a ww_class property. -* -* The current task must not be on the wait list. -*/ -static void __sched -__ww_mutex_check_waiters (struct mutex *lock,struct ww_acquire_ctx *ww_ctx) -{ -struct mutex_waiter *cur; - -lockdep_assert_held (&lock->wait_lock); - -list_for_each_entry (cur, &lock->wait_list, list) { -if (!cur->ww_ctx) -continue ; - -if (__ww_mutex_die (lock, cur, ww_ctx) || -__ww_mutex_wound (lock, cur->ww_ctx, ww_ctx)) -break ; - } -} - -/* -* After acquiring lock with fastpath, where we do not hold wait_lock, set ctx -* and wake up any waiters so they can recheck. -*/ -static __always_inline void -ww_mutex_set_context_fastpath (struct ww_mutex *lock,struct ww_acquire_ctx *ctx) -{ -ww_mutex_lock_acquired (lock, ctx); - -/* -* The lock->ctx update should be visible on all cores before -* the WAITERS check is done, otherwise contended waiters might be -* missed. The contended waiters will either see ww_ctx == NULL -* and keep spinning, or it will acquire wait_lock, add itself -* to waiter list and sleep. -*/ -smp_mb ();/* See comments above and below. */ - -/* -* [W] ww->ctx = ctx [W] MUTEX_FLAG_WAITERS -* MB MB -* [R] MUTEX_FLAG_WAITERS [R] ww->ctx -* -* The memory barrier above pairs with the memory barrier in -* __ww_mutex_add_waiter() and makes sure we either observe ww->ctx -* and/or !empty list. -*/ -if (likely (!(atomic_long_read (&lock->base.owner) &MUTEX_FLAG_WAITERS ))) -return ; - -/* -* Uh oh, we raced in fastpath, check if any of the waiters need to -* die or wound us. -*/ -raw_spin_lock (&lock->base.wait_lock); -__ww_mutex_check_waiters (&lock->base, ctx); -raw_spin_unlock (&lock->base.wait_lock); -} +#include "ww_mutex.h" #ifdef CONFIG_MUTEX_SPIN_ON_OWNER
@@ -755,166 +547,6 @@ void __sched ww_mutex_unlock(struct ww_m }EXPORT_SYMBOL (ww_mutex_unlock); - -static __always_inline int __sched -__ww_mutex_kill (struct mutex *lock,struct ww_acquire_ctx *ww_ctx) -{ -if (ww_ctx->acquired >0 ) { -#ifdef CONFIG_DEBUG_MUTEXES -struct ww_mutex *ww; - - ww = container_of(lock, struct ww_mutex, base); -DEBUG_LOCKS_WARN_ON (ww_ctx->contending_lock); - ww_ctx->contending_lock = ww; -#endif -return -EDEADLK ; - } - -return 0 ; -} - - -/* -* Check the wound condition for the current lock acquire. -* -* Wound-Wait: If we're wounded, kill ourself. -* -* Wait-Die: If we're trying to acquire a lock already held by an older -* context, kill ourselves. -* -* Since __ww_mutex_add_waiter() orders the wait-list on stamp, we only have to -* look at waiters before us in the wait-list. -*/ -static inline int __sched -__ww_mutex_check_kill (struct mutex *lock,struct mutex_waiter *waiter, -struct ww_acquire_ctx *ctx) -{ - struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); -struct ww_acquire_ctx *hold_ctx =READ_ONCE (ww->ctx); -struct mutex_waiter *cur; - -if (ctx->acquired ==0 ) -return 0 ; - -if (!ctx->is_wait_die) { -if (ctx->wounded) -return __ww_mutex_kill (lock, ctx); - -return 0 ; - } - -if (hold_ctx &&__ww_ctx_stamp_after (ctx, hold_ctx)) -return __ww_mutex_kill (lock, ctx); - -/* -* If there is a waiter in front of us that has a context, then its -* stamp is earlier than ours and we must kill ourself. -*/ - cur = waiter; -list_for_each_entry_continue_reverse (cur, &lock->wait_list, list) { -if (!cur->ww_ctx) -continue ; - -return __ww_mutex_kill (lock, ctx); - } - -return 0 ; -} - -/* -* Add @waiter to the wait-list, keep the wait-list ordered by stamp, smallest -* first. Such that older contexts are preferred to acquire the lock over -* younger contexts. -* -* Waiters without context are interspersed in FIFO order. -* -* Furthermore, for Wait-Die kill ourself immediately when possible (there are -* older contexts already waiting) to avoid unnecessary waiting and for -* Wound-Wait ensure we wound the owning context when it is younger. -*/ -static inline int __sched -__ww_mutex_add_waiter (struct mutex_waiter *waiter, -struct mutex *lock, -struct ww_acquire_ctx *ww_ctx) -{ -struct mutex_waiter *cur; -struct list_head *pos; -bool is_wait_die; - -if (!ww_ctx) { -__mutex_add_waiter (lock, waiter, &lock->wait_list); -return 0 ; - } - - is_wait_die = ww_ctx->is_wait_die; - -/* -* Add the waiter before the first waiter with a higher stamp. -* Waiters without a context are skipped to avoid starving -* them. Wait-Die waiters may die here. Wound-Wait waiters -* never die here, but they are sorted in stamp order and -* may wound the lock holder. -*/ - pos = &lock->wait_list; -list_for_each_entry_reverse (cur, &lock->wait_list, list) { -if (!cur->ww_ctx) -continue ; - -if (__ww_ctx_stamp_after (ww_ctx, cur->ww_ctx)) { -/* -* Wait-Die: if we find an older context waiting, there -* is no point in queueing behind it, as we'd have to -* die the moment it would acquire the lock. -*/ -if (is_wait_die) { -int ret =__ww_mutex_kill (lock, ww_ctx); - -if (ret) -return ret; - } - -break ; - } - - pos = &cur->list; - -/* Wait-Die: ensure younger waiters die. */ -__ww_mutex_die (lock, cur, ww_ctx); - } - -__mutex_add_waiter (lock, waiter, pos); - -/* -* Wound-Wait: if we're blocking on a mutex owned by a younger context, -* wound that such that we might proceed. -*/ -if (!is_wait_die) { - struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); - -/* -* See ww_mutex_set_context_fastpath(). Orders setting -* MUTEX_FLAG_WAITERS vs the ww->ctx load, -* such that either we or the fastpath will wound @ww->ctx. -*/ -smp_mb (); -__ww_mutex_wound (lock, ww_ctx, ww->ctx); - } - -return 0 ; -} - -static void __ww_mutex_unlock (struct ww_mutex *lock) -{ -if (lock->ctx) { -#ifdef CONFIG_DEBUG_MUTEXES -DEBUG_LOCKS_WARN_ON (!lock->ctx->acquired); -#endif -if (lock->ctx->acquired >0 ) - lock->ctx->acquired--; - lock->ctx =NULL ; - } -} -/* * Lock a mutex (possibly interruptible), slowpath: */
kernel/locking/ww_mutex.h new file +369 -0
369 lines not on this page · raw message
PATCH 40/63 locking/ww_mutex: Remove __sched annotation
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
None of these functions will be on the stack when we block in schedule(), hence __sched is not needed.
--- kernel/locking/ww_mutex.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
---
kernel/locking/ww_mutex.h +6 -6
@@ -62,7 +62,7 @@ ww_mutex_lock_acquired(struct ww_mutex ** transaction than @b and depending on algorithm either needs to wait for * @b or die. */ -static inline bool __sched +static inline bool __ww_ctx_stamp_after (struct ww_acquire_ctx *a,struct ww_acquire_ctx *b) {
@@ -77,7 +77,7 @@ static inline bool __sched* already (ctx->acquired > 0), because __ww_mutex_add_waiter() and * __ww_mutex_check_kill() wake any but the earliest context. */ -static bool __sched +static bool __ww_mutex_die (struct mutex *lock,struct mutex_waiter *waiter,struct ww_acquire_ctx *ww_ctx) {
@@ -154,7 +154,7 @@ static bool __ww_mutex_wound(struct mute* * The current task must not be on the wait list. */ -static void __sched +static void __ww_mutex_check_waiters (struct mutex *lock,struct ww_acquire_ctx *ww_ctx) {struct mutex_waiter *cur;
@@ -210,7 +210,7 @@ ww_mutex_set_context_fastpath(struct ww_raw_spin_unlock (&lock->base.wait_lock); } -static __always_inline int __sched +static __always_inline int__ww_mutex_kill (struct mutex *lock,struct ww_acquire_ctx *ww_ctx) {if (ww_ctx->acquired >0 ) {
@@ -238,7 +238,7 @@ static __always_inline int __sched* Since __ww_mutex_add_waiter() orders the wait-list on stamp, we only have to * look at waiters before us in the wait-list. */ -static inline int __sched +static inline int __ww_mutex_check_kill (struct mutex *lock,struct mutex_waiter *waiter,struct ww_acquire_ctx *ctx) {
@@ -285,7 +285,7 @@ static inline int __sched* older contexts already waiting) to avoid unnecessary waiting and for * Wound-Wait ensure we wound the owning context when it is younger. */ -static inline int __sched +static inline int __ww_mutex_add_waiter (struct mutex_waiter *waiter,struct mutex *lock,struct ww_acquire_ctx *ww_ctx)
PATCH 41/63 locking/ww_mutex: Abstract waiter iteration
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Split out the waiter iteration functions so they can be substituted for a rtmutex based ww_mutex later.
--- kernel/locking/ww_mutex.h | 57 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 4 deletions(-)
---
kernel/locking/ww_mutex.h +53 -4
@@ -1,5 +1,49 @@/* SPDX-License-Identifier: GPL-2.0-only */ +static inline struct mutex_waiter * +__ww_waiter_first (struct mutex *lock) +{ +struct mutex_waiter *w; + + w = list_first_entry(&lock->wait_list, struct mutex_waiter, list); +if (list_entry_is_head (w, &lock->wait_list, list)) +return NULL ; + +return w; +} + +static inline struct mutex_waiter * +__ww_waiter_next (struct mutex *lock,struct mutex_waiter *w) +{ + w =list_next_entry (w, list); +if (list_entry_is_head (w, &lock->wait_list, list)) +return NULL ; + +return w; +} + +static inline struct mutex_waiter * +__ww_waiter_prev (struct mutex *lock,struct mutex_waiter *w) +{ + w =list_prev_entry (w, list); +if (list_entry_is_head (w, &lock->wait_list, list)) +return NULL ; + +return w; +} + +static inline struct mutex_waiter * +__ww_waiter_last (struct mutex *lock) +{ +struct mutex_waiter *w; + + w = list_last_entry(&lock->wait_list, struct mutex_waiter, list); +if (list_entry_is_head (w, &lock->wait_list, list)) +return NULL ; + +return w; +} +/* * Wait-Die: * The newer transactions are killed when:
@@ -161,7 +205,9 @@ static voidlockdep_assert_held (&lock->wait_lock); -list_for_each_entry (cur, &lock->wait_list, list) { +for (cur =__ww_waiter_first (lock); cur; + cur =__ww_waiter_next (lock, cur)) { +if (!cur->ww_ctx)continue ;
@@ -263,8 +309,9 @@ static inline int* If there is a waiter in front of us that has a context, then its * stamp is earlier than ours and we must kill ourself. */ - cur = waiter; -list_for_each_entry_continue_reverse (cur, &lock->wait_list, list) { +for (cur =__ww_waiter_prev (lock, waiter); cur; + cur =__ww_waiter_prev (lock, cur)) { +if (!cur->ww_ctx)continue ;
@@ -309,7 +356,9 @@ static inline int* may wound the lock holder. */ pos = &lock->wait_list; -list_for_each_entry_reverse (cur, &lock->wait_list, list) { +for (cur =__ww_waiter_last (lock); cur; + cur =__ww_waiter_prev (lock, cur)) { +if (!cur->ww_ctx)continue ;
PATCH 42/63 locking/ww_mutex: Abstract waiter enqueueing
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
The upcoming rtmutex based ww_mutex needs a different handling for enqueueing a waiter. Split it out into a helper function.
--- kernel/locking/ww_mutex.h | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
---
kernel/locking/ww_mutex.h +13 -6
@@ -44,6 +44,15 @@ static inline struct mutex_waiter *return w; } +static inline void +__ww_waiter_add (struct mutex *lock,struct mutex_waiter *waiter,struct mutex_waiter *pos) +{ +struct list_head *p = &lock->wait_list; +if (pos) + p = &pos->list; +__mutex_add_waiter (lock, waiter, p); +} +/* * Wait-Die: * The newer transactions are killed when:
@@ -337,12 +346,11 @@ static inline intstruct mutex *lock, struct ww_acquire_ctx *ww_ctx) { - struct mutex_waiter *cur; -struct list_head *pos; + struct mutex_waiter *cur, *pos = NULL;bool is_wait_die;if (!ww_ctx) { -__mutex_add_waiter (lock, waiter, &lock->wait_list); +__ww_waiter_add (lock, waiter,NULL );return 0 ; }
@@ -355,7 +363,6 @@ static inline int* never die here, but they are sorted in stamp order and * may wound the lock holder. */ - pos = &lock->wait_list;for (cur =__ww_waiter_last (lock); cur; cur =__ww_waiter_prev (lock, cur)) {
@@ -378,13 +385,13 @@ static inline intbreak ; } - pos = &cur->list; + pos = cur;/* Wait-Die: ensure younger waiters die. */ __ww_mutex_die (lock, cur, ww_ctx); } -__mutex_add_waiter (lock, waiter, pos); +__ww_waiter_add (lock, waiter, pos);/* * Wound-Wait: if we're blocking on a mutex owned by a younger context,
PATCH 43/63 locking/ww_mutex: Abstract mutex accessors
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Move the mutex related access from various ww_mutex functions into helper functions so they can be substituted for rtmutex based ww_mutex later.
--- kernel/locking/ww_mutex.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
---
kernel/locking/ww_mutex.h +14 -2
@@ -53,6 +53,18 @@ static inline void__mutex_add_waiter (lock, waiter, p); } +static inline struct task_struct * +__ww_mutex_owner (struct mutex *lock) +{ +return __mutex_owner (lock); +} + +static inline bool +__ww_mutex_has_waiters (struct mutex *lock) +{ +return atomic_long_read (&lock->owner) &MUTEX_FLAG_WAITERS ; +} +/* * Wait-Die: * The newer transactions are killed when:
@@ -157,7 +169,7 @@ static bool __ww_mutex_wound(struct mutestruct ww_acquire_ctx *ww_ctx, struct ww_acquire_ctx *hold_ctx) { - struct task_struct *owner = __mutex_owner(lock); + struct task_struct *owner = __ww_mutex_owner(lock);lockdep_assert_held (&lock->wait_lock);
@@ -253,7 +265,7 @@ ww_mutex_set_context_fastpath(struct ww_* __ww_mutex_add_waiter() and makes sure we either observe ww->ctx * and/or !empty list. */ -if (likely (!(atomic_long_read (&lock->base.owner) &MUTEX_FLAG_WAITERS ))) +if (likely (!__ww_mutex_has_waiters (&lock->base)))return ;/*
PATCH 44/63 locking/ww_mutex: Abstract mutex types
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Some ww_mutex helper functions use pointers for the underlying mutex and mutex_waiter. The upcoming rtmutex based implementation needs to share these functions. Add and use defines for the types and replace the direct types in the affected functions.
--- kernel/locking/ww_mutex.h | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
---
kernel/locking/ww_mutex.h +13 -10
@@ -1,5 +1,8 @@/* SPDX-License-Identifier: GPL-2.0-only */ +#define MUTEX mutex +#define MUTEX_WAITER mutex_waiter +static inline struct mutex_waiter *__ww_waiter_first (struct mutex *lock) {
@@ -143,7 +146,7 @@ static inline bool* __ww_mutex_check_kill() wake any but the earliest context. */ static bool -__ww_mutex_die (struct mutex *lock,struct mutex_waiter *waiter, +__ww_mutex_die (struct MUTEX *lock,struct MUTEX_WAITER *waiter,struct ww_acquire_ctx *ww_ctx) {if (!ww_ctx->is_wait_die)
@@ -165,7 +168,7 @@ static bool* the lock holders. Even if multiple waiters may wound the lock holder, * it's sufficient that only one does. */ -static bool __ww_mutex_wound (struct mutex *lock, +static bool __ww_mutex_wound (struct MUTEX *lock,struct ww_acquire_ctx *ww_ctx,struct ww_acquire_ctx *hold_ctx) {
@@ -220,9 +223,9 @@ static bool __ww_mutex_wound(struct mute* The current task must not be on the wait list. */ static void -__ww_mutex_check_waiters (struct mutex *lock,struct ww_acquire_ctx *ww_ctx) +__ww_mutex_check_waiters (struct MUTEX *lock,struct ww_acquire_ctx *ww_ctx) { -struct mutex_waiter *cur; +struct MUTEX_WAITER *cur;lockdep_assert_held (&lock->wait_lock);
@@ -278,7 +281,7 @@ ww_mutex_set_context_fastpath(struct ww_ } static __always_inline int -__ww_mutex_kill (struct mutex *lock,struct ww_acquire_ctx *ww_ctx) +__ww_mutex_kill (struct MUTEX *lock,struct ww_acquire_ctx *ww_ctx) {if (ww_ctx->acquired >0 ) {#ifdef CONFIG_DEBUG_MUTEXES
@@ -306,12 +309,12 @@ static __always_inline int* look at waiters before us in the wait-list. */ static inline int -__ww_mutex_check_kill (struct mutex *lock,struct mutex_waiter *waiter, +__ww_mutex_check_kill (struct MUTEX *lock,struct MUTEX_WAITER *waiter,struct ww_acquire_ctx *ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);struct ww_acquire_ctx *hold_ctx =READ_ONCE (ww->ctx); -struct mutex_waiter *cur; +struct MUTEX_WAITER *cur;if (ctx->acquired ==0 )return 0 ;
@@ -354,11 +357,11 @@ static inline int* Wound-Wait ensure we wound the owning context when it is younger. */ static inline int -__ww_mutex_add_waiter (struct mutex_waiter *waiter, -struct mutex *lock, +__ww_mutex_add_waiter (struct MUTEX_WAITER *waiter, +struct MUTEX *lock,struct ww_acquire_ctx *ww_ctx) { -struct mutex_waiter *cur, *pos =NULL ; +struct MUTEX_WAITER *cur, *pos =NULL ;bool is_wait_die;if (!ww_ctx) {
PATCH 45/63 locking/ww_mutex: Abstract internal lock access
In reply to Thomas GleixnerAccessing the internal wait_lock of mutex and rtmutex is slightly different. Provide helper functions for that.
--- include/linux/ww_mutex.h | 13 +++++++++---- kernel/locking/ww_mutex.h | 23 +++++++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-)
include/linux/ww_mutex.h +9 -4
@@ -19,6 +19,11 @@#include <linux/mutex.h> +#define WW_MUTEX_BASE mutex +#define ww_mutex_base_init (l,n,k) __mutex_init(l,n,k) +#define ww_mutex_base_trylock (l) mutex_trylock(l) +#define ww_mutex_base_is_locked (b) mutex_is_locked((b)) +struct ww_class {atomic_long_t stamp;struct lock_class_key acquire_key;
@@ -29,7 +34,7 @@ struct ww_class { };struct ww_mutex { -struct mutex base; +struct WW_MUTEX_BASE base;struct ww_acquire_ctx *ctx;#ifdef CONFIG_DEBUG_MUTEXES struct ww_class *ww_class;
@@ -82,7 +87,7 @@ struct ww_acquire_ctx {static inline void ww_mutex_init (struct ww_mutex *lock,struct ww_class *ww_class) { -__mutex_init (&lock->base, ww_class->mutex_name, &ww_class->mutex_key); +ww_mutex_base_init (&lock->base, ww_class->mutex_name, &ww_class->mutex_key); lock->ctx =NULL ;#ifdef CONFIG_DEBUG_MUTEXES lock->ww_class = ww_class;
@@ -330,7 +335,7 @@ extern void ww_mutex_unlock(struct ww_mu*/ static inline int __must_check ww_mutex_trylock (struct ww_mutex *lock) { -return mutex_trylock (&lock->base); +return ww_mutex_base_trylock (&lock->base); }/***
@@ -354,7 +359,7 @@ static inline void ww_mutex_destroy(stru*/ static inline bool ww_mutex_is_locked (struct ww_mutex *lock) { -return mutex_is_locked (&lock->base); +return ww_mutex_base_is_locked (&lock->base); }#endif
kernel/locking/ww_mutex.h +19 -4
@@ -68,6 +68,21 @@ static inline boolreturn atomic_long_read (&lock->owner) &MUTEX_FLAG_WAITERS ; } +static inline void lock_wait_lock (struct mutex *lock) +{ +raw_spin_lock (&lock->wait_lock); +} + +static inline void unlock_wait_lock (struct mutex *lock) +{ +raw_spin_unlock (&lock->wait_lock); +} + +static inline void lockdep_assert_wait_lock_held (struct mutex *lock) +{ +lockdep_assert_held (&lock->wait_lock); +} +/* * Wait-Die: * The newer transactions are killed when:
@@ -174,7 +189,7 @@ static bool __ww_mutex_wound(struct MUTE {struct task_struct *owner =__ww_mutex_owner (lock); -lockdep_assert_held (&lock->wait_lock); +lockdep_assert_wait_lock_held (lock);/* * Possible through __ww_mutex_add_waiter() when we race with
@@ -227,7 +242,7 @@ static void {struct MUTEX_WAITER *cur; -lockdep_assert_held (&lock->wait_lock); +lockdep_assert_wait_lock_held (lock);for (cur =__ww_waiter_first (lock); cur; cur =__ww_waiter_next (lock, cur)) {
@@ -275,9 +290,9 @@ ww_mutex_set_context_fastpath(struct ww_* Uh oh, we raced in fastpath, check if any of the waiters need to * die or wound us. */ -raw_spin_lock (&lock->base.wait_lock); +lock_wait_lock (&lock->base);__ww_mutex_check_waiters (&lock->base, ctx); -raw_spin_unlock (&lock->base.wait_lock); +unlock_wait_lock (&lock->base); }static __always_inline int
PATCH 46/63 locking/ww_mutex: Implement rt_mutex accessors
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Provide the type defines and the helper inlines for rtmutex based ww_mutexes.
--- kernel/locking/ww_mutex.h | 80 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+)
---
kernel/locking/ww_mutex.h +80 -0
@@ -1,5 +1,7 @@/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef WW_RT +#define MUTEX mutex#define MUTEX_WAITER mutex_waiter
@@ -83,6 +85,82 @@ static inline void lockdep_assert_wait_llockdep_assert_held (&lock->wait_lock); } +#else /* WW_RT */ + +#define MUTEX rt_mutex +#define MUTEX_WAITER rt_mutex_waiter + +static inline struct rt_mutex_waiter * +__ww_waiter_first (struct rt_mutex *lock) +{ +struct rb_node *n =rb_first (&lock->rtmutex.waiters.rb_root); +if (!n) +return NULL ; + return rb_entry(n, struct rt_mutex_waiter, tree_entry); +} + +static inline struct rt_mutex_waiter * +__ww_waiter_next (struct rt_mutex *lock,struct rt_mutex_waiter *w) +{ +struct rb_node *n =rb_next (&w->tree_entry); +if (!n) +return NULL ; + return rb_entry(n, struct rt_mutex_waiter, tree_entry); +} + +static inline struct rt_mutex_waiter * +__ww_waiter_prev (struct rt_mutex *lock,struct rt_mutex_waiter *w) +{ +struct rb_node *n =rb_prev (&w->tree_entry); +if (!n) +return NULL ; + return rb_entry(n, struct rt_mutex_waiter, tree_entry); +} + +static inline struct rt_mutex_waiter * +__ww_waiter_last (struct rt_mutex *lock) +{ +struct rb_node *n =rb_last (&lock->rtmutex.waiters.rb_root); +if (!n) +return NULL ; + return rb_entry(n, struct rt_mutex_waiter, tree_entry); +} + +static inline void +__ww_waiter_add (struct rt_mutex *lock,struct rt_mutex_waiter *waiter,struct rt_mutex_waiter *pos) +{ +/* RT unconditionally adds the waiter first and then removes it on error */ +} + +static inline struct task_struct * +__ww_mutex_owner (struct rt_mutex *lock) +{ +return rt_mutex_owner (&lock->rtmutex); +} + +static inline bool +__ww_mutex_has_waiters (struct rt_mutex *lock) +{ +return rt_mutex_has_waiters (&lock->rtmutex); +} + +static inline void lock_wait_lock (struct rt_mutex *lock) +{ +raw_spin_lock (&lock->rtmutex.wait_lock); +} + +static inline void unlock_wait_lock (struct rt_mutex *lock) +{ +raw_spin_unlock (&lock->rtmutex.wait_lock); +} + +static inline void lockdep_assert_wait_lock_held (struct rt_mutex *lock) +{ +lockdep_assert_held (&lock->rtmutex.wait_lock); +} + +#endif /* WW_RT */ +/* * Wait-Die: * The newer transactions are killed when:
@@ -169,7 +247,9 @@ static boolif (waiter->ww_ctx->acquired >0 &&__ww_ctx_stamp_after (waiter->ww_ctx, ww_ctx)) { +#ifndef WW_RT debug_mutex_wake_waiter (lock, waiter); +#endif wake_up_process (waiter->task); }
PATCH 47/63 locking/ww_mutex: Add RT priority to W/W order
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
RTmutex based ww_mutexes cannot order based on timestamp. They have to order based on priority. Add the necessary decision logic.
--- kernel/locking/ww_mutex.h | 64 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 15 deletions(-)
---
kernel/locking/ww_mutex.h +49 -15
@@ -219,19 +219,54 @@ ww_mutex_lock_acquired(struct ww_mutex * }/* -* Determine if context @a is 'after' context @b. IOW, @a is a younger -* transaction than @b and depending on algorithm either needs to wait for -* @b or die. +* Determine if @a is 'less' than @b. IOW, either @a is a lower priority task +* or, when of equal priority, a younger transaction than @b. +* +* Depending on the algorithm, @a will either need to wait for @b, or die. */ static inline bool -__ww_ctx_stamp_after (struct ww_acquire_ctx *a,struct ww_acquire_ctx *b) +__ww_ctx_less (struct ww_acquire_ctx *a,struct ww_acquire_ctx *b) { +/* +* Can only do the RT prio for WW_RT because task->prio isn't stable due to PI, +* so the wait_list ordering will go wobbly. rt_mutex re-queues the waiter and +* isn't affected by this. +*/ +#ifdef WW_RT +/* kernel prio; less is more */ +int a_prio = a->task->prio; +int b_prio = b->task->prio; + +if (dl_prio (a_prio) ||dl_prio (b_prio)) { + +if (a_prio > b_prio) +return true; + +if (a_prio < b_prio) +return false; + +/* equal static prio */ + +if (dl_prio (a_prio)) { +if (dl_time_before (b->task->dl.deadline, + a->task->dl.deadline)) +return true; + +if (dl_time_before (a->task->dl.deadline, + b->task->dl.deadline)) +return false; + } + +/* equal prio */ + } +#endif +/* FIFO order tie break -- bigger is younger */ return (signed long )(a->stamp - b->stamp) >0 ; }/* -* Wait-Die; wake a younger waiter context (when locks held) such that it can +* Wait-Die; wake a lesser waiter context (when locks held) such that it can * die. * * Among waiters with context, only the first one can have other locks acquired
@@ -245,8 +280,7 @@ static boolif (!ww_ctx->is_wait_die)return false; -if (waiter->ww_ctx->acquired >0 && -__ww_ctx_stamp_after (waiter->ww_ctx, ww_ctx)) { +if (waiter->ww_ctx->acquired >0 &&__ww_ctx_less (waiter->ww_ctx, ww_ctx)) {#ifndef WW_RT debug_mutex_wake_waiter (lock, waiter);#endif
@@ -257,10 +291,10 @@ static bool }/* -* Wound-Wait; wound a younger @hold_ctx if it holds the lock. +* Wound-Wait; wound a lesser @hold_ctx if it holds the lock. * -* Wound the lock holder if there are waiters with older transactions than -* the lock holders. Even if multiple waiters may wound the lock holder, +* Wound the lock holder if there are waiters with more important transactions +* than the lock holders. Even if multiple waiters may wound the lock holder, * it's sufficient that only one does. */ static bool __ww_mutex_wound(struct MUTEX *lock,
@@ -287,7 +321,7 @@ static bool __ww_mutex_wound(struct MUTEif (!owner)return false; -if (ww_ctx->acquired >0 &&__ww_ctx_stamp_after (hold_ctx, ww_ctx)) { +if (ww_ctx->acquired >0 &&__ww_ctx_less (hold_ctx, ww_ctx)) { hold_ctx->wounded =1 ;/*
@@ -306,8 +340,8 @@ static bool __ww_mutex_wound(struct MUTE }/* -* We just acquired @lock under @ww_ctx, if there are later contexts waiting -* behind us on the wait-list, check if they need to die, or wound us. +* We just acquired @lock under @ww_ctx, if there are more important contexts +* waiting behind us on the wait-list, check if they need to die, or wound us. * * See __ww_mutex_add_waiter() for the list-order construction; basically the * list is ordered by stamp, smallest (oldest) first.
@@ -421,7 +455,7 @@ static inline intreturn 0 ; } -if (hold_ctx &&__ww_ctx_stamp_after (ctx, hold_ctx)) +if (hold_ctx &&__ww_ctx_less (ctx, hold_ctx))return __ww_mutex_kill (lock, ctx);/*
@@ -479,7 +513,7 @@ static inline intif (!cur->ww_ctx)continue ; -if (__ww_ctx_stamp_after (ww_ctx, cur->ww_ctx)) { +if (__ww_ctx_less (ww_ctx, cur->ww_ctx)) {/* * Wait-Die: if we find an older context waiting, there * is no point in queueing behind it, as we'd have to
PATCH 48/63 locking/ww_mutex: Add rt_mutex based lock type and accessors
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Provide the defines for RT mutex based ww_mutexes and fixup the debug logic so it's either enabled by DEBUG_MUTEXES or DEBUG_RT_MUTEXES on RT kernels.
--- include/linux/ww_mutex.h | 33 ++++++++++++++++++++++++--------- kernel/locking/ww_mutex.h | 6 +++--- 2 files changed, 27 insertions(+), 12 deletions(-)
---
include/linux/ww_mutex.h +24 -9
@@ -18,11 +18,24 @@#define __LINUX_WW_MUTEX_H#include <linux/mutex.h> +#include <linux/rtmutex.h> +#if defined(CONFIG_DEBUG_MUTEXES ) || \ + (defined(CONFIG_PREEMPT_RT ) && defined(CONFIG_DEBUG_RT_MUTEXES )) +#define DEBUG_WW_MUTEXES +#endif + +#ifndef CONFIG_PREEMPT_RT #define WW_MUTEX_BASE mutex#define ww_mutex_base_init (l,n,k) __mutex_init(l,n,k)#define ww_mutex_base_trylock (l) mutex_trylock(l)#define ww_mutex_base_is_locked (b) mutex_is_locked((b)) +#else +#define WW_MUTEX_BASE rt_mutex +#define ww_mutex_base_init (l,n,k) __rt_mutex_init(l,n,k) +#define ww_mutex_base_trylock (l) rt_mutex_trylock(l) +#define ww_mutex_base_is_locked (b) rt_mutex_base_is_locked(&(b)->rtmutex) +#endif struct ww_class {atomic_long_t stamp;
@@ -36,7 +49,7 @@ struct ww_class {struct ww_mutex {struct WW_MUTEX_BASE base;struct ww_acquire_ctx *ctx; -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES struct ww_class *ww_class;#endif };
@@ -47,10 +60,10 @@ struct ww_acquire_ctx {unsigned int acquired;unsigned short wounded;unsigned short is_wait_die; -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES unsigned int done_acquire;struct ww_class *ww_class; -struct ww_mutex *contending_lock; +void *contending_lock;#endif #ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map dep_map;
@@ -89,7 +102,7 @@ static inline void ww_mutex_init(struct {ww_mutex_base_init (&lock->base, ww_class->mutex_name, &ww_class->mutex_key); lock->ctx =NULL ; -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES lock->ww_class = ww_class;#endif }
@@ -126,7 +139,7 @@ static inline void ww_acquire_init(struc ctx->acquired =0 ; ctx->wounded = false; ctx->is_wait_die = ww_class->is_wait_die; -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES ctx->ww_class = ww_class; ctx->done_acquire =0 ; ctx->contending_lock =NULL ;
@@ -156,7 +169,7 @@ static inline void ww_acquire_init(struc*/ static inline void ww_acquire_done (struct ww_acquire_ctx *ctx) { -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES lockdep_assert_held (ctx);DEBUG_LOCKS_WARN_ON (ctx->done_acquire);
@@ -176,7 +189,7 @@ static inline void ww_acquire_fini(struc#ifdef CONFIG_DEBUG_LOCK_ALLOC mutex_release (&ctx->dep_map, _THIS_IP_);#endif -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES DEBUG_LOCKS_WARN_ON (ctx->acquired);if (!IS_ENABLED (CONFIG_PROVE_LOCKING ))/*
@@ -282,7 +295,7 @@ static inline void ww_mutex_lock_slow(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { int ret; -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES DEBUG_LOCKS_WARN_ON (!ctx->contending_lock);#endif ret =ww_mutex_lock (lock, ctx);
@@ -318,7 +331,7 @@ static inline int __must_check ww_mutex_lock_slow_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES DEBUG_LOCKS_WARN_ON(!ctx->contending_lock);#endif return ww_mutex_lock_interruptible (lock, ctx);
@@ -348,7 +361,9 @@ static inline int __must_check ww_mutex_*/ static inline void ww_mutex_destroy (struct ww_mutex *lock) { +#ifndef CONFIG_PREEMPT_RT mutex_destroy (&lock->base); +#endif }/**
kernel/locking/ww_mutex.h +3 -3
@@ -180,7 +180,7 @@ static inline void lockdep_assert_wait_l static __always_inline voidww_mutex_lock_acquired (struct ww_mutex *ww,struct ww_acquire_ctx *ww_ctx) { -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES /* * If this WARN_ON triggers, you used ww_mutex_lock to acquire, * but released with a normal mutex_unlock in this call.
@@ -413,7 +413,7 @@ static __always_inline int __ww_mutex_kill(struct MUTEX *lock, struct ww_acquire_ctx *ww_ctx) {if (ww_ctx->acquired >0 ) { -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base);
@@ -559,7 +559,7 @@ static inline intstatic inline void __ww_mutex_unlock (struct ww_mutex *lock) {if (lock->ctx) { -#ifdef CONFIG_DEBUG_MUTEXES +#ifdef DEBUG_WW_MUTEXES DEBUG_LOCKS_WARN_ON (!lock->ctx->acquired);#endif if (lock->ctx->acquired >0 )
PATCH 49/63 locking/rtmutex: Extend the rtmutex core to support ww_mutex
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Add a ww acquire context pointer to the waiter and various functions and add the ww_mutex related invocations to the proper spots in the locking code similar to the mutex based variant.
--- kernel/locking/rtmutex.c | 115 ++++++++++++++++++++++++++++++++++++---- kernel/locking/rtmutex_api.c | 4 - kernel/locking/rtmutex_common.h | 2 kernel/locking/rwsem.c | 2 4 files changed, 110 insertions(+), 13 deletions(-)
kernel/locking/rtmutex.c +105 -10
@@ -17,9 +17,44 @@#include <linux/sched/signal.h> #include <linux/sched/rt.h> #include <linux/sched/wake_q.h> +#include <linux/ww_mutex.h> #include "rtmutex_common.h" +#ifndef WW_RT +# define build_ww_mutex () (false) +# define ww_container_of (rtm) NULL + +static inline int __ww_mutex_add_waiter (struct rt_mutex_waiter *waiter, +struct rt_mutex *lock, +struct ww_acquire_ctx *ww_ctx) +{ +return 0 ; +} + +static inline void __ww_mutex_check_waiters (struct rt_mutex *lock, +struct ww_acquire_ctx *ww_ctx) +{ +} + +static inline void ww_mutex_lock_acquired (struct ww_mutex *lock, +struct ww_acquire_ctx *ww_ctx) +{ +} + +static inline int __ww_mutex_check_kill (struct rt_mutex *lock, +struct rt_mutex_waiter *waiter, +struct ww_acquire_ctx *ww_ctx) +{ +return 0 ; +} + +#else +# define build_ww_mutex () (true) +# define ww_container_of (rtm) container_of(rtm, struct ww_mutex, base) +# include "ww_mutex.h" +#endif +/* * lock->owner state tracking: *
@@ -291,7 +326,24 @@ static __always_inline int rt_mutex_wait static __always_inline bool __waiter_less(struct rb_node *a, const struct rb_node *b) { -return rt_mutex_waiter_less (__node_2_waiter (a),__node_2_waiter (b)); +struct rt_mutex_waiter *aw =__node_2_waiter (a); +struct rt_mutex_waiter *bw =__node_2_waiter (b); + +if (rt_mutex_waiter_less (aw, bw)) +return 1 ; +if (rt_mutex_waiter_less (bw, aw)) +return 0 ; + +/* NOTE: relies on waiter->ww_ctx being set before insertion */ +if (build_ww_mutex () && aw->ww_ctx) { +if (!bw->ww_ctx) +return 1 ; + +return (signed long )(aw->ww_ctx->stamp - + bw->ww_ctx->stamp) <0 ; + } + +return 0 ; }static __always_inline void
@@ -939,6 +991,7 @@ try_to_take_rt_mutex(struct rt_mutex_bas static int __sched task_blocks_on_rt_mutex(struct rt_mutex_base *lock,struct rt_mutex_waiter *waiter,struct task_struct *task, +struct ww_acquire_ctx *ww_ctx,enum rtmutex_chainwalk chwalk) {struct task_struct *owner =rt_mutex_owner (lock);
@@ -975,6 +1028,16 @@ static int __sched task_blocks_on_rt_mutraw_spin_unlock (&task->pi_lock); +if (build_ww_mutex () && ww_ctx) { +struct rt_mutex *rtm; + +/* Check whether the waiter should backout immediately */ + rtm = container_of(lock, struct rt_mutex, rtmutex); + res =__ww_mutex_add_waiter (waiter, rtm, ww_ctx); +if (res) +return res; + } +if (!owner)return 0 ;
@@ -1260,6 +1323,7 @@ static void __sched remove_waiter(struct/** * rt_mutex_slowlock_block() - Perform the wait-wake-try-to-take loop * @lock: the rt_mutex to take +* @ww_ctx: WW mutex context pointer * @state: the state the task should block in (TASK_INTERRUPTIBLE * or TASK_UNINTERRUPTIBLE) * @timeout: the pre-initialized and started timer, or NULL for none
@@ -1268,10 +1332,12 @@ static void __sched remove_waiter(struct* Must be called with lock->wait_lock held and interrupts disabled */ static int __sched rt_mutex_slowlock_block(struct rt_mutex_base *lock, +struct ww_acquire_ctx *ww_ctx,unsigned int state,struct hrtimer_sleeper *timeout,struct rt_mutex_waiter *waiter) { + struct rt_mutex *rtm = container_of(lock, struct rt_mutex, rtmutex);int ret =0 ;for (;;) {
@@ -1288,6 +1354,12 @@ static int __sched rt_mutex_slowlock_blobreak ; } +if (build_ww_mutex () && ww_ctx) { + ret =__ww_mutex_check_kill (rtm, waiter, ww_ctx); +if (ret) +break ; + } +raw_spin_unlock_irq (&lock->wait_lock);schedule ();
@@ -1310,6 +1382,9 @@ static void __sched rt_mutex_handle_deadif (res != -EDEADLOCK || detect_deadlock)return ; +if (build_ww_mutex () && w->ww_ctx) +return ; +/* * Yell loudly and stop the task right here. */
@@ -1323,31 +1398,46 @@ static void __sched rt_mutex_handle_dead/** * __rt_mutex_slowlock - Locking slowpath invoked with lock::wait_lock held * @lock: The rtmutex to block lock +* @ww_ctx: WW mutex context pointer * @state: The task state for sleeping * @chwalk: Indicator whether full or partial chainwalk is requested * @waiter: Initializer waiter for blocking */ static int __sched __rt_mutex_slowlock(struct rt_mutex_base *lock, +struct ww_acquire_ctx *ww_ctx,unsigned int state,enum rtmutex_chainwalk chwalk,struct rt_mutex_waiter *waiter) { + struct rt_mutex *rtm = container_of(lock, struct rt_mutex, rtmutex); +struct ww_mutex *ww =ww_container_of (rtm);int ret;lockdep_assert_held (&lock->wait_lock);/* Try to acquire the lock again: */ -if (try_to_take_rt_mutex (lock, current,NULL )) +if (try_to_take_rt_mutex (lock, current,NULL )) { +if (build_ww_mutex () && ww_ctx) { +__ww_mutex_check_waiters (rtm, ww_ctx); +ww_mutex_lock_acquired (ww, ww_ctx); + }return 0 ; + }set_current_state (state); - ret =task_blocks_on_rt_mutex (lock, waiter, current, chwalk); - + ret =task_blocks_on_rt_mutex (lock, waiter, current, ww_ctx, chwalk);if (likely (!ret)) - ret =rt_mutex_slowlock_block (lock, state,NULL , waiter); + ret =rt_mutex_slowlock_block (lock, ww_ctx, state,NULL , waiter); -if (unlikely (ret)) { +if (likely (!ret)) { +/* acquired the lock */ +if (build_ww_mutex () && ww_ctx) { +if (!ww_ctx->is_wait_die) +__ww_mutex_check_waiters (rtm, ww_ctx); +ww_mutex_lock_acquired (ww, ww_ctx); + } + }else {__set_current_state (TASK_RUNNING );remove_waiter (lock, waiter);rt_mutex_handle_deadlock (ret, chwalk, waiter);
@@ -1362,14 +1452,17 @@ static int __sched __rt_mutex_slowlock(s }static inline int __rt_mutex_slowlock_locked (struct rt_mutex_base *lock, +struct ww_acquire_ctx *ww_ctx,unsigned int state) {struct rt_mutex_waiter waiter;int ret;rt_mutex_init_waiter (&waiter); + waiter.ww_ctx = ww_ctx; - ret =__rt_mutex_slowlock (lock, state,RT_MUTEX_MIN_CHAINWALK , &waiter); + ret =__rt_mutex_slowlock (lock, ww_ctx, state,RT_MUTEX_MIN_CHAINWALK , + &waiter);debug_rt_mutex_free_waiter (&waiter);return ret;
@@ -1378,9 +1471,11 @@ static inline int __rt_mutex_slowlock_lo/* * rt_mutex_slowlock - Locking slowpath invoked when fast path fails * @lock: The rtmutex to block lock +* @ww_ctx: WW mutex context pointer * @state: The task state for sleeping */ static int __sched rt_mutex_slowlock(struct rt_mutex_base *lock, +struct ww_acquire_ctx *ww_ctx,unsigned int state) {unsigned long flags;
@@ -1395,7 +1490,7 @@ static int __sched rt_mutex_slowlock(str* irqsave/restore variants. */ raw_spin_lock_irqsave (&lock->wait_lock, flags); - ret =__rt_mutex_slowlock_locked (lock, state); + ret =__rt_mutex_slowlock_locked (lock, ww_ctx, state);raw_spin_unlock_irqrestore (&lock->wait_lock, flags);return ret;
@@ -1407,7 +1502,7 @@ static __always_inline int __rt_mutex_loif (likely (rt_mutex_cmpxchg_acquire (lock,NULL , current)))return 0 ; - return rt_mutex_slowlock(lock, state); + return rt_mutex_slowlock(lock, NULL, state); }#endif /* RT_MUTEX_BUILD_MUTEX */
@@ -1434,7 +1529,7 @@ static void __sched rtlock_slowlock_lock/* Save current state and set state to TASK_RTLOCK_WAIT */ current_save_and_set_rtlock_wait_state (); -task_blocks_on_rt_mutex (lock, &waiter, current,RT_MUTEX_MIN_CHAINWALK ); +task_blocks_on_rt_mutex (lock, &waiter, current,NULL ,RT_MUTEX_MIN_CHAINWALK );for (;;) {/* Try to acquire the lock again. */
kernel/locking/rtmutex_api.c +2 -2
@@ -267,7 +267,7 @@ int __sched __rt_mutex_start_proxy_lock(return 1 ;/* We enforce deadlock detection for futexes */ - ret =task_blocks_on_rt_mutex (lock, waiter, task, + ret =task_blocks_on_rt_mutex (lock, waiter, task,NULL ,RT_MUTEX_FULL_CHAINWALK );if (ret && !rt_mutex_owner (lock)) {
@@ -343,7 +343,7 @@ int __sched rt_mutex_wait_proxy_lock(strraw_spin_lock_irq (&lock->wait_lock);/* sleep on the mutex */ set_current_state (TASK_INTERRUPTIBLE ); - ret =rt_mutex_slowlock_block (lock,TASK_INTERRUPTIBLE , to, waiter); + ret =rt_mutex_slowlock_block (lock,NULL ,TASK_INTERRUPTIBLE , to, waiter);/* * try_to_take_rt_mutex() sets the waiter bit unconditionally. We might * have to fix that up.
kernel/locking/rtmutex_common.h +2 -0
@@ -28,6 +28,7 @@* @wake_state: Wakeup state to use (TASK_NORMAL or TASK_RTLOCK_WAIT) * @prio: Priority of the waiter * @deadline: Deadline of the waiter if applicable +* @ww_ctx: WW context pointer */ struct rt_mutex_waiter {struct rb_node tree_entry;
@@ -37,6 +38,7 @@ struct rt_mutex_waiter {unsigned int wake_state;int prio;u64 deadline; +struct ww_acquire_ctx *ww_ctx; };/**
kernel/locking/rwsem.c +1 -1
@@ -1360,7 +1360,7 @@ static inline void __downgrade_write(str __rt_mutex_lock(rtm, state)#define rwbase_rtmutex_slowlock_locked (rtm, state) \ - __rt_mutex_slowlock_locked(rtm, state) + __rt_mutex_slowlock_locked(rtm, NULL, state)#define rwbase_rtmutex_unlock (rtm) \ __rt_mutex_unlock(rtm)
PATCH 50/63 locking/ww_mutex: Implement rtmutex based ww_mutex API functions
In reply to Thomas GleixnerFrom: Peter Zijlstra <[email protected]>
Add the actual ww_mutex API functions which replace the mutex based variant on RT enabled kernels.
--- kernel/locking/Makefile | 2 - kernel/locking/ww_rt_mutex.c | 76 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-)
---
kernel/locking/Makefile +1 -1
@@ -25,7 +25,7 @@ obj-$(CONFIG_LOCK_SPIN_ON_OWNER) += osq_ obj-$(CONFIG_PROVE_LOCKING) += spinlock.o obj-$(CONFIG_QUEUED_SPINLOCKS) += qspinlock.o obj-$(CONFIG_RT_MUTEXES) += rtmutex_api.o -obj-$(CONFIG_PREEMPT_RT) += spinlock_rt.o +obj-$(CONFIG_PREEMPT_RT) += spinlock_rt.o ww_rt_mutex.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock_debug.o obj-$(CONFIG_QUEUED_RWLOCKS) += qrwlock.o
kernel/locking/ww_rt_mutex.c new file +76 -0
76 lines not on this page · raw message
Re: [patch 50/63] locking/ww_mutex: Implement rtmutex based ww_mutex API functions
On Fri, 2021-07-30 at 15:50 +0200, Thomas Gleixner wrote:
...
Quoted 3 lines
kernel/locking/ww_rt_mutex.c new file +0
-0
...
Quoted 5 lines
kernel/locking/ww_rt_mutex.c new file +5
-0+int __sched +ww_mutex_lock_interruptible (struct ww_mutex *lock,struct ww_acquire_ctx *ctx) +{ + return __ww_rt_mutex_lock(lock, ctx, TASK_UNINTERRUPTIBLE, _RET_IP_); +} ^^typo
Re: [patch 50/63] locking/ww_mutex: Implement rtmutex based ww_mutex API functions
Quoted 7 lines · Mike Galbraith
On Fri, 2021-07-30 at 15:50 +0200, Thomas Gleixner wrote:
...
Quoted 3 lines
kernel/locking/ww_rt_mutex.c new file +0
-0...
Quoted 5 lines
kernel/locking/ww_rt_mutex.c
Quoted 5 lines
kernel/locking/ww_rt_mutex.c new file +5
-0+int __sched +ww_mutex_lock_interruptible (struct ww_mutex *lock,struct ww_acquire_ctx *ctx) +{ + return __ww_rt_mutex_lock(lock, ctx, TASK_UNINTERRUPTIBLE, _RET_IP_); +} ^^typo
Ooops.
PATCH 51/63 locking/rtmutex: Add mutex variant for RT
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
Add the necessary defines, helpers and API functions for replacing mutex on a PREEMPT_RT enabled kernel with a rtmutex based variant.
No functional change when CONFIG_PREEMPT_RT=n
--- include/linux/mutex.h | 67 ++++++++++++++++++++--- kernel/locking/mutex.c | 4 + kernel/locking/rtmutex_api.c | 122 +++++++++++++++++++++++++++++++++++++++++++ lib/Kconfig.debug | 11 ++- 4 files changed, 188 insertions(+), 16 deletions(-)
---
include/linux/mutex.h +57 -10
@@ -20,6 +20,18 @@#include <linux/osq_lock.h> #include <linux/debug_locks.h> +#ifdef CONFIG_DEBUG_LOCK_ALLOC +# define __DEP_MAP_MUTEX_INITIALIZER (lockname) \ + , .dep_map = { \ + .name = #lockname, \ + .wait_type_inner = LD_WAIT_SLEEP, \ + } +#else +# define __DEP_MAP_MUTEX_INITIALIZER (lockname) +#endif + +#ifndef CONFIG_PREEMPT_RT +/* * Simple, straightforward mutexes with strict semantics: *
@@ -93,16 +105,6 @@ do { \ __mutex_init((mutex), #mutex, &__key); \ } while (0) -#ifdef CONFIG_DEBUG_LOCK_ALLOC -# define __DEP_MAP_MUTEX_INITIALIZER (lockname) \ - , .dep_map = { \ - .name = #lockname, \ - .wait_type_inner = LD_WAIT_SLEEP, \ - } -#else -# define __DEP_MAP_MUTEX_INITIALIZER (lockname) -#endif -#define __MUTEX_INITIALIZER (lockname) \ { .owner = ATOMIC_LONG_INIT(0) \ , .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(lockname.wait_lock) \
@@ -124,6 +126,51 @@ extern void __mutex_init(struct mutex *l*/ extern bool mutex_is_locked (struct mutex *lock); +#else /* !CONFIG_PREEMPT_RT */ +/* +* Preempt-RT variant based on rtmutexes. +*/ +#include <linux/rtmutex.h> + +struct mutex { +struct rt_mutex_base rtmutex; +#ifdef CONFIG_DEBUG_LOCK_ALLOC +struct lockdep_map dep_map; +#endif +}; + +#define __MUTEX_INITIALIZER (mutexname) \ +{ \ + .rtmutex = __RT_MUTEX_BASE_INITIALIZER(mutexname.rtmutex) \ + __DEP_MAP_MUTEX_INITIALIZER(mutexname) \ +} + +#define DEFINE_MUTEX (mutexname) \ + struct mutex mutexname = __MUTEX_INITIALIZER(mutexname) + +extern void __mutex_rt_init (struct mutex *lock,const char *name, +struct lock_class_key *key); +extern int mutex_trylock (struct mutex *lock); + +static inline void mutex_destroy (struct mutex *lock) { } + +#define mutex_is_locked (l) rt_mutex_base_is_locked(&(l)->rtmutex) + +#define mutex_init (mutex) \ +do { \ + static struct lock_class_key __key; \ + \ + rt_mutex_base_init(&(mutex)->rtmutex); \ + __mutex_rt_init((mutex), #mutex, &__key); \ +} while (0) + +#define __mutex_init (mutex, name, key) \ +do { \ + rt_mutex_base_init(&(mutex)->rtmutex); \ + __mutex_rt_init((mutex), name, key); \ +} while (0) +#endif /* CONFIG_PREEMPT_RT */ +/* * See kernel/locking/mutex.c for detailed documentation of these APIs. * Also see Documentation/locking/mutex-design.rst.
kernel/locking/mutex.c +3 -1
@@ -30,6 +30,7 @@#include <linux/debug_locks.h> #include <linux/osq_lock.h> +#ifndef CONFIG_PREEMPT_RT #include "mutex.h" void
@@ -1078,7 +1079,8 @@ ww_mutex_lock_interruptible(struct ww_mu }EXPORT_SYMBOL (ww_mutex_lock_interruptible); -#endif +#endif /* !CONFIG_DEBUG_LOCK_ALLOC */ +#endif /* !CONFIG_PREEMPT_RT */ /** * atomic_dec_and_mutex_lock - return holding mutex if we dec to 0
kernel/locking/rtmutex_api.c +122 -0
125 lines not on this page · raw message
lib/Kconfig.debug +6 -5
@@ -1235,7 +1235,7 @@ config PROVE_LOCKING depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT select LOCKDEP select DEBUG_SPINLOCK - select DEBUG_MUTEXES + select DEBUG_MUTEXES if !PREEMPT_RT select DEBUG_RT_MUTEXES if RT_MUTEXES select DEBUG_RWSEMS select DEBUG_WW_MUTEX_SLOWPATH
@@ -1299,7 +1299,7 @@ config LOCK_STAT depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT select LOCKDEP select DEBUG_SPINLOCK - select DEBUG_MUTEXES + select DEBUG_MUTEXES if !PREEMPT_RT select DEBUG_RT_MUTEXES if RT_MUTEXES select DEBUG_LOCK_ALLOC default n
@@ -1335,7 +1335,7 @@ config DEBUG_SPINLOCK config DEBUG_MUTEXES bool "Mutex debugging: basic checks" - depends on DEBUG_KERNEL + depends on DEBUG_KERNEL && !PREEMPT_RT help This feature allows mutex semantics violations to be detected and reported.
@@ -1345,7 +1345,8 @@ config DEBUG_WW_MUTEX_SLOWPATH depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT select DEBUG_LOCK_ALLOC select DEBUG_SPINLOCK - select DEBUG_MUTEXES + select DEBUG_MUTEXES if !PREEMPT_RT + select DEBUG_RT_MUTEXES if PREEMPT_RT help This feature enables slowpath testing for w/w mutex users by injecting additional -EDEADLK wound/backoff cases. Together with
@@ -1368,7 +1369,7 @@ config DEBUG_LOCK_ALLOC bool "Lock debugging: detect incorrect freeing of live locks" depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT select DEBUG_SPINLOCK - select DEBUG_MUTEXES + select DEBUG_MUTEXES if !PREEMPT_RT select DEBUG_RT_MUTEXES if RT_MUTEXES select LOCKDEP help
PATCH 52/63 lib/test_lockup: Adapt to changed variables.
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
The inner parts of certain locks (mutex, rwlocks) changed due to a rework for RT and non RT code. Most users remain unaffected, but those who fiddle around in the inner parts need to be updated.
Match the struct names to the new layout.
--- lib/test_lockup.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
---
lib/test_lockup.c +4 -4
@@ -485,13 +485,13 @@ static int __init test_lockup_init(void) offsetof(spinlock_t, lock.wait_lock.magic), SPINLOCK_MAGIC) || test_magic(lock_rwlock_ptr, - offsetof(rwlock_t, rtmutex.wait_lock.magic), + offsetof(rwlock_t, rwbase.rtmutex.wait_lock.magic), SPINLOCK_MAGIC) || test_magic(lock_mutex_ptr, - offsetof(struct mutex, lock.wait_lock.magic), + offsetof(struct mutex, rtmutex.wait_lock.magic), SPINLOCK_MAGIC) || test_magic(lock_rwsem_ptr, - offsetof(struct rw_semaphore, rtmutex.wait_lock.magic), + offsetof(struct rw_semaphore, rwbase.rtmutex.wait_lock.magic), SPINLOCK_MAGIC))return -EINVAL ;#else
@@ -502,7 +502,7 @@ static int __init test_lockup_init(void) offsetof(rwlock_t, magic), RWLOCK_MAGIC) ||test_magic (lock_mutex_ptr, - offsetof(struct mutex, wait_lock.rlock.magic), + offsetof(struct mutex, wait_lock.magic),SPINLOCK_MAGIC ) ||test_magic (lock_rwsem_ptr, offsetof(struct rw_semaphore, wait_lock.magic),
PATCH 53/63 futex: Validate waiter correctly in futex_proxy_trylock_atomic()
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The loop in futex_requeue() has a sanity check for the waiter which is missing in futex_proxy_trylock_atomic(). In theory the key2 check is sufficient, but futexes are cursed so add it for completness and paranoia sake.
--- kernel/futex.c | 4 ++++ 1 file changed, 4 insertions(+)
---
kernel/futex.c +4 -0
@@ -1879,6 +1879,10 @@ futex_proxy_trylock_atomic(u32 __user *pif (!top_waiter)return 0 ; +/* Ensure that this is a waiter sitting in futex_wait_requeue_pi() */ +if (!top_waiter->rt_waiter || top_waiter->pi_state) + ret = -EINVAL ; +/* Ensure we requeue to the expected futex. */ if (!match_futex (top_waiter->requeue_pi_key, key2))return -EINVAL ;
PATCH 54/63 futex: Cleanup stale comments
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The futex key reference mechanism is long gone. Cleanup the stale comments which still mention it.
---
V2: Cleanup more key ref comments - Andre
--- kernel/futex.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
---
kernel/futex.c +7 -11
@@ -1354,7 +1354,7 @@ static int lock_pi_update_atomic(u32 __u* - 1 - acquired the lock; * - <0 - error * -* The hb->lock and futex_key refs shall be held by the caller. +* The hb->lock must be held by the caller. * * @exiting is only set when the return value is -EBUSY. If so, this holds * a refcount on the exiting task on return and the caller needs to drop it
@@ -2618,8 +2618,7 @@ static void futex_wait_queue_me(struct f* * Setup the futex_q and locate the hash_bucket. Get the futex value and * compare it with the expected value. Handle atomic faults internally. -* Return with the hb lock held and a q.key reference on success, and unlocked -* with no q.key reference on failure. +* Return with the hb lock held on success, and unlocked on failure. * * Return: * - 0 - uaddr contains val and hb has been locked;
@@ -2697,8 +2696,8 @@ static int futex_wait(u32 __user *uaddr, current->timer_slack_ns); retry:/* -* Prepare to wait on uaddr. On success, holds hb lock and increments -* q.key refs. +* Prepare to wait on uaddr. On success, it holds hb->lock and q +* is initialized. */ ret =futex_wait_setup (uaddr, val, flags, &q, &hb);if (ret)
@@ -2709,7 +2708,6 @@ static int futex_wait(u32 __user *uaddr,/* If we were woken (and unqueued), we succeeded, whatever. */ ret =0 ; -/* unqueue_me() drops q.key ref */ if (!unqueue_me (&q)) goto out; ret = -ETIMEDOUT ;
@@ -3202,8 +3200,8 @@ static int futex_wait_requeue_pi(u32 __u q.requeue_pi_key = &key2;/* -* Prepare to wait on uaddr. On success, increments q.key (key1) ref -* count. +* Prepare to wait on uaddr. On success, it holds hb->lock and q +* is initialized. */ ret =futex_wait_setup (uaddr, val, flags, &q, &hb);if (ret)
@@ -3232,9 +3230,7 @@ static int futex_wait_requeue_pi(u32 __u* In order for us to be here, we know our q.key == key2, and since * we took the hb->lock above, we also know that futex_requeue() has * completed and we no longer have to concern ourselves with a wakeup -* race with the atomic proxy lock acquisition by the requeue code. The -* futex_requeue dropped our key1 reference and incremented our key2 -* reference count. +* race with the atomic proxy lock acquisition by the requeue code. */ /*
PATCH 55/63 futex: Correct the number of requeued waiters for PI
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The accounting is wrong when either the PI sanity check or the requeue PI operation fails. Adjust it in the failure path.
Will be simplified in the next step.
--- kernel/futex.c | 4 ++++ 1 file changed, 4 insertions(+)
---
kernel/futex.c +4 -0
@@ -2128,6 +2128,8 @@ static int futex_requeue(u32 __user *uad/* Ensure we requeue to the expected futex for requeue_pi. */ if (requeue_pi && !match_futex (this->requeue_pi_key, &key2)) { +/* Don't account for it */ + task_count--; ret = -EINVAL ;break ; }
@@ -2169,6 +2171,8 @@ static int futex_requeue(u32 __user *uad*/ this->pi_state =NULL ;put_pi_state (pi_state); +/* Don't account for it */ + task_count--;/* * We stop queueing more waiters and let user * space deal with the mess.
PATCH 56/63 futex: Restructure futex_requeue()
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
No point in taking two more 'requeue_pi' conditionals just to get to the requeue. Same for the requeue_pi case just the other way round.
No functional change.
--- kernel/futex.c | 90 +++++++++++++++++++++++++-------------------------------- 1 file changed, 41 insertions(+), 49 deletions(-)
---
kernel/futex.c +41 -49
@@ -2116,20 +2116,17 @@ static int futex_requeue(u32 __user *uadbreak ; } -/* -* Wake nr_wake waiters. For requeue_pi, if we acquired the -* lock, we already woke the top_waiter. If not, it will be -* woken by futex_unlock_pi(). -*/ -if (++task_count <= nr_wake && !requeue_pi) { -mark_wake_futex (&wake_q, this); +/* Plain futexes just wake or requeue and are done */ +if (!requeue_pi) { +if (++task_count <= nr_wake) +mark_wake_futex (&wake_q, this); +else +requeue_futex (this, hb1, hb2, &key2);continue ; }/* Ensure we requeue to the expected futex for requeue_pi. */ -if (requeue_pi && !match_futex (this->requeue_pi_key, &key2)) { -/* Don't account for it */ - task_count--; +if (!match_futex (this->requeue_pi_key, &key2)) { ret = -EINVAL ;break ; }
@@ -2137,50 +2134,45 @@ static int futex_requeue(u32 __user *uad/* * Requeue nr_requeue waiters and possibly one more in the case * of requeue_pi if we couldn't acquire the lock atomically. +* +* Prepare the waiter to take the rt_mutex. Take a refcount +* on the pi_state and store the pointer in the futex_q +* object of the waiter. */ -if (requeue_pi) { +get_pi_state (pi_state); + this->pi_state = pi_state; + ret =rt_mutex_start_proxy_lock (&pi_state->pi_mutex, + this->rt_waiter, this->task); +if (ret ==1 ) { +/* +* We got the lock. We do neither drop the refcount +* on pi_state nor clear this->pi_state because the +* waiter needs the pi_state for cleaning up the +* user space value. It will drop the refcount +* after doing so. +*/ +requeue_pi_wake_futex (this, &key2, hb2); + task_count++; +continue ; + }else if (ret) { +/* +* rt_mutex_start_proxy_lock() detected a potential +* deadlock when we tried to queue that waiter. +* Drop the pi_state reference which we took above +* and remove the pointer to the state from the +* waiters futex_q object. +*/ + this->pi_state =NULL ; +put_pi_state (pi_state);/* -* Prepare the waiter to take the rt_mutex. Take a -* refcount on the pi_state and store the pointer in -* the futex_q object of the waiter. +* We stop queueing more waiters and let user space +* deal with the mess. */ -get_pi_state (pi_state); - this->pi_state = pi_state; - ret =rt_mutex_start_proxy_lock (&pi_state->pi_mutex, - this->rt_waiter, - this->task); -if (ret ==1 ) { -/* -* We got the lock. We do neither drop the -* refcount on pi_state nor clear -* this->pi_state because the waiter needs the -* pi_state for cleaning up the user space -* value. It will drop the refcount after -* doing so. -*/ -requeue_pi_wake_futex (this, &key2, hb2); -continue ; - }else if (ret) { -/* -* rt_mutex_start_proxy_lock() detected a -* potential deadlock when we tried to queue -* that waiter. Drop the pi_state reference -* which we took above and remove the pointer -* to the state from the waiters futex_q -* object. -*/ - this->pi_state =NULL ; -put_pi_state (pi_state); -/* Don't account for it */ - task_count--; -/* -* We stop queueing more waiters and let user -* space deal with the mess. -*/ -break ; - } +break ; } +/* Waiter is queued, move it to hb2 */ requeue_futex (this, hb1, hb2, &key2); + task_count++; }/*
PATCH 57/63 futex: Clarify comment in futex_requeue()
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The comment about the restriction of the number of waiters to wake for the REQUEUE_PI case is confusing at best. Rewrite it.
--- kernel/futex.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-)
---
kernel/futex.c +20 -8
@@ -1957,15 +1957,27 @@ static int futex_requeue(u32 __user *uad*/ if (refill_pi_state_cache ())return -ENOMEM ; +/* -* requeue_pi must wake as many tasks as it can, up to nr_wake -* + nr_requeue, since it acquires the rt_mutex prior to -* returning to userspace, so as to not leave the rt_mutex with -* waiters and no owner. However, second and third wake-ups -* cannot be predicted as they involve race conditions with the -* first wake and a fault while looking up the pi_state. Both -* pthread_cond_signal() and pthread_cond_broadcast() should -* use nr_wake=1. +* futex_requeue() allows the caller to define the number +* of waiters to wake up via the @nr_wake argument. With +* REQUEUE_PI waking up more than one waiter is creating +* more problems than it solves. Waking up a waiter makes +* only sense if the PI futex @uaddr2 is uncontended as +* this allows the requeue code to acquire the futex +* @uaddr2 before waking the waiter. The waiter can then +* return to user space without further action. A secondary +* wakeup would just make the futex_wait_requeue_pi() +* handling more complex because that code would have to +* look up pi_state and do more or less all the handling +* which the requeue code has to do for the to be requeued +* waiters. So restrict the number of waiters to wake to +* one and only wake it up when the PI futex is +* uncontended. Otherwise requeue it and let the unlock of +* the PI futex handle the wakeup. +* +* All REQUEUE_PI users, e.g. pthread_cond_signal() and +* pthread_cond_broadcast() must use nr_wake=1. */ if (nr_wake !=1 )return -EINVAL ;
PATCH 58/63 futex: Prevent requeue_pi() lock nesting issue on RT
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
The requeue_pi() operation on RT kernels creates a problem versus the task::pi_blocked_on state when a waiter is woken early (signal, timeout) and that early wake up interleaves with the requeue_pi() operation.
When the requeue manages to block the waiter on the rtmutex which is associated to the second futex, then a concurrent early wakeup of that waiter faces the problem that it has to acquire the hash bucket spinlock, which is not an issue on non-RT kernels, but on RT kernels spinlocks are substituted by 'sleeping' spinlocks based on rtmutex. If the hash bucket lock is contended then blocking on that spinlock would result in a impossible situation: blocking on two locks at the same time (the hash bucket lock and the rtmutex representing the PI futex).
It was considered to make the hash bucket locks raw_spinlocks, but especially requeue operations with a large amount of waiters can introduce significant latencies, so that's not an option for RT.
The RT tree carried a solution which (ab)used task::pi_blocked_on to store the information about an ongoing requeue and an early wakeup which worked, but required to add checks for these special states all over the place.
The distangling of an early wakeup of a waiter for a requeue_pi() operation is already looking at quite some different states and the task::pi_blocked_on magic just expanded that to a hard to understand 'state machine'.
This can be avoided by keeping track of the waiter/requeue state in the futex_q object itself.
Add a requeue_state field to struct futex_q with the following possible states:
Q_REQUEUE_PI_NONE
Q_REQUEUE_PI_IGNORE
Q_REQUEUE_PI_IN_PROGRESS
Q_REQUEUE_PI_WAIT
Q_REQUEUE_PI_DONE
Q_REQUEUE_PI_LOCKED
The waiter starts with state = NONE and the following state transitions are valid:
On the waiter side:
Q_REQUEUE_PI_NONE -> Q_REQUEUE_PI_IGNORE
Q_REQUEUE_PI_IN_PROGRESS -> Q_REQUEUE_PI_WAIT
On the requeue side:
Q_REQUEUE_PI_NONE -> Q_REQUEUE_PI_INPROGRESS
Q_REQUEUE_PI_IN_PROGRESS -> Q_REQUEUE_PI_DONE/LOCKED
Q_REQUEUE_PI_IN_PROGRESS -> Q_REQUEUE_PI_NONE (requeue failed)
Q_REQUEUE_PI_WAIT -> Q_REQUEUE_PI_DONE/LOCKED
Q_REQUEUE_PI_WAIT -> Q_REQUEUE_PI_IGNORE (requeue failed)
The requeue side ignores a waiter with state Q_REQUEUE_PI_IGNORE as this signals that the waiter is already on the way out. It also means that the waiter is still on the 'wait' futex, i.e. uaddr1.
The waiter side signals early wakeup to the requeue side either through setting state to Q_REQUEUE_PI_IGNORE or to Q_REQUEUE_PI_WAIT depending on the current state. In case of Q_REQUEUE_PI_IGNORE it can immediately proceed to take the hash bucket lock of uaddr1. If it set state to WAIT, which means the wakeup is interleaving with a requeue in progress it has to wait for the requeue side to change the state. Either to DONE/LOCKED or to IGNORE. DONE/LOCKED means the waiter q is now on the uaddr2 futex and either blocked (DONE) or has acquired it (LOCKED). IGNORE is set by the requeue side when the requeue attempt failed via deadlock detection and therefore the waiter's futex_q is still on the uaddr1 futex.
While this is not strictly required on !RT making this unconditional has the benefit of common code and it also allows the waiter to avoid taking the hash bucket lock on the way out in certain cases, which reduces contention.
Add the required helpers required for the state transitions, invoke them at the right places and restructure the futex_wait_requeue_pi() code to handle the return from wait (early or not) based on the state machine values.
On !RT enabled kernels the waiter spin waits for the state going from Q_REQUEUE_PI_WAIT to some other state, on RT enabled kernels this is handled by rcuwait_wait_event() and the corresponding wake up on the requeue side.
--- kernel/futex.c | 349 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 278 insertions(+), 71 deletions(-)
---
kernel/futex.c +278 -71
@@ -197,6 +197,8 @@ struct futex_pi_state {* @rt_waiter: rt_waiter storage for use with requeue_pi * @requeue_pi_key: the requeue_pi target futex key * @bitset: bitset for the optional bitmasked wakeup +* @requeue_state: State field for futex_requeue_pi() +* @requeue_wait: RCU wait for futex_requeue_pi() (RT only) * * We use this hashed waitqueue, instead of a normal wait_queue_entry_t, so * we can wake only the relevant ones (hashed queues may be shared).
@@ -219,6 +221,10 @@ struct futex_q {struct rt_mutex_waiter *rt_waiter;union futex_key *requeue_pi_key;u32 bitset; +atomic_t requeue_state; +#ifdef CONFIG_PREEMPT_RT +struct rcuwait requeue_wait; +#endif } __randomize_layout;static const struct futex_q futex_q_init = {
@@ -1793,6 +1799,158 @@ void requeue_futex(struct futex_q *q, st q->key = *key2; } +/* +* On PREEMPT_RT, the hash bucket lock is a 'sleeping' spinlock with an +* underlying rtmutex. The task which is about to be requeued could have +* just woken up (timeout, signal). After the wake up the task has to +* acquire hash bucket lock, which is held by the requeue code. As a task +* can only be blocked on _ONE_ rtmutex at a time, the proxy lock blocking +* and the hash bucket lock blocking would collide and corrupt state. +* +* On !PREEMPT_RT this is not a problem and everything could be serialized +* on hash bucket lock, but aside of having the benefit of common code, +* this allows to avoid doing the requeue when the task is already on the +* way out and taking the hash bucket lock of the original uaddr1 when the +* requeue has been completed. +* +* The following state transitions are valid: +* +* On the waiter side: +* Q_REQUEUE_PI_NONE -> Q_REQUEUE_PI_IGNORE +* Q_REQUEUE_PI_IN_PROGRESS -> Q_REQUEUE_PI_WAIT +* +* On the requeue side: +* Q_REQUEUE_PI_NONE -> Q_REQUEUE_PI_INPROGRESS +* Q_REQUEUE_PI_IN_PROGRESS -> Q_REQUEUE_PI_DONE/LOCKED +* Q_REQUEUE_PI_IN_PROGRESS -> Q_REQUEUE_PI_NONE (requeue failed) +* Q_REQUEUE_PI_WAIT -> Q_REQUEUE_PI_DONE/LOCKED +* Q_REQUEUE_PI_WAIT -> Q_REQUEUE_PI_IGNORE (requeue failed) +* +* The requeue side ignores a waiter with state Q_REQUEUE_PI_IGNORE as this +* signals that the waiter is already on the way out. It also means that +* the waiter is still on the 'wait' futex, i.e. uaddr1. +* +* The waiter side signals early wakeup to the requeue side either through +* setting state to Q_REQUEUE_PI_IGNORE or to Q_REQUEUE_PI_WAIT depending +* on the current state. In case of Q_REQUEUE_PI_IGNORE it can immediately +* proceed to take the hash bucket lock of uaddr1. If it set state to WAIT, +* which means the wakeup is interleaving with a requeue in progress it has +* to wait for the requeue side to change the state. Either to DONE/LOCKED +* or to IGNORE. DONE/LOCKED means the waiter q is now on the uaddr2 futex +* and either blocked (DONE) or has acquired it (LOCKED). IGNORE is set by +* the requeue side when the requeue attempt failed via deadlock detection +* and therefore the waiter q is still on the uaddr1 futex. +*/ +enum { +Q_REQUEUE_PI_NONE =0 , +Q_REQUEUE_PI_IGNORE , +Q_REQUEUE_PI_IN_PROGRESS , +Q_REQUEUE_PI_WAIT , +Q_REQUEUE_PI_DONE , +Q_REQUEUE_PI_LOCKED , +}; + +static inline bool futex_requeue_pi_prepare (struct futex_q *q, +struct futex_pi_state *pi_state) +{ +int cur, res, new; + +/* +* Set state to Q_REQUEUE_PI_IN_PROGRESS unless an early wakeup has +* already set Q_REQUEUE_PI_IGNORE to signal that requeue should +* ignore the waiter. +*/ +for (cur =atomic_read (&q->requeue_state);; cur = res) { +if (cur ==Q_REQUEUE_PI_IGNORE ) +return false; + +/* +* futex_proxy_trylock_atomic() might have set it to +* IN_PROGRESS and a interleaved early wake to WAIT. +* +* It was considered to have an extra state for that +* trylock, but that would just add more conditionals +* all over the place for a dubious value. +*/ +if (cur !=Q_REQUEUE_PI_NONE ) +break ; + + new =Q_REQUEUE_PI_IN_PROGRESS ; + res =atomic_cmpxchg (&q->requeue_state, cur, new); +if (likely (cur == res)) +break ; + } + q->pi_state = pi_state; +return true; +} + +static inline void futex_requeue_pi_complete (struct futex_q *q,int locked) +{ +int cur, res, new; + +for (cur =atomic_read (&q->requeue_state);; cur = res) { +if (locked >=0 ) { +/* Requeue succeeded. Set DONE or LOCKED */ + new =Q_REQUEUE_PI_DONE + locked; + }else if (cur ==Q_REQUEUE_PI_IN_PROGRESS ) { +/* Deadlock, no early wakeup interleave */ + new =Q_REQUEUE_PI_NONE ; + }else { +/* Deadlock, early wakeup interleave. */ + new =Q_REQUEUE_PI_IGNORE ; + } + + res =atomic_cmpxchg (&q->requeue_state, cur, new); +if (likely (cur == res)) +break ; + } + +#ifdef CONFIG_PREEMPT_RT +/* If the waiter interleaved with the requeue let it know */ +if (unlikely (cur ==Q_REQUEUE_PI_WAIT )) +rcuwait_wake_up (&q->requeue_wait); +#endif +} + +static inline int futex_requeue_pi_wakeup_sync (struct futex_q *q) +{ +int cur, new, res; + +for (cur =atomic_read (&q->requeue_state);; cur = res) { +/* Is requeue done already? */ +if (cur >=Q_REQUEUE_PI_DONE ) +break ; + +/* +* If not done, then tell the requeue code to either ignore +* the waiter or to wake it up once the requeue is done. +*/ + new = !cur ?Q_REQUEUE_PI_IGNORE :Q_REQUEUE_PI_WAIT ; + res =atomic_cmpxchg (&q->requeue_state, cur, new); +if (likely (cur == res)) +break ; + } + +/* If the requeue was in progress, wait for it to complete */ +if (cur ==Q_REQUEUE_PI_IN_PROGRESS ) { +#ifdef CONFIG_PREEMPT_RT +rcuwait_wait_event (&q->requeue_wait, +atomic_read (&q->requeue_state) !=Q_REQUEUE_PI_WAIT , +TASK_UNINTERRUPTIBLE ); +#else +while (atomic_read (&q->requeue_state) ==Q_REQUEUE_PI_WAIT ) +cpu_relax (); +#endif + } + +/* +* Requeue is now either prohibited or complete. Reread state +* because during the wait above it might have changed. Nothing +* will modify q->requeue_state after this point. +*/ +return atomic_read (&q->requeue_state); +} +/** * requeue_pi_wake_futex() - Wake a task that acquired the lock during requeue * @q: the futex_q
@@ -1820,6 +1978,8 @@ void requeue_pi_wake_futex(struct futex_ q->lock_ptr = &hb->lock; +/* Signal locked state to the waiter */ +futex_requeue_pi_complete (q,1 );wake_up_state (q->task,TASK_NORMAL ); }
@@ -1887,6 +2047,10 @@ futex_proxy_trylock_atomic(u32 __user *pif (!match_futex (top_waiter->requeue_pi_key, key2))return -EINVAL ; +/* Ensure that this does not race against an early wakeup */ +if (!futex_requeue_pi_prepare (top_waiter,NULL )) +return -EAGAIN ; +/* * Try to take the lock for top_waiter. Set the FUTEX_WAITERS bit in * the contended case or if set_waiters is 1. The pi_state is returned
@@ -1896,8 +2060,22 @@ futex_proxy_trylock_atomic(u32 __user *p ret =futex_lock_pi_atomic (pifutex, hb2, key2, ps, top_waiter->task, exiting, set_waiters);if (ret ==1 ) { +/* Dequeue, wake up and update top_waiter::requeue_state */ requeue_pi_wake_futex (top_waiter, key2, hb2);return vpid; + }else if (ret <0 ) { +/* Rewind top_waiter::requeue_state */ +futex_requeue_pi_complete (top_waiter, ret); + }else { +/* +* futex_lock_pi_atomic() did not acquire the user space +* futex, but managed to establish the proxy lock and pi +* state. top_waiter::requeue_state cannot be fixed up here +* because the waiter is not enqueued on the rtmutex +* yet. This is handled at the callsite depending on the +* result of rt_mutex_start_proxy_lock() which is +* guaranteed to be reached with this function returning 0. +*/ }return ret; }
@@ -2038,6 +2216,8 @@ static int futex_requeue(u32 __user *uad* intend to requeue waiters, force setting the FUTEX_WAITERS * bit. We force this here where we are able to easily handle * faults rather in the requeue loop below. +* +* Updates topwaiter::requeue_state if a top waiter exists. */ ret = futex_proxy_trylock_atomic(uaddr2, hb1, hb2, &key1, &key2, &pi_state,
@@ -2051,6 +2231,27 @@ static int futex_requeue(u32 __user *uad* vpid of the top waiter task. * If the lock was not taken, we have pi_state and an initial * refcount on it. In case of an error we have nothing. +* +* The top waiter's requeue_state is up to date: +* +* - If the lock was acquired atomically (ret > 0), then +* the state is Q_REQUEUE_PI_LOCKED. No matter whether +* the below lookup_pi_state() fails or not requeue_state +* is correct because that waiter is dequeued and woken +* up and nothing can hold it up. +* +* - If the trylock failed with an error (ret < 0) then +* the state is either Q_REQUEUE_PI_NONE, i.e. "nothing +* happened", or Q_REQUEUE_PI_IGNORE when there was an +* interleaved early wakeup. +* +* - If the trylock did not succeed (ret == 0) then the +* state is either Q_REQUEUE_PI_IN_PROGRESS or +* Q_REQUEUE_PI_WAIT if an early wakeup interleaved. +* This will be cleaned up in the loop below, which +* cannot fail because futex_proxy_trylock_atomic() did +* the same sanity checks for requeue_pi as the loop +* below does. */ if (ret >0 ) {WARN_ON (pi_state);
@@ -2076,7 +2277,10 @@ static int futex_requeue(u32 __user *uad/* We hold a reference on the pi state. */ break ; -/* If the above failed, then pi_state is NULL */ +/* +* If the above failed, then pi_state is NULL and +* waiter::requeue_state is correct. +*/ case -EFAULT :double_unlock_hb (hb1, hb2);hb_waiters_dec (hb2);
@@ -2152,21 +2356,39 @@ static int futex_requeue(u32 __user *uad* object of the waiter. */ get_pi_state (pi_state); - this->pi_state = pi_state; + +/* Don't requeue when the waiter is already on the way out. */ +if (!futex_requeue_pi_prepare (this, pi_state)) { +/* +* Early woken waiter signaled that it is on the +* way out. Drop the pi_state reference and try the +* next waiter. @this->pi_state is still NULL. +*/ +put_pi_state (pi_state); +continue ; + } + ret =rt_mutex_start_proxy_lock (&pi_state->pi_mutex, - this->rt_waiter, this->task); + this->rt_waiter, + this->task); +if (ret ==1 ) {/* * We got the lock. We do neither drop the refcount * on pi_state nor clear this->pi_state because the * waiter needs the pi_state for cleaning up the * user space value. It will drop the refcount -* after doing so. +* after doing so. this::requeue_state is updated +* in the wakeup as well. */ requeue_pi_wake_futex (this, &key2, hb2); task_count++; -continue ; - }else if (ret) { + }else if (!ret) { +/* Waiter is queued, move it to hb2 */ +requeue_futex (this, hb1, hb2, &key2); +futex_requeue_pi_complete (this,0 ); + task_count++; + }else {/* * rt_mutex_start_proxy_lock() detected a potential * deadlock when we tried to queue that waiter.
@@ -2176,15 +2398,13 @@ static int futex_requeue(u32 __user *uad*/ this->pi_state =NULL ;put_pi_state (pi_state); +futex_requeue_pi_complete (this, ret);/* * We stop queueing more waiters and let user space * deal with the mess. */ break ; } -/* Waiter is queued, move it to hb2 */ -requeue_futex (this, hb1, hb2, &key2); - task_count++; }/*
@@ -3082,27 +3302,22 @@ static int futex_unlock_pi(u32 __user *u }/** -* handle_early_requeue_pi_wakeup() - Detect early wakeup on the initial futex +* handle_early_requeue_pi_wakeup() - Handle early wakeup on the initial futex * @hb: the hash_bucket futex_q was original enqueued on * @q: the futex_q woken while waiting to be requeued -* @key2: the futex_key of the requeue target futex * @timeout: the timeout associated with the wait (NULL if none) * -* Detect if the task was woken on the initial futex as opposed to the requeue -* target futex. If so, determine if it was a timeout or a signal that caused -* the wakeup and return the appropriate error code to the caller. Must be -* called with the hb lock held. +* Determine the cause for the early wakeup. * * Return: -* - 0 = no early wakeup detected; -* - <0 = -ETIMEDOUT or -ERESTARTNOINTR +* -EWOULDBLOCK or -ETIMEDOUT or -ERESTARTNOINTR */ static inline int handle_early_requeue_pi_wakeup (struct futex_hash_bucket *hb, -struct futex_q *q,union futex_key *key2, +struct futex_q *q,struct hrtimer_sleeper *timeout) { -int ret =0 ; +int ret;/* * With the hb lock held, we avoid races while we process the wakeup.
@@ -3111,22 +3326,21 @@ int handle_early_requeue_pi_wakeup(struc* It can't be requeued from uaddr2 to something else since we don't * support a PI aware source futex for requeue. */ -if (!match_futex (&q->key, key2)) { -WARN_ON (q->lock_ptr && (&hb->lock != q->lock_ptr)); -/* -* We were woken prior to requeue by a timeout or a signal. -* Unqueue the futex_q and determine which it was. -*/ -plist_del (&q->list, &hb->chain); -hb_waiters_dec (hb); +WARN_ON_ONCE (&hb->lock != q->lock_ptr); -/* Handle spurious wakeups gracefully */ - ret = -EWOULDBLOCK ; -if (timeout && !timeout->task) - ret = -ETIMEDOUT ; -else if (signal_pending (current)) - ret = -ERESTARTNOINTR ; - } +/* +* We were woken prior to requeue by a timeout or a signal. +* Unqueue the futex_q and determine which it was. +*/ +plist_del (&q->list, &hb->chain); +hb_waiters_dec (hb); + +/* Handle spurious wakeups gracefully */ + ret = -EWOULDBLOCK ; +if (timeout && !timeout->task) + ret = -ETIMEDOUT ; +else if (signal_pending (current)) + ret = -ERESTARTNOINTR ;return ret; }
@@ -3179,6 +3393,7 @@ static int futex_wait_requeue_pi(u32 __ustruct futex_hash_bucket *hb;union futex_key key2 =FUTEX_KEY_INIT ;struct futex_q q = futex_q_init; +struct rt_mutex_base *pi_mutex;int res, ret;if (!IS_ENABLED (CONFIG_FUTEX_PI ))
@@ -3228,30 +3443,22 @@ static int futex_wait_requeue_pi(u32 __u/* Queue the futex_q, drop the hb lock, wait for wakeup. */ futex_wait_queue_me (hb, &q, to); -spin_lock (&hb->lock); - ret =handle_early_requeue_pi_wakeup (hb, &q, &key2, to); -spin_unlock (&hb->lock); -if (ret) - goto out; - -/* -* In order for us to be here, we know our q.key == key2, and since -* we took the hb->lock above, we also know that futex_requeue() has -* completed and we no longer have to concern ourselves with a wakeup -* race with the atomic proxy lock acquisition by the requeue code. -*/ +switch (futex_requeue_pi_wakeup_sync (&q)) { +case Q_REQUEUE_PI_IGNORE : +/* The waiter is still on uaddr1 */ +spin_lock (&hb->lock); + ret =handle_early_requeue_pi_wakeup (hb, &q, to); +spin_unlock (&hb->lock); +break ; -/* -* Check if the requeue code acquired the second futex for us and do -* any pertinent fixup. -*/ -if (!q.rt_waiter) { +case Q_REQUEUE_PI_LOCKED : +/* The requeue acquired the lock */ if (q.pi_state && (q.pi_state->owner != current)) {spin_lock (q.lock_ptr); ret =fixup_owner (uaddr2, &q, true);/* -* Drop the reference to the pi state which -* the requeue_pi() code acquired for us. +* Drop the reference to the pi state which the +* requeue_pi() code acquired for us. */ put_pi_state (q.pi_state);spin_unlock (q.lock_ptr);
@@ -3261,18 +3468,14 @@ static int futex_wait_requeue_pi(u32 __u*/ ret = ret <0 ? ret :0 ; } - }else { -struct rt_mutex_base *pi_mutex; +break ; -/* -* We have been woken up by futex_unlock_pi(), a timeout, or a -* signal. futex_unlock_pi() will not destroy the lock_ptr nor -* the pi_state. -*/ -WARN_ON (!q.pi_state); +case Q_REQUEUE_PI_DONE : +/* Requeue completed. Current is 'pi_blocked_on' the rtmutex */ pi_mutex = &q.pi_state->pi_mutex; ret =rt_mutex_wait_proxy_lock (pi_mutex, to, &rt_waiter); +/* Current is not longer pi_blocked_on */ spin_lock (q.lock_ptr);if (ret && !rt_mutex_cleanup_proxy_lock (pi_mutex, &rt_waiter)) ret =0 ;
@@ -3292,17 +3495,21 @@ static int futex_wait_requeue_pi(u32 __uunqueue_me_pi (&q);spin_unlock (q.lock_ptr); - } -if (ret == -EINTR ) { -/* -* We've already been requeued, but cannot restart by calling -* futex_lock_pi() directly. We could restart this syscall, but -* it would detect that the user space "val" changed and return -* -EWOULDBLOCK. Save the overhead of the restart and return -* -EWOULDBLOCK directly. -*/ - ret = -EWOULDBLOCK ; +if (ret == -EINTR ) { +/* +* We've already been requeued, but cannot restart +* by calling futex_lock_pi() directly. We could +* restart this syscall, but it would detect that +* the user space "val" changed and return +* -EWOULDBLOCK. Save the overhead of the restart +* and return -EWOULDBLOCK directly. +*/ + ret = -EWOULDBLOCK ; + } +break ; +default : +BUG (); } out:
Peter Zijlstra
maintainerRe: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
Quoted 90 lines · Thomas Gleixner
(quoted diff) +90
-0+static inline bool futex_requeue_pi_prepare(struct futex_q *q, + struct futex_pi_state *pi_state) +{ + int cur, res, new; + + /* + * Set state to Q_REQUEUE_PI_IN_PROGRESS unless an early wakeup has + * already set Q_REQUEUE_PI_IGNORE to signal that requeue should + * ignore the waiter. + */ + for (cur = atomic_read(&q->requeue_state);; cur = res) { + if (cur == Q_REQUEUE_PI_IGNORE) + return false; + + /* + * futex_proxy_trylock_atomic() might have set it to + * IN_PROGRESS and a interleaved early wake to WAIT. + * + * It was considered to have an extra state for that + * trylock, but that would just add more conditionals + * all over the place for a dubious value. + */ + if (cur != Q_REQUEUE_PI_NONE) + break; + + new = Q_REQUEUE_PI_IN_PROGRESS; + res = atomic_cmpxchg(&q->requeue_state, cur, new); + if (likely(cur == res)) + break; + } + q->pi_state = pi_state; + return true; +} + +static inline void futex_requeue_pi_complete(struct futex_q *q, int locked) +{ + int cur, res, new; + + for (cur = atomic_read(&q->requeue_state);; cur = res) { + if (locked >= 0) { + /* Requeue succeeded. Set DONE or LOCKED */ + new = Q_REQUEUE_PI_DONE + locked; + } else if (cur == Q_REQUEUE_PI_IN_PROGRESS) { + /* Deadlock, no early wakeup interleave */ + new = Q_REQUEUE_PI_NONE; + } else { + /* Deadlock, early wakeup interleave. */ + new = Q_REQUEUE_PI_IGNORE; + } + + res = atomic_cmpxchg(&q->requeue_state, cur, new); + if (likely(cur == res)) + break; + } + +#ifdef CONFIG_PREEMPT_RT + /* If the waiter interleaved with the requeue let it know */ + if (unlikely(cur == Q_REQUEUE_PI_WAIT)) + rcuwait_wake_up(&q->requeue_wait); +#endif +} + +static inline int futex_requeue_pi_wakeup_sync(struct futex_q *q) +{ + int cur, new, res; + + for (cur = atomic_read(&q->requeue_state);; cur = res) { + /* Is requeue done already? */ + if (cur >= Q_REQUEUE_PI_DONE) + break; + + /* + * If not done, then tell the requeue code to either ignore + * the waiter or to wake it up once the requeue is done. + */ + new = !cur ? Q_REQUEUE_PI_IGNORE : Q_REQUEUE_PI_WAIT; + res = atomic_cmpxchg(&q->requeue_state, cur, new); + if (likely(cur == res)) + break; + } + + /* If the requeue was in progress, wait for it to complete */ + if (cur == Q_REQUEUE_PI_IN_PROGRESS) { +#ifdef CONFIG_PREEMPT_RT + rcuwait_wait_event(&q->requeue_wait, + atomic_read(&q->requeue_state) != Q_REQUEUE_PI_WAIT, + TASK_UNINTERRUPTIBLE); +#else + while (atomic_read(&q->requeue_state) == Q_REQUEUE_PI_WAIT) + cpu_relax();
Review
Quoted 10 lines
(quoted diff) +10
-0+#endif + } + + /* + * Requeue is now either prohibited or complete. Reread state + * because during the wait above it might have changed. Nothing + * will modify q->requeue_state after this point. + */ + return atomic_read(&q->requeue_state); +}
I did:
- atomic_cmpxchg() -> atomic_try_cmpxchg()
- atomic_read() -> atomic_read_acquire(); which I think is required for at least the futex_requeue_pi_wakeup_sync() >= Q_REQUEUE_PI_DONE case to ensure we observe the state as per whoever set DONE/LOCKED.
- use atomic_cond_read_relaxed()
- removed one ternary operator for (IMO) clearer code.
kernel/futex.c +23 -28
@@ -1853,15 +1853,16 @@ enum {static inline bool futex_requeue_pi_prepare (struct futex_q *q,struct futex_pi_state *pi_state) { -int cur, res, new; +int old, new;/* * Set state to Q_REQUEUE_PI_IN_PROGRESS unless an early wakeup has * already set Q_REQUEUE_PI_IGNORE to signal that requeue should * ignore the waiter. */ -for (cur =atomic_read (&q->requeue_state);; cur = res) { -if (cur ==Q_REQUEUE_PI_IGNORE ) + old =atomic_read_acquire (&q->requeue_state); + do { + if (old == Q_REQUEUE_PI_IGNORE) return false; /*
@@ -1872,74 +1873,68 @@ static inline bool futex_requeue_pi_prep* trylock, but that would just add more conditionals * all over the place for a dubious value. */ -if (cur !=Q_REQUEUE_PI_NONE ) +if (old !=Q_REQUEUE_PI_NONE )break ; new =Q_REQUEUE_PI_IN_PROGRESS ; - res =atomic_cmpxchg (&q->requeue_state, cur, new); -if (likely (cur == res)) -break ; - } + }while (!atomic_try_cmpxchg (&q->requeue_state, &old, new)); + q->pi_state = pi_state;return true; }static inline void futex_requeue_pi_complete (struct futex_q *q,int locked) { -int cur, res, new; +int old, new; -for (cur =atomic_read (&q->requeue_state);; cur = res) { + old =atomic_read_acquire (&q->requeue_state); +do {if (locked >=0 ) {/* Requeue succeeded. Set DONE or LOCKED */ new =Q_REQUEUE_PI_DONE + locked; - }else if (cur ==Q_REQUEUE_PI_IN_PROGRESS ) { + }else if (old ==Q_REQUEUE_PI_IN_PROGRESS ) {/* Deadlock, no early wakeup interleave */ new =Q_REQUEUE_PI_NONE ; }else {/* Deadlock, early wakeup interleave. */ new =Q_REQUEUE_PI_IGNORE ; } - - res =atomic_cmpxchg (&q->requeue_state, cur, new); -if (likely (cur == res)) -break ; - } + }while (!atomic_try_cmpxchg (&q->requeue_state, &old, new));#ifdef CONFIG_PREEMPT_RT /* If the waiter interleaved with the requeue let it know */ -if (unlikely (cur ==Q_REQUEUE_PI_WAIT )) +if (unlikely (old ==Q_REQUEUE_PI_WAIT ))rcuwait_wake_up (&q->requeue_wait);#endif }static inline int futex_requeue_pi_wakeup_sync (struct futex_q *q) { -int cur, new, res; +int old, new; -for (cur =atomic_read (&q->requeue_state);; cur = res) { + old =atomic_read_acquire (&q->requeue_state); +do {/* Is requeue done already? */ -if (cur >=Q_REQUEUE_PI_DONE ) +if (old >=Q_REQUEUE_PI_DONE )break ;/* * If not done, then tell the requeue code to either ignore * the waiter or to wake it up once the requeue is done. */ - new = !cur ?Q_REQUEUE_PI_IGNORE :Q_REQUEUE_PI_WAIT ; - res =atomic_cmpxchg (&q->requeue_state, cur, new); -if (likely (cur == res)) -break ; - } + new =Q_REQUEUE_PI_WAIT ; +if (old ==Q_REQUEUE_PI_NONE ) + new =Q_REQUEUE_PI_IGNORE ; + }while (!atomic_try_cmpxchg (&q->requeue_state, &old, new));/* If the requeue was in progress, wait for it to complete */ -if (cur ==Q_REQUEUE_PI_IN_PROGRESS ) { +if (old ==Q_REQUEUE_PI_IN_PROGRESS ) {#ifdef CONFIG_PREEMPT_RT rcuwait_wait_event (&q->requeue_wait,atomic_read (&q->requeue_state) !=Q_REQUEUE_PI_WAIT ,TASK_UNINTERRUPTIBLE );#else -while (atomic_read (&q->requeue_state) ==Q_REQUEUE_PI_WAIT ) -cpu_relax (); + (void )atomic_cond_read_relaxed (&q->requeue_state,VAL !=Q_REQUEUE_PI_WAIT );#endif }
Peter Zijlstra
maintainerRe: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
kernel/futex.c static inline bool futex_requeue_pi_prep · line 1919
Quoted 9 lines · Peter Zijlstra
static inline int
futex_requeue_pi_wakeup_sync(struct futex_q *q){kernel/futex.c +4
-0+int old, new; + old =atomic_read_acquire (&q->requeue_state); + do { /* Is requeue done already? */ + if (old >= Q_REQUEUE_PI_DONE) break;
I think that can be: return old; for slightly simpler code.
Quoted 21 lines
kernel/futex.c +6
-0/* * If not done, then tell the requeue code to either ignore * the waiter or to wake it up once the requeue is done. */ + new =Q_REQUEUE_PI_WAIT ; +if (old ==Q_REQUEUE_PI_NONE ) + new =Q_REQUEUE_PI_IGNORE ; + }while (!atomic_try_cmpxchg (&q->requeue_state, &old, new));/* If the requeue was in progress, wait for it to complete */ +if (old ==Q_REQUEUE_PI_IN_PROGRESS ) {#ifdef CONFIG_PREEMPT_RT rcuwait_wait_event (&q->requeue_wait,atomic_read (&q->requeue_state) !=Q_REQUEUE_PI_WAIT ,TASK_UNINTERRUPTIBLE );#else + (void )atomic_cond_read_relaxed (&q->requeue_state,VAL !=Q_REQUEUE_PI_WAIT );#endif }
Re: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
Quoted 5 lines · Peter Zijlstra
Quoted 4 lines · Peter Zijlstra
static inline int
futex_requeue_pi_wakeup_sync(struct futex_q *q){(quoted diff) +1
-0+ int old, new;
Quoted 7 lines
kernel/futex.c static inline bool futex_requeue_pi_prep · line 1919
Quoted 5 lines
kernel/futex.c +3
-0+ old =atomic_read_acquire (&q->requeue_state); + do { /* Is requeue done already? */ + if (old >= Q_REQUEUE_PI_DONE) break;I think that can be: return old; for slightly simpler code.
Yes.
Re: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
In reply to Peter ZijlstraQuoted 4 lines · Peter Zijlstra
On Fri, Jul 30, 2021 at 03:51:05PM +0200, Thomas Gleixner wrote:
I did:-
atomic_cmpxchg()->atomic_try_cmpxchg()
Ack.
Quoted 3 lines
-
atomic_read()->atomic_read_acquire(); which I think is required for at least thefutex_requeue_pi_wakeup_sync()>=Q_REQUEUE_PI_DONEcase to ensure we observe the state as per whoever set DONE/LOCKED.
Indeed
Quoted 1 line
- use
atomic_cond_read_relaxed()
Cute
Quoted 1 line
- removed one ternary operator for (IMO) clearer code.
It's definitely more readable this way.
Thanks,
tglx
Peter Zijlstra
maintainerRe: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
In reply to Peter Zijlstrakernel/futex.c enum { · line 1864
Quoted 6 lines · Peter Zijlstra
static inline void
futex_requeue_pi_complete(struct futex_q *q, int locked){kernel/futex.c +3
-0+int old, new; + old =atomic_read_acquire (&q->requeue_state); + do {
if (old == Q_REQUEUE_PI_IGNORE) return;
Quoted 1 line
if (locked >= 0) {
WARN_ON_ONCE(old != Q_REQUEUE_PI_IN_PROGRESS && old != Q_REQUEUE_PI_WAIT)
kernel/futex.c static inline bool futex_requeue_pi_prep · line 1898
Quoted 6 lines
kernel/futex.c +1
-0/* Requeue succeeded. Set DONE or LOCKED */ new =Q_REQUEUE_PI_DONE + locked; + }else if (old ==Q_REQUEUE_PI_IN_PROGRESS ) {/* Deadlock, no early wakeup interleave */ new =Q_REQUEUE_PI_NONE ; }else {
WARN_ON_ONCE(old != Q_REQUEUE_PI_WAIT);
kernel/futex.c static inline bool futex_requeue_pi_prep · line 1909
Quoted 11 lines
kernel/futex.c +2
-0/* Deadlock, early wakeup interleave. */ new =Q_REQUEUE_PI_IGNORE ; } + }while (!atomic_try_cmpxchg (&q->requeue_state, &old, new));#ifdef CONFIG_PREEMPT_RT /* If the waiter interleaved with the requeue let it know */ +if (unlikely (old ==Q_REQUEUE_PI_WAIT ))rcuwait_wake_up (&q->requeue_wait);#endif }
I think...
Re: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
Quoted 26 lines · Peter Zijlstra
kernel/futex.c enum { · line 1864
Quoted 6 lines · Peter Zijlstra
static inline void
futex_requeue_pi_complete(struct futex_q *q, int locked){kernel/futex.c +3
-0+int old, new; + old =atomic_read_acquire (&q->requeue_state); + do {if (old == Q_REQUEUE_PI_IGNORE) return;Quoted 1 line
if (locked >= 0) {
WARN_ON_ONCE(old != Q_REQUEUE_PI_IN_PROGRESS && old != Q_REQUEUE_PI_WAIT)kernel/futex.c static inline bool futex_requeue_pi_prep · line 1898
Quoted 6 lines
kernel/futex.c +1
-0/* Requeue succeeded. Set DONE or LOCKED */ new =Q_REQUEUE_PI_DONE + locked; + }else if (old ==Q_REQUEUE_PI_IN_PROGRESS ) {/* Deadlock, no early wakeup interleave */ new =Q_REQUEUE_PI_NONE ; }else {WARN_ON_ONCE(old != Q_REQUEUE_PI_WAIT);
Quoted 1 line
I think...
Yes, it's matching the valid transition table.
Thanks,
tglx
Peter Zijlstra
maintainerRe: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
In reply to Thomas Gleixnerkernel/futex.c struct futex_q { · line 230
Quoted 11 lines · Thomas Gleixner
kernel/futex.c +4
-0@@ -219,6 +221,10 @@ struct futex_q {struct rt_mutex_waiter *rt_waiter;union futex_key *requeue_pi_key;u32 bitset; +atomic_t requeue_state; +#ifdef CONFIG_PREEMPT_RT +struct rcuwait requeue_wait; +#endif } __randomize_layout;static const struct futex_q futex_q_init = {
Do we want to explicitly initialize requeue_state in futex_q_init? I was looking where we reset the state machine and eventually figured it out, but I'm thinking something more explicit might help avoid this for the next time.
Re: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
Quoted 11 lines · Peter Zijlstra
Quoted 10 lines · Thomas Gleixner
kernel/futex.c +4
-0@@ -219,6 +221,10 @@ struct futex_q {struct rt_mutex_waiter *rt_waiter;union futex_key *requeue_pi_key;u32 bitset; +atomic_t requeue_state; +#ifdef CONFIG_PREEMPT_RT +struct rcuwait requeue_wait; +#endif } __randomize_layout;
Quoted 6 lines
kernel/futex.c struct futex_q { · line 230
Quoted 1 line
kernel/futex.c +0
-0static const struct futex_q futex_q_init = {Do we want to explicitly initialize
requeue_stateinfutex_q_init? I was looking where we reset the state machine and eventually figured it out, but I'm thinking something more explicit might help avoid this for the next time.
Sure.
Peter Zijlstra
maintainerRe: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
In reply to Thomas GleixnerQuoted 59 lines · Thomas Gleixner
kernel/futex.c +11
-25@@ -3082,27 +3302,22 @@ static int futex_unlock_pi(u32 __user *u }/** -* handle_early_requeue_pi_wakeup() - Detect early wakeup on the initial futex +* handle_early_requeue_pi_wakeup() - Handle early wakeup on the initial futex * @hb: the hash_bucket futex_q was original enqueued on * @q: the futex_q woken while waiting to be requeued -* @key2: the futex_key of the requeue target futex * @timeout: the timeout associated with the wait (NULL if none) * -* Detect if the task was woken on the initial futex as opposed to the requeue -* target futex. If so, determine if it was a timeout or a signal that caused -* the wakeup and return the appropriate error code to the caller. Must be -* called with the hb lock held. +* Determine the cause for the early wakeup. * * Return: -* - 0 = no early wakeup detected; -* - <0 = -ETIMEDOUT or -ERESTARTNOINTR +* -EWOULDBLOCK or -ETIMEDOUT or -ERESTARTNOINTR */ static inline int handle_early_requeue_pi_wakeup (struct futex_hash_bucket *hb, -struct futex_q *q,union futex_key *key2, +struct futex_q *q,struct hrtimer_sleeper *timeout) { -int ret =0 ; +int ret;/* * With the hb lock held, we avoid races while we process the wakeup. @@ -3111,22 +3326,21 @@ int handle_early_requeue_pi_wakeup(struc* It can't be requeued from uaddr2 to something else since we don't * support a PI aware source futex for requeue. */ -if (!match_futex (&q->key, key2)) { -WARN_ON (q->lock_ptr && (&hb->lock != q->lock_ptr)); -/* -* We were woken prior to requeue by a timeout or a signal. -* Unqueue the futex_q and determine which it was. -*/ -plist_del (&q->list, &hb->chain); -hb_waiters_dec (hb); +WARN_ON_ONCE (&hb->lock != q->lock_ptr); -/* Handle spurious wakeups gracefully */ - ret = -EWOULDBLOCK ; -if (timeout && !timeout->task) - ret = -ETIMEDOUT ; -else if (signal_pending (current)) - ret = -ERESTARTNOINTR ; - } +/* +* We were woken prior to requeue by a timeout or a signal. +* Unqueue the futex_q and determine which it was. +*/ +plist_del (&q->list, &hb->chain);
kernel/futex.c int handle_early_requeue_pi_wakeup(struc · line 3345
Quoted 10 lines
kernel/futex.c +8
-0+hb_waiters_dec (hb); + +/* Handle spurious wakeups gracefully */ + ret = -EWOULDBLOCK ; +if (timeout && !timeout->task) + ret = -ETIMEDOUT ; +else if (signal_pending (current)) + ret = -ERESTARTNOINTR ;return ret; }
AFAICT this change is a separate cleanup, possible because the only callsite already does that match_futex() test before calling this.
I think it might be worth splitting out, just to reduce the complexity of this patch.
Re: [patch 58/63] futex: Prevent requeue_pi() lock nesting issue on RT
Quoted 11 lines · Peter Zijlstra
Quoted 10 lines · Thomas Gleixner
kernel/futex.c +10
-0+/* +* We were woken prior to requeue by a timeout or a signal. +* Unqueue the futex_q and determine which it was. +*/ +plist_del (&q->list, &hb->chain); +hb_waiters_dec (hb); + +/* Handle spurious wakeups gracefully */ + ret = -EWOULDBLOCK ; +if (timeout && !timeout->task)
Quoted 8 lines
kernel/futex.c int handle_early_requeue_pi_wakeup(struc · line 3345
Quoted 5 lines
kernel/futex.c +3
-0+ ret = -ETIMEDOUT ; + else if (signal_pending(current)) + ret = -ERESTARTNOINTR ;return ret; }AFAICT this change is a separate cleanup, possible because the only callsite already does that
match_futex()test before calling this.
No. It's possible because the state machine guarantees that. The match_futex() in the calling function is unrelated.
Quoted 2 lines
I think it might be worth splitting out, just to reduce the complexity of this patch.
Yes, I did by moving the match_futex() test to the call site as a preparatory change for this one.
Thanks,
tglx
PATCH 59/63 rtmutex: Prevent lockdep false positive with PI futexes
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
On PREEMPT_RT the futex hashbucket spinlock becomes 'sleeping' and rtmutex based. That causes a lockdep false positive because some of the futex functions invoke spin_unlock(&hb->lock) with the wait_lock of the rtmutex associated to the pi_futex held. spin_unlock() in turn takes wait_lock of the rtmutex on which the spinlock is based which makes lockdep notice a lock recursion.
Give the futex/rtmutex wait_lock a seperate key.
--- kernel/locking/rtmutex_api.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
---
kernel/locking/rtmutex_api.c +12 -0
@@ -214,7 +214,19 @@ EXPORT_SYMBOL_GPL(__rt_mutex_init); void __sched rt_mutex_init_proxy_locked(struct rt_mutex_base *lock,struct task_struct *proxy_owner) { +static struct lock_class_key pi_futex_key; +__rt_mutex_base_init (lock); +/* +* On PREEMPT_RT the futex hashbucket spinlock becomes 'sleeping' +* and rtmutex based. That causes a lockdep false positive because +* some of the futex functions invoke spin_unlock(&hb->lock) with +* the wait_lock of the rtmutex associated to the pi_futex held. +* spin_unlock() in turn takes wait_lock of the rtmutex on which +* the spinlock is based which makes lockdep notice a lock +* recursion. Give the futex/rtmutex wait_lock a seperate key. +*/ +lockdep_set_class (&lock->wait_lock, &pi_futex_key);rt_mutex_set_owner (lock, proxy_owner); }
PATCH 60/63 preempt: Adjust PREEMPT_LOCK_OFFSET for RT
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
On PREEMPT_RT regular spinlocks and rwlocks are substituted with rtmutex based constructs. spin/rwlock held regions are preemptible on PREEMPT_RT, so PREEMPT_LOCK_OFFSET has to be 0 to make the various cond_resched_*lock() functions work correctly.
--- include/linux/preempt.h | 4 ++++ 1 file changed, 4 insertions(+)
---
include/linux/preempt.h +4 -0
@@ -121,7 +121,11 @@/* * The preempt_count offset after spin_lock() */ +#if !defined(CONFIG_PREEMPT_RT )#define PREEMPT_LOCK_OFFSET PREEMPT_DISABLE_OFFSET +#else +#define PREEMPT_LOCK_OFFSET 0 +#endif /* * The preempt_count offset needed for things like:
PATCH 61/63 locking/rtmutex: Implement equal priority lock stealing
In reply to Thomas GleixnerFrom: Gregory Haskins <[email protected]>
The current logic only allows lock stealing to occur if the current task is of higher priority than the pending owner.
Signficant throughput improvements can be gained by allowing the lock stealing to include tasks of equal priority when the contended lock is a spin_lock or a rw_lock and the tasks are not in a RT scheduling task.
The assumption was that the system will make faster progress by allowing the task already on the CPU to take the lock rather than waiting for the system to wake up a different task.
This does add a degree of unfairness, but in reality no negative side effects have been observed in the many years that this has been used in the RT kernel.
[ tglx: Refactored and rewritten several times by Steve Rostedt, Sebastian
Siewior and myself ]
--- kernel/locking/rtmutex.c | 52 +++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 17 deletions(-)
---
kernel/locking/rtmutex.c +35 -17
@@ -321,6 +321,26 @@ static __always_inline int rt_mutex_waitreturn 1 ; } +static inline bool rt_mutex_steal (struct rt_mutex_waiter *waiter, +struct rt_mutex_waiter *top_waiter) +{ +if (rt_mutex_waiter_less (waiter, top_waiter)) +return true; + +#ifdef RT_MUTEX_BUILD_SPINLOCKS +/* +* Note that RT tasks are excluded from same priority (lateral) +* steals to prevent the introduction of an unbounded latency. +*/ +if (rt_prio (waiter->prio) ||dl_prio (waiter->prio)) +return false; + +return rt_mutex_waiter_equal (waiter, top_waiter); +#else +return false; +#endif +} +#define __node_2_waiter (node) \ rb_entry((node), struct rt_mutex_waiter, tree_entry)
@@ -906,19 +926,21 @@ try_to_take_rt_mutex(struct rt_mutex_bas* trylock attempt. */ if (waiter) { -/* -* If waiter is not the highest priority waiter of -* @lock, give up. -*/ -if (waiter !=rt_mutex_top_waiter (lock)) -return 0 ; +struct rt_mutex_waiter *top_waiter =rt_mutex_top_waiter (lock);/* -* We can acquire the lock. Remove the waiter from the -* lock waiters tree. +* If waiter is the highest priority waiter of @lock, +* or allowed to steal it, take it over. */ -rt_mutex_dequeue (lock, waiter); - +if (waiter == top_waiter ||rt_mutex_steal (waiter, top_waiter)) { +/* +* We can acquire the lock. Remove the waiter from the +* lock waiters tree. +*/ +rt_mutex_dequeue (lock, waiter); + }else { +return 0 ; + } }else {/* * If the lock has waiters already we check whether @task is
@@ -929,13 +951,9 @@ try_to_take_rt_mutex(struct rt_mutex_bas* not need to be dequeued. */ if (rt_mutex_has_waiters (lock)) { -/* -* If @task->prio is greater than or equal to -* the top waiter priority (kernel view), -* @task lost. -*/ -if (!rt_mutex_waiter_less (task_to_waiter (task), -rt_mutex_top_waiter (lock))) +/* Check whether the trylock can steal it. */ +if (!rt_mutex_steal (task_to_waiter (task), +rt_mutex_top_waiter (lock)))return 0 ;/*
PATCH 62/63 locking/rtmutex: Add adaptive spinwait mechanism
In reply to Thomas GleixnerFrom: Steven Rostedt <[email protected]>
Going to sleep when a spinlock or rwlock is contended can be quite inefficient when the contention time is short and the lock owner is running on a different CPU. The MCS mechanism is not applicable to rtmutex based locks, so provide a simple adaptive spinwait mechanism for the RT specific spin/rwlock implementations.
[ tglx: Provide a contemporary changelog ]
--- kernel/locking/rtmutex.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
---
kernel/locking/rtmutex.c +49 -1
@@ -8,6 +8,11 @@* Copyright (C) 2005-2006 Timesys Corp., Thomas Gleixner <[email protected]> * Copyright (C) 2005 Kihon Technologies Inc., Steven Rostedt * Copyright (C) 2006 Esben Nielsen +* Adaptive Spinlocks: +* Copyright (C) 2008 Novell, Inc., Gregory Haskins, Sven Dietrich, +* and Peter Morreale, +* Adaptive Spinlocks simplification: +* Copyright (C) 2008 Red Hat, Inc., Steven Rostedt <[email protected]> * * See Documentation/locking/rt-mutex-design.rst for details. */
@@ -1529,6 +1534,43 @@ static __always_inline int __rt_mutex_lo* Functions required for spin/rw_lock substitution on RT kernels */ +#ifdef CONFIG_SMP +/* +* Note that owner is a speculative pointer and dereferencing relies +* on rcu_read_lock() and the check against the lock owner. +*/ +static bool rtlock_adaptive_spinwait (struct rt_mutex_base *lock, +struct task_struct *owner) +{ +bool res = true; + +rcu_read_lock (); +for (;;) { +/* Owner changed. Trylock again */ +if (owner !=rt_mutex_owner (lock)) +break ; +/* +* Ensure that owner->on_cpu is dereferenced _after_ +* checking the above to be valid. +*/ +barrier (); +if (!owner->on_cpu) { + res = false; +break ; + } +cpu_relax (); + } +rcu_read_unlock (); +return res; +} +#else +static bool rtlock_adaptive_spinwait (struct rt_mutex_base *lock, +struct task_struct *owner) +{ +return false; +} +#endif +/** * rtlock_slowlock_locked - Slow path lock acquisition for RT locks * @lock: The underlying rt mutex
@@ -1536,6 +1578,7 @@ static __always_inline int __rt_mutex_lo static void __sched rtlock_slowlock_locked(struct rt_mutex_base *lock) {struct rt_mutex_waiter waiter; +struct task_struct *owner;lockdep_assert_held (&lock->wait_lock);
@@ -1554,9 +1597,14 @@ static void __sched rtlock_slowlock_lockif (try_to_take_rt_mutex (lock, current, &waiter))break ; +if (&waiter ==rt_mutex_top_waiter (lock)) + owner =rt_mutex_owner (lock); +else + owner =NULL ;raw_spin_unlock_irq (&lock->wait_lock); -schedule_rtlock (); +if (!owner || !rtlock_adaptive_spinwait (lock, owner)) +schedule_rtlock ();raw_spin_lock_irq (&lock->wait_lock);set_current_state (TASK_RTLOCK_WAIT );
Peter Zijlstra
maintainerRe: [patch 62/63] locking/rtmutex: Add adaptive spinwait mechanism
Quoted 7 lines · Thomas Gleixner
From: Steven Rostedt <[email protected]>
Going to sleep when a spinlock or rwlock is contended can be quite inefficient when the contention time is short and the lock owner is running on a different CPU. The MCS mechanism is not applicable to rtmutex based locks, so provide a simple adaptive spinwait mechanism for the RT specific spin/rwlock implementations.
A better Changelog would explain *why* OSQ does not apply. I'm thinking this ie because the (spin) waiters can be of different priorities and we need to ensure the highest prio waiter gets to win?
AFAICT that isn't true even without OSQ, you just get a thundering herd and the higher prio waiter has a better chance of winning the race but all bets are off either way around.
Quoted 1 line
[ tglx: Provide a contemporary changelog ]
It might be best to squash this and the next patch, this back and forth doesn't make much sense at this point.
Quoted 1 line
+#ifdef
CONFIG_SMP
Existing convention would make that:
#ifdef CONFIG_RTMUTEX_SPIN_ON_OWNER
But I suppose that's indeed not required if we don't use OSQ.
kernel/locking/rtmutex.c static __always_inline int __rt_mutex_lo · line 1543
Quoted 6 lines
kernel/locking/rtmutex.c +6
-0+/* +* Note that owner is a speculative pointer and dereferencing relies +* on rcu_read_lock() and the check against the lock owner. +*/ +static bool rtlock_adaptive_spinwait (struct rt_mutex_base *lock, +struct task_struct *owner)
similarly, this would be:
rt_mutex_spin_on_owner()
Quoted 4 lines
kernel/locking/rtmutex.c +4
-0+{ +bool res = true; + +rcu_read_lock ();
kernel/locking/rtmutex.c static __always_inline int __rt_mutex_lo · line 1557
Quoted 10 lines
kernel/locking/rtmutex.c +10
-0+for (;;) { +/* Owner changed. Trylock again */ +if (owner !=rt_mutex_owner (lock)) +break ; +/* +* Ensure that owner->on_cpu is dereferenced _after_ +* checking the above to be valid. +*/ +barrier (); +if (!owner->on_cpu) {
Esp. when this will be on rtmutex unconditionally, you want to mirror the full set of conditions we also have on mutex_spin_on_owner():
|| need_resched() || vcpu_is_preempted(task_cpu(owner))
kernel/locking/rtmutex.c static __always_inline int __rt_mutex_lo · line 1565
Quoted 8 lines
kernel/locking/rtmutex.c +8
-0+ res = false; +break ; + } +cpu_relax (); + } +rcu_read_unlock (); +return res; +}
Additionally, we could consider adding something that would compare the current prio to the top_waiter prio and terminate the loop if we're found to be of lower prio, but lifetime issues are probably going to make that 'interesting'.
Re: [patch 62/63] locking/rtmutex: Add adaptive spinwait mechanism
Quoted 9 lines · Peter Zijlstra
Quoted 7 lines · Thomas Gleixner
From: Steven Rostedt <[email protected]>
Going to sleep when a spinlock or rwlock is contended can be quite inefficient when the contention time is short and the lock owner is running on a different CPU. The MCS mechanism is not applicable to rtmutex based locks, so provide a simple adaptive spinwait mechanism for the RT specific spin/rwlock implementations.
Quoted 7 lines
A better Changelog would explain *why* OSQ does not apply. I'm thinking this ie because the (spin) waiters can be of different priorities and we need to ensure the highest prio waiter gets to win?
AFAICT that isn't true even without OSQ, you just get a thundering herd and the higher prio waiter has a better chance of winning the race but all bets are off either way around.
Will do.
Quoted 7 lines
Quoted 1 line
+#ifdef
CONFIG_SMPExisting convention would make that:
#ifdef
CONFIG_RTMUTEX_SPIN_ON_OWNERBut I suppose that's indeed not required if we don't use OSQ.
Right.
Quoted 10 lines
kernel/locking/rtmutex.c static __always_inline int __rt_mutex_lo · line 1543
Quoted 6 lines
kernel/locking/rtmutex.c +6
-0+/* +* Note that owner is a speculative pointer and dereferencing relies +* on rcu_read_lock() and the check against the lock owner. +*/ +static bool rtlock_adaptive_spinwait (struct rt_mutex_base *lock, +struct task_struct *owner)similarly, this would be:
rt_mutex_spin_on_owner()
Duh.
Quoted 5 lines
Esp. when this will be on rtmutex unconditionally, you want to mirror the full set of conditions we also have on
mutex_spin_on_owner():|| need_resched() || vcpu_is_preempted(task_cpu(owner))
Sure.
Quoted 5 lines
Quoted 5 lines
kernel/locking/rtmutex.c +5
-0+ res = false; +break ; + } +cpu_relax (); + }
Quoted 8 lines
kernel/locking/rtmutex.c static __always_inline int __rt_mutex_lo · line 1565
Quoted 3 lines
kernel/locking/rtmutex.c +3
-0+rcu_read_unlock (); +return res; +}Additionally, we could consider adding something that would compare the current prio to the
top_waiterprio and terminate the loop if we're found to be of lower prio, but lifetime issues are probably going to make that 'interesting'.
It's only the top priority waiter which can spin. If all of them start spinning then everything goes down the drain.
Thanks,
tglx
PATCH 63/63 locking/rtmutex: Use adaptive spinwait for all rtmutex based locks
In reply to Thomas GleixnerThere is no reason to restrict adaptive spinwait to the rt mutex based 'spinlocks'. Testing on RT shows a 4x impromevemt for hackbench.
---
--- kernel/locking/rtmutex.c | 84 +++++++++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 39 deletions(-)
kernel/locking/rtmutex.c +45 -39
@@ -1277,6 +1277,43 @@ static __always_inline void __rt_mutex_urt_mutex_slowunlock (lock); } +#ifdef CONFIG_SMP +/* +* Note that owner is a speculative pointer and dereferencing relies +* on rcu_read_lock() and the check against the lock owner. +*/ +static bool rtmutex_adaptive_spinwait (struct rt_mutex_base *lock, +struct task_struct *owner) +{ +bool res = true; + +rcu_read_lock (); +for (;;) { +/* Owner changed. Trylock again */ +if (owner !=rt_mutex_owner (lock)) +break ; +/* +* Ensure that owner->on_cpu is dereferenced _after_ +* checking the above to be valid. +*/ +barrier (); +if (!owner->on_cpu) { + res = false; +break ; + } +cpu_relax (); + } +rcu_read_unlock (); +return res; +} +#else +static bool rtmutex_adaptive_spinwait (struct rt_mutex_base *lock, +struct task_struct *owner) +{ +return false; +} +#endif +#ifdef RT_MUTEX_BUILD_MUTEX /* * Functions required for:
@@ -1361,6 +1398,7 @@ static int __sched rt_mutex_slowlock_blo struct rt_mutex_waiter *waiter) { struct rt_mutex *rtm = container_of(lock, struct rt_mutex, rtmutex); +struct task_struct *owner;int ret =0 ;for (;;) {
@@ -1383,9 +1421,14 @@ static int __sched rt_mutex_slowlock_blobreak ; } +if (waiter ==rt_mutex_top_waiter (lock)) + owner =rt_mutex_owner (lock); +else + owner =NULL ;raw_spin_unlock_irq (&lock->wait_lock); -schedule (); +if (!owner || !rtmutex_adaptive_spinwait (lock, owner)) +schedule ();raw_spin_lock_irq (&lock->wait_lock);set_current_state (state);
@@ -1534,43 +1577,6 @@ static __always_inline int __rt_mutex_lo* Functions required for spin/rw_lock substitution on RT kernels */ -#ifdef CONFIG_SMP -/* -* Note that owner is a speculative pointer and dereferencing relies -* on rcu_read_lock() and the check against the lock owner. -*/ -static bool rtlock_adaptive_spinwait (struct rt_mutex_base *lock, -struct task_struct *owner) -{ -bool res = true; - -rcu_read_lock (); -for (;;) { -/* Owner changed. Trylock again */ -if (owner !=rt_mutex_owner (lock)) -break ; -/* -* Ensure that owner->on_cpu is dereferenced _after_ -* checking the above to be valid. -*/ -barrier (); -if (!owner->on_cpu) { - res = false; -break ; - } -cpu_relax (); - } -rcu_read_unlock (); -return res; -} -#else -static bool rtlock_adaptive_spinwait (struct rt_mutex_base *lock, -struct task_struct *owner) -{ -return false; -} -#endif -/** * rtlock_slowlock_locked - Slow path lock acquisition for RT locks * @lock: The underlying rt mutex
@@ -1603,7 +1609,7 @@ static void __sched rtlock_slowlock_lock owner =NULL ;raw_spin_unlock_irq (&lock->wait_lock); -if (!owner || !rtlock_adaptive_spinwait (lock, owner)) +if (!owner || !rtmutex_adaptive_spinwait (lock, owner))schedule_rtlock ();raw_spin_lock_irq (&lock->wait_lock);
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [patch 30/63] locking/spinlock: Provide RT variant
Quoted 13 lines · Hillf Danton
On Fri, 30 Jul 2021 15:50:37 +0200
Quoted 3 lines
+ * The RT substitutions explicitly disable migration and take
+ *rcu_read_lock()across the lock held section.
+ */My stupid question is how it works in scenarios like spinlock chain
spin_lock(&foo->lock); spin_lock(&bar->lock); do something for foo and bar; spin_unlock(&bar->lock); spin_unlock(&foo->lock);
Quoted 3 lines
with rcu read lock held across the lock protected section because rcu lock has preempt disabled and that will disallow scheduling while acquiring the inner lock.
If CONFIG_PREEMPT_RCU=y rcu_read_lock() does not disable preemption. This is even true for !RT kernels with CONFIG_PREEMPT=y
Thanks,
tglx
Quoted 54 lines · Thomas Gleixner
Folks,
the following series is an update to V1 which can be found here:
https://lore.kernel.org/r/[email protected]
It contains the bulk of the PREEMPT-RT locking infrastructure. In PREEMPT-RT enabled kernels the following locking primitives are substituted by RT-Mutex based variants:
mutex,
rw_semaphore, spinlock, rwlocksemaphores are not substituted because they do not provide strict owner semantics.
ww_mutexesare also not substituted because the usage sites are not really RT relevant and it would require a full reimplementation to make them work correctly based on rtmutex. That might change in the future, but for now utilizing the existing variant is considered a safe and sane choice.Of course
raw_spinlocksare not touched either as they protect low level operations in the scheduler, timers and hardware access.The most interesting parts of the series which need a lot of eyeballs are:
- the scheduler bits which provide the infrastructure for spinlock and rwlock substitution to ensure that the task state is preserved when blocking on such a lock and a regular wakeup is handled correctly and not lost
- the rtmutex core implementation to handle lock contention on spinlocks and rwlocks correctly vs. the task state
- the rw_semaphore/rwlock substitutions which utilize the same implementation vs. the reader/writer handling
- the isolation of the
ww_mutexcode which allows to build it stand alone. The typedef based solution might look a bit odd on the first glance, but that turned out to be the least intrusive variant.- the PI futex related bits to handle the interaction between blocking on the underlying rtmutex and contention on the hash bucket lock which is converted to a 'sleeping spinlock'.
The rest surely needs a thorough review as well, but those parts are pretty straight forward. Quite some code restructuring and the actual wrapper functions to replace the existing !RT implementations.
The series survived quite some internal testing in RT kernels and is part of the recent 5.14-rc3-rt2 release.
For !RT kernels there is no functional change.
Quoted 7 lines
The series is also available from git:
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.gitrtmutexand fully integrated into the v5.14-rc3-rt2 release:
Thomas,
While reviewing/testing this series, I ended up finding some (simple) typos, reporting them here:
patch 2:
unneccesary -> unnecessary
patches 2/3/27/29/31:
accross -> across
patch 4:
optimze -> optimize
patches 4/15:
seperately -> separately
patches 5(2x)/12/17/18:
seperate -> separate
patch 11:
appropiate -> appropriate
patch 15:
indentical -> identical
patch 34:
Preperatory -> Preparatory
patch 53:
completness -> completeness
patch 61:
Signficant -> Significant
continuing the testing...
-- Daniel