lkml.org 
[lkml]   [2022]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC PATCH 2/8] workqueue: Make create_worker() safe against prematurely wakeups
 i

On Thu, Aug 4, 2022 at 8:35 PM Hillf Danton <hdanton@sina.com> wrote:
>
> On Thu, 4 Aug 2022 16:41:29 +0800 Lai Jiangshan wrote:
> >
> > @@ -1942,6 +1943,7 @@ static struct worker *create_worker(struct worker_pool *pool)
> > goto fail;
> >
> > worker->id = id;
> > + worker->pool = pool;
> >
> > if (pool->cpu >= 0)
> > snprintf(id_buf, sizeof(id_buf), "%d:%d%s", pool->cpu, id,
> > @@ -1949,6 +1951,7 @@ static struct worker *create_worker(struct worker_pool *pool)
> > else
> > snprintf(id_buf, sizeof(id_buf), "u%d:%d", pool->id, id);
> >
> > + reinit_completion(&pool->created);
> > worker->task = kthread_create_on_node(worker_thread, worker, pool->node,
> > "kworker/%s", id_buf);
> > if (IS_ERR(worker->task))
> > @@ -1957,15 +1960,9 @@ static struct worker *create_worker(struct worker_pool *pool)
> > set_user_nice(worker->task, pool->attrs->nice);
> > kthread_bind_mask(worker->task, pool->attrs->cpumask);
> >
> > - /* successful, attach the worker to the pool */
> > - worker_attach_to_pool(worker, pool);
> > -
> > /* start the newly created worker */
> > - raw_spin_lock_irq(&pool->lock);
> > - worker->pool->nr_workers++;
> > - worker_enter_idle(worker);
> > wake_up_process(worker->task);
> > - raw_spin_unlock_irq(&pool->lock);
> > + wait_for_completion(&pool->created);
> >
> > return worker;
>
> cpu0 cpu1 cpu2
> === === ===
> complete
>
> reinit_completion
> wait_for_completion

reinit_completion() and wait_for_completion() are both in
create_worker(). create_worker() itself is mutually exclusive
which means no two create_worker()s running at the same time
for the same pool.

No work item can be added before the first initial create_worker()
returns for a new or first-online per-cpu pool, so there would be no
manager for the pool during the first initial create_worker().

The manager is the only worker who can call create_worker() for a pool
except the first initial create_worker().

And there is always only one manager after the first initial
create_worker().

The document style in some of workqueue code is:
"/* locking rule: what it is */"

For example:
struct list_head worklist; /* L: list of pending works */
which means it is protected by pool->lock.

And for
struct completion created; /* create_worker(): worker created */
it means it is protected by the exclusive create_worker().

>
> Any chance for race above?

\
 
 \ /
  Last update: 2022-08-05 04:31    [W:0.128 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site