Skip to content

Commit 967b494

Browse files
committed
workqueue: Use a kthread_worker to release pool_workqueues
pool_workqueue release path is currently bounced to system_wq; however, this is a bit tricky because this bouncing occurs while holding a pool lock and thus has risk of causing a A-A deadlock. This is currently addressed by the fact that only unbound workqueues use this bouncing path and system_wq is a per-cpu workqueue. While this works, it's brittle and requires a work-around like setting the lockdep subclass for the lock of unbound pools. Besides, future changes will use the bouncing path for per-cpu workqueues too making the current approach unusable. Let's just use a dedicated kthread_worker to untangle the dependency. This is just one more kthread for all workqueues and makes the pwq release logic simpler and more robust. Signed-off-by: Tejun Heo <[email protected]>
1 parent fcecfa8 commit 967b494

File tree

1 file changed

+23
-17
lines changed

1 file changed

+23
-17
lines changed

kernel/workqueue.c

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -257,12 +257,12 @@ struct pool_workqueue {
257257
u64 stats[PWQ_NR_STATS];
258258

259259
/*
260-
* Release of unbound pwq is punted to system_wq. See put_pwq()
261-
* and pwq_unbound_release_workfn() for details. pool_workqueue
262-
* itself is also RCU protected so that the first pwq can be
263-
* determined without grabbing wq->mutex.
260+
* Release of unbound pwq is punted to a kthread_worker. See put_pwq()
261+
* and pwq_unbound_release_workfn() for details. pool_workqueue itself
262+
* is also RCU protected so that the first pwq can be determined without
263+
* grabbing wq->mutex.
264264
*/
265-
struct work_struct unbound_release_work;
265+
struct kthread_work unbound_release_work;
266266
struct rcu_head rcu;
267267
} __aligned(1 << WORK_STRUCT_FLAG_BITS);
268268

@@ -395,6 +395,13 @@ static struct workqueue_attrs *unbound_std_wq_attrs[NR_STD_WORKER_POOLS];
395395
/* I: attributes used when instantiating ordered pools on demand */
396396
static struct workqueue_attrs *ordered_wq_attrs[NR_STD_WORKER_POOLS];
397397

398+
/*
399+
* I: kthread_worker to release pwq's. pwq release needs to be bounced to a
400+
* process context while holding a pool lock. Bounce to a dedicated kthread
401+
* worker to avoid A-A deadlocks.
402+
*/
403+
static struct kthread_worker *pwq_release_worker;
404+
398405
struct workqueue_struct *system_wq __read_mostly;
399406
EXPORT_SYMBOL(system_wq);
400407
struct workqueue_struct *system_highpri_wq __read_mostly;
@@ -1366,14 +1373,10 @@ static void put_pwq(struct pool_workqueue *pwq)
13661373
if (WARN_ON_ONCE(!(pwq->wq->flags & WQ_UNBOUND)))
13671374
return;
13681375
/*
1369-
* @pwq can't be released under pool->lock, bounce to
1370-
* pwq_unbound_release_workfn(). This never recurses on the same
1371-
* pool->lock as this path is taken only for unbound workqueues and
1372-
* the release work item is scheduled on a per-cpu workqueue. To
1373-
* avoid lockdep warning, unbound pool->locks are given lockdep
1374-
* subclass of 1 in get_unbound_pool().
1376+
* @pwq can't be released under pool->lock, bounce to a dedicated
1377+
* kthread_worker to avoid A-A deadlocks.
13751378
*/
1376-
schedule_work(&pwq->unbound_release_work);
1379+
kthread_queue_work(pwq_release_worker, &pwq->unbound_release_work);
13771380
}
13781381

13791382
/**
@@ -3965,7 +3968,6 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
39653968
if (!pool || init_worker_pool(pool) < 0)
39663969
goto fail;
39673970

3968-
lockdep_set_subclass(&pool->lock, 1); /* see put_pwq() */
39693971
copy_workqueue_attrs(pool->attrs, attrs);
39703972
pool->node = target_node;
39713973

@@ -3999,10 +4001,10 @@ static void rcu_free_pwq(struct rcu_head *rcu)
39994001
}
40004002

40014003
/*
4002-
* Scheduled on system_wq by put_pwq() when an unbound pwq hits zero refcnt
4003-
* and needs to be destroyed.
4004+
* Scheduled on pwq_release_worker by put_pwq() when an unbound pwq hits zero
4005+
* refcnt and needs to be destroyed.
40044006
*/
4005-
static void pwq_unbound_release_workfn(struct work_struct *work)
4007+
static void pwq_unbound_release_workfn(struct kthread_work *work)
40064008
{
40074009
struct pool_workqueue *pwq = container_of(work, struct pool_workqueue,
40084010
unbound_release_work);
@@ -4110,7 +4112,8 @@ static void init_pwq(struct pool_workqueue *pwq, struct workqueue_struct *wq,
41104112
INIT_LIST_HEAD(&pwq->inactive_works);
41114113
INIT_LIST_HEAD(&pwq->pwqs_node);
41124114
INIT_LIST_HEAD(&pwq->mayday_node);
4113-
INIT_WORK(&pwq->unbound_release_work, pwq_unbound_release_workfn);
4115+
kthread_init_work(&pwq->unbound_release_work,
4116+
pwq_unbound_release_workfn);
41144117
}
41154118

41164119
/* sync @pwq with the current state of its associated wq and link it */
@@ -6433,6 +6436,9 @@ static void __init wq_cpu_intensive_thresh_init(void)
64336436
if (wq_cpu_intensive_thresh_us != ULONG_MAX)
64346437
return;
64356438

6439+
pwq_release_worker = kthread_create_worker(0, "pool_workqueue_release");
6440+
BUG_ON(IS_ERR(pwq_release_worker));
6441+
64366442
/*
64376443
* The default of 10ms is derived from the fact that most modern (as of
64386444
* 2023) processors can do a lot in 10ms and that it's just below what

0 commit comments

Comments
 (0)