Message ID | 20231024214625.6483-5-frederic@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce89:0:b0:403:3b70:6f57 with SMTP id p9csp2220123vqx; Tue, 24 Oct 2023 14:47:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEB9ZTME3Wk8AVUHciEgNVt2xvU7PBwVYWzWb/zLCm+e9fWaV1SHpSwMEUCGg4MciYhK2d3 X-Received: by 2002:a17:90a:e409:b0:27d:1126:1ff6 with SMTP id hv9-20020a17090ae40900b0027d11261ff6mr23520518pjb.8.1698184035781; Tue, 24 Oct 2023 14:47:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698184035; cv=none; d=google.com; s=arc-20160816; b=r+/rGyQEhYWQ+iwolIpqfVYT845C7Ro1qnH+ABiHJ08p5RP8I26g1Smrwm8agMkmWL gx2z4u2cGLJe8hqaQ3uy9WbvO2Pc2DBnNreuASCrz62wpOBrgKlGjJgW1TepbUKi+tpM +pk9GZLdWwkmZC8nLBaEO7IuqWXrjGPq3rcPDKMvBjspV18M34/7rRwnyiicmWnmA9ab JN+Q7LlMrFw0nRyizwTDilu8j5nEkh5vxRb/YVOtvnxwE8N2SIBay0adGKTc3F2kugo5 hCmgYV8SblKKBhSF+ASC0hkGv5f8A96dCpZ+xyl9M4kMalrtTDR6mGR3iF/mwo0tuYVU M+Kg== 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:message-id:date:subject:cc:to:from :dkim-signature; bh=MYiFxL/NR4y9QBhU4XPHSky0NxYDbU+lNetAadSXM1g=; fh=vRLsh+mlJ/3erNsj9qJLuZWu4oX6irbICpSWFnY9W1o=; b=vHN2LNc0FJ0pG84SZGE8SNa1p6aEn310aZr160wUa7QEOg9APLeloRPyRKhR8kuGkz o0T1Nobiq3z0r1fNfuN/Zib2K2ejiGNTqyGySl5gNw+XsB/Yoz1IewUG8NKfWlpEgudW e6Wu0YWDSxKbvmbi+fGs4xjOPRZEfb07Zfce7OxaP4697fq5rESOChJYi0R+OT1dz/DW QqVqXPV1/Z1ISUsPo1fS7+nv9m4eoYCiH06zPjIEqZgkw3xjxZfpTdEZBV9og+iIx82X pTJ8AatcGp1UQ//cwbDuzkmSUEEfmILpPwCwQuMfTcy+RumHA35CD8DySWqn319qJabv CC7Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=EDaO3ZmC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id b1-20020a17090a9bc100b0027491203b43si11070689pjw.189.2023.10.24.14.47.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Oct 2023 14:47:15 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=EDaO3ZmC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 957F380220A6; Tue, 24 Oct 2023 14:47:13 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344499AbjJXVq5 (ORCPT <rfc822;aposhian.dev@gmail.com> + 27 others); Tue, 24 Oct 2023 17:46:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344477AbjJXVqw (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 24 Oct 2023 17:46:52 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C101AE8; Tue, 24 Oct 2023 14:46:49 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EE75DC433C7; Tue, 24 Oct 2023 21:46:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1698184009; bh=AqHnLeB5xem2kiRsFLowge6w/ot0AAmqtV9xGzicirc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EDaO3ZmCJO3fKoF+gIfWqQroBqbgxx5f8hX2+JrCOc4JsdJNI38tcDbzDbxDd9IiD lQQjafqLkkawzo6WRYBYq6iBDRLc6rw/K82bL4QufhCMuuMtuiIDHLj1HTZLkWwhv0 Y7mPAxMD5NRzchUhYgtYdpyVNGyV6qMUsXDOOPc0ShJDQv6YARd0YrIOFvg4uFaCm6 1ngaB9X+/AarJdFS3TRUPmWau++XTxbJE6uZMiH0YYWipraB8z3fgsqMiLJkFXYbza ZaAmHAP183sCvV3XzoyR81WsOfdotWNwXasO1UWQeKF/ElKrOC4OzjJFDnqlnpa/LJ dKFkktu+IKyVw== From: Frederic Weisbecker <frederic@kernel.org> To: LKML <linux-kernel@vger.kernel.org> Cc: Frederic Weisbecker <frederic@kernel.org>, Boqun Feng <boqun.feng@gmail.com>, Joel Fernandes <joel@joelfernandes.org>, Josh Triplett <josh@joshtriplett.org>, Lai Jiangshan <jiangshanlai@gmail.com>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Neeraj Upadhyay <neeraj.upadhyay@amd.com>, "Paul E . McKenney" <paulmck@kernel.org>, Steven Rostedt <rostedt@goodmis.org>, Uladzislau Rezki <urezki@gmail.com>, Zqiang <qiang.zhang1211@gmail.com>, rcu <rcu@vger.kernel.org>, "Liam R . Howlett" <Liam.Howlett@oracle.com>, Peter Zijlstra <peterz@infradead.org> Subject: [PATCH 4/4] sched: Exclude CPU boot code from PF_IDLE area Date: Tue, 24 Oct 2023 23:46:25 +0200 Message-ID: <20231024214625.6483-5-frederic@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231024214625.6483-1-frederic@kernel.org> References: <20231024214625.6483-1-frederic@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 morse.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 (morse.vger.email [0.0.0.0]); Tue, 24 Oct 2023 14:47:13 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780675023493096998 X-GMAIL-MSGID: 1780675023493096998 |
Series |
rcu: Fix PF_IDLE related issues v2
|
|
Commit Message
Frederic Weisbecker
Oct. 24, 2023, 9:46 p.m. UTC
The commit:
cff9b2332ab7 ("kernel/sched: Modify initial boot task idle setup")
has changed the semantics of what is to be considered an idle task in
such a way that only the actual idle loop is accounted as PF_IDLE. The
intent is to exclude the CPU boot code from that coverage.
However this doesn't clear the flag when the CPU goes down. Therefore
when the CPU goes up again, its boot code is part of the PF_IDLE zone.
Make sure this flag behave consistently and clear the flag when a CPU
exits from the idle loop. If anything, RCU-tasks relies on it to exclude
CPU boot code from its quiescent states.
Fixes: cff9b2332ab7 ("kernel/sched: Modify initial boot task idle setup")
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
include/linux/sched.h | 2 +-
kernel/cpu.c | 4 ++++
kernel/sched/idle.c | 1 -
3 files changed, 5 insertions(+), 2 deletions(-)
Comments
On Tue, Oct 24, 2023 at 11:46:25PM +0200, Frederic Weisbecker wrote: > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 8885be2c143e..ad18962b921d 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1945,7 +1945,7 @@ extern struct task_struct *idle_task(int cpu); > */ > static __always_inline bool is_idle_task(const struct task_struct *p) > { > - return !!(p->flags & PF_IDLE); > + return !!(READ_ONCE(p->flags) & PF_IDLE); > } > > extern struct task_struct *curr_task(int cpu); > diff --git a/kernel/cpu.c b/kernel/cpu.c > index 3b9d5c7eb4a2..3a1991010f4e 100644 > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -1394,7 +1394,9 @@ void cpuhp_report_idle_dead(void) > { > struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); > > + WRITE_ONCE(current->flags, current->flags & ~PF_IDLE); > BUG_ON(st->state != CPUHP_AP_OFFLINE); > + > rcutree_report_cpu_dead(); > st->state = CPUHP_AP_IDLE_DEAD; > /* > @@ -1642,6 +1644,8 @@ void cpuhp_online_idle(enum cpuhp_state state) > { > struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); > > + WRITE_ONCE(current->flags, current->flags | PF_IDLE); > + > /* Happens for the boot cpu */ > if (state != CPUHP_AP_ONLINE_IDLE) > return; Without changing *ALL* ->flags stores to WRITE_ONCE() I don't see the point of this. Also, since we only care about a single bit, how does store tearing affect things? Not to mention if we're really paranoid, what are the SMP ordering considerations :-) [ also, PF_ is used for Protocol Family, Page Flag and Process Flag, grepping is a pain in the arse :-( ]
Le Wed, Oct 25, 2023 at 10:48:33AM +0200, Peter Zijlstra a écrit : > On Tue, Oct 24, 2023 at 11:46:25PM +0200, Frederic Weisbecker wrote: > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > index 8885be2c143e..ad18962b921d 100644 > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -1945,7 +1945,7 @@ extern struct task_struct *idle_task(int cpu); > > */ > > static __always_inline bool is_idle_task(const struct task_struct *p) > > { > > - return !!(p->flags & PF_IDLE); > > + return !!(READ_ONCE(p->flags) & PF_IDLE); > > } > > > > extern struct task_struct *curr_task(int cpu); > > diff --git a/kernel/cpu.c b/kernel/cpu.c > > index 3b9d5c7eb4a2..3a1991010f4e 100644 > > --- a/kernel/cpu.c > > +++ b/kernel/cpu.c > > @@ -1394,7 +1394,9 @@ void cpuhp_report_idle_dead(void) > > { > > struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); > > > > + WRITE_ONCE(current->flags, current->flags & ~PF_IDLE); > > BUG_ON(st->state != CPUHP_AP_OFFLINE); > > + > > rcutree_report_cpu_dead(); > > st->state = CPUHP_AP_IDLE_DEAD; > > /* > > @@ -1642,6 +1644,8 @@ void cpuhp_online_idle(enum cpuhp_state state) > > { > > struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); > > > > + WRITE_ONCE(current->flags, current->flags | PF_IDLE); > > + > > /* Happens for the boot cpu */ > > if (state != CPUHP_AP_ONLINE_IDLE) > > return; > > Without changing *ALL* ->flags stores to WRITE_ONCE() I don't see the > point of this. Also, since we only care about a single bit, how does > store tearing affect things? > > Not to mention if we're really paranoid, what are the SMP ordering > considerations :-) > > [ also, PF_ is used for Protocol Family, Page Flag and Process Flag, > grepping is a pain in the arse :-( ] Indeed. Also cpuhp_online_idle() is called with preemption disabled and cpuhp_report_idle_dead() with interrupts disabled. As for idle injection in play_idle_precise(), the flag is set and cleared with preemption disabled. This means that all writes are in an RCU read side critical section that RCU-tasks pre-gp's synchronize_rcu() waits for. So I don't think we need those WRITE_ONCE/READ_ONCE. Paul are you ok with that? Thanks.
diff --git a/include/linux/sched.h b/include/linux/sched.h index 8885be2c143e..ad18962b921d 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1945,7 +1945,7 @@ extern struct task_struct *idle_task(int cpu); */ static __always_inline bool is_idle_task(const struct task_struct *p) { - return !!(p->flags & PF_IDLE); + return !!(READ_ONCE(p->flags) & PF_IDLE); } extern struct task_struct *curr_task(int cpu); diff --git a/kernel/cpu.c b/kernel/cpu.c index 3b9d5c7eb4a2..3a1991010f4e 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -1394,7 +1394,9 @@ void cpuhp_report_idle_dead(void) { struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); + WRITE_ONCE(current->flags, current->flags & ~PF_IDLE); BUG_ON(st->state != CPUHP_AP_OFFLINE); + rcutree_report_cpu_dead(); st->state = CPUHP_AP_IDLE_DEAD; /* @@ -1642,6 +1644,8 @@ void cpuhp_online_idle(enum cpuhp_state state) { struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); + WRITE_ONCE(current->flags, current->flags | PF_IDLE); + /* Happens for the boot cpu */ if (state != CPUHP_AP_ONLINE_IDLE) return; diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index 5007b25c5bc6..342f58a329f5 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -373,7 +373,6 @@ EXPORT_SYMBOL_GPL(play_idle_precise); void cpu_startup_entry(enum cpuhp_state state) { - current->flags |= PF_IDLE; arch_cpu_idle_prepare(); cpuhp_online_idle(state); while (1)