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 DFE8CC6FD18 for ; Tue, 28 Mar 2023 19:43:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 662916B0071; Tue, 28 Mar 2023 15:43:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 637E66B0072; Tue, 28 Mar 2023 15:43:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 527196B0074; Tue, 28 Mar 2023 15:43:10 -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 446566B0071 for ; Tue, 28 Mar 2023 15:43:10 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 10B7080B48 for ; Tue, 28 Mar 2023 19:43:10 +0000 (UTC) X-FDA: 80619330540.25.B95D129 Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) by imf15.hostedemail.com (Postfix) with ESMTP id 2526CA0004 for ; Tue, 28 Mar 2023 19:43:07 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=TIEXaRQD; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.42 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=1680032588; 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=nqIPDr+ZqfoCi8eQc3DTNr+ThF98a/RBvzrvNFqWYDM=; b=vwaxhJ+RKyEed1Zbv73icr+RSYO8KPZpU5FpOJR29RuISz/R06wit40rBIyh3845Tlh6mj FY38l48V9H9KgrtwWIDutiMqbb+DpOT5skzvDXv0MsOCHVR1t7Wfxnc6uboPg6JXCHd5ID uOMTZzTJdtV0VpmIyTIm3DkPgH6+RiY= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=TIEXaRQD; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.42 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=1680032588; a=rsa-sha256; cv=none; b=PQ8bBisuzakKLV1bA/IjuvOpk3EDsTRsFAOr4K1BkRpcjTGSL17RB0xsOplwnud7j5V969 9sZKRcpaN2Ja3vQlqC4P4vrzbFaq/J5lS6b4Nihky/8ouGuXbRltb0x9HbdLQNZRmO1TPf pUTKkytrvZu8MeXRVAJZ9FP6aVHPwMQ= Received: by mail-ed1-f42.google.com with SMTP id eg48so54130483edb.13 for ; Tue, 28 Mar 2023 12:43:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680032586; 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=nqIPDr+ZqfoCi8eQc3DTNr+ThF98a/RBvzrvNFqWYDM=; b=TIEXaRQDBAZ6mBWE2R4TELhNaijNMDedZH3kg6n0ljtndAxJLXXqyFQwcErH6B46Lz U9BN+2qoq3bDs+Ypbo1k5NzI/RQIygK7ia03C7cfrRO0ByWfBXsdRCT0alHvfQTfmvRe qv9IocyC7Cjb3idOoIKKeNX5YVur0irro0khGXutbTnhV67ua5rmPZ8USHumeZuOnxd8 bw+LUtnZMfNIRTh+++D2tITNO4f5XHgKLCv401nXpXS49KzLZKP/0ycEI4wvThEYgq34 SdVY6fUmkfV2iCF/Uacl+jKfcVLSr1puNKsWanMsSlWfcXyo+ZpeNqbzs0La62nvhfVM xa2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680032586; 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=nqIPDr+ZqfoCi8eQc3DTNr+ThF98a/RBvzrvNFqWYDM=; b=Ctp1kmlVeukjimNY0eWqYVC9AGTaLRMWMEmFAmzyKLJ3uSxPZ4on5GVN5Rg+bZM5Fq I3HgBpp3cDFRnLED63KwjJkX8Modm3uEzVQVzBM50SnT1nETQHxdLUUXB4kCYnmmPfpv 0Bi8WeK6WslAKIsMqgCf70zE0ihgyGA1q0ZzNazJFGQCXrJqoQYHzuWG/xu1rrefEOdG WZEk2K/UYEUsYiFQWCsUOdkO5aLsoe46p+ASnW4TzSmUsPXMh4mjmYIjmz7G78qa1aTo 6dpH6SLamzDWlKPyTyPwH9S9MqKNF1j5O1Tx1pocOV5bqjZxypknTwHclpje7vpUmndM UuNg== X-Gm-Message-State: AAQBX9fzXhX6bxBbNSOTuiYTjBmzNI6GUUklHBXRCQud7/qlP1FRqgrR Cmc2L/XxkDrJ0BRa0ck4GIlY3vZ4+YIhU/A15tRRSw== X-Google-Smtp-Source: AKy350Y+r8YQubYTs8R7PJEitaPs1q0OlTz1iemnZQ7KI+3/VnM/MXrQyknZBxEGhPVmnt+/5hUtEtoV/kv4VWDou8E= X-Received: by 2002:a50:a444:0:b0:500:5463:35de with SMTP id v4-20020a50a444000000b00500546335demr8511392edb.8.1680032586465; Tue, 28 Mar 2023 12:43:06 -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: From: Yosry Ahmed Date: Tue, 28 Mar 2023 12:42:30 -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-Stat-Signature: q76q8u81iy8jqb3xjq3847p35akxqcu9 X-Rspam-User: X-Rspamd-Queue-Id: 2526CA0004 X-Rspamd-Server: rspam06 X-HE-Tag: 1680032587-112553 X-HE-Meta: U2FsdGVkX1+ODOJDCxsn2oRCh0mwrkZQVwbi0JNFOc3sRfVomsIm7A2sT/4Jov0209FkVNCODTvRdKU9YvEOjhs5jwy+qJcTD6y/q1Lw52Lx1PTUfpDdi5fVHQKzJaoCLb6RbMLLZ/iBgmJGYSOaICJxTvmWIKUmWP600Kmy7lQmL1ehkgsK7ltP3gvnOs77PzUgNaCSSBI37Uc0STtyr2TF53b8RNoDIcz1dlKWo7Zy+lswndlLqOXN6w0/5eJe8robkbrqvJ9VTpuBNVrMmmFAep/Mlq4Pv7JhKdZtPMMjM1n7m3O/uY5i7NV5wMmygV8rlLcljO7xRj7oQgw8rIuKnJmCnthQVdrpXWhn5mtReUuy552kVBBjGJ1qHper0PYIv2NATSJE0KRTIqKDTagCVtpO0nXJvbU2JF7wNdzi3Zs/H23Na1GJzMRZC6mz8yEP7GsBBKeHvBqWh12d7jMU2gxU+UbWi7bZEDLCY/lYLuQQfuxhS5WDNSniSgneF4FmpSsTiKr9eNdVX+ekEQCs7JCgz6SXxoK8lzpWftJIYn46GHWJxDrqTATtTf7kFFDBUzcfhgZO1U+mS9Lghmd2uj/beAmsjmOm9760xqPk23RLaB/S+DZJ2YObMBSjl80dBb8VevjoHq4IEJiWDqe6O1ORUrRZNR/gHXBZ+fsTgzF0KY3Bk2p77NoeZ5TWrrZSekxXt+aiYm7iZrmaZbW7wHcacCgO0PV8kfMTCW66pGMlUDWYViReM+qUWSzMeMDUdRBuf/Euc+p4iwtybNmDT7N3ITAHe0pokLkhIvYZlOpN6t8Wvor3smlUYm6Yai3Cq+QLgIsl/cjoc+jlrxVaNo5u6gBKjnaWD/nIxBKFX9WpGOgDp2G6jGkvwXIeNslU0k2h0wDUvmhaqvnn+e1DhYA/h8WGWXHuCPvuUnjvum1wZphQfaIKKLycQARszFjqgR99Aq+K348MXdM bwq3FFy8 f+juyAClb7O2QEizGrs63BMDjIJ5KXfz473dVJn3Msj1yyYaJJ+nqeqQWF4nyj8RDTnmU0z2B/Pn+rD95TcSGju3jk5gy1dAzvLUfxDnB4k5KkNlO6SxRFv7J5kWmBz1wuVGYtdr+bBFuksjTvGVZBOaMIU/yiGCQLqrLgNNsJgVV/U3gGiLpBbSaB5TA+LkTKHsVSArF6fTyPPDde84EK6WlskDeFUaCTGYIf2Ty7ZJd1eLG/gmxr+QLhCsWtheOjn7s7sOxu8nwf4OPs2LceiBL0yohgz5RqenDcUPKC+GEG/aqMxkasONK7A== 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: On Tue, Mar 28, 2023 at 12:34=E2=80=AFPM Yosry Ahmed wrote: > > On Tue, Mar 28, 2023 at 12:28=E2=80=AFPM Shakeel Butt wrote: > > > > On Tue, Mar 28, 2023 at 11:53=E2=80=AFAM Yosry Ahmed wrote: > > > > > [...] > > > > > + if (atomic_xchg(&stats_flush_ongoing, 1)) > > > > > > > > Have you profiled this? I wonder if we should replace the above wit= h > > > > > > > > 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 notabl= e > > > difference between before and after the patch series -- but maybe som= e > > > specific access patterns cause a regression, not sure. > > > > > > Does an atomic_cmpxchg() satisfy the same purpose? it's easier to rea= d > > > / more concise I guess. > > > > > > Something like > > > > > > if (atomic_cmpxchg(&stats_flush_ongoing, 0, 1)) > > > > > > WDYT? > > > > > > > No, I don't think cmpxchg will be any different from xchg(). On x86, > > the cmpxchg will always write to stats_flush_ongoing and depending on > > the comparison result, it will either be 0 or 1 here. > > > > If you see the implementation of queued_spin_trylock(), it does the > > same as well. > > Interesting. I thought cmpxchg by definition will compare first and > only do the write if stats_flush_ongoing =3D=3D 0 in this case. > > I thought queued_spin_trylock() was doing an atomic_read() first to > avoid the LOCK instruction unnecessarily the lock is held by someone > else. Anyway, perhaps it's better to follow what queued_spin_trylock() is doing, even if only to avoid locking the cache line unnecessarily. (Although now that I think about it, I wonder why atomic_cmpxchg doesn't do this by default, food for thought)