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 95165CD98CA for ; Tue, 10 Oct 2023 22:22:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 213658E0001; Tue, 10 Oct 2023 18:22:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 19BD98D00E7; Tue, 10 Oct 2023 18:22:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 016B98E0001; Tue, 10 Oct 2023 18:22:29 -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 E091B8D00E7 for ; Tue, 10 Oct 2023 18:22:29 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 95CD01603D3 for ; Tue, 10 Oct 2023 22:22:29 +0000 (UTC) X-FDA: 81330976818.09.5C6B395 Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by imf13.hostedemail.com (Postfix) with ESMTP id BD48020019 for ; Tue, 10 Oct 2023 22:22:27 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=MCUqPvax; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696976547; 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=D9ytv1y2yvSa4MvbD/ZRdqp1EA4N80KA40csq2dEErQ=; b=eeW14KOR8zVf6LhRIM8BT/wfc5evMTrj/IOkAS+7orAhOKLyragpFvs1JhkvcvMQtC+qYZ YK8txnm5uAo520TUVTRTDowCUyeyxCS2SWNkb869WW358N7b99R7Dfv+zYVJ177A15roRz Dcs1/v/uy3aziAL6qctdcvcVuTAu2QA= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=MCUqPvax; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696976547; a=rsa-sha256; cv=none; b=CwLMgs4SbtrUv43/GB438UIOHKyEG4+0cPnfHCu9Vkl8hKKaschwiGPMvzdUeIuwiWk+Q+ yGji+Jq8cuXWIaxlRyWnrpX3vOE6bYJLYaEi4Rx2w3IpJwEIwcVnvGNV+WT6dNZf48Xg+A yzJgCGp3To40IbXK2I2T37xPcF9mzzc= Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-99c3c8adb27so1047847666b.1 for ; Tue, 10 Oct 2023 15:22:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1696976546; x=1697581346; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=D9ytv1y2yvSa4MvbD/ZRdqp1EA4N80KA40csq2dEErQ=; b=MCUqPvax9kYNHEKSAwG/fJEXkGw26xYVSv99eBMaQ7LURnkc2540L1f2zFj1ohGlZX nbrr+3SbeqLvGHFct/Ld81npWCxVkQTIqS47bil1Mux53pSEVRbleUketNurqPdZiDMp sZka9nBqxQZMW44viYT7TzBDLXf+z66o6JwAKYeamBq8y7fuCw3kWZdV4TXMBeq5US95 icbAbKZFah4CfAMtFMt3Bk2AEE61obH47rXjJzV4QFm18Lt2SwmZreEpW5NfLSkipPv/ YY2NcVLCBQBnixoIUrlYUGLKLYFfRBNtWX7xJKrBLEquH1ij438KlLziiQ8uNprpPyiC BpVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696976546; x=1697581346; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=D9ytv1y2yvSa4MvbD/ZRdqp1EA4N80KA40csq2dEErQ=; b=lEpUDbZ9bEcPj+ruw3A1wRj/f8GAXxd65rJYVUWu0uF9u9kk2f6ooVPMCIH2ZCe7r2 R0c6W/TfhChOz79XflTGx17Rpm1LmltDcOf/5gLsLD7E4j5GJ2uF3fb5VLIfgIa3Ram5 1qwU4+iwaE9a6q0sFchlJj1OA7yDw2bVwgRRaJ45YDFzTzQbBAlfRzQUNiP2GsbtiBc5 U6tpxF/pebwV9HW/BqH5pjn8umg8qNZefGLb5PbD+5qKfQS0PWPVuhaIrwXkpFb3yZ0Q ZSKN0gwbtme09B2DwfdEkFDUhq3JCHUbr9DlfbuonUckWBBIT1apWbFacsexOP97/K4G ONeg== X-Gm-Message-State: AOJu0YwjZ/TMpv6KOzjJOi8z5gsEhMbsW2+SD+YpSimoDXh6NJAdG78x LxZhWLxOTqSJ1CUSKTB990KxOUoM3BEfmAF13Qnodg== X-Google-Smtp-Source: AGHT+IGiBCpXck43PPvlJ3+rqPaZEMLPOQYkkdbd1Jrk+SG9At/A+vBYi5VVZg79jP3TkAytynbWMp6WW63no4Z9iIY= X-Received: by 2002:a17:906:5349:b0:9b2:8b14:7a20 with SMTP id j9-20020a170906534900b009b28b147a20mr18547846ejo.45.1696976546026; Tue, 10 Oct 2023 15:22:26 -0700 (PDT) MIME-Version: 1.0 References: <20231010032117.1577496-1-yosryahmed@google.com> <20231010032117.1577496-4-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Tue, 10 Oct 2023 15:21:47 -0700 Message-ID: Subject: Re: [PATCH v2 3/5] mm: memcg: make stats flushing threshold per-memcg To: Shakeel Butt Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Ivan Babrou , Tejun Heo , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Waiman Long , kernel-team@cloudflare.com, Wei Xu , Greg Thelen , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: BD48020019 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: d843bjaaqirre9c9y34rempnpjywupza X-HE-Tag: 1696976547-712418 X-HE-Meta: U2FsdGVkX1/BHlKYLB+mSzpNbEtKHAq2cLQQO/csEA37o/fkaeGALpsdiCaF4W13oJDnPazT8BOjYkBqRYB4HZXDjOVKqPNq4AX5rZLia3lFFTVnAJGH6ryytarL5TYkiAFA0coUZF3hEmKgKT1bgVq8VospIxz7nqMHofOylq5DecUjlzlgDpH97nH5nwkaBCg9jGHafDMAuoHwbLgmM9v1lsMU2ZHNg2XwT5l3yvm2UBfEpapaSx51ROqFVegfLoIjOFFUAPkmYmdUT9i4+d9cF+hojkN3emIEM4itra4Z0B7OjKal9YrHP0SiJKmzegMHEoYtCURTrbwTsp0rLmcoUCUBQJqoNsSMreMVd/McYg0HcaHLzzGF+SeqTqNTW9LIFW/p+EEtnFbWiE2Vc8/rRafIX2zar907lttkwGhVO4AjI27CJSZ11JThQEdsNmslB691crd3XBtYKOrBm4QOmieDi6PHqZ+pgIaUnl4UtYDoyxg23zMyVXBL+86NZl0B0Vj+C3HFPNM9DSYdsTEezSKNSTaxG3h0tPlXLkZ8njYXbJK5QdVIQXBKxST5zXqc9yuVDnU2nI4D9n+NNGwemvzNXK24I9in2Mf8MIHq576X6ccQcv/yoMpnafNqgCDmf6Ywa5GejzNVcRMnu/fEogAy+srre0R4eYVIl991S09jegEfPsqNGRMfRLqQ0pP00Kv26YdFKN8y0Y1QV90WR8jOksfkQy8afUaxE7Erta6FcievFBj/8al83/Uce0laUHWoe+ETPNiqqN6obRc0ION9JYUpYryRWUiU1MGzOiLAQ2VgdLyCevA1kcFh4j3JKHKoQZ8ceANJXOxOo8mwhltWYGFvG+79iDwcTHA3tt42TKhdPrFycFDu+lhnQl1lt2AAAEq6UIys4RaJC7suFHjF51p76XDO57QRUkbtuAl4KbXdb6mWJC7+YbOVTcMBW/IostXZOx/dZHe nqNFl8WH dMniO94CoAiqbC+iUmOcslKpLiZNHPXO5qShW1ZfQD0wjkOLW+rhHExtrFMNHIXmbtGjNXoVGesKuZ7BRnU1GLjuyIyHfAp1mmZiMSl1AzGZpkTpM473EiXRRF7CszIBiPzTz7GOLKtY7Iy6q+ZnwYY9IzhEZz2O5ko+v X-Bogosity: Ham, tests=bogofilter, spamicity=0.000016, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Oct 10, 2023 at 2:02=E2=80=AFPM Yosry Ahmed = wrote: > > On Tue, Oct 10, 2023 at 1:45=E2=80=AFPM Shakeel Butt wrote: > > > > On Mon, Oct 9, 2023 at 8:21=E2=80=AFPM Yosry Ahmed wrote: > > > > > > A global counter for the magnitude of memcg stats update is maintaine= d > > > on the memcg side to avoid invoking rstat flushes when the pending > > > updates are not significant. This avoids unnecessary flushes, which a= re > > > not very cheap even if there isn't a lot of stats to flush. It also > > > avoids unnecessary lock contention on the underlying global rstat loc= k. > > > > > > Make this threshold per-memcg. The scheme is followed where percpu (n= ow > > > also per-memcg) counters are incremented in the update path, and only > > > propagated to per-memcg atomics when they exceed a certain threshold. > > > > > > This provides two benefits: > > > (a) On large machines with a lot of memcgs, the global threshold can = be > > > reached relatively fast, so guarding the underlying lock becomes less > > > effective. Making the threshold per-memcg avoids this. > > > > > > (b) Having a global threshold makes it hard to do subtree flushes, as= we > > > cannot reset the global counter except for a full flush. Per-memcg > > > counters removes this as a blocker from doing subtree flushes, which > > > helps avoid unnecessary work when the stats of a small subtree are > > > needed. > > > > > > Nothing is free, of course. This comes at a cost: > > > (a) A new per-cpu counter per memcg, consuming NR_CPUS * NR_MEMCGS * = 4 > > > bytes. The extra memory usage is insigificant. > > > > > > (b) More work on the update side, although in the common case it will > > > only be percpu counter updates. The amount of work scales with the > > > number of ancestors (i.e. tree depth). This is not a new concept, add= ing > > > a cgroup to the rstat tree involves a parent loop, so is charging. > > > Testing results below show no significant regressions. > > > > > > (c) The error margin in the stats for the system as a whole increases > > > from NR_CPUS * MEMCG_CHARGE_BATCH to NR_CPUS * MEMCG_CHARGE_BATCH * > > > NR_MEMCGS. This is probably fine because we have a similar per-memcg > > > error in charges coming from percpu stocks, and we have a periodic > > > flusher that makes sure we always flush all the stats every 2s anyway= . > > > > > > This patch was tested to make sure no significant regressions are > > > introduced on the update path as follows. The following benchmarks we= re > > > ran in a cgroup that is 4 levels deep (/sys/fs/cgroup/a/b/c/d), which= is > > > deeper than a usual setup: > > > > > > (a) neper [1] with 1000 flows and 100 threads (single machine). The > > > values in the table are the average of server and client throughputs = in > > > mbps after 30 iterations, each running for 30s: > > > > > > tcp_rr tcp_stream > > > Base 9504218.56 357366.84 > > > Patched 9656205.68 356978.39 > > > Delta +1.6% -0.1% > > > Standard Deviation 0.95% 1.03% > > > > > > An increase in the performance of tcp_rr doesn't really make sense, b= ut > > > it's probably in the noise. The same tests were ran with 1 flow and 1 > > > thread but the throughput was too noisy to make any conclusions (the > > > averages did not show regressions nonetheless). > > > > > > Looking at perf for one iteration of the above test, __mod_memcg_stat= e() > > > (which is where memcg_rstat_updated() is called) does not show up at = all > > > without this patch, but it shows up with this patch as 1.06% for tcp_= rr > > > and 0.36% for tcp_stream. > > > > > > (b) "stress-ng --vm 0 -t 1m --times --perf". I don't understand > > > stress-ng very well, so I am not sure that's the best way to test thi= s, > > > but it spawns 384 workers and spits a lot of metrics which looks nice= :) > > > I picked a few ones that seem to be relevant to the stats update path= . I > > > also included cache misses as this patch introduce more atomics that = may > > > bounce between cpu caches: > > > > > > Metric Base Patched Delta > > > Cache Misses 3.394 B/sec 3.433 B/sec +1.14% > > > Cache L1D Read 0.148 T/sec 0.154 T/sec +4.05% > > > Cache L1D Read Miss 20.430 B/sec 21.820 B/sec +6.8% > > > Page Faults Total 4.304 M/sec 4.535 M/sec +5.4% > > > Page Faults Minor 4.304 M/sec 4.535 M/sec +5.4% > > > Page Faults Major 18.794 /sec 0.000 /sec > > > Kmalloc 0.153 M/sec 0.152 M/sec -0.65% > > > Kfree 0.152 M/sec 0.153 M/sec +0.65% > > > MM Page Alloc 4.640 M/sec 4.898 M/sec +5.56% > > > MM Page Free 4.639 M/sec 4.897 M/sec +5.56% > > > Lock Contention Begin 0.362 M/sec 0.479 M/sec +32.32% > > > Lock Contention End 0.362 M/sec 0.479 M/sec +32.32% > > > page-cache add 238.057 /sec 0.000 /sec > > > page-cache del 6.265 /sec 6.267 /sec -0.03% > > > > > > This is only using a single run in each case. I am not sure what to > > > make out of most of these numbers, but they mostly seem in the noise > > > (some better, some worse). The lock contention numbers are interestin= g. > > > I am not sure if higher is better or worse here. No new locks or lock > > > sections are introduced by this patch either way. > > > > > > Looking at perf, __mod_memcg_state() shows up as 0.00% with and witho= ut > > > this patch. This is suspicious, but I verified while stress-ng is > > > running that all the threads are in the right cgroup. > > > > > > (3) will-it-scale page_fault tests. These tests (specifically > > > per_process_ops in page_fault3 test) detected a 25.9% regression befo= re > > > for a change in the stats update path [2]. These are the > > > numbers from 30 runs (+ is good): > > > > > > LABEL | MEAN | MEDIAN | STDDEV = | > > > ------------------------------+-------------+-------------+----------= --- > > > page_fault1_per_process_ops | | | = | > > > (A) base | 265207.738 | 262941.000 | 12112.379= | > > > (B) patched | 249249.191 | 248781.000 | 8767.457 = | > > > | -6.02% | -5.39% | = | > > > page_fault1_per_thread_ops | | | = | > > > (A) base | 241618.484 | 240209.000 | 10162.207= | > > > (B) patched | 229820.671 | 229108.000 | 7506.582 = | > > > | -4.88% | -4.62% | = | > > > page_fault1_scalability | | | > > > (A) base | 0.03545 | 0.035705 | 0.0015837= | > > > (B) patched | 0.029952 | 0.029957 | 0.0013551= | > > > | -9.29% | -9.35% | = | > > > > This much regression is not acceptable. > > > > In addition, I ran netperf with the same 4 level hierarchy as you have > > run and I am seeing ~11% regression. > > Interesting, I thought neper and netperf should be similar. Let me try > to reproduce this. > > Thanks for testing! > > > > > More specifically on a machine with 44 CPUs (HT disabled ixion machine)= : > > > > # for server > > $ netserver -6 > > > > # 22 instances of netperf clients > > $ netperf -6 -H ::1 -l 60 -t TCP_SENDFILE -- -m 10K > > > > (averaged over 4 runs) > > > > base (next-20231009): 33081 MBPS > > patched: 29267 MBPS > > > > So, this series is not acceptable unless this regression is resolved. I tried this on a machine with 72 cpus (also ixion), running both netserver and netperf in /sys/fs/cgroup/a/b/c/d as follows: # echo "+memory" > /sys/fs/cgroup/cgroup.subtree_control # mkdir /sys/fs/cgroup/a # echo "+memory" > /sys/fs/cgroup/a/cgroup.subtree_control # mkdir /sys/fs/cgroup/a/b # echo "+memory" > /sys/fs/cgroup/a/b/cgroup.subtree_control # mkdir /sys/fs/cgroup/a/b/c # echo "+memory" > /sys/fs/cgroup/a/b/c/cgroup.subtree_control # mkdir /sys/fs/cgroup/a/b/c/d # echo 0 > /sys/fs/cgroup/a/b/c/d/cgroup.procs # ./netserver -6 # echo 0 > /sys/fs/cgroup/a/b/c/d/cgroup.procs # for i in $(seq 10); do ./netperf -6 -H ::1 -l 60 -t TCP_SENDFILE -- -m 10K; done Base: 540000 262144 10240 60.00 54613.89 540000 262144 10240 60.00 54940.52 540000 262144 10240 60.00 55168.86 540000 262144 10240 60.00 54800.15 540000 262144 10240 60.00 54452.55 540000 262144 10240 60.00 54501.60 540000 262144 10240 60.00 55036.11 540000 262144 10240 60.00 52018.91 540000 262144 10240 60.00 54877.78 540000 262144 10240 60.00 55342.38 Average: 54575.275 Patched: 540000 262144 10240 60.00 53694.86 540000 262144 10240 60.00 54807.68 540000 262144 10240 60.00 54782.89 540000 262144 10240 60.00 51404.91 540000 262144 10240 60.00 55024.00 540000 262144 10240 60.00 54725.84 540000 262144 10240 60.00 51400.40 540000 262144 10240 60.00 54212.63 540000 262144 10240 60.00 51951.47 540000 262144 10240 60.00 51978.27 Average: 53398.295 That's ~2% regression. Did I do anything incorrectly?