i On Thu, Aug 4, 2022 at 8:35 PM Hillf Danton <hdanton@xxxxxxxx> 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?
