As we add functionality to proxy execution, we may migrate a donor task to a runqueue where it can't run due to cpu affinity. Thus, we must be careful to ensure we return-migrate the task back to a cpu in its cpumask when it becomes unblocked. Peter helpfully provided the following example with pictures: "Suppose we have a ww_mutex cycle: ,-+-* Mutex-1 <-. Task-A ---' | | ,-- Task-B `-> Mutex-2 *-+-' Where Task-A holds Mutex-1 and tries to acquire Mutex-2, and where Task-B holds Mutex-2 and tries to acquire Mutex-1. Then the blocked_on->owner chain will go in circles. Task-A -> Mutex-2 ^ | | v Mutex-1 <- Task-B We need two things: - find_proxy_task() to stop iterating the circle; - the woken task to 'unblock' and run, such that it can back-off and re-try the transaction. Now, the current code [without this patch] does: __clear_task_blocked_on(); wake_q_add(); And surely clearing ->blocked_on is sufficient to break the cycle. Suppose it is Task-B that is made to back-off, then we have: Task-A -> Mutex-2 -> Task-B (no further blocked_on) and it would attempt to run Task-B. Or worse, it could directly pick Task-B and run it, without ever getting into find_proxy_task(). Now, here is a problem because Task-B might not be runnable on the CPU it is currently on; and because !task_is_blocked() we don't get into the proxy paths, so nobody is going to fix this up. Ideally we would have dequeued Task-B alongside of clearing ->blocked_on, but alas, [the lock ordering prevents us from getting the task_rq_lock() and] spoils things." Thus we need more than just a binary concept of the task being blocked on a mutex or not. So allow setting blocked_on to PROXY_WAKING as a special value which specifies the task is no longer blocked, but needs to be evaluated for return migration *before* it can be run. This will then be used in a later patch to handle proxy return-migration.
Showing the changes to kernel (3 of 4 files). Show all
Unified · Split
| @@ -983,7 +983,7 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · 982 unmodified lines | ||
| 983 | 983 | next = waiter->task; |
| 984 | 984 | |
| 985 | 985 | debug_mutex_wake_waiter(lock, waiter); |
| 986 | − clear_task_blocked_on(next, lock); | |
| 986 | + set_task_blocked_on_waking(next, lock); | |
| 987 | 987 | wake_q_add(&wake_q, next); |
| 988 | 988 | } |
| 989 | 989 | |
| @@ -285,11 +285,11 @@ __ww_mutex_die(struct MUTEX *lock, struct MUTEX_WAITER *waiter, · 284 unmodified lines | ||
| 285 | 285 | debug_mutex_wake_waiter(lock, waiter); |
| 286 | 286 | #endif |
| 287 | 287 | /* |
| 288 | − * When waking up the task to die, be sure to clear the | |
| 289 | − * blocked_on pointer. Otherwise we can see circular | |
| 290 | − * blocked_on relationships that can't resolve. | |
| 288 | + * When waking up the task to die, be sure to set the | |
| 289 | + * blocked_on to PROXY_WAKING. Otherwise we can see | |
| 290 | + * circular blocked_on relationships that can't resolve. | |
| 291 | 291 | */ |
| 292 | − clear_task_blocked_on(waiter->task, lock); | |
| 292 | + set_task_blocked_on_waking(waiter->task, lock); | |
| 293 | 293 | wake_q_add(wake_q, waiter->task); |
| 294 | 294 | } |
| 295 | 295 | |
| @@ -339,15 +339,15 @@ static bool __ww_mutex_wound(struct MUTEX *lock, · 43 unmodified lines | ||
| 339 | 339 | */ |
| 340 | 340 | if (owner != current) { |
| 341 | 341 | /* |
| 342 | − * When waking up the task to wound, be sure to clear the | |
| 343 | − * blocked_on pointer. Otherwise we can see circular | |
| 344 | − * blocked_on relationships that can't resolve. | |
| 342 | + * When waking up the task to wound, be sure to set the | |
| 343 | + * blocked_on to PROXY_WAKING. Otherwise we can see | |
| 344 | + * circular blocked_on relationships that can't resolve. | |
| 345 | 345 | * |
| 346 | 346 | * NOTE: We pass NULL here instead of lock, because we |
| 347 | 347 | * are waking the mutex owner, who may be currently |
| 348 | 348 | * blocked on a different mutex. |
| 349 | 349 | */ |
| 350 | − clear_task_blocked_on(owner, NULL); | |
| 350 | + set_task_blocked_on_waking(owner, NULL); | |
| 351 | 351 | wake_q_add(wake_q, owner); |
| 352 | 352 | } |
| 353 | 353 | return true; |
| @@ -4239,6 +4239,13 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags) · 4238 unmodified lines | ||
| 4239 | 4239 | ttwu_queue(p, cpu, wake_flags); |
| 4240 | 4240 | } |
| 4241 | 4241 | out: |
| 4242 | + /* | |
| 4243 | + * For now, if we've been woken up, clear the task->blocked_on | |
| 4244 | + * regardless if it was set to a mutex or PROXY_WAKING so the | |
| 4245 | + * task can run. We will need to be more careful later when | |
| 4246 | + * properly handling proxy migration | |
| 4247 | + */ | |
| 4248 | + clear_task_blocked_on(p, NULL); | |
| 4242 | 4249 | if (success) |
| 4243 | 4250 | ttwu_stat(p, task_cpu(p), wake_flags); |
| 4244 | 4251 | |
| @@ -6600,6 +6607,10 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf) · 2355 unmodified lines | ||
| 6600 | 6607 | |
| 6601 | 6608 | /* Follow blocked_on chain. */ |
| 6602 | 6609 | for (p = donor; (mutex = p->blocked_on); p = owner) { |
| 6610 | + /* if its PROXY_WAKING, resched_idle so ttwu can complete */ | |
| 6611 | + if (mutex == PROXY_WAKING) | |
| 6612 | + return proxy_resched_idle(rq); | |
| 6613 | + | |
| 6603 | 6614 | /* |
| 6604 | 6615 | * By taking mutex->wait_lock we hold off concurrent mutex_unlock() |
| 6605 | 6616 | * and ensure @owner sticks around. |
| @@ -6620,6 +6631,11 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf) · 14 unmodified lines | ||
| 6620 | 6631 | |
| 6621 | 6632 | owner = __mutex_owner(mutex); |
| 6622 | 6633 | if (!owner) { |
| 6634 | + /* | |
| 6635 | + * If there is no owner, clear blocked_on | |
| 6636 | + * and return p so it can run and try to | |
| 6637 | + * acquire the lock | |
| 6638 | + */ | |
| 6623 | 6639 | __clear_task_blocked_on(p, mutex); |
| 6624 | 6640 | return p; |
| 6625 | 6641 | } |