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 78B0DC282EC for ; Mon, 17 Mar 2025 21:55:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CE28C280002; Mon, 17 Mar 2025 17:54:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C91D3280001; Mon, 17 Mar 2025 17:54:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B812E280002; Mon, 17 Mar 2025 17:54:58 -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 9B811280001 for ; Mon, 17 Mar 2025 17:54:58 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 2508D1A06DD for ; Mon, 17 Mar 2025 21:54:59 +0000 (UTC) X-FDA: 83232398718.11.7589F21 Received: from out-187.mta1.migadu.com (out-187.mta1.migadu.com [95.215.58.187]) by imf16.hostedemail.com (Postfix) with ESMTP id 364FC18000F for ; Mon, 17 Mar 2025 21:54:56 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=Hl9rHEnk; spf=pass (imf16.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.187 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1742248497; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=T8zp66IVL2VPZNUMDSB5F2rdrquIj0RbhStyEr5Jlho=; b=WGRjkJSQKFCWj7KBpul9Y87pi/rw9Wfku2lEwE9tQOMgZ80g2L9k+m+7VWUDBTjc0AVI9v L3NoPYL74KqsEeQfK78AO+nxGJlvsxYdVDFNFRme9alXqfMb0LHeOqTEya7pX5V5+OXVqn GDPEH5Vwfg7EyZ2qxi6eqZmzImDYMqY= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=Hl9rHEnk; spf=pass (imf16.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.187 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742248497; a=rsa-sha256; cv=none; b=hQC+cu41nQTThlzYjngnn/0xnJUcynF1pQZVIXwzyKU0LJgPzSADBg74kWww5PYfmzRfCF s2Y1o0VcAX6lmfatfzEGxEZOn9spp2FqEqBrPnEF9395UJ+1u1V4VDhS3ir7LndpdMdXJH 8fGXDlFl63gO78XOHnFku3PEjWAZLMs= Date: Mon, 17 Mar 2025 14:54:45 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1742248495; 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: in-reply-to:in-reply-to:references:references; bh=T8zp66IVL2VPZNUMDSB5F2rdrquIj0RbhStyEr5Jlho=; b=Hl9rHEnk46eedn3Q3OYJUjuR6n+mHfz4G4ox+piDVlybnzegawJ8HkN6AxH381K/PtK+F8 3etfdwWnmelGvnAUK3NcVzOkrqmrR1AtfAjy6ZSFLpGwY4Hz6De+AyxaRO2Wrtm6OIvvdH mJDsyw76RRjDeMDhuKFI+rBzTgVL/H4= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Vlastimil Babka Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Sebastian Andrzej Siewior , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Meta kernel team Subject: Re: [PATCH 7/9] memcg: use __mod_memcg_state in drain_obj_stock Message-ID: <6n7rsw565dy4kt7yxmik5kpxdz2b5h2bdsysfvi2rwmvl4juml@npkqfiyzfqua> References: <20250315174930.1769599-1-shakeel.butt@linux.dev> <20250315174930.1769599-8-shakeel.butt@linux.dev> <7d50a14a-edfb-410d-840e-17876806a63b@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7d50a14a-edfb-410d-840e-17876806a63b@suse.cz> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Queue-Id: 364FC18000F X-Rspamd-Server: rspam03 X-Stat-Signature: i17pgnoo369or7oqt7u37td1mgrdjhjj X-HE-Tag: 1742248496-571896 X-HE-Meta: U2FsdGVkX1+Eky3pw77f+HbuvzhS/1xULpo0puk2VdldruT4OvFQx0dZT70DWXtWcmWHZs3j/rpFfeVolPhwRps5ycceRWkiUZz14Gwxa5jAeCvz9f4/3W+AOgMT0/HSyL9DEYtueq9eMpHe/p1Yoro37UtTXEhsPlR4dncnPXOGZeFwLK1iJy1BAiE6Ab7P4BJLTWgEKsmd5FPfNbeI3NYxfz00OIMbaN8NLNI5yfCLkZVUlWdpgsr3KrH1cK/xeQixKsU39ZpmFx4QpeSMAUfHhG4caVVZuQhYe04Zqxy2zHjyfe9CibPRW46JkXelPnPGH0m6YYidmSyRTV0UvD7XWA6zwI52H/IQJ6J9MHTgQowDQtFRXyIi2i5TUOR5BoyPMjEGDHjQkeClZE7QAoNmzCFP6VOiY0yemZfPhjzXtVQIWS+6qa6/YS6rHOnpkJB8vYGdTDBwUwCgdBfXm1mIs3LCaQTq9Ih/jOknVlXOG56FpWpolt1nFsOgbsZmXrN6Z7P72V1LPWl1WDQvKgyrO2+jVJFKi/CM6Ls26glFfJqSM/2sFuSLVvKa3L+LbYsx4fKwX2KGow6DNSI05SvUxF7j09shDhLwEkFr33v4oW/5ROMnXNnFFxEmvnlu1EDeZY82PAl3oIXlsmTm8XRskiBTxN5DuQ2YDj/UmYtZGgYbG5tKny6n7/gb1z11jNKexvcYkpUawc6J1nlkWsUruVprvKXOB8QeKV0beBJwfLH/SJjaQFfp+nhNcbJeFXWuHjMYsCwPzDACmguYDEpxmNT+oca7ChIqjcDNg5H0IK1DwpQNjVug2N/JqJBOUILaBeGGEq3GKP2Fp1db3Tu+6RX3HSLQNpQzcel25Fq8vKRc6IA27WsIoMR3vGNJp7ZfqjXLbJn229FKkGZCtfUOcbqAbXXapxV7j8GgzU76Dz+6hsHLiymnOj4H8RzWE2iG9Bj0sFEmFF0cnZd UHLpXJiZ CTq2mIwNEqog69MWZdAOWbXKkhCSvZldtM6Mx6olIkbweRPzXoSvkAhbxFfxIjIkssVQ2VI1tKu420tTWzjN1+4U4oZbJ8TitkOtfy29N3Or4BR62VGn+aUGlK6hGW+3rrMz6Skj/K+o95sQpvrsBAccUAVLhIRCuUJy278FKn8L5pV6m+yHhbAjkOAyJzO9heuBrikDWMpnyjbceflcS4YI9anNpSyuDGJ2U 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 Mon, Mar 17, 2025 at 09:56:39PM +0100, Vlastimil Babka wrote: > On 3/15/25 18:49, Shakeel Butt wrote: > > For non-PREEMPT_RT kernels, drain_obj_stock() is always called with irq > > disabled, so we can use __mod_memcg_state() instead of > > mod_memcg_state(). For PREEMPT_RT, we need to add memcg_stats_[un]lock > > in __mod_memcg_state(). > > > > Signed-off-by: Shakeel Butt > > Reviewed-by: Sebastian Andrzej Siewior > > I've asked in the RFC and from Sebastian's answer I think my question was > misunderstod, so let me try again. > > After this patch we'll have from mod_memcg_state(): > > mod_memcg_state() > local_irq_save(flags); > __mod_memcg_state() > memcg_stats_lock(); <- new and unnecessary? > > Instead of modifying __mod_memcg_state() we could be more targetted and just > do in drain_obj_stock(): > > memcg_stats_lock(); > __mod_memcg_state(); > memcg_stats_unlock(); > > Am I missing something? This seems unnecessary because this patch is adding the first user of __mod_memcg_state() but I think maintainability is better with memcg_stats_[un]lock() inside __mod_memcg_state(). Let's take the example of __mod_memcg_lruvec_state(). It is being called from places where non-RT kernel, the irqs are disabled through spin_lock_irq*, so on RT kernel, the irq would not be disabled and thus explicitly need preemption disabled. What if in future __mod_memcg_state() is being used by a caller which assumes preemption is disabled through irq disable. The RT kernel would be buggy there. I am not sure if it is easy to force the future users to explicitly add memcg_stats_[un]lock() across the call to __mod_memcg_state().