locking, sched: The PREEMPT-RT locking infrastructure
Subsystems (8)
[tip: locking/core] lib/test_lockup: Adapt to changed variables
In reply to Thomas GleixnerThe following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: c49f7ece4617807c5de06857d196c825aadf60d5 Gitweb: https://git.kernel.org/tip/c49f7ece4617807c5de06857d196c825aadf60d5 Author: Sebastian Andrzej Siewior <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:03 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:33 +02:00
lib/test_lockup: Adapt to changed variables
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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
lib/test_lockup.c +3 -3
@@ -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
PATCH v5 57/72 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 completeness and paranoia sake.
--- kernel/futex.c | 7 +++++++ 1 file changed, 7 insertions(+)
---
kernel/futex.c +7 -0
@@ -1879,6 +1879,13 @@ futex_proxy_trylock_atomic(u32 __user *pif (!top_waiter)return 0 ; +/* +* Ensure that this is a waiter sitting in futex_wait_requeue_pi() +* and waiting on the 'waitqueue' futex which is always !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 ;
[tip: locking/core] futex: Validate waiter correctly in futex_proxy_trylock_atomic()
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: dc7109aaa233d83b573f75763a9f1ae207042a53 Gitweb: https://git.kernel.org/tip/dc7109aaa233d83b573f75763a9f1ae207042a53 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:04 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:36 +02:00
futex: Validate waiter correctly in futex_proxy_trylock_atomic()
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 completeness and paranoia sake.
--- kernel/futex.c | 7 +++++++ 1 file changed, 7 insertions(+)
kernel/futex.c +7 -0
@@ -1879,6 +1879,13 @@ futex_proxy_trylock_atomic(u32 __user *pifutex, struct futex_hash_bucket *hb1,if (!top_waiter)return 0 ; +/* +* Ensure that this is a waiter sitting in futex_wait_requeue_pi() +* and waiting on the 'waitqueue' futex which is always !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 v5 58/72 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
@@ -2621,8 +2621,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;
@@ -2700,8 +2699,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)
@@ -2712,7 +2711,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 ;
@@ -3205,8 +3203,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)
@@ -3235,9 +3233,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. */ /*
[tip: locking/core] futex: Clean up stale comments
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: c363b7ed79253d5b53494197f6ae625cff64694f Gitweb: https://git.kernel.org/tip/c363b7ed79253d5b53494197f6ae625cff64694f Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:06 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:39 +02:00
futex: Clean up stale comments
The futex key reference mechanism is long gone. Clean up the stale comments which still mention it.
--- 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 __user *uaddr, u32 uval, u32 newval)* - 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
@@ -2621,8 +2621,7 @@ static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,* * 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;
@@ -2700,8 +2699,8 @@ static int futex_wait(u32 __user *uaddr, unsigned int flags, u32 val, 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)
@@ -2712,7 +2711,6 @@ retry:/* 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 ;
@@ -3205,8 +3203,8 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags, 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)
@@ -3235,9 +3233,7 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,* 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 v5 59/72 futex: Clarify futex_requeue() PI handling
In reply to Thomas GleixnerWhen requeuing to a PI futex then the requeue code tries to trylock the PI futex on behalf of the topmost waiter on the inner 'waitqueue' futex. If that succeeds then PI state has to be allocated in order to requeue further waiters to the PI futex.
The comment and the code are confusing as the PI state allocation uses lookup_pi_state() which either attaches to an existing waiter or to the owner. As the PI futex was just acquired, there cannot be a waiter on the PI futex because the hash bucket lock is held.
Clarify the comment and use attach_to_pi_owner() directly. As the task on which behalf the PI futex has been acquired is guaranteed to be alive and not exiting, this call must succeed. Add a WARN_ON() in case that fails.
---
V4: New patch
--- kernel/futex.c | 61 +++++++++++++++++++++------------------------------------ 1 file changed, 23 insertions(+), 38 deletions(-)
kernel/futex.c +23 -38
@@ -1299,27 +1299,6 @@ static int attach_to_pi_owner(u32 __userreturn 0 ; } -static int lookup_pi_state (u32 __user *uaddr,u32 uval, -struct futex_hash_bucket *hb, -union futex_key *key,struct futex_pi_state **ps, -struct task_struct **exiting) -{ -struct futex_q *top_waiter =futex_top_waiter (hb, key); - -/* -* If there is a waiter on that futex, validate it and -* attach to the pi_state when the validation succeeds. -*/ -if (top_waiter) -return attach_to_pi_state (uaddr, uval, top_waiter->pi_state, ps); - -/* -* We are the first waiter - try to look up the owner based on -* @uval and attach to it. -*/ -return attach_to_pi_owner (uaddr, uval, key, ps, exiting); -} -static int lock_pi_update_atomic (u32 __user *uaddr,u32 uval,u32 newval) {int err;
@@ -2038,8 +2017,8 @@ static int futex_requeue(u32 __user *uad* At this point the top_waiter has either taken uaddr2 or is * waiting on it. If the former, then the pi_state will not * exist yet, look it up one more time to ensure we have a -* reference to it. If the lock was taken, ret contains the -* vpid of the top waiter task. +* reference to it. If the lock was taken, @ret contains the +* 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. */
@@ -2047,19 +2026,25 @@ static int futex_requeue(u32 __user *uadWARN_ON (pi_state); task_count++;/* -* If we acquired the lock, then the user space value -* of uaddr2 should be vpid. It cannot be changed by -* the top waiter as it is blocked on hb2 lock if it -* tries to do so. If something fiddled with it behind -* our back the pi state lookup might unearth it. So -* we rather use the known value than rereading and -* handing potential crap to lookup_pi_state. +* If futex_proxy_trylock_atomic() acquired the +* user space futex, then the user space value +* @uaddr2 has been set to the @hb1's top waiter +* task VPID. This task is guaranteed to be alive +* and cannot be exiting because it is either +* sleeping or blocked on @hb2 lock. +* +* The @uaddr2 futex cannot have waiters either as +* otherwise futex_proxy_trylock_atomic() would not +* have succeeded. * -* If that call succeeds then we have pi_state and an -* initial refcount on it. +* In order to requeue waiters to @hb2, pi state is +* required. Hand in the VPID value (@ret) and +* allocate PI state with an initial refcount on +* it. */ - ret =lookup_pi_state (uaddr2, ret, hb2, &key2, - &pi_state, &exiting); + ret =attach_to_pi_owner (uaddr2, ret, &key2, &pi_state, + &exiting); +WARN_ON (ret); }switch (ret) {
@@ -2183,9 +2168,9 @@ static int futex_requeue(u32 __user *uad }/* -* We took an extra initial reference to the pi_state either -* in futex_proxy_trylock_atomic() or in lookup_pi_state(). We -* need to drop it here again. +* We took an extra initial reference to the pi_state either in +* futex_proxy_trylock_atomic() or in attach_to_pi_owner(). We need +* to drop it here again. */ put_pi_state (pi_state);
@@ -2364,7 +2349,7 @@ static int __fixup_pi_state_owner(u32 __* Modifying pi_state _before_ the user space value would leave the * pi_state in an inconsistent state when we fault here, because we * need to drop the locks to handle the fault. This might be observed -* in the PID check in lookup_pi_state. +* in the PID checks when attaching to PI state . */ retry:if (!argowner) {
[tip: locking/core] futex: Clarify futex_requeue() PI handling
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: f6f4ec00b57a2c950235435bff8e888daafad5af Gitweb: https://git.kernel.org/tip/f6f4ec00b57a2c950235435bff8e888daafad5af Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:07 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:41 +02:00
futex: Clarify futex_requeue() PI handling
When requeuing to a PI futex, then the requeue code tries to trylock the PI futex on behalf of the topmost waiter on the inner 'waitqueue' futex. If that succeeds, then PI state has to be allocated in order to requeue further waiters to the PI futex.
The comment and the code are confusing, as the PI state allocation uses lookup_pi_state(), which either attaches to an existing waiter or to the owner. As the PI futex was just acquired, there cannot be a waiter on the PI futex because the hash bucket lock is held.
Clarify the comment and use attach_to_pi_owner() directly. As the task on which behalf the PI futex has been acquired is guaranteed to be alive and not exiting, this call must succeed. Add a WARN_ON() in case that fails.
--- kernel/futex.c | 61 ++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 38 deletions(-)
kernel/futex.c +23 -38
@@ -1299,27 +1299,6 @@ static int attach_to_pi_owner(u32 __user *uaddr, u32 uval, union futex_key *key,return 0 ; } -static int lookup_pi_state (u32 __user *uaddr,u32 uval, -struct futex_hash_bucket *hb, -union futex_key *key,struct futex_pi_state **ps, -struct task_struct **exiting) -{ -struct futex_q *top_waiter =futex_top_waiter (hb, key); - -/* -* If there is a waiter on that futex, validate it and -* attach to the pi_state when the validation succeeds. -*/ -if (top_waiter) -return attach_to_pi_state (uaddr, uval, top_waiter->pi_state, ps); - -/* -* We are the first waiter - try to look up the owner based on -* @uval and attach to it. -*/ -return attach_to_pi_owner (uaddr, uval, key, ps, exiting); -} -static int lock_pi_update_atomic (u32 __user *uaddr,u32 uval,u32 newval) {int err;
@@ -2038,8 +2017,8 @@ retry_private:* At this point the top_waiter has either taken uaddr2 or is * waiting on it. If the former, then the pi_state will not * exist yet, look it up one more time to ensure we have a -* reference to it. If the lock was taken, ret contains the -* vpid of the top waiter task. +* reference to it. If the lock was taken, @ret contains the +* 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. */
@@ -2047,19 +2026,25 @@ retry_private:WARN_ON (pi_state); task_count++;/* -* If we acquired the lock, then the user space value -* of uaddr2 should be vpid. It cannot be changed by -* the top waiter as it is blocked on hb2 lock if it -* tries to do so. If something fiddled with it behind -* our back the pi state lookup might unearth it. So -* we rather use the known value than rereading and -* handing potential crap to lookup_pi_state. +* If futex_proxy_trylock_atomic() acquired the +* user space futex, then the user space value +* @uaddr2 has been set to the @hb1's top waiter +* task VPID. This task is guaranteed to be alive +* and cannot be exiting because it is either +* sleeping or blocked on @hb2 lock. +* +* The @uaddr2 futex cannot have waiters either as +* otherwise futex_proxy_trylock_atomic() would not +* have succeeded. * -* If that call succeeds then we have pi_state and an -* initial refcount on it. +* In order to requeue waiters to @hb2, pi state is +* required. Hand in the VPID value (@ret) and +* allocate PI state with an initial refcount on +* it. */ - ret =lookup_pi_state (uaddr2, ret, hb2, &key2, - &pi_state, &exiting); + ret =attach_to_pi_owner (uaddr2, ret, &key2, &pi_state, + &exiting); +WARN_ON (ret); }switch (ret) {
@@ -2183,9 +2168,9 @@ retry_private: }/* -* We took an extra initial reference to the pi_state either -* in futex_proxy_trylock_atomic() or in lookup_pi_state(). We -* need to drop it here again. +* We took an extra initial reference to the pi_state either in +* futex_proxy_trylock_atomic() or in attach_to_pi_owner(). We need +* to drop it here again. */ put_pi_state (pi_state);
@@ -2364,7 +2349,7 @@ static int __fixup_pi_state_owner(u32 __user *uaddr, struct futex_q *q,* Modifying pi_state _before_ the user space value would leave the * pi_state in an inconsistent state when we fault here, because we * need to drop the locks to handle the fault. This might be observed -* in the PID check in lookup_pi_state. +* in the PID checks when attaching to PI state . */ retry:if (!argowner) {
PATCH v5 60/72 futex: Remove bogus condition for requeue PI
In reply to Thomas GleixnerFor requeue PI it's required to establish PI state for the PI futex to which waiters are requeued. This either acquires the user space futex on behalf of the top most waiter on the inner 'waitqueue' futex or attaches to the PI state of an existing waiter or creates on attached to the owner of the futex.
This code can retry in case of failure, but retry can never happen when the pi state was successfully created. The condition to run this code is:
(task_count - nr_wake) < nr_requeue
which is always true because:
task_count = 0
nr_wake = 1
nr_requeue >= 0
Remove it all together.
---
V4: New patch
--- kernel/futex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/futex.c +1 -1
@@ -2000,7 +2000,7 @@ static int futex_requeue(u32 __user *uad } } -if (requeue_pi && (task_count - nr_wake < nr_requeue)) { +if (requeue_pi) {struct task_struct *exiting =NULL ;/*
[tip: locking/core] futex: Remove bogus condition for requeue PI
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 8e74633dcefb280f2cefb49b7201d99650243d96 Gitweb: https://git.kernel.org/tip/8e74633dcefb280f2cefb49b7201d99650243d96 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:09 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:44 +02:00
futex: Remove bogus condition for requeue PI
For requeue PI it's required to establish PI state for the PI futex to which waiters are requeued. This either acquires the user space futex on behalf of the top most waiter on the inner 'waitqueue' futex, or attaches to the PI state of an existing waiter, or creates on attached to the owner of the futex.
This code can retry in case of failure, but retry can never happen when the pi state was successfully created. The condition to run this code is:
(task_count - nr_wake) < nr_requeue
which is always true because:
task_count = 0
nr_wake = 1
nr_requeue >= 0
Remove it completely.
--- kernel/futex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/futex.c +1 -1
@@ -2000,7 +2000,7 @@ retry_private: } } -if (requeue_pi && (task_count - nr_wake < nr_requeue)) { +if (requeue_pi) {struct task_struct *exiting =NULL ;/*
PATCH v5 61/72 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
@@ -2116,6 +2116,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 ; }
@@ -2157,6 +2159,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.
[tip: locking/core] futex: Correct the number of requeued waiters for PI
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 59c7ecf1544e1841b5be8847e81bc9842f838e7e Gitweb: https://git.kernel.org/tip/59c7ecf1544e1841b5be8847e81bc9842f838e7e Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:10 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:46 +02:00
futex: Correct the number of requeued waiters for PI
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
@@ -2116,6 +2116,8 @@ retry_private:/* 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 ; }
@@ -2157,6 +2159,8 @@ retry_private:*/ 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 v5 62/72 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
@@ -2104,20 +2104,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 ; }
@@ -2125,50 +2122,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++; }/*
[tip: locking/core] futex: Restructure futex_requeue()
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 64b7b715f7f92ae3233446b4a4cdda3524fcd4b0 Gitweb: https://git.kernel.org/tip/64b7b715f7f92ae3233446b4a4cdda3524fcd4b0 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:12 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:49 +02:00
futex: Restructure futex_requeue()
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
@@ -2104,20 +2104,17 @@ retry_private:break ; } -/* -* 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 ; }
@@ -2125,50 +2122,45 @@ retry_private:/* * 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 ) {/* -* 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 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. */ -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 ; - } +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); +/* +* 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++; }/*
PATCH v5 63/72 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
@@ -1939,15 +1939,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 ;
[tip: locking/core] futex: Clarify comment in futex_requeue()
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: c18eaa3aca43688a3aee199d85ce4227686a29b6 Gitweb: https://git.kernel.org/tip/c18eaa3aca43688a3aee199d85ce4227686a29b6 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:14 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:51 +02:00
futex: Clarify comment in futex_requeue()
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
@@ -1939,15 +1939,27 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,*/ 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 v5 64/72 futex: Reorder sanity checks in futex_requeue()
In reply to Thomas GleixnerNo point in allocating memory when the input parameters are bogus. Validate all parameters before proceeding.
---
V4: New patch
--- kernel/futex.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
kernel/futex.c +7 -7
@@ -1934,13 +1934,6 @@ static int futex_requeue(u32 __user *uadreturn -EINVAL ;/* -* requeue_pi requires a pi_state, try to allocate it now -* without any locks in case it fails. -*/ -if (refill_pi_state_cache ()) -return -ENOMEM ; - -/* * 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
@@ -1963,6 +1956,13 @@ static int futex_requeue(u32 __user *uad*/ if (nr_wake !=1 )return -EINVAL ; + +/* +* requeue_pi requires a pi_state, try to allocate it now +* without any locks in case it fails. +*/ +if (refill_pi_state_cache ()) +return -ENOMEM ; } retry:
[tip: locking/core] futex: Reorder sanity checks in futex_requeue()
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: d69cba5c719b0c551f6380ec5da4ed8c20a3815a Gitweb: https://git.kernel.org/tip/d69cba5c719b0c551f6380ec5da4ed8c20a3815a Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:15 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:54 +02:00
futex: Reorder sanity checks in futex_requeue()
No point in allocating memory when the input parameters are bogus. Validate all parameters before proceeding.
--- kernel/futex.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
kernel/futex.c +7 -7
@@ -1934,13 +1934,6 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,return -EINVAL ;/* -* requeue_pi requires a pi_state, try to allocate it now -* without any locks in case it fails. -*/ -if (refill_pi_state_cache ()) -return -ENOMEM ; - -/* * 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
@@ -1963,6 +1956,13 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,*/ if (nr_wake !=1 )return -EINVAL ; + +/* +* requeue_pi requires a pi_state, try to allocate it now +* without any locks in case it fails. +*/ +if (refill_pi_state_cache ()) +return -ENOMEM ; } retry:
PATCH v5 65/72 futex: Simplify handle_early_requeue_pi_wakeup()
In reply to Thomas GleixnerMove the futex key match out of handle_early_requeue_pi_wakeup() which allows to simplify that function. The upcoming state machine for requeue_pi() will make that go away.
---
V3: New patch - Suggested by Peter
--- kernel/futex.c | 50 +++++++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 27 deletions(-)
kernel/futex.c +23 -27
@@ -3070,27 +3070,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.
@@ -3099,22 +3094,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); - -/* Handle spurious wakeups gracefully */ - ret = -EWOULDBLOCK ; -if (timeout && !timeout->task) - ret = -ETIMEDOUT ; -else if (signal_pending (current)) - ret = -ERESTARTNOINTR ; - } +WARN_ON_ONCE (&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); + +/* Handle spurious wakeups gracefully */ + ret = -EWOULDBLOCK ; +if (timeout && !timeout->task) + ret = -ETIMEDOUT ; +else if (signal_pending (current)) + ret = -ERESTARTNOINTR ;return ret; }
@@ -3217,7 +3211,9 @@ static int futex_wait_requeue_pi(u32 __ufutex_wait_queue_me (hb, &q, to);spin_lock (&hb->lock); - ret =handle_early_requeue_pi_wakeup (hb, &q, &key2, to); +/* Is @q still queued on uaddr1? */ +if (!match_futex (&q->key, key2)) + ret =handle_early_requeue_pi_wakeup (hb, &q, to);spin_unlock (&hb->lock);if (ret) goto out;
[tip: locking/core] futex: Simplify handle_early_requeue_pi_wakeup()
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 6231acbd0802e76580c71ceb52c09646d42170fb Gitweb: https://git.kernel.org/tip/6231acbd0802e76580c71ceb52c09646d42170fb Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:17 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:57 +02:00
futex: Simplify handle_early_requeue_pi_wakeup()
Move the futex key match out of handle_early_requeue_pi_wakeup() which allows to simplify that function. The upcoming state machine for requeue_pi() will make that go away.
--- kernel/futex.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-)
kernel/futex.c +22 -26
@@ -3070,27 +3070,22 @@ pi_faulted: }/** -* 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.
@@ -3099,22 +3094,21 @@ int handle_early_requeue_pi_wakeup(struct futex_hash_bucket *hb,* 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; }
@@ -3217,7 +3211,9 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,futex_wait_queue_me (hb, &q, to);spin_lock (&hb->lock); - ret =handle_early_requeue_pi_wakeup (hb, &q, &key2, to); +/* Is @q still queued on uaddr1? */ +if (!match_futex (&q->key, key2)) + ret =handle_early_requeue_pi_wakeup (hb, &q, to);spin_unlock (&hb->lock);if (ret) goto out;
PATCH v5 66/72 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.
---
V3: Folded Peter's improvements
--- kernel/futex.c | 308 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 259 insertions(+), 49 deletions(-)
---
kernel/futex.c +259 -49
@@ -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,12 +221,68 @@ 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; +/* +* 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 const struct futex_q futex_q_init = {/* list gets initialized in queue_me()*/ - .key =FUTEX_KEY_INIT , - .bitset =FUTEX_BITSET_MATCH_ANY + .key =FUTEX_KEY_INIT , + .bitset =FUTEX_BITSET_MATCH_ANY , + .requeue_state =ATOMIC_INIT (Q_REQUEUE_PI_NONE ), };/*
@@ -1772,6 +1830,108 @@ void requeue_futex(struct futex_q *q, st q->key = *key2; } +static inline bool futex_requeue_pi_prepare (struct futex_q *q, +struct futex_pi_state *pi_state) +{ +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. +*/ + old =atomic_read_acquire (&q->requeue_state); +do { +if (old ==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 (old !=Q_REQUEUE_PI_NONE ) +break ; + + new =Q_REQUEUE_PI_IN_PROGRESS ; + }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 old, new; + + old =atomic_read_acquire (&q->requeue_state); +do { +if (old ==Q_REQUEUE_PI_IGNORE ) +return ; + +if (locked >=0 ) { +/* Requeue succeeded. Set DONE or LOCKED */ +WARN_ON_ONCE (old !=Q_REQUEUE_PI_IN_PROGRESS && + old !=Q_REQUEUE_PI_WAIT ); + 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 { +/* Deadlock, early wakeup interleave. */ +WARN_ON_ONCE (old !=Q_REQUEUE_PI_WAIT ); + 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 +} + +static inline int futex_requeue_pi_wakeup_sync (struct futex_q *q) +{ +int old, new; + + old =atomic_read_acquire (&q->requeue_state); +do { +/* Is requeue done already? */ +if (old >=Q_REQUEUE_PI_DONE ) +return old; + +/* +* 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 + } + +/* +* 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
@@ -1799,6 +1959,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 ); }
@@ -1869,6 +2031,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
@@ -1878,8 +2044,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; }
@@ -2020,6 +2200,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,
@@ -2033,6 +2215,24 @@ 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. +* +* - 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);
@@ -2064,7 +2264,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);
@@ -2140,21 +2343,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.
@@ -2164,15 +2385,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++; }/*
@@ -3161,6 +3380,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 ))
@@ -3210,32 +3430,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); -/* Is @q still queued on uaddr1? */ -if (!match_futex (&q->key, key2)) +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); -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. -*/ +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);
@@ -3245,18 +3455,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 ;
@@ -3276,17 +3482,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:
[tip: locking/core] futex: Prevent requeue_pi() lock nesting issue on RT
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 07d91ef510fb16a2e0ca7453222105835b7ba3b8 Gitweb: https://git.kernel.org/tip/07d91ef510fb16a2e0ca7453222105835b7ba3b8 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:18 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:05:59 +02:00
futex: Prevent requeue_pi() lock nesting issue on RT
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 | 308 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 259 insertions(+), 49 deletions(-)
kernel/futex.c +259 -49
@@ -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,12 +221,68 @@ 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; +/* +* 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 const struct futex_q futex_q_init = {/* list gets initialized in queue_me()*/ - .key =FUTEX_KEY_INIT , - .bitset =FUTEX_BITSET_MATCH_ANY + .key =FUTEX_KEY_INIT , + .bitset =FUTEX_BITSET_MATCH_ANY , + .requeue_state =ATOMIC_INIT (Q_REQUEUE_PI_NONE ), };/*
@@ -1772,6 +1830,108 @@ void requeue_futex(struct futex_q *q, struct futex_hash_bucket *hb1, q->key = *key2; } +static inline bool futex_requeue_pi_prepare (struct futex_q *q, +struct futex_pi_state *pi_state) +{ +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. +*/ + old =atomic_read_acquire (&q->requeue_state); +do { +if (old ==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 (old !=Q_REQUEUE_PI_NONE ) +break ; + + new =Q_REQUEUE_PI_IN_PROGRESS ; + }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 old, new; + + old =atomic_read_acquire (&q->requeue_state); +do { +if (old ==Q_REQUEUE_PI_IGNORE ) +return ; + +if (locked >=0 ) { +/* Requeue succeeded. Set DONE or LOCKED */ +WARN_ON_ONCE (old !=Q_REQUEUE_PI_IN_PROGRESS && + old !=Q_REQUEUE_PI_WAIT ); + 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 { +/* Deadlock, early wakeup interleave. */ +WARN_ON_ONCE (old !=Q_REQUEUE_PI_WAIT ); + 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 +} + +static inline int futex_requeue_pi_wakeup_sync (struct futex_q *q) +{ +int old, new; + + old =atomic_read_acquire (&q->requeue_state); +do { +/* Is requeue done already? */ +if (old >=Q_REQUEUE_PI_DONE ) +return old; + +/* +* 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 + } + +/* +* 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
@@ -1799,6 +1959,8 @@ void requeue_pi_wake_futex(struct futex_q *q, union futex_key *key, q->lock_ptr = &hb->lock; +/* Signal locked state to the waiter */ +futex_requeue_pi_complete (q,1 );wake_up_state (q->task,TASK_NORMAL ); }
@@ -1869,6 +2031,10 @@ futex_proxy_trylock_atomic(u32 __user *pifutex, struct futex_hash_bucket *hb1,if (!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
@@ -1878,8 +2044,22 @@ futex_proxy_trylock_atomic(u32 __user *pifutex, struct futex_hash_bucket *hb1, 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; }
@@ -2020,6 +2200,8 @@ retry_private:* 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,
@@ -2033,6 +2215,24 @@ retry_private:* 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. +* +* - 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);
@@ -2064,7 +2264,10 @@ retry_private:/* 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);
@@ -2140,21 +2343,39 @@ retry_private:* 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.
@@ -2164,15 +2385,13 @@ retry_private:*/ 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++; }/*
@@ -3161,6 +3380,7 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,struct 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 ))
@@ -3210,32 +3430,22 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,/* Queue the futex_q, drop the hb lock, wait for wakeup. */ futex_wait_queue_me (hb, &q, to); -spin_lock (&hb->lock); -/* Is @q still queued on uaddr1? */ -if (!match_futex (&q->key, key2)) +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); -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. -*/ +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);
@@ -3245,18 +3455,14 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,*/ 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 ;
@@ -3276,17 +3482,21 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,unqueue_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:
PATCH v5 67/72 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 separate 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 separate key. +*/ +lockdep_set_class (&lock->wait_lock, &pi_futex_key);rt_mutex_set_owner (lock, proxy_owner); }
[tip: locking/core] locking/rtmutex: Prevent lockdep false positive with PI futexes
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 51711e825a6d1b2fe7ca46bb06d08c25d97656ee Gitweb: https://git.kernel.org/tip/51711e825a6d1b2fe7ca46bb06d08c25d97656ee Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:20 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:06:02 +02:00
locking/rtmutex: Prevent lockdep false positive with PI futexes
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 separate 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 separate key. +*/ +lockdep_set_class (&lock->wait_lock, &pi_futex_key);rt_mutex_set_owner (lock, proxy_owner); }
PATCH v5 68/72 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:
[tip: locking/core] preempt: Adjust PREEMPT_LOCK_OFFSET for RT
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 015680aa4c5d784513d0a9728bc52ec7c4a64227 Gitweb: https://git.kernel.org/tip/015680aa4c5d784513d0a9728bc52ec7c4a64227 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:22 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:06:04 +02:00
preempt: Adjust PREEMPT_LOCK_OFFSET for RT
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 v5 69/72 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.
Significant 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
@@ -338,6 +338,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)
@@ -932,19 +952,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
@@ -955,13 +977,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 ;/*
[tip: locking/core] locking/rtmutex: Implement equal priority lock stealing
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 48eb3f4fcfd35495a8357459aa6fe437aa430b00 Gitweb: https://git.kernel.org/tip/48eb3f4fcfd35495a8357459aa6fe437aa430b00 Author: Gregory Haskins <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:23 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:06:07 +02:00
locking/rtmutex: Implement equal priority lock stealing
The current logic only allows lock stealing to occur if the current task is of higher priority than the pending owner.
Significant 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
@@ -338,6 +338,26 @@ static __always_inline int rt_mutex_waiter_equal(struct rt_mutex_waiter *left,return 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)
@@ -932,19 +952,21 @@ try_to_take_rt_mutex(struct rt_mutex_base *lock, struct task_struct *task,* 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
@@ -955,13 +977,9 @@ try_to_take_rt_mutex(struct rt_mutex_base *lock, struct task_struct *task,* 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 v5 70/72 locking/rtmutex: Add adaptive spinwait mechanism
In reply to Thomas GleixnerFrom: Steven Rostedt <[email protected]>
Going to sleep when locks are contended can be quite inefficient when the contention time is short and the lock owner is running on a different CPU.
The MCS mechanism cannot be used because MCS is strictly FIFO ordered while for rtmutex based locks the waiter ordering is priority based.
Provide a simple adaptive spinwait mechanism which currently restricts the spinning to the top priority waiter.
[ tglx: Provide a contemporary changelog, extended it to all rtmutex based
locks and updated it to match the other spin on owner implementations ]
---
V4: Rename to rtmutex_spin_on_owner() (PeterZ)
Check for top waiter changes and rewrite comment (Davidlohr)
V3: Fold the extension for regular sleeping locks and add the missing spin
wait checks (PeterZ)
--- kernel/locking/rtmutex.c | 67 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 2 deletions(-)
---
kernel/locking/rtmutex.c +65 -2
@@ -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. */
@@ -1297,6 +1302,52 @@ static __always_inline void __rt_mutex_urt_mutex_slowunlock (lock); } +#ifdef CONFIG_SMP +static bool rtmutex_spin_on_owner (struct rt_mutex_base *lock, +struct rt_mutex_waiter *waiter, +struct task_struct *owner) +{ +bool res = true; + +rcu_read_lock (); +for (;;) { +/* If owner changed, trylock again. */ +if (owner !=rt_mutex_owner (lock)) +break ; +/* +* Ensure that @owner is dereferenced after checking that +* the lock owner still matches @owner. If that fails, +* @owner might point to freed memory. If it still matches, +* the rcu_read_lock() ensures the memory stays valid. +*/ +barrier (); +/* +* Stop spinning when: +* - the lock owner has been scheduled out +* - current is not longer the top waiter +* - current is requested to reschedule (redundant +* for CONFIG_PREEMPT_RCU=y) +* - the VCPU on which owner runs is preempted +*/ +if (!owner->on_cpu || waiter !=rt_mutex_top_waiter (lock) || +need_resched () ||vcpu_is_preempted (task_cpu (owner))) { + res = false; +break ; + } +cpu_relax (); + } +rcu_read_unlock (); +return res; +} +#else +static bool rtmutex_spin_on_owner (struct rt_mutex_base *lock, +struct rt_mutex_waiter *waiter, +struct task_struct *owner) +{ +return false; +} +#endif +#ifdef RT_MUTEX_BUILD_MUTEX /* * Functions required for:
@@ -1381,6 +1432,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 (;;) {
@@ -1403,9 +1455,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_spin_on_owner (lock, waiter, owner)) +schedule ();raw_spin_lock_irq (&lock->wait_lock);set_current_state (state);
@@ -1561,6 +1618,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);
@@ -1579,9 +1637,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 || !rtmutex_spin_on_owner (lock, &waiter, owner)) +schedule_rtlock ();raw_spin_lock_irq (&lock->wait_lock);set_current_state (TASK_RTLOCK_WAIT );
[tip: locking/core] locking/rtmutex: Add adaptive spinwait mechanism
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 992caf7f17243d736fc996770bac6566103778f6 Gitweb: https://git.kernel.org/tip/992caf7f17243d736fc996770bac6566103778f6 Author: Steven Rostedt <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:25 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:06:11 +02:00
locking/rtmutex: Add adaptive spinwait mechanism
Going to sleep when locks are contended can be quite inefficient when the contention time is short and the lock owner is running on a different CPU.
The MCS mechanism cannot be used because MCS is strictly FIFO ordered while for rtmutex based locks the waiter ordering is priority based.
Provide a simple adaptive spinwait mechanism which currently restricts the spinning to the top priority waiter.
[ tglx: Provide a contemporary changelog, extended it to all rtmutex based
locks and updated it to match the other spin on owner implementations ]
--- kernel/locking/rtmutex.c | 67 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 2 deletions(-)
kernel/locking/rtmutex.c +65 -2
@@ -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. */
@@ -1297,6 +1302,52 @@ static __always_inline void __rt_mutex_unlock(struct rt_mutex_base *lock)rt_mutex_slowunlock (lock); } +#ifdef CONFIG_SMP +static bool rtmutex_spin_on_owner (struct rt_mutex_base *lock, +struct rt_mutex_waiter *waiter, +struct task_struct *owner) +{ +bool res = true; + +rcu_read_lock (); +for (;;) { +/* If owner changed, trylock again. */ +if (owner !=rt_mutex_owner (lock)) +break ; +/* +* Ensure that @owner is dereferenced after checking that +* the lock owner still matches @owner. If that fails, +* @owner might point to freed memory. If it still matches, +* the rcu_read_lock() ensures the memory stays valid. +*/ +barrier (); +/* +* Stop spinning when: +* - the lock owner has been scheduled out +* - current is not longer the top waiter +* - current is requested to reschedule (redundant +* for CONFIG_PREEMPT_RCU=y) +* - the VCPU on which owner runs is preempted +*/ +if (!owner->on_cpu || waiter !=rt_mutex_top_waiter (lock) || +need_resched () ||vcpu_is_preempted (task_cpu (owner))) { + res = false; +break ; + } +cpu_relax (); + } +rcu_read_unlock (); +return res; +} +#else +static bool rtmutex_spin_on_owner (struct rt_mutex_base *lock, +struct rt_mutex_waiter *waiter, +struct task_struct *owner) +{ +return false; +} +#endif +#ifdef RT_MUTEX_BUILD_MUTEX /* * Functions required for:
@@ -1381,6 +1432,7 @@ static int __sched rt_mutex_slowlock_block(struct rt_mutex_base *lock, struct rt_mutex_waiter *waiter) { struct rt_mutex *rtm = container_of(lock, struct rt_mutex, rtmutex); +struct task_struct *owner;int ret =0 ;for (;;) {
@@ -1403,9 +1455,14 @@ static int __sched rt_mutex_slowlock_block(struct rt_mutex_base *lock,break ; } +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_spin_on_owner (lock, waiter, owner)) +schedule ();raw_spin_lock_irq (&lock->wait_lock);set_current_state (state);
@@ -1561,6 +1618,7 @@ static __always_inline int __rt_mutex_lock(struct rt_mutex_base *lock, 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);
@@ -1579,9 +1637,14 @@ static void __sched rtlock_slowlock_locked(struct rt_mutex_base *lock)if (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 || !rtmutex_spin_on_owner (lock, &waiter, owner)) +schedule_rtlock ();raw_spin_lock_irq (&lock->wait_lock);set_current_state (TASK_RTLOCK_WAIT );
PATCH v5 71/72 locking/spinlock/rt: Prepare for RT local_lock
In reply to Thomas GleixnerAdd the static and runtime initializer mechanics to support the RT variant of local_lock, which requires the lock type in the lockdep map to be set to LD_LOCK_PERCPU.
---
V5: New patch
--- include/linux/spinlock_rt.h | 24 ++++++++++++++++-------- include/linux/spinlock_types.h | 6 ++++++ include/linux/spinlock_types_raw.h | 8 ++++++++ kernel/locking/spinlock_rt.c | 7 +++++-- 4 files changed, 35 insertions(+), 10 deletions(-)
include/linux/spinlock_rt.h +16 -8
@@ -8,20 +8,28 @@#ifdef CONFIG_DEBUG_LOCK_ALLOC extern void __rt_spin_lock_init (spinlock_t *lock,const char *name, -struct lock_class_key *key); +struct lock_class_key *key,bool percpu);#else static inline void __rt_spin_lock_init (spinlock_t *lock,const char *name, -struct lock_class_key *key) +struct lock_class_key *key,bool percpu) { }#endif -#define spin_lock_init (slock) \ -do { \ - static struct lock_class_key __key; \ - \ - rt_mutex_base_init(&(slock)->lock); \ - __rt_spin_lock_init(slock, #slock, &__key); \ +#define spin_lock_init (slock) \ +do { \ + static struct lock_class_key __key; \ + \ + rt_mutex_base_init(&(slock)->lock); \ + __rt_spin_lock_init(slock, #slock, &__key, false); \ +} while (0) + +#define local_spin_lock_init (slock) \ +do { \ + static struct lock_class_key __key; \ + \ + rt_mutex_base_init(&(slock)->lock); \ + __rt_spin_lock_init(slock, #slock, &__key, true); \ } while (0)extern void rt_spin_lock (spinlock_t *lock);
include/linux/spinlock_types.h +6 -0
@@ -60,6 +60,12 @@ typedef struct spinlock { SPIN_DEP_MAP_INIT(name) \ } +#define __LOCAL_SPIN_LOCK_UNLOCKED (name) \ + { \ + .lock = __RT_MUTEX_BASE_INITIALIZER(name.lock), \ + LOCAL_SPIN_DEP_MAP_INIT(name) \ + } +#define DEFINE_SPINLOCK (name) \ spinlock_t name = __SPIN_LOCK_UNLOCKED(name)
include/linux/spinlock_types_raw.h +8 -0
@@ -37,9 +37,17 @@ typedef struct raw_spinlock { .name = #lockname, \ .wait_type_inner = LD_WAIT_CONFIG, \ } + +# define LOCAL_SPIN_DEP_MAP_INIT (lockname) \ + .dep_map = { \ + .name = #lockname, \ + .wait_type_inner = LD_WAIT_CONFIG, \ + .lock_type = LD_LOCK_PERCPU, \ + }#else # define RAW_SPIN_DEP_MAP_INIT (lockname)# define SPIN_DEP_MAP_INIT (lockname) +# define LOCAL_SPIN_DEP_MAP_INIT (lockname)#endif #ifdef CONFIG_DEBUG_SPINLOCK
kernel/locking/spinlock_rt.c +5 -2
@@ -120,10 +120,13 @@ EXPORT_SYMBOL(rt_spin_trylock_bh);#ifdef CONFIG_DEBUG_LOCK_ALLOC void __rt_spin_lock_init (spinlock_t *lock,const char *name, -struct lock_class_key *key) +struct lock_class_key *key,bool percpu) { +u8 type = percpu ?LD_LOCK_PERCPU :LD_LOCK_NORMAL ; +debug_check_no_locks_freed ((void *)lock,sizeof (*lock)); -lockdep_init_map_wait (&lock->dep_map, name, key,0 ,LD_WAIT_CONFIG ); +lockdep_init_map_type (&lock->dep_map, name, key,0 ,LD_WAIT_CONFIG , +LD_WAIT_INV , type); }EXPORT_SYMBOL (__rt_spin_lock_init);#endif
[tip: locking/core] locking/spinlock/rt: Prepare for RT local_lock
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 31552385f8e9d0869117014bf8e55ba0497e3ec8 Gitweb: https://git.kernel.org/tip/31552385f8e9d0869117014bf8e55ba0497e3ec8 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:27 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:06:13 +02:00
locking/spinlock/rt: Prepare for RT local_lock
Add the static and runtime initializer mechanics to support the RT variant of local_lock, which requires the lock type in the lockdep map to be set to LD_LOCK_PERCPU.
--- include/linux/spinlock_rt.h | 24 ++++++++++++++++-------- include/linux/spinlock_types.h | 6 ++++++ include/linux/spinlock_types_raw.h | 8 ++++++++ kernel/locking/spinlock_rt.c | 7 +++++-- 4 files changed, 35 insertions(+), 10 deletions(-)
include/linux/spinlock_rt.h +16 -8
@@ -8,20 +8,28 @@#ifdef CONFIG_DEBUG_LOCK_ALLOC extern void __rt_spin_lock_init (spinlock_t *lock,const char *name, -struct lock_class_key *key); +struct lock_class_key *key,bool percpu);#else static inline void __rt_spin_lock_init (spinlock_t *lock,const char *name, -struct lock_class_key *key) +struct lock_class_key *key,bool percpu) { }#endif -#define spin_lock_init (slock) \ -do { \ - static struct lock_class_key __key; \ - \ - rt_mutex_base_init(&(slock)->lock); \ - __rt_spin_lock_init(slock, #slock, &__key); \ +#define spin_lock_init (slock) \ +do { \ + static struct lock_class_key __key; \ + \ + rt_mutex_base_init(&(slock)->lock); \ + __rt_spin_lock_init(slock, #slock, &__key, false); \ +} while (0) + +#define local_spin_lock_init (slock) \ +do { \ + static struct lock_class_key __key; \ + \ + rt_mutex_base_init(&(slock)->lock); \ + __rt_spin_lock_init(slock, #slock, &__key, true); \ } while (0)extern void rt_spin_lock (spinlock_t *lock);
include/linux/spinlock_types.h +6 -0
@@ -60,6 +60,12 @@ typedef struct spinlock { SPIN_DEP_MAP_INIT(name) \ } +#define __LOCAL_SPIN_LOCK_UNLOCKED (name) \ + { \ + .lock = __RT_MUTEX_BASE_INITIALIZER(name.lock), \ + LOCAL_SPIN_DEP_MAP_INIT(name) \ + } +#define DEFINE_SPINLOCK (name) \ spinlock_t name = __SPIN_LOCK_UNLOCKED(name)
include/linux/spinlock_types_raw.h +8 -0
@@ -37,9 +37,17 @@ typedef struct raw_spinlock { .name = #lockname, \ .wait_type_inner = LD_WAIT_CONFIG, \ } + +# define LOCAL_SPIN_DEP_MAP_INIT (lockname) \ + .dep_map = { \ + .name = #lockname, \ + .wait_type_inner = LD_WAIT_CONFIG, \ + .lock_type = LD_LOCK_PERCPU, \ + }#else # define RAW_SPIN_DEP_MAP_INIT (lockname)# define SPIN_DEP_MAP_INIT (lockname) +# define LOCAL_SPIN_DEP_MAP_INIT (lockname)#endif #ifdef CONFIG_DEBUG_SPINLOCK
kernel/locking/spinlock_rt.c +5 -2
@@ -120,10 +120,13 @@ EXPORT_SYMBOL(rt_spin_trylock_bh);#ifdef CONFIG_DEBUG_LOCK_ALLOC void __rt_spin_lock_init (spinlock_t *lock,const char *name, -struct lock_class_key *key) +struct lock_class_key *key,bool percpu) { +u8 type = percpu ?LD_LOCK_PERCPU :LD_LOCK_NORMAL ; +debug_check_no_locks_freed ((void *)lock,sizeof (*lock)); -lockdep_init_map_wait (&lock->dep_map, name, key,0 ,LD_WAIT_CONFIG ); +lockdep_init_map_type (&lock->dep_map, name, key,0 ,LD_WAIT_CONFIG , +LD_WAIT_INV , type); }EXPORT_SYMBOL (__rt_spin_lock_init);#endif
PATCH v5 72/72 locking/local_lock: Add PREEMPT_RT support
In reply to Thomas GleixnerFrom: Thomas Gleixner <[email protected]>
On PREEMPT_RT enabled kernels local_lock maps to a per CPU 'sleeping' spinlock which protects the critical section while staying preemptible. CPU locality is established by disabling migration.
Provide the necessary types and macros to substitute the non-RT variant.
---
V5: New patch
--- include/linux/local_lock_internal.h | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
include/linux/local_lock_internal.h +48 -0
@@ -6,6 +6,8 @@#include <linux/percpu-defs.h> #include <linux/lockdep.h> +#ifndef CONFIG_PREEMPT_RT + typedef struct { #ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map dep_map;
@@ -95,3 +97,49 @@ do { \local_lock_release (this_cpu_ptr (lock)); \local_irq_restore (flags); \ } while (0) + +#else /* !CONFIG_PREEMPT_RT */ + +/* +* On PREEMPT_RT local_lock maps to a per CPU spinlock which protects the +* critical section while staying preemptible. +*/ +typedef struct { +spinlock_t lock; +}local_lock_t ; + +#define INIT_LOCAL_LOCK (lockname) { \ + __LOCAL_SPIN_LOCK_UNLOCKED((lockname).lock) \ + } + +#define __local_lock_init (l) \ + do { \ + local_spin_lock_init(&(l)->lock); \ + } while (0) + +#define __local_lock (__lock) \ + do { \ + migrate_disable(); \ + spin_lock(&(this_cpu_ptr((__lock)))->lock); \ + } while (0) + +#define __local_lock_irq (lock) __local_lock(lock) + +#define __local_lock_irqsave (lock, flags) \ + do { \ + typecheck(unsigned long, flags); \ + flags = 0; \ + __local_lock(lock); \ + } while (0) + +#define __local_unlock (__lock) \ + do { \ + spin_unlock(&(this_cpu_ptr((__lock)))->lock); \ + migrate_enable(); \ + } while (0) + +#define __local_unlock_irq (lock) __local_unlock(lock) + +#define __local_unlock_irqrestore (lock, flags) __local_unlock(lock) + +#endif /* CONFIG_PREEMPT_RT */
[tip: locking/core] locking/local_lock: Add PREEMPT_RT support
The following commit has been merged into the locking/core branch of tip:
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 026659b9774e4c586baeb457557fcfc4e0ad144b Gitweb: https://git.kernel.org/tip/026659b9774e4c586baeb457557fcfc4e0ad144b Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 15 Aug 2021 23:29:28 +02:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Tue, 17 Aug 2021 19:08:49 +02:00
locking/local_lock: Add PREEMPT_RT support
On PREEMPT_RT enabled kernels local_lock maps to a per CPU 'sleeping' spinlock which protects the critical section while staying preemptible. CPU locality is established by disabling migration.
Provide the necessary types and macros to substitute the non-RT variant.
--- include/linux/local_lock_internal.h | 44 ++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+)
include/linux/local_lock_internal.h +44 -0
@@ -6,6 +6,8 @@#include <linux/percpu-defs.h> #include <linux/lockdep.h> +#ifndef CONFIG_PREEMPT_RT + typedef struct { #ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map dep_map;
@@ -95,3 +97,45 @@ do { \local_lock_release (this_cpu_ptr (lock)); \local_irq_restore (flags); \ } while (0) + +#else /* !CONFIG_PREEMPT_RT */ + +/* +* On PREEMPT_RT local_lock maps to a per CPU spinlock, which protects the +* critical section while staying preemptible. +*/ +typedef spinlock_t local_lock_t ; + +#define INIT_LOCAL_LOCK (lockname) __LOCAL_SPIN_LOCK_UNLOCKED((lockname)) + +#define __local_lock_init (l) \ + do { \ + local_spin_lock_init((l)); \ + } while (0) + +#define __local_lock (__lock) \ + do { \ + migrate_disable(); \ + spin_lock(this_cpu_ptr((__lock))); \ + } while (0) + +#define __local_lock_irq (lock) __local_lock(lock) + +#define __local_lock_irqsave (lock, flags) \ + do { \ + typecheck(unsigned long, flags); \ + flags = 0; \ + __local_lock(lock); \ + } while (0) + +#define __local_unlock (__lock) \ + do { \ + spin_unlock(this_cpu_ptr((__lock))); \ + migrate_enable(); \ + } while (0) + +#define __local_unlock_irq (lock) __local_unlock(lock) + +#define __local_unlock_irqrestore (lock, flags) __local_unlock(lock) + +#endif /* CONFIG_PREEMPT_RT */
Quoted 2 lines · Thomas Gleixner
The series survived internal testing in RT kernels and is part of the upcoming v5.14-rc6-rt9 release.
v5.14-rc6-rt10 to be exact, -rt9 was an intermediate release containing the update to -rc6.
Sebastian