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 00D12C54E49 for ; Tue, 12 Mar 2024 04:04:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5625C6B0087; Tue, 12 Mar 2024 00:04:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 514E26B00A1; Tue, 12 Mar 2024 00:04:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3B2826B0087; Tue, 12 Mar 2024 00:04:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 2946D6B007E for ; Tue, 12 Mar 2024 00:04:38 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E828DC0FDE for ; Tue, 12 Mar 2024 04:04:37 +0000 (UTC) X-FDA: 81887045394.12.6700B11 Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) by imf19.hostedemail.com (Postfix) with ESMTP id 2A8761A000B for ; Tue, 12 Mar 2024 04:04:36 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=vJaiGv4H; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of 3U9TvZQoKCPAqgkjqSZeWVYggYdW.Ugedafmp-eecnSUc.gjY@flex--yosryahmed.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3U9TvZQoKCPAqgkjqSZeWVYggYdW.Ugedafmp-eecnSUc.gjY@flex--yosryahmed.bounces.google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710216276; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=DKGHJnbXSzQyoajwNYJ2VuXwhIvgziwgeBLpVKyZTQA=; b=MaiinoEPXNCHKYSm3EoFSx7ocCGjYeeGnKISgGZ3CKeCXe2FtJOKNTF6sI6x4K+okSyEib cA9OSIdc+KSr6sge0ihsPa8aZ71twUVNvZkmONib0hInh6EhUb37Z6isv/5eJXaWCwwdbo ZMa9B4yB7optXD5ubeW6ecalq5wxsJY= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=vJaiGv4H; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of 3U9TvZQoKCPAqgkjqSZeWVYggYdW.Ugedafmp-eecnSUc.gjY@flex--yosryahmed.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3U9TvZQoKCPAqgkjqSZeWVYggYdW.Ugedafmp-eecnSUc.gjY@flex--yosryahmed.bounces.google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710216276; a=rsa-sha256; cv=none; b=YayYL8oVE0+vR7El6qjb+cvgfJBwv9iR2eRwvDQbkxMdRAEBQDnDV0igBx1YiHuw4ZAy+1 yEnu7XmwTTI2P/w1dHSim7nkJ0GsBZrPxYg72YXEJzbzRl326Bv3UKAqyTsfhIhm1lN5ho 18jWCz94hyV3Pe13pPm8keEPj3f42rE= Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-dc6ceade361so6682114276.0 for ; Mon, 11 Mar 2024 21:04:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1710216275; x=1710821075; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=DKGHJnbXSzQyoajwNYJ2VuXwhIvgziwgeBLpVKyZTQA=; b=vJaiGv4Hbvl76zA2PYjoQWYol/V4gXVBMlCHL/uESgaDNM4++yaIk1+AQqhPk8VzEq s7vnNJO42V9E+5MafGCrslkURISUwEQcv8Mlx6v7iS6ZGomNJwNWdSbPr4gfTmrtlv4d 5PKn83NIGqgxuMllPcg9+LUqPWTEPD5yhxGqkSQNd5+CFCMDr8JErQ9M9ETUup+LkcjL LpajbLifIm1chFosOkeFmgu/fcYg41d8/HgcWb8XOE3coA/BHQuuLzWaN7VlIJy4W6MX LNwhzJleQMZKIk2uejQCN80+HPCyw0EUAdbna9Y5WRD9Z1OoLHkc3DP4rOLfB70i3vOq FAgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710216275; x=1710821075; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=DKGHJnbXSzQyoajwNYJ2VuXwhIvgziwgeBLpVKyZTQA=; b=AskKo4o/KLJCWz4SDAcN0SMuu2cYTw6c2YJJHY1gkk6fesOl7n/lTS0RiPIRgzw/Dm 1BlEblOK4CAZyQM15iqUE9GqEr9K93iL7tEYfesWhsa298DlSouCtO20UBCObBO7WpAF DkI9m2Zs2RMv2jFzmns8GOfAZ6DlYus108NAxytIGzcdI3Y2OAgvZyQLFz9EA3fmzw8L LwQIOhFnIYMVJGR52X/C2ci6ejroufJSD8SlW+bMK1tdjTaoZmOV4Ml1aMJCfNytZV1J 7szuaIe31mKdjPvtZC3uPeHZ/N8YdVBGZIKITee5eBxuCrE5Ak5/75rlezKpO/D0d4Su TMgw== X-Forwarded-Encrypted: i=1; AJvYcCWhMUHPFEHp/y/YkJ+E3uAamxmfp4ITfwj3ctiPj35abr0JQU2FiVd4NsjdMGr/k2jXfOvcVqqFD4jRj4XUv40BWmY= X-Gm-Message-State: AOJu0YyCcjdNWqFdgQNtrqeDY9fPltz74HhcfJCxyfoDPm8KGzwBi2ZQ KRXSXQy3J6apHpx2CynnpS2+s04wHC9auB7KsRIhfWksVdbCChIRgI8g3QlFsiltJMKKhjBB7zu WvwzEi1byECycSMdHbw== X-Google-Smtp-Source: AGHT+IHSM/RCW6tdjFD+iaaWS3QoWG/+A1iS87q9c84LEMGLBoDbTTaanEuiOY0eoHyvPbAVrOaVG1EO9mlzIkNe X-Received: from yosry.c.googlers.com ([fda3:e722:ac3:cc00:20:ed76:c0a8:29b4]) (user=yosryahmed job=sendgmr) by 2002:a25:c712:0:b0:dcf:6b50:9bd7 with SMTP id w18-20020a25c712000000b00dcf6b509bd7mr2202554ybe.7.1710216275352; Mon, 11 Mar 2024 21:04:35 -0700 (PDT) Date: Tue, 12 Mar 2024 04:04:33 +0000 In-Reply-To: <20240312023411.GA22705@cmpxchg.org> Mime-Version: 1.0 References: <20240311161214.1145168-1-hannes@cmpxchg.org> <20240312023411.GA22705@cmpxchg.org> Message-ID: Subject: Re: [PATCH 1/2] mm: zswap: optimize zswap pool size tracking From: Yosry Ahmed To: Johannes Weiner Cc: Andrew Morton , Nhat Pham , Chengming Zhou , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" X-Rspamd-Queue-Id: 2A8761A000B X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: yc8uy88c6rxzcg647uwb4hji35oc48dc X-HE-Tag: 1710216275-969198 X-HE-Meta: U2FsdGVkX1/8QkJTQ4XomNP0x3/CfOBTLVnsVWHNE9eMMJco8Ex/a/7N9vnOCHhThvlttkheJvEhmU0P96lRXKJc/RnsbVE3KegRz2FJZCnGLpYln7KMGOe1/XxH8TQO4MnM8fbUOaO+6TUZOMBAnJsD7EyMJjtq+7AVU44SstLIRUd550L7C6g+tckXcxLBBn5stWfs3KjYnmMVHnB0Eu/iVlsmCk2AGNAxNvq1IXYDB8WM5shQ0rqa0J7OlVEGMtCfFQScfc1X3YkOV7lTZgOtAoKk88l+UiwMOtfOJiPgs26IuZWzDWStKY2Dncu6A+I5G+EZoRhU9fCdpTlSLY9plWl6mdCNBquZuOMgIK9/eW7SfDK4TD9oDRraqdtrPaW3gy36sB0D39n7QMTEcKSWKFZxsAyeoge0cGHzstOpW86mnlV44VWVTqtLSRXzvo1nZcPlOSxRI2Ay8PijFtyyWzTIjGymHobQDz9nHk9s23MdDny8BFfziZ4c4IDVSVTlxc5EZHBByae84NLgykD4R8OKN4KHQbby6zsNBuMXkbtAP/iJI67W4XBAZQCKFgu6EYl0yH1EKaSP5vBEfztXpy+MurQ95nHJrGjOGDJYDhpdnfCv5bXC/p/d8HnTaUOmtAvLK4CDasFZPit9rGmOBBaeP/eOoG463n1I8n4miZx+HmKg0FuEPDWdbGsZp2tWAfXKjYtWtOjogKO7i7BwdZCNeLASgUkAG19WY/olcR3xiuqTVeVX9ZVj4O8TtuF4X0zq8uuBZLAPJ8ZBrrzmNMbB+4wxUxMQiIiTcig737qgVRbc7v5ea5Wx6vwuoo1kBBHSXF74wYPt/pOQo0MnSz4WbhyilGdQ728RYyd/jrNGTFMT8VaowmCJ3zpaP+Dt7pSWKtLPZAuvHyFBEu+m/c/11UspenMvmwDaFZPpU2g5bHqlq5qZwmk0eNrTZvDwqFm/VM3eTs0Mhqb jEl0GnAv TzNCLKXUYluATiLp9klCczVecgM/zWvl7jonNQKmL5bBhk+ohT+FBad7J8eCKjxR1lHd3w0pzgiYm5Ma+3PRcCetUuk8FlcMhqeRK8xRx7X/eEnp9YnV8WatB3X3iCAguNKSVG5Dhrc7Tmz7cfKKMb2LaIp/ShuawolGXM/jMYqghKs4j8/mxcVkfWsBoe7JiHmrxC7QIct6lhoHfI9t8kUMsU80vcgnEw0fRJhks2zalrL+HRfDV7sJ+cH3eKvsD1NEGUlxNR7PC54pc/CiNi0GFWCDZ/Q9/IcIWgy70P/CAAgePwQuKnYLaOqAxsluZTF7/gIqdxZGuwSwYjOhhZG73Rtt8z7hjHc5gYzpj2Kab2yNql5x0+BoqTwfHhJic+kBFgbEXYfTnZWR3JLqkbG/LCiccZrGATgOkNxFCt+yReoj67/h1buvn40/MYWtUBgK0iD0uXA02iqzXeS4Xk1ftexJ8DW8k6f7WxVmkaVCwoCqGVdrRxUiODb1GqHzdQ9UFOBGgLXM+8HFt6iK4PVF7HUN+Pewuk2SllRR/P6W83DvamPzNIRRbBNRi1XXh96xNnVY5wBbcp5SqjX9Ol3f4L4Yq6DK9KAtHjbqa0VKQQ7na/uKKDh5oAmowpHBAtjlN 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: List-Subscribe: List-Unsubscribe: On Mon, Mar 11, 2024 at 10:34:11PM -0400, Johannes Weiner wrote: > On Mon, Mar 11, 2024 at 10:09:35PM +0000, Yosry Ahmed wrote: > > On Mon, Mar 11, 2024 at 12:12:13PM -0400, Johannes Weiner wrote: > > > Profiling the munmap() of a zswapped memory region shows 50%(!) of the > > > total cycles currently going into updating the zswap_pool_total_size. > > > > Yikes. I have always hated that size update scheme FWIW. > > > > I have also wondered whether it makes sense to just maintain the number > > of pages in zswap as an atomic, like zswap_stored_pages. I guess your > > proposed scheme is even cheaper for the load/invalidate paths because we > > do nothing at all. It could be an option if the aggregation in other > > paths ever becomes a problem, but we would need to make sure it > > doesn't regress the load/invalidate paths. Just sharing some thoughts. > > Agree with you there. I actually tried doing it that way at first, but > noticed zram uses zs_get_total_pages() and actually wants a per-pool > count. I didn't want the backend to have to update two atomics, so I > settled for this version. Could be useful to document this context if you send a v2. This version is a big improvement anyway, so hopefully we don' t need to revisit. > > > > There are three consumers of this counter: > > > - store, to enforce the globally configured pool limit > > > - meminfo & debugfs, to report the size to the user > > > - shrink, to determine the batch size for each cycle > > > > > > Instead of aggregating everytime an entry enters or exits the zswap > > > pool, aggregate the value from the zpools on-demand: > > > > > > - Stores aggregate the counter anyway upon success. Aggregating to > > > check the limit instead is the same amount of work. > > > > > > - Meminfo & debugfs might benefit somewhat from a pre-aggregated > > > counter, but aren't exactly hotpaths. > > > > > > - Shrinking can aggregate once for every cycle instead of doing it for > > > every freed entry. As the shrinker might work on tens or hundreds of > > > objects per scan cycle, this is a large reduction in aggregations. > > > > > > The paths that benefit dramatically are swapin, swapoff, and > > > unmaps. There could be millions of pages being processed until > > > somebody asks for the pool size again. This eliminates the pool size > > > updates from those paths entirely. > > > > This looks like a big win, thanks! I wonder if you have any numbers of > > perf profiles to share. That would be nice to have, but I think the > > benefit is clear regardless. > > I deleted the perf files already, but can re-run it tomorrow. Thanks! > > > I also like the implicit cleanup when we switch to maintaining the > > number of pages rather than bytes. The code looks much better with all > > the shifts and divisions gone :) > > > > I have a couple of comments below. With them addressed, feel free to > > add: > > Acked-by: Yosry Ahmed > > Thanks! > > > > @@ -1385,6 +1365,10 @@ static void shrink_worker(struct work_struct *w) > > > { > > > struct mem_cgroup *memcg; > > > int ret, failures = 0; > > > + unsigned long thr; > > > + > > > + /* Reclaim down to the accept threshold */ > > > + thr = zswap_max_pages() * zswap_accept_thr_percent / 100; > > > > This calculation is repeated twice, so I'd rather keep a helper for it > > as an alternative to zswap_can_accept(). Perhaps zswap_threshold_page() > > or zswap_acceptance_pages()? > > Sounds good. I went with zswap_accept_thr_pages(). Even better. > > > > @@ -1711,6 +1700,13 @@ void zswap_swapoff(int type) > > > > > > static struct dentry *zswap_debugfs_root; > > > > > > +static int debugfs_get_total_size(void *data, u64 *val) > > > +{ > > > + *val = zswap_total_pages() * PAGE_SIZE; > > > + return 0; > > > +} > > > +DEFINE_DEBUGFS_ATTRIBUTE(total_size_fops, debugfs_get_total_size, NULL, "%llu"); > > > > I think we are missing a newline here to maintain the current format > > (i.e "%llu\n"). > > Oops, good catch! I had verified the debugfs file (along with the > others) with 'grep . *', which hides that this is missing. Fixed up. > > Thanks for taking a look. The incremental diff is below. I'll run the > tests and recapture the numbers tomorrow, then send v2. LGTM. Feel free to carry the Ack forward.