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 7ADEFC282EC for ; Fri, 14 Mar 2025 11:58:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 20D55280002; Fri, 14 Mar 2025 07:58:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1942B280001; Fri, 14 Mar 2025 07:58:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 00D1A280002; Fri, 14 Mar 2025 07:58:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id CEFEA280001 for ; Fri, 14 Mar 2025 07:58:06 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 58D9D140872 for ; Fri, 14 Mar 2025 11:58:08 +0000 (UTC) X-FDA: 83220008256.12.BBC5F9D Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf19.hostedemail.com (Postfix) with ESMTP id 876241A0005 for ; Fri, 14 Mar 2025 11:58:06 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=GJHImAf3; dkim=pass header.d=linutronix.de header.s=2020e header.b=XrDfQ8Ms; dmarc=pass (policy=none) header.from=linutronix.de; spf=pass (imf19.hostedemail.com: domain of bigeasy@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=bigeasy@linutronix.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741953486; 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-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=8uewKNoT8JWDI3vksDZ1lfM+BZl6yGTaXxxJLZKOzx0=; b=VO15ym1HBDZOIy5g0Z8RFsMflYUKGo0+F6N2GyeJnneAbnys0cI2ngox1ShGD+8yDPTajf Ckm3gjnshzXG+CpkSIyQFSG6moP85710VAq9ykmOdPGZQDWNv9jus7u7geu1gHK8Mgz005 +Ggsfh+Ub+hd9JXpEXjkBzyurzjnzkg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741953486; a=rsa-sha256; cv=none; b=6/TpcVzmp7hbpwTcK4u/AYXgEVIH5MA+TVNlEAw5f+Ssv87IGdQthGNIJOe13HlfqZc9iW vtAqP+hqfRsKRHXpB8WaIN6sVmwh1hx+nVzPUztHWN5/nRkvFFNKtipRGlWQr50fQ0DBeA TFPPP+2HiX6KsUBTwVD+7IspURgqubI= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=GJHImAf3; dkim=pass header.d=linutronix.de header.s=2020e header.b=XrDfQ8Ms; dmarc=pass (policy=none) header.from=linutronix.de; spf=pass (imf19.hostedemail.com: domain of bigeasy@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=bigeasy@linutronix.de Date: Fri, 14 Mar 2025 12:58:02 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1741953484; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8uewKNoT8JWDI3vksDZ1lfM+BZl6yGTaXxxJLZKOzx0=; b=GJHImAf3Y7UB1RzvBxLOtqJc4jRh/h+gxXQP7hVvQ/uY/ozVfmBAfaT5PfjDbUEAMAEzQf bVUsG69XNEjcbVktmfv5cUUhbUXkOV1kn+I4LergArsjOqTRm7Z0L2FpJdi8y6AhZMLAbU zu9OzdHZShuCHv+RwE7UXK/SIfi8IC3f2a+ZT0zMZfv2mMT53i8AsWG64l8Mh9cvHeCK0i zhb9MIRl9kbT/AVN58rnwvDktH164Oajfi6GVfdriyNWMh8xGFV75Da5ypk6GadFo5nvpc WpCx8iKXJp8hOJ1Fw+l+i8b207JoNIKHeK2ihdkh9rwZPkf0YCcC6M7y/z7ASQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1741953484; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8uewKNoT8JWDI3vksDZ1lfM+BZl6yGTaXxxJLZKOzx0=; b=XrDfQ8MsdqFP7ajJekANqTpOTS7uitbjh6jCB/v6tJ8WB2HHfVYXFwxVhWlu66lnTyqneO nIYvnaaaA+Xkn7Aw== From: Sebastian Andrzej Siewior To: Vlastimil Babka Cc: Shakeel Butt , Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Meta kernel team Subject: Re: [RFC PATCH 10/10] memcg: no more irq disabling for stock locks Message-ID: <20250314115802.DESa-C1z@linutronix.de> References: <20250314061511.1308152-1-shakeel.butt@linux.dev> <20250314061511.1308152-11-shakeel.butt@linux.dev> <9e1e3877-55ae-4546-a5c1-08ea730ea638@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <9e1e3877-55ae-4546-a5c1-08ea730ea638@suse.cz> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 876241A0005 X-Stat-Signature: gm4jbers7xid68kkc9g7tcixniryna5j X-HE-Tag: 1741953486-35489 X-HE-Meta: U2FsdGVkX1/z8myqhrX7fTXL61+bvMuGJOES6067fFUci9F/hOPsBRbUN6Cbe/FB6CGQe9BGqrmGgiimgkbTLvW+ZeFQ0NAy2F33YLcQu6fweCoDql1i/Ydk7HE4QzafDrTc34ewxdUgBlk7LajVRJteqD4OC167TbjInawfvK/2ebRH+GYLCrUF8NGOYmjZPpUGkeBDX4Vq7bCuenfHeffjNHS92vpPn3B91LFP/Z6aJY9E1vSaLLQzUBh7Aka+N5crjX14X6wKuNmqpdVrgXpg/a/PMdUKrJpRDFuP+h/32XO1tnXUAp0NP7FCjlU9S5NDLkL24Tbdv+gKC9QV0j5B1Wr1l7I77zRjf7/NFNtd/a2FE6dzhKgZjdTbzK9MBx2ncmKzOT9+EvvfODt8Lqf731E0RFuhbPr5NG3uprpzRm2PsB9Dwr2YublMuKvnQEPDsUkw9NaTbuSADzznu8TDDdymv1B6ISktFg/LTdxLa2CLDVCJpV6GM5BGCaBk65ec5paiFZWh8WBrbKopBvaCH0PgfR67K0zYvkCBN+u4aXvxyOKmamcoI7I4eVFDFKZ30oMIWXliJNZmNIaczXwmmOki6d6MShYR5OuMtydcY8gKO2WAHmVtXRuhFJk6zkQyO9iM+CJ+aEhT4gtzSIeQVSxA8/xbKMSSjDxw51cZBGjJE1N84JT9T7NiosMsKR7vxBT5PE2vepWo83HptOD5YHCv4Vmw4b1o4fk+lZXQVbXfQxOVVyzmBtBCrDOF9f953rtH8QAzwkYu56TTBdM+57clVP+laOCX535P1oOt4JQga/C1c8NW+u20+gN3v4bmOxpkNYcz1FSK4JkYGNYeMqG+dSrYY7JlU4+fhxxIUSmTZP8hPdRGJI5mFagKKfb9QKkuZBhIBkzom7Jdy2mTYMt7GOoJKF4XkWrID6Q0vDLwbLB6m53UkGyPI1D09Is+Jc+84Q/VADQ7HaY D/cz2nfs 3oyS6x2rkivtty5k3bxl7Fn5PzhMG4cWaK7ikoe9C9vPR9Vilvbl37SjAyC5VQoH8y8hTERYBCeDfiSxQ/AV9LeyVjVulbD/vZic7KTjoVY3hNu+VkSYecy5AS2pwb5lU5VqjRmAzhva5IU4kG4CUOJ/VddOc5GRsZsmjURqYNMHlj2PculwdGWgDwBRrJ7Am0MVi X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2025-03-14 11:54:34 [+0100], Vlastimil Babka wrote: > On 3/14/25 07:15, Shakeel Butt wrote: > > Let's switch all memcg_stock locks acquire and release places to not > > disable and enable irqs. There are two still functions (i.e. > > mod_objcg_state() and drain_obj_stock) which needs to disable irqs to > > update the stats on non-RT kernels. For now add a simple wrapper for > > that functionality. >=20 > BTW, which part of __mod_objcg_mlstate() really needs disabled irqs and n= ot > just preemption? I see it does rcu_read_lock() anyway, which disables > preemption. Then in __mod_memcg_lruvec_state() we do some __this_cpu_add() > updates. I think these also are fine with just disabled preemption as they > are atomic vs irqs (but don't need LOCK prefix to be atomic vs other cpus > updates). __this_cpu_add() is not safe if also used in interrupt context. Some architectures (not x86) implemented as read, add, write. this_cpu_add()() does the same but disables interrupts during the operation. So __this_cpu_add() should not be used if interrupts are not disabled and a modification can happen from interrupt context. > Is it just memcg_rstat_updated() which does READ_ONCE/WRITE_ONCE? Could we > perhaps just change it to operations where disabled preemption is enough? >=20 > > Signed-off-by: Shakeel Butt =E2=80=A6 > > @@ -2757,6 +2745,28 @@ static void replace_stock_objcg(struct memcg_sto= ck_pcp *stock, > > WRITE_ONCE(stock->cached_objcg, objcg); > > } > > =20 > > +static unsigned long rt_lock(void) > > +{ No, we don't name it rt_lock(). We have local_lock() for this exact reason. And migrate_disable() does not protect vs re-enter of the function on the CPU while local_irq_save() does. > > +#ifdef CONFIG_PREEMPT_RT > > + migrate_disable(); > > + return 0; > > +#else > > + unsigned long flags =3D 0; > > + > > + local_irq_save(flags); > > + return flags; > > +#endif > > +} > > + > > +static void rt_unlock(unsigned long flags) > > +{ > > +#ifdef CONFIG_PREEMPT_RT > > + migrate_enable(); > > +#else > > + local_irq_restore(flags); > > +#endif > > +} > > + > > static void mod_objcg_state(struct obj_cgroup *objcg, struct pglist_da= ta *pgdat, > > enum node_stat_item idx, int nr) > > { Sebastian