From: Lai Jiangshan <[email protected]>
worker_thread() always does the recheck after getting the manager role,
so the recheck in the maybe_create_worker() is unneeded and is removed.
A piece of comment for maybe_create_worker() is removed because it is
not true anymore after the recheck is removed.
A piece of comment for manage_workers() is removed because there is
already another piece of comment explaining the same thing which is
more accurate.
Signed-off-by: Lai Jiangshan <[email protected]>
---
kernel/workqueue.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 64dc1833d11a..0d9844b81482 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2091,9 +2091,6 @@ static void pool_mayday_timeout(struct timer_list *t)
* sent to all rescuers with works scheduled on @pool to resolve
* possible allocation deadlock.
*
- * On return, need_to_create_worker() is guaranteed to be %false and
- * may_start_working() %true.
- *
* LOCKING:
* raw_spin_lock_irq(pool->lock) which may be released and regrabbed
* multiple times. Does GFP_KERNEL allocations. Called only from
@@ -2103,7 +2100,6 @@ static void maybe_create_worker(struct worker_pool *pool)
__releases(&pool->lock)
__acquires(&pool->lock)
{
-restart:
raw_spin_unlock_irq(&pool->lock);
/* if we don't make progress in MAYDAY_INITIAL_TIMEOUT, call for help */
@@ -2121,13 +2117,6 @@ __acquires(&pool->lock)
del_timer_sync(&pool->mayday_timer);
raw_spin_lock_irq(&pool->lock);
- /*
- * This is necessary even after a new worker was just successfully
- * created as @pool->lock was dropped and the new worker might have
- * already become busy.
- */
- if (need_to_create_worker(pool))
- goto restart;
}
/**
@@ -2138,10 +2127,6 @@ __acquires(&pool->lock)
* to. At any given time, there can be only zero or one manager per
* pool. The exclusion is handled automatically by this function.
*
- * The caller can safely start processing works on false return. On
- * true return, it's guaranteed that need_to_create_worker() is false
- * and may_start_working() is true.
- *
* CONTEXT:
* raw_spin_lock_irq(pool->lock) which may be released and regrabbed
* multiple times. Does GFP_KERNEL allocations.
--
2.19.1.6.gb485710b
On Thu, Aug 04, 2022 at 04:41:34PM +0800, Lai Jiangshan wrote:
> worker_thread() always does the recheck after getting the manager role,
> so the recheck in the maybe_create_worker() is unneeded and is removed.
So, before if multiple workers need to be created, a single manager would
create them all. After, we'd end up daisy chaining, right? One manager
creates one worker and goes to process one work item. The new worker wakes
up and becomes the manager and creates another worker and so on. That
doesn't seem like a desirable behavior.
Thanks.
--
tejun
On Wed, Aug 17, 2022 at 6:08 AM Tejun Heo <[email protected]> wrote:
>
> On Thu, Aug 04, 2022 at 04:41:34PM +0800, Lai Jiangshan wrote:
> > worker_thread() always does the recheck after getting the manager role,
> > so the recheck in the maybe_create_worker() is unneeded and is removed.
>
> So, before if multiple workers need to be created, a single manager would
> create them all. After, we'd end up daisy chaining, right? One manager
> creates one worker and goes to process one work item. The new worker wakes
> up and becomes the manager and creates another worker and so on. That
> doesn't seem like a desirable behavior.
>
The recheck is always in the same pool lock critical section, so the
behavior isn't changed before/after this patch.
On Thu, Aug 18, 2022 at 10:44:02PM +0800, Lai Jiangshan wrote:
> On Wed, Aug 17, 2022 at 6:08 AM Tejun Heo <[email protected]> wrote:
> >
> > On Thu, Aug 04, 2022 at 04:41:34PM +0800, Lai Jiangshan wrote:
> > > worker_thread() always does the recheck after getting the manager role,
> > > so the recheck in the maybe_create_worker() is unneeded and is removed.
> >
> > So, before if multiple workers need to be created, a single manager would
> > create them all. After, we'd end up daisy chaining, right? One manager
> > creates one worker and goes to process one work item. The new worker wakes
> > up and becomes the manager and creates another worker and so on. That
> > doesn't seem like a desirable behavior.
> >
>
> The recheck is always in the same pool lock critical section, so the
> behavior isn't changed before/after this patch.
Ah, right you are.
Thanks.
--
tejun