From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F7F6C25B06 for ; Sat, 6 Aug 2022 08:03:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 031EE6B0071; Sat, 6 Aug 2022 04:03:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F23696B0072; Sat, 6 Aug 2022 04:03:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E126D8E0001; Sat, 6 Aug 2022 04:03:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id D0F836B0071 for ; Sat, 6 Aug 2022 04:03:13 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id A1107C079A for ; Sat, 6 Aug 2022 08:03:13 +0000 (UTC) X-FDA: 79768427466.24.F62EF2A Received: from r3-25.sinamail.sina.com.cn (r3-25.sinamail.sina.com.cn [202.108.3.25]) by imf01.hostedemail.com (Postfix) with SMTP id DDEF940145 for ; Sat, 6 Aug 2022 08:03:09 +0000 (UTC) Received: from unknown (HELO localhost.localdomain)([114.249.59.48]) by sina.com (172.16.97.23) with ESMTP id 62EE201400011265; Sat, 6 Aug 2022 16:02:30 +0800 (CST) X-Sender: hdanton@sina.com X-Auth-ID: hdanton@sina.com X-SMAIL-MID: 65309654920007 From: Hillf Danton To: Lai Jiangshan Cc: LKML , linux-mm@kvack.org, Petr Mladek , Peter Zijlstra Subject: Re: [RFC PATCH 2/8] workqueue: Make create_worker() safe against prematurely wakeups Date: Sat, 6 Aug 2022 16:02:51 +0800 Message-Id: <20220806080251.1871-1-hdanton@sina.com> In-Reply-To: References: <20220804084135.92425-1-jiangshanlai@gmail.com> <20220804084135.92425-3-jiangshanlai@gmail.com> <20220804123520.1660-1-hdanton@sina.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1659772993; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WmIhJck8AJzDxTnbe4l5ziJhyOp4l8OgXi1+05m3zmo=; b=WinaLMemRWJf6MG+y9xgeC0oLL4/mJgzRIB9QrrQHLthI/BwU4fPY79BrI0b/kyn68m2Y7 18cVYWJNGtSK0lDJIK4lnH+Yqm4kEoXae6xCbkV4ruM7t+O22UpvAwfwETD2L2eknS/gD8 iEXq+HpsMGT+M1wVLjSKzB43l7rKYyo= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of hdanton@sina.com designates 202.108.3.25 as permitted sender) smtp.mailfrom=hdanton@sina.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1659772993; a=rsa-sha256; cv=none; b=Pzu+Uawx/DfjAHXQ8BSQrcPzNMs6jLy5aktBCzUsu/sehgijWHZg366DMVXO37ZdaMJrFE qevnFgNRVxrmvFQp0/Lvpv9VeAIrejCe78sIi3Z9381wH1EWbBifyGpzn9+YNcJyp7yowx wn/sjyhz0tBqvNwx0JPx78yHIGAF5YY= X-Stat-Signature: odsb9n3bfq53bztzybtfuzzqrgazucnn X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: DDEF940145 Authentication-Results: imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of hdanton@sina.com designates 202.108.3.25 as permitted sender) smtp.mailfrom=hdanton@sina.com; dmarc=none X-HE-Tag: 1659772989-599383 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000038, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, 5 Aug 2022 10:30:10 +0800 Lai Jiangshan wrote: > On Thu, Aug 4, 2022 at 8:35 PM Hillf Danton 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. Then want to know the reasons why complete() in combination with wait_for_completion() OTOH fails to work for you without reinit. > > 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?