Message ID | tencent_49AFDBA31F885906234219591097D42ABE08@qq.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2010:b0:403:3b70:6f57 with SMTP id fe16csp1106016vqb; Fri, 20 Oct 2023 07:44:02 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHNCQ2+4j+EyZFo4i4kThbdr1k1YZOKyMgT+KyCS+kXEabjZLKyk57kQvod4Le3wpp+2Coo X-Received: by 2002:a05:6358:7209:b0:166:e6da:5441 with SMTP id h9-20020a056358720900b00166e6da5441mr2070812rwa.6.1697813041818; Fri, 20 Oct 2023 07:44:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697813041; cv=none; d=google.com; s=arc-20160816; b=HVLC+gCmcYp3LufTL4X44XZJw4DwxiVacZvJKu09UlvJptan5B1nuDbY3Vq/2eaylE srgkzIKqoJJy0XHRRsrnfEGaKsMx1tcP+4w4d7ZYzi2w80zqUJracqpdkK73b2ilK1bj mSrvQy0uY37FXrjOEtinVkKpAb6eWJzYVcvmuknBXa26HfJld/S6iPNWwbd2/vD0k/4k RmlX+3sT4D+ilBu0vaxdmJU93BJUR3Kx4aO4CZC2gxT/7oh0cgXNoNJ4qFZYGhEaZiwv AT9MBvmKaPyC1Z34F8/7++yAowa/TgrTGWLEkmOUARTk+cgfA1m6fZuWT+CGpVtac5b6 yw9w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:date:subject:to:from:message-id :dkim-signature; bh=K83F6SskCJLMqeCBk7w6O0CFlOxHTQk9Oh1VXwnu6bQ=; fh=PGaP26rljTYcjLituiXWjvRiYsB22YGWGH+YkC/057Y=; b=KWuqMtU9artoZqGUCz+7cR/9WfnWDIf3t+5a/i8tZckeDeWb//psT+yzVI8PSky5lW kIYvTfwylYYmnrb994HhmS0Tpp0vGNV3yYi5VqHOYLSe3GU0W/Baun/q9d0XxLGxLJYJ MQ5F2+plgPOK56vfEXoDY/WRmdZA05GFMA7ysrrEXtbRK3FviOtIyqrQe0SRi4KRtyNL LPsjxUyBv5JWAXhOe2ZjiZ4LPS15yke/dJU17gBM2NaPJrP1zn2wz+nzibw00lAIjYdT 0Zvi+gxRMaC8sK9RtLO32xbGXyM/EMgIdkm13eWqKVYdo7jCNU4/jL9v5Xe09ksshv1P NhAg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@foxmail.com header.s=s201512 header.b=ZUHL2clx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=foxmail.com Received: from howler.vger.email (howler.vger.email. [2620:137:e000::3:4]) by mx.google.com with ESMTPS id s194-20020a632ccb000000b005b86140eabfsi1949996pgs.186.2023.10.20.07.44.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Oct 2023 07:44:01 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4; Authentication-Results: mx.google.com; dkim=pass header.i=@foxmail.com header.s=s201512 header.b=ZUHL2clx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=foxmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id 68F798369F5B; Fri, 20 Oct 2023 07:43:59 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377537AbjJTOns (ORCPT <rfc822;a1648639935@gmail.com> + 26 others); Fri, 20 Oct 2023 10:43:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36746 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1377429AbjJTOnr (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 20 Oct 2023 10:43:47 -0400 X-Greylist: delayed 138733 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Fri, 20 Oct 2023 07:43:43 PDT Received: from out203-205-251-53.mail.qq.com (out203-205-251-53.mail.qq.com [203.205.251.53]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D0187106 for <linux-kernel@vger.kernel.org>; Fri, 20 Oct 2023 07:43:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foxmail.com; s=s201512; t=1697813021; bh=K83F6SskCJLMqeCBk7w6O0CFlOxHTQk9Oh1VXwnu6bQ=; h=From:To:Subject:Date:In-Reply-To:References; b=ZUHL2clxRG+MPwnLwAlGNm+vCWDYPoYi4XesKmvDoPxKz54FcJMi+v6U9eIKLxNSx RLkYZUY0okAmgsMYSgb+F9G0id4C7Bc8p93LblAEicfiM18hDd2ZhRQK5hLvFCc+NV k4WuQmFX8eph8hbb6wSQ2bnnprVu24cDfHKRFKbo= Received: from rtoax.lan ([120.245.114.36]) by newxmesmtplogicsvrszb6-0.qq.com (NewEsmtp) with SMTP id AE3AEEEC; Fri, 20 Oct 2023 22:43:35 +0800 X-QQ-mid: xmsmtpt1697813017tr3admgz9 Message-ID: <tencent_49AFDBA31F885906234219591097D42ABE08@qq.com> X-QQ-XMAILINFO: MmpliBmRb3iCu0TFj/+Ks7ktWUvfayv2uurkgvqnyHovMHS0AUi4113gtu1fIz odH7cfi7qgLWd9vSrZh4Xyvw3YXJrb+zou0eOojNscTk6JxHjBDllg070sKwdX0AcrOg+Lu/vF8j 1bx60T6nDJvETAde7g/zs7V9dcDelygJZQwIIu+eXb1Z0KDHUbh4f8avtWiGp3ZIChMU+dyCMzP0 M47McCn4SQURXb+xPDUqkusXnqGx/K9sO5Jq9bVJjv+ppiKia5yMrHGVF2PjZvnJZwjI7052a+Gn PQU94362WzsBO7geGKIlD4tS8eNtjjDV5ZfR8vlofuIGR+b0/kIBaW4T9aCE5Z1OE90IlxmIBOMD L5Z9aylWK9GY1+YNrsVHIoxQIhYXdXCnvSsxevnEh+9vanX1QwVpgbeQREo+ph8toilGhQQBlpB7 Sxm9GVZvC3PXRJRF/ISCG/8dAcrxvMKyBdD3H8dZcQkL0Rrgt6I89sMcBeMJo1NvngZaVEYkUGjf 2sluDofD8ibXe6IRv82F4JAyxh5MBdbjncy3wnltrEWGB/XfsrLe4qg22vqHU7UVYds46nsMqB8w HS8Zt7SzQC1QPFcZJ8cLxC05kW8in7VQZK+wofBsm82mEftCM8IFYgFVt4epzfHLJKyaRDntFgkY 1I8b3730I5aaXdKomPdOW/EAFZHDzGIJoNYqBrWuIncD9PVYbfEjZsBpeRRfksfzeGjt/jVcbRVn k72To2fWg6EHQkshHPBJAQ/vSVzaB7Yd7YoxPGFz8Pxql6X2HqFccMGI6XItw3HVsLdmTJxpApY5 M8tpiqXnBXM/PMZWUUEibiWaSYmVOdRU5Y7a+C+wP29r4NUP4zkiSOOMCe8mgHNb5oXx2g2LPO+/ TfrL2Rb0+y3cxAlhUnBFV2BcVYHTLY8HKoI9S/XBS0zX8Lrx13IXVcdznk/5tVMEwvRwizEldvjw cdzJdk2MhK7Tpzh1TXzOV5CHMMyia4SJZh6LQIXTgDGh7KOf6uYYUzO8YRBT5UVpbUUCwVKFyYS2 GDMRAkJw== X-QQ-XMRINFO: OWPUhxQsoeAVDbp3OJHYyFg= From: Rong Tao <rtoax@foxmail.com> To: mark.rutland@arm.com, elver@google.com, linux-kernel@vger.kernel.org, peterz@infradead.org, rongtao@cestc.cn, rtoax@foxmail.com, tglx@linutronix.de Subject: [PATCH 1/2] stop_machine: Use non-atomic read multi_stop_data::state clearly Date: Fri, 20 Oct 2023 22:43:33 +0800 X-OQ-MSGID: <ec679f9a9877849ee649da1b355f995e71d42737.1697811778.git.rongtao@cestc.cn> X-Mailer: git-send-email 2.41.0 In-Reply-To: <cover.1697811778.git.rongtao@cestc.cn> References: <cover.1697811778.git.rongtao@cestc.cn> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Fri, 20 Oct 2023 07:43:59 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780286008166385158 X-GMAIL-MSGID: 1780286008166385158 |
Series |
[1/2] stop_machine: Use non-atomic read multi_stop_data::state clearly
|
|
Commit Message
Rong Tao
Oct. 20, 2023, 2:43 p.m. UTC
From: Rong Tao <rongtao@cestc.cn> commit b1fc58333575 ("stop_machine: Avoid potential race behaviour") solved the race behaviour problem, to better show that race behaviour does not exist, pass the 'curstate' directly to ack_state() instead of refetching msdata->state in ack_state(). Signed-off-by: Rong Tao <rongtao@cestc.cn> --- kernel/stop_machine.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
Comments
On Fri, Oct 20, 2023 at 10:43:33PM +0800, Rong Tao wrote: > From: Rong Tao <rongtao@cestc.cn> > > commit b1fc58333575 ("stop_machine: Avoid potential race behaviour") > solved the race behaviour problem, to better show that race behaviour > does not exist, pass the 'curstate' directly to ack_state() instead of > refetching msdata->state in ack_state(). > I'd prefer if we make this: | stop_machine: pass curstate to ack_state() | | The multi_cpu_stop() state machine uses multi_stop_data::state to hold | the current state, and this is read and written atomically except in | ack_state(), which performs a non-atomic read. | | As ack_state() only performs this non-atomic read when there is a single | writer, this is benign, but it makes reasoning about the state machine a | little harder. | | Remove the non-atomic read and pass the (atomically read) curstate in | instead. This makes it clear that we do not expect any racy writes, and | avoids a redundant load. With that wording: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > Signed-off-by: Rong Tao <rongtao@cestc.cn> > --- > kernel/stop_machine.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c > index cedb17ba158a..268c2e581698 100644 > --- a/kernel/stop_machine.c > +++ b/kernel/stop_machine.c > @@ -188,10 +188,11 @@ static void set_state(struct multi_stop_data *msdata, > } > > /* Last one to ack a state moves to the next state. */ > -static void ack_state(struct multi_stop_data *msdata) > +static void ack_state(struct multi_stop_data *msdata, > + enum multi_stop_state curstate) > { > if (atomic_dec_and_test(&msdata->thread_ack)) > - set_state(msdata, msdata->state + 1); > + set_state(msdata, curstate + 1); > } > > notrace void __weak stop_machine_yield(const struct cpumask *cpumask) > @@ -242,7 +243,7 @@ static int multi_cpu_stop(void *data) > default: > break; > } > - ack_state(msdata); > + ack_state(msdata, curstate); > } else if (curstate > MULTI_STOP_PREPARE) { > /* > * At this stage all other CPUs we depend on must spin > -- > 2.41.0 >
Thanks for your advice, Mark. Your commit information is clearer and easier to understand, I will use it in the next patch version, thank you. Rong Tao
On 10/24/23 6:46 PM, Mark Rutland wrote: > On Fri, Oct 20, 2023 at 10:43:33PM +0800, Rong Tao wrote: >> From: Rong Tao <rongtao@cestc.cn> >> >> commit b1fc58333575 ("stop_machine: Avoid potential race behaviour") >> solved the race behaviour problem, to better show that race behaviour >> does not exist, pass the 'curstate' directly to ack_state() instead of >> refetching msdata->state in ack_state(). >> > I'd prefer if we make this: > > | stop_machine: pass curstate to ack_state() > | > | The multi_cpu_stop() state machine uses multi_stop_data::state to hold > | the current state, and this is read and written atomically except in > | ack_state(), which performs a non-atomic read. > | > | As ack_state() only performs this non-atomic read when there is a single > | writer, this is benign, but it makes reasoning about the state machine a > | little harder. > | > | Remove the non-atomic read and pass the (atomically read) curstate in > | instead. This makes it clear that we do not expect any racy writes, and > | avoids a redundant load. > > With that wording: > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > Mark. Hi, Mark, I just submit a single patch [0] individually, not as a patchset. please review. thank you. Rong Tao [0] https://lore.kernel.org/lkml/tencent_FB1D31CEC045E837ABE5B25CC5E37575F405@qq.com/ > >> Signed-off-by: Rong Tao <rongtao@cestc.cn> >> --- >> kernel/stop_machine.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c >> index cedb17ba158a..268c2e581698 100644 >> --- a/kernel/stop_machine.c >> +++ b/kernel/stop_machine.c >> @@ -188,10 +188,11 @@ static void set_state(struct multi_stop_data *msdata, >> } >> >> /* Last one to ack a state moves to the next state. */ >> -static void ack_state(struct multi_stop_data *msdata) >> +static void ack_state(struct multi_stop_data *msdata, >> + enum multi_stop_state curstate) >> { >> if (atomic_dec_and_test(&msdata->thread_ack)) >> - set_state(msdata, msdata->state + 1); >> + set_state(msdata, curstate + 1); >> } >> >> notrace void __weak stop_machine_yield(const struct cpumask *cpumask) >> @@ -242,7 +243,7 @@ static int multi_cpu_stop(void *data) >> default: >> break; >> } >> - ack_state(msdata); >> + ack_state(msdata, curstate); >> } else if (curstate > MULTI_STOP_PREPARE) { >> /* >> * At this stage all other CPUs we depend on must spin >> -- >> 2.41.0 >>
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c index cedb17ba158a..268c2e581698 100644 --- a/kernel/stop_machine.c +++ b/kernel/stop_machine.c @@ -188,10 +188,11 @@ static void set_state(struct multi_stop_data *msdata, } /* Last one to ack a state moves to the next state. */ -static void ack_state(struct multi_stop_data *msdata) +static void ack_state(struct multi_stop_data *msdata, + enum multi_stop_state curstate) { if (atomic_dec_and_test(&msdata->thread_ack)) - set_state(msdata, msdata->state + 1); + set_state(msdata, curstate + 1); } notrace void __weak stop_machine_yield(const struct cpumask *cpumask) @@ -242,7 +243,7 @@ static int multi_cpu_stop(void *data) default: break; } - ack_state(msdata); + ack_state(msdata, curstate); } else if (curstate > MULTI_STOP_PREPARE) { /* * At this stage all other CPUs we depend on must spin