Received: by 2002:a05:6358:11c7:b0:104:8066:f915 with SMTP id i7csp4849507rwl; Tue, 28 Mar 2023 12:11:33 -0700 (PDT) X-Google-Smtp-Source: AK7set8DF1W5LQQTJuGfJGTTWvqZNQ/Y+gwd33K5JT0FeLBEQAmp3HtoFC2s8Y8vuN3JHl90L3cc X-Received: by 2002:aa7:8f3c:0:b0:627:6328:79d7 with SMTP id y28-20020aa78f3c000000b00627632879d7mr13598903pfr.34.1680030693165; Tue, 28 Mar 2023 12:11:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680030693; cv=none; d=google.com; s=arc-20160816; b=tENI30OHaPmFNPteFMo+NLm5yjk2g1B1EPYIN+kim5je4VRsvSu+glY8BBTtrAkhL+ 4B1MfgfoXT3W+Dg+p0hdIxiZNaf0EG6o9XDYzKfggg9vSyB2XsPUEgzibNXXn38Dp9kG 0O3ZEQL9vJDUNR4+0E1eltDx/NL+ntOM6Z87LEk3W5EMix7AgY3H9Ev30AfXIuqJLOVd 7jPsuKYQ3FCyqV9p53X5smX/42HNIS3f9GNzpWvp0kanrjsm5rq8XEdcYGKvWaHiP1qJ kj2DLRMLWqOzLLGHF/9WfYlYE21lXRHZS71LKJbvxCRrETUqNnP60q5KrIE5fX+vcXz1 Y75A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=Knz0ICKyVQWqTN98QRlsl94SOUBbAgJSfpknOWcWIz8=; b=EUlYCx7nZd9Q31o/XxgI3IwdxzcJxuGt/QbRdKCSonnfT8RwiiqCXTqj7UqBYCKada NX4+KZwSSvxod5aEhrpBxchsKOgxKTa/lSzrEZPUwnlwvfSJLMxChIhHZavyLvGVJl6I yDxmOSeD4ef/+hHwe78TekK+SdpfnIr1jNXjnl445BTbmlRmQ8FamaH7WAuM6ct6Q0GD i7uUDEhX6yIzXWRwJo9gXbkxEAA257+v2JjgAghzIDB8I7Y3FQfo2sL4fkmWUTRmFwQY f8X/D6PPANHKfS4ffAyaAOk+skNrhBLAwtB8/6oSsNKCG5fl7JXamBEp8Cna+KncSmPW UR5w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=eBFPS5up; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id t11-20020a056a0021cb00b00628a75401bdsi16549033pfj.142.2023.03.28.12.11.21; Tue, 28 Mar 2023 12:11:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=eBFPS5up; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229891AbjC1Sxa (ORCPT + 99 others); Tue, 28 Mar 2023 14:53:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55250 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229686AbjC1Sx3 (ORCPT ); Tue, 28 Mar 2023 14:53:29 -0400 Received: from mail-ed1-x529.google.com (mail-ed1-x529.google.com [IPv6:2a00:1450:4864:20::529]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CDFC1110 for ; Tue, 28 Mar 2023 11:53:27 -0700 (PDT) Received: by mail-ed1-x529.google.com with SMTP id eg48so53615787edb.13 for ; Tue, 28 Mar 2023 11:53:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680029606; 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=Knz0ICKyVQWqTN98QRlsl94SOUBbAgJSfpknOWcWIz8=; b=eBFPS5upevzYowGb39P08hDPA5ByzP8CKwYQMFgOKj+6IYq2Vkfhm5oL5cVrbZhHzf eQHkB8C313lW8nsV9BpZNUZGGFqiswJwY5FMsPU6z/cd/O9XwAIDMFqeGPfx35miDyrF +bGkozLip971EeAk9h6UC0HOXtdfFwRiZJkEUoXuLJ3fBqgLl1/lbS6hfhsmfL9XRhH+ KR9XbqwM57zERf5JS531f6qDu8XI49Nmmt2jHNhErVh4imKzomY5+lfULAhHKaJb3jM2 Rye/OtfKqLqEEIMO08YuFZgxXxl9NZRbCLkFj6dKWKMGFVVM0dFa3ui8jA4/djrvfILk dA1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680029606; 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=Knz0ICKyVQWqTN98QRlsl94SOUBbAgJSfpknOWcWIz8=; b=UloDPd/iJE6DonFS6OBLDEEAzLa6qdWv3vLDJON6w2ZUfxOp3ZtJoXcX12m77YFLcW zQHBJ7NBqrLvOkvVwtOUCpjYUBuaikDE5ZmjQOU8My771LkwJzpIMOQlxZKSVDXpMotA 1KmV4CdgMlFj8RqMho14XFh9S+ZO0JW3njYD9uE7i+0ap6aejZh9WtPROPE6aA/izL2x TWWfl04v8k+Pb3PVfFuE8VtHtAJL64RCE+YwjIQsg2z5sjTDh4RE5OfrLw2EtjK9bYsB XKPo2RYsXIqm0qN9KGRHcW9VUUhFmNaSNDrNfHf9Odr65yn089NUkeCKXLac7ctD8Qlw tPcQ== X-Gm-Message-State: AAQBX9eSfsmYus9QNMPLAGdg+yT7LUwIKxwi1urXH5Ex51j86G2DDnuc hq5LVhSeH/8oVE3JlQ5cM/9vJNXGJjj70rt/drjepQ== X-Received: by 2002:a17:907:9870:b0:8b1:28f6:8ab3 with SMTP id ko16-20020a170907987000b008b128f68ab3mr8771567ejc.15.1680029606204; Tue, 28 Mar 2023 11:53:26 -0700 (PDT) MIME-Version: 1.0 References: <20230328061638.203420-1-yosryahmed@google.com> <20230328061638.203420-6-yosryahmed@google.com> <20230328141523.txyhl7wt7wtvssea@google.com> In-Reply-To: <20230328141523.txyhl7wt7wtvssea@google.com> From: Yosry Ahmed Date: Tue, 28 Mar 2023 11:52:50 -0700 Message-ID: Subject: Re: [PATCH v1 5/9] memcg: replace stats_flush_lock with an atomic To: Shakeel Butt Cc: Tejun Heo , Josef Bacik , Jens Axboe , Zefan Li , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Vasily Averin , cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, bpf@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-15.7 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,ENV_AND_HDR_SPF_MATCH, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 28, 2023 at 7:15=E2=80=AFAM Shakeel Butt = wrote: > > On Tue, Mar 28, 2023 at 06:16:34AM +0000, Yosry Ahmed wrote: > [...] > > @@ -585,8 +585,8 @@ mem_cgroup_largest_soft_limit_node(struct mem_cgrou= p_tree_per_node *mctz) > > */ > > static void flush_memcg_stats_dwork(struct work_struct *w); > > static DECLARE_DEFERRABLE_WORK(stats_flush_dwork, flush_memcg_stats_dw= ork); > > -static DEFINE_SPINLOCK(stats_flush_lock); > > static DEFINE_PER_CPU(unsigned int, stats_updates); > > +static atomic_t stats_flush_ongoing =3D ATOMIC_INIT(0); > > static atomic_t stats_flush_threshold =3D ATOMIC_INIT(0); > > static u64 flush_next_time; > > > > @@ -636,15 +636,18 @@ static inline void memcg_rstat_updated(struct mem= _cgroup *memcg, int val) > > > > static void __mem_cgroup_flush_stats(void) > > { > > - unsigned long flag; > > - > > - if (!spin_trylock_irqsave(&stats_flush_lock, flag)) > > + /* > > + * We always flush the entire tree, so concurrent flushers can ju= st > > + * skip. This avoids a thundering herd problem on the rstat globa= l lock > > + * from memcg flushers (e.g. reclaim, refault, etc). > > + */ > > + if (atomic_xchg(&stats_flush_ongoing, 1)) > > Have you profiled this? I wonder if we should replace the above with > > if (atomic_read(&stats_flush_ongoing) || atomic_xchg(&stats_flush= _ongoing, 1)) I profiled the entire series with perf and I haven't noticed a notable difference between before and after the patch series -- but maybe some specific access patterns cause a regression, not sure. Does an atomic_cmpxchg() satisfy the same purpose? it's easier to read / more concise I guess. Something like if (atomic_cmpxchg(&stats_flush_ongoing, 0, 1)) WDYT? > > to not always dirty the cacheline. This would not be an issue if there > is no cacheline sharing but I suspect percpu stats_updates is sharing > the cacheline with it and may cause false sharing with the parallel stat > updaters (updaters only need to read the base percpu pointer). > > Other than that the patch looks good.