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 B0947C369D9 for ; Wed, 30 Apr 2025 11:42:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 91DE46B008C; Wed, 30 Apr 2025 07:42:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8A4A16B00BA; Wed, 30 Apr 2025 07:42:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6F98D6B00BB; Wed, 30 Apr 2025 07:42:52 -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 4B55E6B008C for ; Wed, 30 Apr 2025 07:42:52 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 4D7761A150E for ; Wed, 30 Apr 2025 11:42:52 +0000 (UTC) X-FDA: 83390523384.03.C18528F Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf22.hostedemail.com (Postfix) with ESMTP id A25B0C0011 for ; Wed, 30 Apr 2025 11:42:49 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b="l54wFE/F"; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5BKmIgpf; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b="l54wFE/F"; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5BKmIgpf; dmarc=none; spf=pass (imf22.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746013370; a=rsa-sha256; cv=none; b=rrh5SC3fYHJNer2xHTScHlpj+Oiz72WbwftaUXcvJTFfXNN4wetP92R/4gSEyfjp0HwzvS h21MUhGnc7Q4WFOAukbhja1NM72AXnWRqRzkCj+uZpkLlzkSBkdRKX+X4w+P6V/z+v3NHB BoucwWLPxWu6xbEUaPF4CGqSqRPrKV8= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b="l54wFE/F"; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5BKmIgpf; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b="l54wFE/F"; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5BKmIgpf; dmarc=none; spf=pass (imf22.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1746013370; 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=DMdUf+G8z9UwnFqiI+Ohf1SNPY5q3W5xNuNqMGtKlLI=; b=h/OYsvEFhV2JFXUyJzpZ/AHj4cPG/dPC3H5Ljepuo7NGno7DX1JEODhJtwizX097dnmCZT e9ysizVX6VNKaizFUBRsuuQwf3veqhbZf4NYXCczCwXru5fBrp2Px0YkiWW4g1uadjhCrC Z3sODZBEJROyU5y42+vexnTh3B6uNEs= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id C17BD21200; Wed, 30 Apr 2025 11:42:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1746013367; h=from:from:reply-to: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=DMdUf+G8z9UwnFqiI+Ohf1SNPY5q3W5xNuNqMGtKlLI=; b=l54wFE/Fyyayk3OovslwsQ4asa1Pi4AgvAffXiZwgZOzttH/oxC6jk5LjGchupx3kPFXu3 1Dv6JpgyR2B2/d3tYnXBDgrPzVao0HiIpRChPS1nNLDwz7u75B57Bq7GzxtbiXk3IvA2IN 4HcJjTvXQVTQ/ds84rUN4C3XKP9jh3I= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1746013367; h=from:from:reply-to: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=DMdUf+G8z9UwnFqiI+Ohf1SNPY5q3W5xNuNqMGtKlLI=; b=5BKmIgpfXflKaDnFG8V+ORenoH4P1/jhNQAt+HptR/T4clMTesTivFo7cDDi58rCuVgla/ DwZdggVsGeegzCDA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1746013367; h=from:from:reply-to: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=DMdUf+G8z9UwnFqiI+Ohf1SNPY5q3W5xNuNqMGtKlLI=; b=l54wFE/Fyyayk3OovslwsQ4asa1Pi4AgvAffXiZwgZOzttH/oxC6jk5LjGchupx3kPFXu3 1Dv6JpgyR2B2/d3tYnXBDgrPzVao0HiIpRChPS1nNLDwz7u75B57Bq7GzxtbiXk3IvA2IN 4HcJjTvXQVTQ/ds84rUN4C3XKP9jh3I= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1746013367; h=from:from:reply-to: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=DMdUf+G8z9UwnFqiI+Ohf1SNPY5q3W5xNuNqMGtKlLI=; b=5BKmIgpfXflKaDnFG8V+ORenoH4P1/jhNQAt+HptR/T4clMTesTivFo7cDDi58rCuVgla/ DwZdggVsGeegzCDA== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id A858F139E7; Wed, 30 Apr 2025 11:42:47 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id K3zJKLcMEmhsUAAAD6G6ig (envelope-from ); Wed, 30 Apr 2025 11:42:47 +0000 Message-ID: Date: Wed, 30 Apr 2025 13:42:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/4] memcg: separate local_trylock for memcg and obj Content-Language: en-US To: Shakeel Butt , Andrew Morton Cc: 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 , bpf References: <20250429230428.1935619-1-shakeel.butt@linux.dev> <20250429230428.1935619-3-shakeel.butt@linux.dev> From: Vlastimil Babka In-Reply-To: <20250429230428.1935619-3-shakeel.butt@linux.dev> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Action: no action X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: A25B0C0011 X-Stat-Signature: nkautqxfqfgz64f81twq3gc9aw51ete6 X-Rspam-User: X-HE-Tag: 1746013369-311655 X-HE-Meta: U2FsdGVkX1/1Cyk9i4V+cN6cn/DuPl9P1k/isV3/6SVAjykx2GWTO4GoVTQQXrsNUYFWkTfFjoT5e8U4JYgsQboCrAZFaV5X6MTlbhOJOZWC14RbXtiJL+2RS2YDm8PAyvPz54dYVCXEZpYpoHIlXulPKWT3CltH4NK02LL8qXcnV8V3tSSuYyIeftKVpH8Zo/aWxxnG6oBdJmi5yFn6Kg9ArgegnzNF3/AerJiWkC0HGIZCJYzbDECfsizqzv87BjOfDuDqJQeIMFZMcn2Cn3L1mKCxLKT0p8RtHgvYXuH3cotsemwLL1i0lFvcu2i0HC25I/7vC+FtUosSxRrBjEZJRAhW6/YznqIDXGGMbdaePJLjZCv985oEaMB7+qe3tQtfymLoDsFII8vEnUJTXsJSNIaVrztt4ulKjKwCNX2V2yNqsfNJ24oqMi8L8st4dBrhKjNRg/c5UbJsTlpfroyyOYGqjxtwaqPn45T8qthYOrq27Ee3Lkm5bQTEfERG+br2ZokUC9Q7Ftb4Ea2f+4ahVEQsS5S9GQWgOvISJ4fgvoEnXtROJdweNuP3HLXv+3L+mBSJ5k8E/77fgVEoiPLvaLJg1PJp0/9ppL1/r2gXr3Tm0NmcpEqNDXGI3Lzmu3NP9vBMDkID+q1gWlBhxRrpZCUBCkB7ROJysiFrhNH/lXGc6LszVwrgnmtuOcmSnI6tsRzZuvHgJuo4yGo+5daCL+QBgeod7o95ce+ajb2V7stUZKoLYOh7iCj/ZtXVc112o5PR/q02prxziQaU8F6F0iQsu/EEhsf0X8nM/yTD3r7+6QfvIUSqJRqppFKSodVa36D05+PYfSDmLQytdUD1cleBlvp2Arrc+OOqWl9SNuMTGSQLQVTKdV/xYiMor3RvqrSnGKlkGnrhOFvchmKQQVeNIeSr3Ef/ptVUodhhLXk1i9ETKlH3pEH3TuA498OCzFU0UzUt2gJ8e6N Uwq5VMd2 2IBhED7chjfjdBZKns+DFgNYy0nZEYbLYS+aCo+ruVV0vduXodHI0eNerqgjDk+kE6o1nFnHkdAtIzZvS7NMgNnLDJGfZmf6JNdjNTCrCtPGyYIU0mHIcDe2lFc0oVdbslqk6ylwnutxmGcfC4TGfj1XfOWecIfpi0RCii1TQGqw0Ewq6PgQ+x7dAzBS4n45Ax2sLsbF+jVDsNAx/12X62cIfxwiQ8SthxMickL9m6xBtU4xk8HL6d4WD9xUJPzSbeHHWB896ePydWcLo9aIkuw20KjPfcYWD6stYUahGGMCSvVWlL/Z7R5g8MzHEM273pre0u0GcOfP+W0EAoWIxVmfVlA== 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 4/30/25 01:04, Shakeel Butt wrote: > The per-cpu stock_lock protects cached memcg and cached objcg and their > respective fields. However there is no dependency between these fields > and it is better to have fine grained separate locks for cached memcg > and cached objcg. This decoupling of locks allows us to make the memcg > charge cache and objcg charge cache to be nmi safe independently. > > At the moment, memcg charge cache is already nmi safe and this > decoupling will allow to make memcg charge cache work without disabling > irqs. > > Signed-off-by: Shakeel Butt > --- > mm/memcontrol.c | 52 +++++++++++++++++++++++++++---------------------- > 1 file changed, 29 insertions(+), 23 deletions(-) > @@ -1883,19 +1885,22 @@ static void drain_local_stock(struct work_struct *dummy) > struct memcg_stock_pcp *stock; > unsigned long flags; > > - /* > - * The only protection from cpu hotplug (memcg_hotplug_cpu_dead) vs. > - * drain_stock races is that we always operate on local CPU stock > - * here with IRQ disabled > - */ > - local_lock_irqsave(&memcg_stock.stock_lock, flags); > + if (WARN_ONCE(!in_task(), "drain in non-task context")) > + return; > > + preempt_disable(); > stock = this_cpu_ptr(&memcg_stock); > + > + local_lock_irqsave(&memcg_stock.obj_lock, flags); > drain_obj_stock(stock); > + local_unlock_irqrestore(&memcg_stock.obj_lock, flags); > + > + local_lock_irqsave(&memcg_stock.memcg_lock, flags); > drain_stock_fully(stock); > - clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags); > + local_unlock_irqrestore(&memcg_stock.memcg_lock, flags); > > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags); > + clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags); > + preempt_enable(); This usage of preempt_disable() looks rather weird and makes RT unhappy as the local lock is a mutex, so it gives you this: BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48 I know the next patch removes it again but for bisectability purposes it should be avoided. Instead of preempt_disable() we can extend the local lock scope here? > } > > static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > @@ -1918,10 +1923,10 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > VM_WARN_ON_ONCE(mem_cgroup_is_root(memcg)); > > if (nr_pages > MEMCG_CHARGE_BATCH || > - !local_trylock_irqsave(&memcg_stock.stock_lock, flags)) { > + !local_trylock_irqsave(&memcg_stock.memcg_lock, flags)) { > /* > * In case of larger than batch refill or unlikely failure to > - * lock the percpu stock_lock, uncharge memcg directly. > + * lock the percpu memcg_lock, uncharge memcg directly. > */ > memcg_uncharge(memcg, nr_pages); > return; > @@ -1953,7 +1958,7 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > WRITE_ONCE(stock->nr_pages[i], nr_pages); > } > > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags); > + local_unlock_irqrestore(&memcg_stock.memcg_lock, flags); > } > > static bool is_drain_needed(struct memcg_stock_pcp *stock, > @@ -2028,11 +2033,12 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu) > > stock = &per_cpu(memcg_stock, cpu); > > - /* drain_obj_stock requires stock_lock */ > - local_lock_irqsave(&memcg_stock.stock_lock, flags); > + /* drain_obj_stock requires obj_lock */ > + local_lock_irqsave(&memcg_stock.obj_lock, flags); > drain_obj_stock(stock); > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags); > + local_unlock_irqrestore(&memcg_stock.obj_lock, flags); > > + /* no need for the local lock */ > drain_stock_fully(stock); > > return 0; > @@ -2885,7 +2891,7 @@ static bool consume_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes, > unsigned long flags; > bool ret = false; > > - local_lock_irqsave(&memcg_stock.stock_lock, flags); > + local_lock_irqsave(&memcg_stock.obj_lock, flags); > > stock = this_cpu_ptr(&memcg_stock); > if (objcg == READ_ONCE(stock->cached_objcg) && stock->nr_bytes >= nr_bytes) { > @@ -2896,7 +2902,7 @@ static bool consume_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes, > __account_obj_stock(objcg, stock, nr_bytes, pgdat, idx); > } > > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags); > + local_unlock_irqrestore(&memcg_stock.obj_lock, flags); > > return ret; > } > @@ -2985,7 +2991,7 @@ static void refill_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes, > unsigned long flags; > unsigned int nr_pages = 0; > > - local_lock_irqsave(&memcg_stock.stock_lock, flags); > + local_lock_irqsave(&memcg_stock.obj_lock, flags); > > stock = this_cpu_ptr(&memcg_stock); > if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */ > @@ -3007,7 +3013,7 @@ static void refill_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes, > stock->nr_bytes &= (PAGE_SIZE - 1); > } > > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags); > + local_unlock_irqrestore(&memcg_stock.obj_lock, flags); > > if (nr_pages) > obj_cgroup_uncharge_pages(objcg, nr_pages);