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 39CD4C2BD09 for ; Mon, 24 Jun 2024 12:46:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4574C6B03D6; Mon, 24 Jun 2024 08:46:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 407006B03D7; Mon, 24 Jun 2024 08:46:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2CE446B03D8; Mon, 24 Jun 2024 08:46:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 0ECED6B03D6 for ; Mon, 24 Jun 2024 08:46:47 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 63C86A328C for ; Mon, 24 Jun 2024 12:46:46 +0000 (UTC) X-FDA: 82265756412.08.AA45F6B Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by imf18.hostedemail.com (Postfix) with ESMTP id 75A941C000F for ; Mon, 24 Jun 2024 12:46:44 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="ZER/t/dh"; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719233197; 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=Ok/L+qsaLydWCAvp7RmGLtdoqWXMsaAGOBxzFb7HiAY=; b=HIM5YTwu0W8G0jw5Orha6x+sA10tfW7UeavoXRbTKc3m04nFX/o3L4Xd4EeSwONfjorsXD O6I1CabCYshArCCEE07qwiPnNpanbqtN4k1zdlTrhOVIQ52T/O7SSiiU2DTfxBDKYoa2Vx AAC5Vcj1vXai3+X6R2gzwK4G/jpolwY= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="ZER/t/dh"; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719233197; a=rsa-sha256; cv=none; b=b0k42GBYJrFU0TDg+9xBGTd7oqjYupri3QkrkO7yAlEnD36JebLuc7KDAF5fPOq7+mVEtZ XH4SAzwheTKe0AjBzM1TH6LfP66kxH8/aIs5fFnujnQegWBLqLIIaoti9lRJxOf+m9Sxh3 wJHpqCXOSvwJf/E2TSJXFgyFTPDRoFY= Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-57d1679ee6eso7539597a12.1 for ; Mon, 24 Jun 2024 05:46:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1719233203; x=1719838003; 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=Ok/L+qsaLydWCAvp7RmGLtdoqWXMsaAGOBxzFb7HiAY=; b=ZER/t/dhlSpFzMdCZ0VFlwojp6neWOLszzW1UGteW8Ro7wl9p9+jHtCHx8JlAIt0+M ok5UnSWrQz4EKtsweGb2MXWaIZ7vvsaCsj4IGoi8b9ZVryzKhluJB3Gk2PET8QNty0ae 8KRYp4C17amRKjh5sW6bRgTXfExxskmpq6+Lj2YBx9eHinKCs5U1K6i/xHMIzN1qluCq 3U6Ov16bovOlEwv9bwN6TEoXqroF48OFtya+S6pIDpj/64YO6gywVD4I3HW0rsb+c/kV m+oBd9k2FtJtVKAgOo8pFdm+OUxDCePz/0Eh5k3liRmChxP/wYn+gId6CI/3i5oHlQe7 sBrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719233203; x=1719838003; 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=Ok/L+qsaLydWCAvp7RmGLtdoqWXMsaAGOBxzFb7HiAY=; b=fKokVIZ07MP85NeiGtKcgb5FAvqqIuzGiF7bA3qkf/tuvd91kjOXnSHE/RFH5PbkDG am+XXHpsd9Bmpw6irGypuPnEpAcjiZDaIUblN0ijdu4cidvpLZihmyR5as+4JVgdSItL xqZ7UxfzF/cn19MnkNzYQr9C/Hs1nrjmWgNDxaAwHDMJC3XVNTle8Q7jG/adSgJuQFsD oAbr5ltY8cArK7bacFHRuD3N+fefzXraaeTOBfwUZJgppgReO2n+pp38/DBfpZCg/kRj fq9TwAirvjkIWTpgriUmyFjaGUoX/Mg2IKEG6tyGdEMZO0NEY1/W8dHscdcWBAF0yRCa yj2Q== X-Forwarded-Encrypted: i=1; AJvYcCUxIph1EF41dYNoSrSpj1cejdwXNorCrFic22gF99dZSvsF0AX/CcBKYrTdIAKH74FdA7uPR8FVb1O4WMmXkGDBBNI= X-Gm-Message-State: AOJu0Yx7OArWMHO/s/jmM92dJgBDYI9lDhpPtCdbE8G3/lVdGUFWfGeR lEYfVeavd7jEFZSSnzB1snamF4P0EzRqav4pKNVAalFgTk8YKQtBmZPl3dbES21vDRYELMits+e mSLVYUL/4qT8YepyC9kJQlfrDlgEfGAfTrApK X-Google-Smtp-Source: AGHT+IHuyeA4mdFUDWcWZKcP0BDWsHVxB+XqBMp+KhF/Y8CrEqvVvO6dlvDLHOe5mhsSuFkDFseTix+/TpNAlSbOzEo= X-Received: by 2002:a17:906:bf48:b0:a72:5d7f:dd4a with SMTP id a640c23a62f3a-a725d7fdddemr116449566b.25.1719233202070; Mon, 24 Jun 2024 05:46:42 -0700 (PDT) MIME-Version: 1.0 References: <171923011608.1500238.3591002573732683639.stgit@firesoul> In-Reply-To: <171923011608.1500238.3591002573732683639.stgit@firesoul> From: Yosry Ahmed Date: Mon, 24 Jun 2024 05:46:05 -0700 Message-ID: Subject: Re: [PATCH V2] cgroup/rstat: Avoid thundering herd problem by kswapd across NUMA nodes To: Jesper Dangaard Brouer Cc: tj@kernel.org, cgroups@vger.kernel.org, shakeel.butt@linux.dev, hannes@cmpxchg.org, lizefan.x@bytedance.com, longman@redhat.com, kernel-team@cloudflare.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 75A941C000F X-Stat-Signature: 7z9dkwaj6xnwn4q3fqpfrcbgdszmqucu X-Rspam-User: X-HE-Tag: 1719233204-395520 X-HE-Meta: U2FsdGVkX19jAZQ1Oe29RDXbRWECjAJ4DcidF2Zwmvv2v/AiCngKfB+CSul51pt10guqMJsXNRtXDidiOVqqLhVqu8CZbGRlWICmJOwQcfIsc5+rEF0m+zDwS8cLcR/wt8Tix3/EXyBpCingO7FqF4bry7sHfYJa/jkeW3nTR/Je7QQgkVHDohffmwc6MlPtXAzcmYda21cGmc0MekSGJ4sW/DrwZncKgWlMTHh5MLMqeRt0EsKHja2b4pf76uZRwnwXgPT6w3G18O2LD5orow5RIkI/GsIfI2SrJbPe+HNTa+9SKxNJ7PrdDTSOLEfzWpJzRJGGfZXawgXsJ3qrald2Q0n7O5Qe0QCAGRXGIGfUChe8vusA7RHg2TWDeeNKsNU1QlhHOKCrP062kp047O02C/3eWGrb4quUpPjwR1q/Ey2sdm0XWw4wG8++6jtuspg9Jp4IXzQlG4k5dJbutcHllMjg6+azFxnOXGxOMylrOMMOvnD4FRH+YEyDJVQTZFISzPitlS6Xs7Z9EjDYoytu1tzV2EP7AnNc49RKBVbhT2emmSc1ttRBUaMAXY0qVJOqhCd/TrNvWGdh3LuxYge0trTCspo6wDGv0pyu1GaTPXU4p1B2+M01P1h19UIaNhiCQDpZrWvunOvPcpuzex+AsENVN+42ZXDM02foBmo9kO+M+uXChcSWL6IlGaN3Lqzp0YMe73HI4UdDv3UhhXGHrWannbuutJbdrlCDLHdMnY1fS/JXzWVLX6cI0XtN9LoQ8Sd25I3t5e5kRF5UYO370LxMzN5LgiFRYGCqQOr+ngKJ8DglDNp/C5VPH8UobIuXT8s7Kssn06uFPz3HNbvb8azdYOAduTxGiZ/ZKNVaY4u0XZmGGrYKIklnJnHtPt355jXVULz+ZCFcgvqPCuPtb2GAD9MLSFwopvUThv4LMUV81x/xbYg1Ii65HobalokC08pQxSSfj7Bunni NBXxTX+Y M9nHaj/Bo9onxtKPt+UNKaa2/gx9LNSqKePwL1t+tlwAkq31JMvxwn4qNGt5NZ1IHYL/z1Qv2nhCcMKjMbrzBan5dshNVATPcyavr2tZz/54ZR/kYnsO3YNV2wURDF6JNYFQYkgzW4G8TQe7NO7ImunA7p7YiyN4Mpn1wW5/p6WCoAKml9Xqv6Z5TbHU+rFPBeUXrzWfcQNiBP1aJyGmU/tEzhF3Ke90Qxv2cLbK1+prCnQXEdVJj8apjDAW06LR6O18y X-Bogosity: Ham, tests=bogofilter, spamicity=0.000020, 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, Jun 24, 2024 at 4:55=E2=80=AFAM Jesper Dangaard Brouer wrote: > > Avoid lock contention on the global cgroup rstat lock caused by kswapd > starting on all NUMA nodes simultaneously. At Cloudflare, we observed > massive issues due to kswapd and the specific mem_cgroup_flush_stats() > call inlined in shrink_node, which takes the rstat lock. > > On our 12 NUMA node machines, each with a kswapd kthread per NUMA node, > we noted severe lock contention on the rstat lock. This contention > causes 12 CPUs to waste cycles spinning every time kswapd runs. > Fleet-wide stats (/proc/N/schedstat) for kthreads revealed that we are > burning an average of 20,000 CPU cores fleet-wide on kswapd, primarily > due to spinning on the rstat lock. > > To help reviewer follow code: When the Per-CPU-Pages (PCP) freelist is > empty, __alloc_pages_slowpath calls wake_all_kswapds(), causing all > kswapdN threads to wake up simultaneously. The kswapd thread invokes > shrink_node (via balance_pgdat) triggering the cgroup rstat flush > operation as part of its work. This results in kernel self-induced rstat > lock contention by waking up all kswapd threads simultaneously. > Leveraging this detail: balance_pgdat() have NULL value in > target_mem_cgroup, this cause mem_cgroup_flush_stats() to do flush with > root_mem_cgroup. > > To resolve the kswapd issue, we generalized the "stats_flush_ongoing" > concept to apply to all users of cgroup rstat, not just memcg. This > concept was originally reverted in commit 7d7ef0a4686a ("mm: memcg: > restore subtree stats flushing"). If there is an ongoing rstat flush, > limited to the root cgroup, the flush is skipped. This is effective as > kswapd operates on the root tree, sufficiently mitigating the thundering > herd problem. > > This lowers contention on the global rstat lock, although limited to the > root cgroup. Flushing cgroup subtree's can still lead to lock contention. > > Fixes: 7d7ef0a4686a ("mm: memcg: restore subtree stats flushing"). > Signed-off-by: Jesper Dangaard Brouer > --- > V1: https://lore.kernel.org/all/171898037079.1222367.13467317484793748519= .stgit@firesoul/ > RFC: https://lore.kernel.org/all/171895533185.1084853.3033751561302228252= .stgit@firesoul/ > > include/linux/cgroup.h | 5 +++++ > kernel/cgroup/rstat.c | 25 +++++++++++++++++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h > index 2150ca60394b..ad41cca5c3b6 100644 > --- a/include/linux/cgroup.h > +++ b/include/linux/cgroup.h > @@ -499,6 +499,11 @@ static inline struct cgroup *cgroup_parent(struct cg= roup *cgrp) > return NULL; > } > > +static inline bool cgroup_is_root(struct cgroup *cgrp) > +{ > + return cgroup_parent(cgrp) =3D=3D NULL; > +} > + > /** > * cgroup_is_descendant - test ancestry > * @cgrp: the cgroup to be tested > diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c > index fb8b49437573..2591840b6dc1 100644 > --- a/kernel/cgroup/rstat.c > +++ b/kernel/cgroup/rstat.c > @@ -11,6 +11,7 @@ > > static DEFINE_SPINLOCK(cgroup_rstat_lock); > static DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_cpu_lock); > +static atomic_t root_rstat_flush_ongoing =3D ATOMIC_INIT(0); > > static void cgroup_base_stat_flush(struct cgroup *cgrp, int cpu); > > @@ -350,8 +351,25 @@ __bpf_kfunc void cgroup_rstat_flush(struct cgroup *c= grp) > { > might_sleep(); > > + /* > + * This avoids thundering herd problem on global rstat lock. When= an > + * ongoing flush of the entire tree is in progress, then skip flu= sh. > + */ > + if (atomic_read(&root_rstat_flush_ongoing)) > + return; > + > + /* Grab right to be ongoing flusher, return if loosing race */ > + if (cgroup_is_root(cgrp) && > + atomic_xchg(&root_rstat_flush_ongoing, 1)) > + return; > + I am assuming this supersedes your other patch titled "[PATCH RFC] cgroup/rstat: avoid thundering herd problem on root cgrp", so I will only respond here. I have two comments: - There is no reason why this should be limited to the root cgroup. We can keep track of the cgroup being flushed, and use cgroup_is_descendant() to find out if the cgroup we want to flush is a descendant of it. We can use a pointer and cmpxchg primitives instead of the atomic here IIUC. - More importantly, I am not a fan of skipping the flush if there is an ongoing one. For all we know, the ongoing flush could have just started and the stats have not been flushed yet. This is another example of non deterministic behavior that could be difficult to debug. I tried a similar approach before where we sleep and wait for the ongoing flush to complete instead, without contending on the lock, using completions [1]. Although that patch has a lot of complexity, I think just using completions to wait for the ongoing flush may be the right way to go, assuming it also helps with the problem you are facing. [1]https://lore.kernel.org/lkml/20230913073846.1528938-4-yosryahmed@google.= com/ > __cgroup_rstat_lock(cgrp, -1); > + > cgroup_rstat_flush_locked(cgrp); > + > + if (cgroup_is_root(cgrp)) > + atomic_set(&root_rstat_flush_ongoing, 0); > + > __cgroup_rstat_unlock(cgrp, -1); > } > > @@ -362,13 +380,20 @@ __bpf_kfunc void cgroup_rstat_flush(struct cgroup *= cgrp) > * Flush stats in @cgrp's subtree and prevent further flushes. Must be > * paired with cgroup_rstat_flush_release(). > * > + * Current invariant, not called with root cgrp. > + * > * This function may block. > */ > void cgroup_rstat_flush_hold(struct cgroup *cgrp) > __acquires(&cgroup_rstat_lock) > { > might_sleep(); > + > __cgroup_rstat_lock(cgrp, -1); > + > + if (atomic_read(&root_rstat_flush_ongoing)) > + return; > + > cgroup_rstat_flush_locked(cgrp); > } > > >