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 B2D5FC369A2 for ; Thu, 10 Apr 2025 01:20:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C8C916B029B; Wed, 9 Apr 2025 21:20:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C3AB36B029C; Wed, 9 Apr 2025 21:20:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ADBCA6B029D; Wed, 9 Apr 2025 21:20:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 914C16B029B for ; Wed, 9 Apr 2025 21:20:44 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 61B2BC168D for ; Thu, 10 Apr 2025 01:20:44 +0000 (UTC) X-FDA: 83316379608.28.9FEB3B3 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf26.hostedemail.com (Postfix) with ESMTP id 0AA8014000D for ; Thu, 10 Apr 2025 01:20:41 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=duJz8iUi; spf=pass (imf26.hostedemail.com: domain of llong@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=llong@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744248042; 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=TI1ZtxlFbxcIExSutdibRhMPPAZuBxi2k7bBBZAj2gc=; b=YMUXnzFuPS/R7rGQhMiSDWFR7bA0/ijLtliMm0hHY+X2tYCJBbj4xnjqQ3/lk7y0WkfqAH KpqQzvmm/caTIicUzal9F8zJAsqLrT+XWhW9PzLohLX0iUn+QkQ6aVS/TqdWDasXfNiCtc ossoxE+SRAFJvlSBsyABT6dOQvGdVeU= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=duJz8iUi; spf=pass (imf26.hostedemail.com: domain of llong@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=llong@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744248042; a=rsa-sha256; cv=none; b=QiKW4H1gR23pglfotNrrT9Ny+NQxa+gdhi8spMFOfb44wzGR4UZ3xmnrV3nmrl5sMTAkmg 76kdLiL1N3ZfPhw2UGmxtwlQckKrtuyURe4JSLDAviO2k1tkYAMWihYKyuFZulejSS02e+ Cgq6EYtw2u0rd85AB/YaBafXvJgN78Q= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1744248041; 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=TI1ZtxlFbxcIExSutdibRhMPPAZuBxi2k7bBBZAj2gc=; b=duJz8iUi9eOXx8J+KVNP/kAi8yN7m82ylK9h9jDoZRNb3FpwRcdQVUsIrKNtS0vJj9PTd0 nykoOOodADvpBOPFd0pdGg9LGI9SvaUqCs8lk0HAdfUoG61C0+0tJ/XRFrAvimDgjXfrCO vuxqcjNfv6jq8yJwnhDKcCwSbvgba1s= Received: from mail-il1-f199.google.com (mail-il1-f199.google.com [209.85.166.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-679-lgkcgjI5Np6aQS641A5IGg-1; Wed, 09 Apr 2025 21:20:37 -0400 X-MC-Unique: lgkcgjI5Np6aQS641A5IGg-1 X-Mimecast-MFC-AGG-ID: lgkcgjI5Np6aQS641A5IGg_1744248037 Received: by mail-il1-f199.google.com with SMTP id e9e14a558f8ab-3d586b968cfso8835615ab.1 for ; Wed, 09 Apr 2025 18:20:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744248037; x=1744852837; h=in-reply-to:content-language:references:cc:to:subject:user-agent :mime-version:date:message-id:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=TI1ZtxlFbxcIExSutdibRhMPPAZuBxi2k7bBBZAj2gc=; b=cxjPmmJo54ELKallbINPKc862OUBCb0GP8t94+3T+IGuFzyI3KeWM59+1uR7EtD6GV GaOc0zJ791XHK14so3GDJvFyBWBdtNujiInws5jcBKsVjrEJ+7kb9RznqhoV4BE+ZVAi 1r3KFLRqz9qMm6BauWbCOikl76BcyChjeVW4qLG2/+GPR3F9pQ9K57D7ogjuI3Sqpdjf jWSSCa7jBK3h9hfW9BZ8omctXMDNWlDGKU5PSsDK2pIzeVCotja3WDIxOPCgOGW14A6H qHwQQ9DoBzU4ZNGl0Eskh7JAvdeitjKi1eWBgES+Sktgwv4t2db4a/5o9FW15BWscV5R k3jA== X-Forwarded-Encrypted: i=1; AJvYcCXp1A8YUS37YgOnUMtdivLQx3ajps+E1/8tkHe5gpvbXeD4PY0TQG6Sk3C0jb5swXdpDRE6HYTeHQ==@kvack.org X-Gm-Message-State: AOJu0Yyw//2pVwoxEIJbb/AmnM05J3nAfOhLvC36EilQgKgBjxlGKGAq DQ/2aA0xaVzaIZLXxB0QuUlG7E8K+2onNB67CH3ys6eg92Haf2L47/tL1FsWL+/MP7D3kVoEFCi 2IG7w9ohpokMIhkbEEB1jJBriLAc9+eXIfjvtjeq4+XKVvD9G X-Gm-Gg: ASbGncuwL0GaKc624VDGEtucnVvZY5AXgnVtuDJTkO/oZ+KQ8f7lsQui3Z5jsgSNxLk vEmegO/Z7TyNwHd6AqnWa+KqbMygOgXk604zMFqDdtMXDL8EbHDqN+M1Pt/M+7k3jZgazj5+oT1 tTSg3z7Wc9EGfA7TXWtNfFbG7ihaDOAt7HoioP+OCR396dOFpFip1PoDDRP2qgFqJ1CrD72Gdx+ a0NuPrz3soozuiRSOKByVuLwrBwgukI1XzKyxjYf+DuUZJY2TW/ug3wiB7TbTKMTF9/h08iOiwq EoTKQ5gC2JeJt14h3yEkPjipo2PC+SPeuAcyBGIcb9IOB2N2A7F0sdatjw== X-Received: by 2002:a05:6e02:3c86:b0:3d4:341a:441d with SMTP id e9e14a558f8ab-3d7e46f9a43mr11315345ab.10.1744248036871; Wed, 09 Apr 2025 18:20:36 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG9PtiatOK/LkHjmNFD7PisHtmqEBZ/QPB4g+xBNPNPepPTbx1Q8d6udsT4IxhuWJNLJfZLdQ== X-Received: by 2002:a05:6e02:3c86:b0:3d4:341a:441d with SMTP id e9e14a558f8ab-3d7e46f9a43mr11315155ab.10.1744248036392; Wed, 09 Apr 2025 18:20:36 -0700 (PDT) Received: from ?IPV6:2601:188:c100:5710:315f:57b3:b997:5fca? ([2601:188:c100:5710:315f:57b3:b997:5fca]) by smtp.gmail.com with ESMTPSA id e9e14a558f8ab-3d7dc592069sm5262425ab.72.2025.04.09.18.20.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 09 Apr 2025 18:20:35 -0700 (PDT) From: Waiman Long X-Google-Original-From: Waiman Long Message-ID: <2f9d478d-3359-40c8-8607-be906eaeea04@redhat.com> Date: Wed, 9 Apr 2025 21:20:34 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] memcg: optimize memcg_rstat_updated To: Shakeel Butt , Andrew Morton Cc: Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Yosry Ahmed , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Meta kernel team References: <20250409234908.940121-1-shakeel.butt@linux.dev> In-Reply-To: <20250409234908.940121-1-shakeel.butt@linux.dev> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: Gf8HhLveLo-ksb028ljlk2XH6istfK-dgbemXaIgdZQ_1744248037 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="------------FOQphYEUW0hr01DoyQy8WFR1" Content-Language: en-US X-Rspamd-Queue-Id: 0AA8014000D X-Rspamd-Server: rspam05 X-Rspam-User: X-Stat-Signature: pczb6nscwm6egc3bj91n88diu31iuxre X-HE-Tag: 1744248041-789018 X-HE-Meta: U2FsdGVkX1/jT6QQ+jZeVDZuAHXXmDrk4b8mVydkjIepdWs+9wX+bU0HxCbATu5rWQOGA5PXD+U/+gtRDH6ueOwEWuhHuB+z/8sPT7El+k/CsZwv2pY86EWzf/jPUwZXUmlcj8M6X3cnqTQ3feCBm9aAzy2uZ/KmaAPSqVu0GKrlYc9TRJR2XZf7fbxsBDjEZ6vK5RPLku4DyAriza/T9k1zb2GOPYB6tpeiPBBMyF/BT6clwqTO8kAOpbRl55F0qLEr58vrGDe0YluVK3NXeAjIbIHknUjibaXqawr4Q/p9/1ncAItQMYJ+d1T048A7gzGZYqemhxFaawvvgCXUG+LLuAxTkSORc8X1cwLAHswS5ZBcnjEl1gwKrWcINnXuBQD0YsAJrRqH5SA6y4LqHzikvPeb0Yy95OasG/QOcOLRwayRCFcGsiLJef+vYB/oCYD4kW6PztoaiytNuSN9lnm2zdaxozimPish8BrPKG9JII+gN28pzGfZOx2tWZapLCsEKaiTbowoHUIgkcwXYHnxi8DWrX4rk93PwvIFGpcoiwaFhmqTNuJSlzTxQ2E4EqEk5adXfPWoRrUPiHf1BMJc38s2yJf4Fn0dkuYoqEnt4/aEAgRu8yAVLnoQ1ajQzsZQFXpasDixdX2Ar7SlC7iUEov6H4W7ZAKQhKNQAltnEWODPc2nbn8V8CqD66VTrWeSSa1MAB00hc2Ufhdko1R/S9x3Io7Rqz2D4lVxYpGEDrlVTfsSTMj5+Exo9j5nPV6fz+TG2164tc0dluQ/48A7rPti0tS5G9AKHHTU7mYcRZ6OJiMSItuVv1elWCV6gsWGJ5u0UwQ7OEiVqNsBVknpQNL/ra9rxqBvf3DkagwtFSdQiPeH9HXvn7Fcq/M0h2m9kY9w24INww4lZxDcZxGUtjrPIe7JCXiLVexa2ktHKpNwk7B1s5hkfFuMRZY3cBg+NHqe4Wo7z2ibQdg oQhxgT7Q btbWSLANa+dq1tOFISV9MtdqJTQymwpGYSgj3B0sGS1I5MQ9kVhZ8OpIWsntmIdKQpSmZhnDifTMahyddFKNowYaDpkZpOqfq7oIPSkVA+1emJPhtzxMitDm6cXgEN9yB6RlB7BIqKrHNtKYetrCRxAS6NNK8mGXUnaaEQ9SldWQ/nU7nMAX9QhnJyMw+7Fpt/QCFtyLKsrA2ovgdQL5ZAUFElYu2MTkRCF5hb9J3NWZFHg+32ZythZQTtjclN+qL3XfG+zIJuL1XbA/YnHfrYm1X3m/urICcDmtXW2jkDteKb1Vb6lCUUkLXy5RvP2lIRWbBYfG5YeOj8cx6RxYZ4x2kAaUbJ++dU054vupcYfVqhFrU/RmVwfHcUnmbCap2JlDn++0Zg4mo/eBv5GGup0XLuEJdiNfdXmD49rPEvPffz+Xiwrs4BRX2mGgoEj1vUYjeb0QLgjsAtd61Bp4LV0xFdwLCPMTPgIGpB2OjK8Ph7v8EomYkv20CoULVLZPn+6XfM9JsYg4dQBG1s5Npau0dusZXCPg/oRkwUQcvdvuG7qMP81wwfauDUhn4EIRNzJQfXjuG8Na2lTBExI7iYlqRCxvYSPmZ4OF1099DH9XbvlOvo2fc0AVcKKs4IKk4TXhY 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: This is a multi-part message in MIME format. --------------FOQphYEUW0hr01DoyQy8WFR1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 4/9/25 7:49 PM, Shakeel Butt wrote: > Currently the kernel maintains the stats updates per-memcg which is > needed to implement stats flushing threshold. On the update side, the > update is added to the per-cpu per-memcg update of the given memcg and > all of its ancestors. However when the given memcg has passed the > flushing threshold, all of its ancestors should have passed the > threshold as well. There is no need to traverse up the memcg tree to > maintain the stats updates. > > Perf profile collected from our fleet shows that memcg_rstat_updated is > one of the most expensive memcg function i.e. a lot of cumulative CPU > is being spent on it. So, even small micro optimizations matter a lot. > > Signed-off-by: Shakeel Butt > --- > mm/memcontrol.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 421740f1bcdc..ea3e40e589df 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -585,18 +585,20 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val) > cgroup_rstat_updated(memcg->css.cgroup, cpu); > statc = this_cpu_ptr(memcg->vmstats_percpu); > for (; statc; statc = statc->parent) { > + /* > + * If @memcg is already flushable then all its ancestors are > + * flushable as well and also there is no need to increase > + * stats_updates. > + */ > + if (!memcg_vmstats_needs_flush(statc->vmstats)) > + break; > + Do you mean "if (memcg_vmstats_needs_flush(statc->vmstats))"? Cheers, Longman > stats_updates = READ_ONCE(statc->stats_updates) + abs(val); > WRITE_ONCE(statc->stats_updates, stats_updates); > if (stats_updates < MEMCG_CHARGE_BATCH) > continue; > > - /* > - * If @memcg is already flush-able, increasing stats_updates is > - * redundant. Avoid the overhead of the atomic update. > - */ > - if (!memcg_vmstats_needs_flush(statc->vmstats)) > - atomic64_add(stats_updates, > - &statc->vmstats->stats_updates); > + atomic64_add(stats_updates, &statc->vmstats->stats_updates); > WRITE_ONCE(statc->stats_updates, 0); > } > } --------------FOQphYEUW0hr01DoyQy8WFR1 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit
On 4/9/25 7:49 PM, Shakeel Butt wrote:
Currently the kernel maintains the stats updates per-memcg which is
needed to implement stats flushing threshold. On the update side, the
update is added to the per-cpu per-memcg update of the given memcg and
all of its ancestors. However when the given memcg has passed the
flushing threshold, all of its ancestors should have passed the
threshold as well. There is no need to traverse up the memcg tree to
maintain the stats updates.

Perf profile collected from our fleet shows that memcg_rstat_updated is
one of the most expensive memcg function i.e. a lot of cumulative CPU
is being spent on it. So, even small micro optimizations matter a lot.

Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 mm/memcontrol.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 421740f1bcdc..ea3e40e589df 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -585,18 +585,20 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
 	cgroup_rstat_updated(memcg->css.cgroup, cpu);
 	statc = this_cpu_ptr(memcg->vmstats_percpu);
 	for (; statc; statc = statc->parent) {
+		/*
+		 * If @memcg is already flushable then all its ancestors are
+		 * flushable as well and also there is no need to increase
+		 * stats_updates.
+		 */
+		if (!memcg_vmstats_needs_flush(statc->vmstats))
+			break;
+

Do you mean "if (memcg_vmstats_needs_flush(statc->vmstats))"?

Cheers, Longman 

 		stats_updates = READ_ONCE(statc->stats_updates) + abs(val);
 		WRITE_ONCE(statc->stats_updates, stats_updates);
 		if (stats_updates < MEMCG_CHARGE_BATCH)
 			continue;
 
-		/*
-		 * If @memcg is already flush-able, increasing stats_updates is
-		 * redundant. Avoid the overhead of the atomic update.
-		 */
-		if (!memcg_vmstats_needs_flush(statc->vmstats))
-			atomic64_add(stats_updates,
-				     &statc->vmstats->stats_updates);
+		atomic64_add(stats_updates, &statc->vmstats->stats_updates);
 		WRITE_ONCE(statc->stats_updates, 0);
 	}
 }
--------------FOQphYEUW0hr01DoyQy8WFR1--