Received-SPF: pass (google.com: domain of linux-kernel+bounces-135167-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Received-SPF: Pass (protection.outlook.com: domain of arm.com designates
 63.35.35.123 as permitted sender) receiver=protection.outlook.com;
 client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com;
 pr=C
From: Yeo Reum Yun <YeoReum.Yun@arm.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Ingo Molnar
	<mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>, Steven Rostedt
	<rostedt@goodmis.org>, Vincent Guittot <vincent.guittot@linaro.org>, Juri
 Lelli <juri.lelli@redhat.com>, Dietmar Eggemann <Dietmar.Eggemann@arm.com>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>, Daniel Bristot
 de Oliveira <bristot@redhat.com>, Valentin Schneider <vschneid@redhat.com>,
	Catalin Marinas <Catalin.Marinas@arm.com>, Mark Rutland
	<Mark.Rutland@arm.com>, Will Deacon <will@kernel.org>, Aaron Lu
	<aaron.lu@intel.com>
Subject: Re: [PATCH] sched: Add missing memory barrier in switch_mm_cid
Thread-Topic: [PATCH] sched: Add missing memory barrier in switch_mm_cid
Thread-Index: AQHacWpg4RzUFZ0isEaP3JURG6Kl97E+2mIUgB9zq54=
Date: Mon, 8 Apr 2024 09:38:05 +0000
Message-ID:
 <AM0PR08MB428921B21B38D517A8A61FC4FB002@AM0PR08MB4289.eurprd08.prod.outlook.com>
References: <20240308150719.676738-1-mathieu.desnoyers@efficios.com>
 <AM0PR08MB42895432478A020E2C5B6C5FFB2C2@AM0PR08MB4289.eurprd08.prod.outlook.com>
In-Reply-To:
 <AM0PR08MB42895432478A020E2C5B6C5FFB2C2@AM0PR08MB4289.eurprd08.prod.outlook.com>
Accept-Language: en-GB, en-US
Content-Language: en-GB
msip_labels:
x-ms-traffictypediagnostic:
	AM0PR08MB4289:EE_|GVXPR08MB7680:EE_|DU2PEPF00028D01:EE_|AS4PR08MB7735:EE_
x-checkrecipientrouted: true
nodisclaimer: true
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam-Untrusted: BCL:0;
X-Microsoft-Antispam-Message-Info-Original:
 HM7LmNYRJ77sDz9mwYzXEnyDVoM1TAmZLjwhD46os4qPVk6tfqLNCVnzkdn3JkKyNIhzKAx0UHHyPHT1INo7wn1Wh4EgYOzacmvgYOYDQgGb5ppgRX5lpJ3vv/RFlWjBPWNd0Oz4VgHfq8WEKMVOV1JvOucdRwaM4I1koQAzG7uhV05CgpCkAs82pMuGl5N90aYcThmTFdrV8WZKqUE4xxeshDtPhq92moAGMYPfh4/Skva7OMCDuq2JoHXOp54fRvqQTaiTFtDl/+GpLZ1Zo7GhsE253yr29chI+S1fLrtyYZLgD1CL20d6P/O31vfw9XTCffetjqvduXA+HZSWm8o4HqO5aC35Nj6ivvikrTG53/ew/3pHevry1jyyn62o3QEJXJgTr6smj9WQVDALFeS7QSIby4cr2b5yiH/vHMgny37ByDWu/8XqtPSUMF4TlovjTJ3UOFHpddLp41bDDaUPdmwgE3fJ9KuWbRijTQsMcxkTV7e4cPRYeEaMHySLFfcxUzS44lBywzlpweK9m+kCdqOO1ipndch7qwHjeJAG1/5Bl3gdO9v85R3NQnZd+ZJt7rORms9SmlcTds2UFHY6VLxlxonqbsXcXfCNmQ4=
X-Forefront-Antispam-Report-Untrusted:
 CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AM0PR08MB4289.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(7416005)(366007)(376005)(1800799015);DIR:OUT;SFP:1102;
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: GVXPR08MB7680
X-EOPAttributedMessage: 0
X-MS-Exchange-Transport-CrossTenantHeadersStripped:
 DU2PEPF00028D01.eurprd03.prod.outlook.com
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 895f9def-60d9-4f8f-1bb5-08dc57afa1e7
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info:
	bxEZKjTGwRJ9yam1rno9PCXykpm2i1Ja5vDjxGskLjEKpW3kUQmc2YrHVxz9GqCPouSzPPqk6HL5jBUza3GH9P+W6TxykIMHzNfTkRzRNgHYBLCfstz135FwMm5ssyzFP1QcQ9DJrOEfCNUv/xDrXVzCaRlL6f8vEudx3+3uuh01QGIN2P97Khyvm5KNz2GkX5R5yznbTYT2jb7yxpGlWA5rgmKglZF4R/te4YgpdIQLrx0sQ68k/aL2ZbyAGfPFsYnBBEfnec1hpKghiOZ+nVzYR3+gva5RKJ4yuJh512ymLnzkKc4TNAuWcn+v+HjDUXiC5NKsOHi9aY1ows9PWUuxhlIsg8irI8Q8wGQgIFn/LdSG3QjHvs8JA9aSnVYZJumPU07tXLjOXmTzL0br2z5/7B8IohCf8PrOFk42lhCbhU3GNFpcAgjgCGTx6JqJrWUqikOvhqI4mfGUEqzR3qTQpvLjG9guKvX8KqOPqII8A66LrbHIOzZrc46n17dkFr/oLTlT0tF/EvrZ+I3dGvkq3iFIVnAm1Nk2aD2VY4OpCuxRvk6Nxohwxx15X0uTcMO9kPn3XrcpU6JiqpUV+yM/WrjikyOEyUaFJr9q9BDOCeFVxEmjs9/ORcGW+8wTch0JMeeyieGDHspRe96luYL13g+oOSG7wG0DJoaLv0RGQ8qP1M1Bnsl8N0G6yvNo7L4DxPIOzERlv0JbqE7r9g==
X-Forefront-Antispam-Report:
	CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(13230031)(376005)(36860700004)(82310400014)(1800799015);DIR:OUT;SFP:1101;
X-OriginatorOrg: arm.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2024 09:38:22.6938
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 895f9def-60d9-4f8f-1bb5-08dc57afa1e7
X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com]
X-MS-Exchange-CrossTenant-AuthSource:
	DU2PEPF00028D01.eurprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS4PR08MB7735

Gentle ping...

________________________________________
From: Yeo Reum Yun <YeoReum.Yun@arm.com>
Sent: 19 March 2024 09:20
To: Mathieu Desnoyers; Ingo Molnar; Peter Zijlstra
Cc: linux-kernel@vger.kernel.org; stable@vger.kernel.org; Steven Rostedt; V=
incent Guittot; Juri Lelli; Dietmar Eggemann; Ben Segall; Mel Gorman; Danie=
l Bristot de Oliveira; Valentin Schneider; Catalin Marinas; Mark Rutland; W=
ill Deacon; Aaron Lu
Subject: Re: [PATCH] sched: Add missing memory barrier in switch_mm_cid

Gentle ping.

________________________________________
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Sent: 08 March 2024 15:07
To: Ingo Molnar; Peter Zijlstra
Cc: linux-kernel@vger.kernel.org; Mathieu Desnoyers; Yeo Reum Yun; stable@v=
ger.kernel.org; Steven Rostedt; Vincent Guittot; Juri Lelli; Dietmar Eggema=
nn; Ben Segall; Mel Gorman; Daniel Bristot de Oliveira; Valentin Schneider;=
 Catalin Marinas; Mark Rutland; Will Deacon; Aaron Lu
Subject: [PATCH] sched: Add missing memory barrier in switch_mm_cid

Many architectures' switch_mm() (e.g. arm64) do not have an smp_mb()
which the core scheduler code has depended upon since commit:

    commit 223baf9d17f25 ("sched: Fix performance regression introduced by =
mm_cid")

If switch_mm() doesn't call smp_mb(), sched_mm_cid_remote_clear() can
unset the actively used cid when it fails to observe active task after it
sets lazy_put.

There *is* a memory barrier between storing to rq->curr and _return to
userspace_ (as required by membarrier), but the rseq mm_cid has stricter
requirements: the barrier needs to be issued between store to rq->curr
and switch_mm_cid(), which happens earlier than:

- spin_unlock(),
- switch_to().

So it's fine when the architecture switch_mm happens to have that barrier
already, but less so when the architecture only provides the full barrier
in switch_to() or spin_unlock().

It is a bug in the rseq switch_mm_cid() implementation. All architectures
that don't have memory barriers in switch_mm(), but rather have the full
barrier either in finish_lock_switch() or switch_to() have them too late
for the needs of switch_mm_cid().

Introduce a new smp_mb__after_switch_mm(), defined as smp_mb() in the
generic barrier.h header, and use it in switch_mm_cid() for scheduler
transitions where switch_mm() is expected to provide a memory barrier.

Architectures can override smp_mb__after_switch_mm() if their
switch_mm() implementation provides an implicit memory barrier.
Override it with a no-op on x86 which implicitly provide this memory
barrier by writing to CR3.

Link: https://lore.kernel.org/lkml/20240305145335.2696125-1-yeoreum.yun@arm=
com/
Reported-by: levi.yun <yeoreum.yun@arm.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fixes: 223baf9d17f2 ("sched: Fix performance regression introduced by mm_ci=
d")
Cc: <stable@vger.kernel.org> # 6.4.x
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: levi.yun <yeoreum.yun@arm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Aaron Lu <aaron.lu@intel.com>
---
 arch/x86/include/asm/barrier.h |  3 +++
 include/asm-generic/barrier.h  |  8 ++++++++
 kernel/sched/sched.h           | 20 ++++++++++++++------
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.=
h
index 35389b2af88e..0d5e54201eb2 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -79,6 +79,9 @@ do {                                                     =
             \
 #define __smp_mb__before_atomic()      do { } while (0)
 #define __smp_mb__after_atomic()       do { } while (0)

+/* Writing to CR3 provides a full memory barrier in switch_mm(). */
+#define smp_mb__after_switch_mm()      do { } while (0)
+
 #include <asm-generic/barrier.h>

 /*
diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index 961f4d88f9ef..5a6c94d7a598 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -296,5 +296,13 @@ do {                                                  =
                     \
 #define io_stop_wc() do { } while (0)
 #endif

+/*
+ * Architectures that guarantee an implicit smp_mb() in switch_mm()
+ * can override smp_mb__after_switch_mm.
+ */
+#ifndef smp_mb__after_switch_mm
+#define smp_mb__after_switch_mm()      smp_mb()
+#endif
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __ASM_GENERIC_BARRIER_H */
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 2e5a95486a42..044d842c696c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -79,6 +79,8 @@
 # include <asm/paravirt_api_clock.h>
 #endif

+#include <asm/barrier.h>
+
 #include "cpupri.h"
 #include "cpudeadline.h"

@@ -3481,13 +3483,19 @@ static inline void switch_mm_cid(struct rq *rq,
                 * between rq->curr store and load of {prev,next}->mm->pcpu=
_cid[cpu].
                 * Provide it here.
                 */
-               if (!prev->mm)                          // from kernel
+               if (!prev->mm) {                        // from kernel
                        smp_mb();
-               /*
-                * user -> user transition guarantees a memory barrier thro=
ugh
-                * switch_mm() when current->mm changes. If current->mm is
-                * unchanged, no barrier is needed.
-                */
+               } else {                                // from user
+                       /*
+                        * user -> user transition relies on an implicit
+                        * memory barrier in switch_mm() when
+                        * current->mm changes. If the architecture
+                        * switch_mm() does not have an implicit memory
+                        * barrier, it is emitted here.  If current->mm
+                        * is unchanged, no barrier is needed.
+                        */
+                       smp_mb__after_switch_mm();
+               }
        }
        if (prev->mm_cid_active) {
                mm_cid_snapshot_time(rq, prev->mm);
--
2.39.2

IMPORTANT NOTICE: The contents of this email and any attachments are confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease notify the sender immediately and do not disclose the contents to any=
 other person, use it for any purpose, or store or copy the information in =
any medium. Thank you.