worker destruction includes these parts of code: adjust pool's stats remove the worker from idle list detach the worker from the pool kthread_stop() to wait for the worker's task exit free the worker struct We can find out that there is no essential work to do after kthread_stop(), which means destroy_worker() doesn't need to wait for the worker's task exit, so we can remove kthread_stop() and free the worker struct in the worker exiting path. However, put_unbound_pool() still needs to sync the all the workers' destruction before destroying the pool; otherwise, the workers may access to the invalid pool when they are exiting. So we also move the code of "detach the worker" to the exiting path and let put_unbound_pool() to sync with this code via detach_completion. The code of "detach the worker" is wrapped in a new function "worker_detach_from_pool()" although worker_detach_from_pool() is only called once (in worker_thread()) after this patch, but we need to wrap it for these reasons: 1) The code of "detach the worker" is not short enough to unfold them in worker_thread(). 2) the name of "worker_detach_from_pool()" is self-comment, and we add some comments above the function. 3) it will be shared by rescuer in later patch which allows rescuer and normal thread use the same attach/detach frameworks. The worker id is freed when detaching which happens before the worker is fully dead, but this id of the dying worker may be re-used for a new worker, so the dying worker's task name is changed to "worker/dying" to avoid two or several workers having the same name. Since "detach the worker" is moved out from destroy_worker(), destroy_worker() doesn't require manager_mutex, so the "lockdep_assert_held(&pool->manager_mutex)" in destroy_worker() is removed, and destroy_worker() is not protected by manager_mutex in put_unbound_pool(). tj: Minor description updates.
Unified · Split
| @@ -163,6 +163,7 @@ struct worker_pool { · 162 unmodified lines | ||
| 163 | 163 | struct mutex manager_arb; /* manager arbitration */ |
| 164 | 164 | struct mutex manager_mutex; /* manager exclusion */ |
| 165 | 165 | struct idr worker_idr; /* M: worker IDs and iteration */ |
| 166 | + struct completion *detach_completion; /* all workers detached */ | |
| 166 | 167 | |
| 167 | 168 | struct workqueue_attrs *attrs; /* I: worker attributes */ |
| 168 | 169 | struct hlist_node hash_node; /* PL: unbound_pool_hash node */ |
| @@ -1687,6 +1688,30 @@ static struct worker *alloc_worker(void) · 1518 unmodified lines | ||
| 1687 | 1688 | return worker; |
| 1688 | 1689 | } |
| 1689 | 1690 | |
| 1691 | +/** | |
| 1692 | + * worker_detach_from_pool() - detach a worker from its pool | |
| 1693 | + * @worker: worker which is attached to its pool | |
| 1694 | + * @pool: the pool @worker is attached to | |
| 1695 | + * | |
| 1696 | + * Undo the attaching which had been done in create_worker(). The caller | |
| 1697 | + * worker shouldn't access to the pool after detached except it has other | |
| 1698 | + * reference to the pool. | |
| 1699 | + */ | |
| 1700 | +static void worker_detach_from_pool(struct worker *worker, | |
| 1701 | + struct worker_pool *pool) | |
| 1702 | +{ | |
| 1703 | + struct completion *detach_completion = NULL; | |
| 1704 | + | |
| 1705 | + mutex_lock(&pool->manager_mutex); | |
| 1706 | + idr_remove(&pool->worker_idr, worker->id); | |
| 1707 | + if (idr_is_empty(&pool->worker_idr)) | |
| 1708 | + detach_completion = pool->detach_completion; | |
| 1709 | + mutex_unlock(&pool->manager_mutex); | |
| 1710 | + | |
| 1711 | + if (detach_completion) | |
| 1712 | + complete(detach_completion); | |
| 1713 | +} | |
| 1714 | + | |
| 1690 | 1715 | /** |
| 1691 | 1716 | * create_worker - create a new workqueue worker |
| 1692 | 1717 | * @pool: pool the new worker will belong to |
| @@ -1815,13 +1840,12 @@ static int create_and_start_worker(struct worker_pool *pool) · 122 unmodified lines | ||
| 1815 | 1840 | * be idle. |
| 1816 | 1841 | * |
| 1817 | 1842 | * CONTEXT: |
| 1818 | − * spin_lock_irq(pool->lock) which is released and regrabbed. | |
| 1843 | + * spin_lock_irq(pool->lock). | |
| 1819 | 1844 | */ |
| 1820 | 1845 | static void destroy_worker(struct worker *worker) |
| 1821 | 1846 | { |
| 1822 | 1847 | struct worker_pool *pool = worker->pool; |
| 1823 | 1848 | |
| 1824 | − lockdep_assert_held(&pool->manager_mutex); | |
| 1825 | 1849 | lockdep_assert_held(&pool->lock); |
| 1826 | 1850 | |
| 1827 | 1851 | /* sanity check frenzy */ |
| @@ -1833,24 +1857,9 @@ static void destroy_worker(struct worker *worker) · 5 unmodified lines | ||
| 1833 | 1857 | pool->nr_workers--; |
| 1834 | 1858 | pool->nr_idle--; |
| 1835 | 1859 | |
| 1836 | − /* | |
| 1837 | − * Once WORKER_DIE is set, the kworker may destroy itself at any | |
| 1838 | − * point. Pin to ensure the task stays until we're done with it. | |
| 1839 | − */ | |
| 1840 | − get_task_struct(worker->task); | |
| 1841 | − | |
| 1842 | 1860 | list_del_init(&worker->entry); |
| 1843 | 1861 | worker->flags |= WORKER_DIE; |
| 1844 | − | |
| 1845 | − idr_remove(&pool->worker_idr, worker->id); | |
| 1846 | − | |
| 1847 | − spin_unlock_irq(&pool->lock); | |
| 1848 | − | |
| 1849 | − kthread_stop(worker->task); | |
| 1850 | − put_task_struct(worker->task); | |
| 1851 | − kfree(worker); | |
| 1852 | − | |
| 1853 | − spin_lock_irq(&pool->lock); | |
| 1862 | + wake_up_process(worker->task); | |
| 1854 | 1863 | } |
| 1855 | 1864 | |
| 1856 | 1865 | static void idle_worker_timeout(unsigned long __pool) |
| @@ -2289,6 +2298,10 @@ woke_up: · 432 unmodified lines | ||
| 2289 | 2298 | spin_unlock_irq(&pool->lock); |
| 2290 | 2299 | WARN_ON_ONCE(!list_empty(&worker->entry)); |
| 2291 | 2300 | worker->task->flags &= ~PF_WQ_WORKER; |
| 2301 | + | |
| 2302 | + set_task_comm(worker->task, "kworker/dying"); | |
| 2303 | + worker_detach_from_pool(worker, pool); | |
| 2304 | + kfree(worker); | |
| 2292 | 2305 | return 0; |
| 2293 | 2306 | } |
| 2294 | 2307 | |
| @@ -3560,6 +3573,7 @@ static void rcu_free_pool(struct rcu_head *rcu) · 1265 unmodified lines | ||
| 3560 | 3573 | */ |
| 3561 | 3574 | static void put_unbound_pool(struct worker_pool *pool) |
| 3562 | 3575 | { |
| 3576 | + DECLARE_COMPLETION_ONSTACK(detach_completion); | |
| 3563 | 3577 | struct worker *worker; |
| 3564 | 3578 | |
| 3565 | 3579 | lockdep_assert_held(&wq_pool_mutex); |
| @@ -3583,15 +3597,21 @@ static void put_unbound_pool(struct worker_pool *pool) · 17 unmodified lines | ||
| 3583 | 3597 | * manager_mutex. |
| 3584 | 3598 | */ |
| 3585 | 3599 | mutex_lock(&pool->manager_arb); |
| 3586 | − mutex_lock(&pool->manager_mutex); | |
| 3587 | − spin_lock_irq(&pool->lock); | |
| 3588 | 3600 | |
| 3601 | + spin_lock_irq(&pool->lock); | |
| 3589 | 3602 | while ((worker = first_worker(pool))) |
| 3590 | 3603 | destroy_worker(worker); |
| 3591 | 3604 | WARN_ON(pool->nr_workers || pool->nr_idle); |
| 3592 | − | |
| 3593 | 3605 | spin_unlock_irq(&pool->lock); |
| 3606 | + | |
| 3607 | + mutex_lock(&pool->manager_mutex); | |
| 3608 | + if (!idr_is_empty(&pool->worker_idr)) | |
| 3609 | + pool->detach_completion = &detach_completion; | |
| 3594 | 3610 | mutex_unlock(&pool->manager_mutex); |
| 3611 | + | |
| 3612 | + if (pool->detach_completion) | |
| 3613 | + wait_for_completion(pool->detach_completion); | |
| 3614 | + | |
| 3595 | 3615 | mutex_unlock(&pool->manager_arb); |
| 3596 | 3616 | |
| 3597 | 3617 | /* shut down the timers */ |