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 1A0FEC6FD1F for ; Thu, 21 Mar 2024 23:58:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A45F86B0098; Thu, 21 Mar 2024 19:58:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9F5426B0099; Thu, 21 Mar 2024 19:58:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8BD746B009A; Thu, 21 Mar 2024 19:58:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 7ABB26B0098 for ; Thu, 21 Mar 2024 19:58:08 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 4C2BEC02AD for ; Thu, 21 Mar 2024 23:58:08 +0000 (UTC) X-FDA: 81922712256.19.E77EFE0 Received: from mail-ej1-f41.google.com (mail-ej1-f41.google.com [209.85.218.41]) by imf09.hostedemail.com (Postfix) with ESMTP id 738DA140006 for ; Thu, 21 Mar 2024 23:58:05 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cB3MY8TS; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.41 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=1711065485; 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=ul906RHRjtTt9ICgM5G5gAfi1fYFVLGfZkDm8s9ACk0=; b=GfNxrYI4S7tnDdVJL8JF91U0+0fi0ypQ5zaYXYadkj7084P50ZnZRQVz5pb94GNBF4rxMR drC+Yz99MzXO9Z2ak7kXlAUuRjgevoe5F2T9vaavtjR225Hy39TBRnqUstJvGM0dsqDrnO 4svUYsgMvI0Vs2xzkn6q2Fu1+RjdyKE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711065485; a=rsa-sha256; cv=none; b=v5vlBPtZb7Urd+Bcf9HfFY5c0QOxLbJ8FFD1i7DBwJMIP7mKJfE2fZLpNbchMmVAY/1Tlm LF2DQGxONsEpzV2OucEK/9C8Q0PZApg15Ezpyq0Ksmtv8QBB0rUKl4mTuBoXayLEogfiOs Qpwd8J+hAVOOZnRc9bgVegTcZ3hICAI= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cB3MY8TS; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.41 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f41.google.com with SMTP id a640c23a62f3a-a4644bde1d4so216302666b.3 for ; Thu, 21 Mar 2024 16:58:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1711065484; x=1711670284; 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=ul906RHRjtTt9ICgM5G5gAfi1fYFVLGfZkDm8s9ACk0=; b=cB3MY8TSVnUFdF4OIj4lTTJZfqyUSo8Bt7Qs95uH9lL8rhybxh+2JpC22LQj4pttrO Pb3C3Yc+sHj8lSR4CTB9EiNSHqcYo2coj5Z4fglPJthCOIQAOTcGmZ1AjKq0vIzmz/kP Q7iB1ctxxRqRBt1YdLFfiKwHm86cSHhHLmN93ycy9h7xe3PxodFcZ+VAxvWYRogT0TLt 0WPh1CPlOJbjM8ObemkdcwfJ3H+BeDsDaYWAOTc8NZUQ3X41rNaownYDFzV+TyJk24DK VNnBME7JgJqKQ/dmm7LjIWnDhjHNBmbqrEiKHrkInLWBcdWF/GLyh4f3KAu9HzsX6gl2 hkxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711065484; x=1711670284; 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=ul906RHRjtTt9ICgM5G5gAfi1fYFVLGfZkDm8s9ACk0=; b=FJmrA3C0DrEU73PJ788zAmh6iCA5gP+Q+0pl4i98MvNurn/Z4NWS/Yxx3SbTyf9oUQ XioA6YnK+bikMBZIorpVBl7E+zsi+ns6bD2L2sa7pQ6vrKYTxlcGn1dah3HaECuco+1p 2dQMZBu5pU6BQNDXd/ro5AC0sasd0RDBjYTtoxg+MywlRPW/ITr9OAu4nT+lRqNnoSIO 73FQdUYD1PbbnLgxJQQ10KU7JUo5ee9ycdKTOuqM4osyk3GMK5a6uk1f0GHPe7sV4tnB 0M0unwZRDnug09Dm+VXwZU0jObrVAmakqj/3ho9XzI9kdWkowSFlH4LRqrFRpgjPWBp5 U2Dg== X-Forwarded-Encrypted: i=1; AJvYcCUM0t8qy+2gOMbqpJfeDKP8tZoniBMII7+mNt9L0oh72TCNrv8zNR5rCv1YJIHeEiAymkecQu1BHBduijurcLG/Jik= X-Gm-Message-State: AOJu0YyfSsUq4bFrM9DD9WsjoD9s152zReIhQtekN/MogrweIbRuq2jl MoDj6FiFO6tdc75RCqPaq/wuoXVXfaS+RFE+j49GiirPJPjeKHlJp5EBZwhdltfpo2YsYcdXm0c dj3RRASnbB8EQFPdXFq85yJ1JAsr0ySumcmSU X-Google-Smtp-Source: AGHT+IF5f1CTZlnm2Xl7nXWvcQsW6QwnCYmk7+9IjBQsblTlSyNewbp8mF70Rfoe45h0EzCiIdM8zWpvtSCe0UPev0k= X-Received: by 2002:a17:906:c349:b0:a47:876:28b9 with SMTP id ci9-20020a170906c34900b00a47087628b9mr532234ejb.42.1711065483601; Thu, 21 Mar 2024 16:58:03 -0700 (PDT) MIME-Version: 1.0 References: <20240320020823.337644-1-yosryahmed@google.com> <20240320020823.337644-2-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Thu, 21 Mar 2024 16:57:27 -0700 Message-ID: Subject: Re: [PATCH 2/2] mm: zswap: remove nr_zswap_stored atomic To: Nhat Pham Cc: Andrew Morton , Johannes Weiner , Chengming Zhou , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 738DA140006 X-Rspam-User: X-Stat-Signature: r4or9o6eggnq9ac1nrb3ep5fohrxmcf3 X-Rspamd-Server: rspam03 X-HE-Tag: 1711065485-910513 X-HE-Meta: U2FsdGVkX18vlIjZBjCDIzuI7SNXPnX+JBtMZf9qziLqWO3Xi4cy+Nxzvkj0+dwH5N/qxZoiU0B+wvS2Q3C9trsQgwbMVM0ZY06r4dNIe4UgSrCIQFxfVrxqG/qBBTMDpVNSNSqYnAeBqwLGvq8BwjAXGfHGHRJmG2Ipe9LB5a53xjVtL0dff18db4Nka3C1H7vBcLPcLg7eP4VW3I/03Y5XECZRVjblTv2x7nlKhjPkJZcUZkg/0CxEkTygHP15ZV2ILNe5sxAs925TRlFsJCvm73i+JWy+Hnd8H/Tnw7hWLd/VeqnHh0bt/J9e8O5ZgHzU8AFAaYd77MbYzu1tStydNyma1kiTqu6f43AG4wwMHDH/ZBL6sZXUA1B9C22J+wp9WP5BRU8kzXEv/ANxfdlnliWDdBWg51ndEguPSq9v6p5zM28kiJPj7wsAYleYE21FmS6XX54gIUaKK/VjhRrP9hN0wCaNbKYNzbKXX21YuJ4+6niYDgSPd7Bsb/TgG8/AsPohJnT4fGp2J2KyOaotA3CdDrBqwIBOgHSthJQ2dCn/OJ2osAuTW16Kws6CpVIdb3annmPgXeX3U75H6oif5IQugtlz9nVQv/HO74EDItIAyhBrVUsp+OBDVVKuCOlvHrqLsc7QdxpFB6MkqpP+AjqIz+sShNPp9wQ1ZmMh//+8QE/SEhDY0ohFnoTIG55pGf3SSwEoKixVQs1nt9gEc2hvwZoOjrdY4w8EVIqJ2iwVojDrsh2A8wO3Io6xjCoMZ2lAzkNkNH4DrZH90I8vxW6r4pMkhZDA6wOphXjulso6gk6e7Kb4uFf9mQTxyiYhD/EODTACR0Mz4UySZaAl+Z8zxwfxGopGb5c2inrlQMGhfyLwP5RfC+wE/Yim+Q4sDeR/qbH/zKa5+kOnGBR7cKCvsHhLWi7YQGzoVEiqtFa7PsJ6vQlSeBTr++VZiMoNexW7Vno3B/3Bv5T rcZxkhQ4 aQCNdYRvfE9hCJb3Bxj58/feU/a1FCMXKv+rqiaK/esQ2XHf/dch2ckE0nQmAX/i+Cpsl+m6CLuGBYWTLFyXZKLmcVAH/iUEozhwFWJzdmAyfh7wP/eo9rm/zePwZX3KWAFhiK1yrG/e9TeHuaU++x7SOtypqL2eaU5SxZQXdvi0zcYwF44zsb5aqviNRUcvqVTsLH9t+XbiMb8rJuD20Rm/OhfgD7R+S5JTVKncOj1qgj4Pt+O4QeqJed+9VSFLML8f9qygtjNbgrfs9HD4jrF+L/sYiEM2r02YMBLqaX9In8+CGBjMtfNVZ2BD2F6mL81q1 X-Bogosity: Ham, tests=bogofilter, spamicity=0.423986, 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 Thu, Mar 21, 2024 at 4:50=E2=80=AFPM Nhat Pham wrote= : > > On Thu, Mar 21, 2024 at 2:09=E2=80=AFPM Yosry Ahmed wrote: > > > > On Tue, Mar 19, 2024 at 7:08=E2=80=AFPM Yosry Ahmed wrote: > > > > > > zswap_nr_stored is used to maintain the number of stored pages in zsw= ap > > > that are not same-filled pages. It is used in zswap_shrinker_count() = to > > > scale the number of freeable compressed pages by the compression rati= o. > > > That is, to reduce the amount of writeback from zswap with higher > > > compression ratios as the ROI from IO diminishes. > > > > > > However, the need for this counter is questionable due to two reasons= : > > > - It is redundant. The value can be inferred from (zswap_stored_pages= - > > > zswap_same_filled_pages). > > Ah, I forgot about this. For context, nr_stored was originally a > zswap_pool-specific stat, but I think Chengming has pulled it out and > converted it into a global pool stat in an earlier patch - yet, > globally, we already have zswap_stored_pages that is (mostly) the same > counter. Thanks for the context. > > Might as well use existing counters (zswap_stored_pages) then, rather > than a newly introduced counter. Probably will shave off a couple > cycles here and there for the atomic increment/decrement :) > > > > - When memcgs are enabled, we use memcg_page_state(memcg, > > > MEMCG_ZSWAPPED), which includes same-filled pages anyway (i.e. > > > equivalent to zswap_stored_pages). > > This is fine I suppose. I was aware of this weird inaccuracy. However, > for the CONFIG_MEMCG case, it was kinda silly to introduce the counter > for per-cgroup same filled zswap pages, just for this one purpose, so > I decided to accept the inaccuracy. > > > > > > > Use zswap_stored_pages instead in zswap_shrinker_count() to keep thin= gs > > > consistent whether memcgs are enabled or not, and add a comment about > > > the number of freeable pages possibly being scaled down more than it > > > should if we have lots of same-filled pages (i.e. inflated compressio= n > > > ratio). > > > > > > Remove nr_zswap_stored and one atomic operation in the store and free > > > paths. > > > > > > Signed-off-by: Yosry Ahmed > > > > Any thoughts on this patch? Should I resend it separately? > > Might be worth resending it separately, but up to you and Andrew! I will resend to add some context and include your R-b, thanks. > > Reviewed-by: Nhat Pham