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 6067EC54E58 for ; Thu, 21 Mar 2024 19:58:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E9A9A6B0088; Thu, 21 Mar 2024 15:58:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E4AA06B0089; Thu, 21 Mar 2024 15:58:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D127B6B008A; Thu, 21 Mar 2024 15:58:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C09216B0088 for ; Thu, 21 Mar 2024 15:58:21 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 9690FA04A4 for ; Thu, 21 Mar 2024 19:58:21 +0000 (UTC) X-FDA: 81922108002.26.3D8B641 Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by imf30.hostedemail.com (Postfix) with ESMTP id AD1538000C for ; Thu, 21 Mar 2024 19:58:19 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Mj9+x4sj; spf=pass (imf30.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.46 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=1711051099; 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=aXMtfnknzUOM6yTFQgeEMj7wqzqcGBcUvrYyxvIjzbM=; b=QZNvwKkSk6RdoMw9J6Ac6CbQrmAiKrY/5CAiaiB/l0j2a1+pX+8kHwQkjmhZvoA9y2/qSE QnQ1HxjSArMJb7m3JDqSSIH6lxmo2yAB1onOVXPWhuW7B+J2SyvMUIvsWJcYiKAKrBgrw1 zmNIIPwWpwMyWSTY/KSIXyJsppt2KnA= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Mj9+x4sj; spf=pass (imf30.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.46 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=1711051099; a=rsa-sha256; cv=none; b=HSkWC3f2OfvenOU/CGv94R5ilpZWhXubIQ1UsVSMYUNTg0MRR78imdch/d3lPvasNBGKur 8nVP0iMwxEyECsCmrrE/ljTwFeeCQmvXnOMwO2SF34CLEh6Xv1uHO6WK1hFix30EGrWX5y PJW5UsTthBwa+Yx1E3p0N01wViFUL0s= Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-56b9e5ed074so1970771a12.3 for ; Thu, 21 Mar 2024 12:58:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1711051098; x=1711655898; 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=aXMtfnknzUOM6yTFQgeEMj7wqzqcGBcUvrYyxvIjzbM=; b=Mj9+x4sjWKVEwZ5iYUbNVNEqV2WAqhC1SGWWPpuzTNeOMKjrMGB3oW6dgfSaK8M8Dk RUxUIUmJHuk8pJqxQK/lSu8rWd0HWJlep+/P3z6AZoStK2LmzXMb8eFfHp2ynvAN/KVv 4FPfxleJWT6u2oUiuL0YEbDuttRDgVgaFKMW5P0r0o3s0UjxbMUDfvr952TJ39AfQYap g7YtAp3pdiiR96nwoh7NBFTReEJUWLjOFt35bC3G/8M9A69nH9LjDVweKBpsatokc980 ZHikbj916ULvgYbFr9QAiRy78Uwei5GWmWKwWeiEYUCvQbrCW2Nf2izkpCVX/SyfJC7l 9flw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711051098; x=1711655898; 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=aXMtfnknzUOM6yTFQgeEMj7wqzqcGBcUvrYyxvIjzbM=; b=nYaH8OSlxArUuAFnmjqDGjXXbxaljM2X+UMB+SNOWr0+7U4h7isFFpMnrQWvO5A3aW OR6CfzCGngIG4qezhbGCB2QXRH1Sz0WpRgjHPP6xlCEDsXGZx1Ku5GcRH2dhHY4VIj5P EV2xG+pNIDXcpv97S+R3X8KdejelUJCfF3VRp9d396SB+kufu32QsgEOcOA+kjBAHTWZ S0r6obkPkXwG6elTrEHi/HjlHUQrUkadccfi02EW/Wl39kC+8t9J5OZTHY+GNHwnsPTu v5eWjXOrhwGhPaRoqnL1l/eo0MptzoNXEBpK81ByVfF1jSFgG5R0iJ0JiLXG7j9Xjg2w avuQ== X-Forwarded-Encrypted: i=1; AJvYcCWGYplrtTK5uj3YNNms7mliyQRrLDJcH1yn8pKYMBnB1GrhEZ+AOk/RrJFXXj+asDe+KFkpnIEF64SsMyY/wGYLMTY= X-Gm-Message-State: AOJu0Yy+GTll6vwNFPjn1VvclZn0hmS6f41tRlyruzcMm4LZ4K5NScoM GNDtcWPzV0Is3nOOChqh7SdMOkI/XyXnvBDBKdhBGF1Hafnd1UuCR3pseSLXxwX+kdUrlqnCb1A 6EF2/vDONpTN0dwfxcpgST0yabX02Byx5q/Ry X-Google-Smtp-Source: AGHT+IFJWJZlOXrL2ICYkmfbWhZ8W8AGF+oJQKbdNz1TB+AeTuTQMW/HKReJDpUyks6cx+m/e8aNbt2M9RUNvEj1SE4= X-Received: by 2002:a17:906:46cc:b0:a46:e0ab:40ff with SMTP id k12-20020a17090646cc00b00a46e0ab40ffmr298721ejs.35.1711051097700; Thu, 21 Mar 2024 12:58:17 -0700 (PDT) MIME-Version: 1.0 References: <20240320210716.GH294822@cmpxchg.org> <20240320211945.GI294822@cmpxchg.org> <7d7b755a-e13e-4267-b1b5-a4e2ff33d6e0@linux.dev> <20240321192912.GB3154@cmpxchg.org> In-Reply-To: <20240321192912.GB3154@cmpxchg.org> From: Yosry Ahmed Date: Thu, 21 Mar 2024 12:57:39 -0700 Message-ID: Subject: Re: [RFC] Storing same-filled pages without a zswap_entry To: Johannes Weiner Cc: Chengming Zhou , Nhat Pham , Chris Li , Linux-MM Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: AD1538000C X-Rspam-User: X-Stat-Signature: 1pkx11386mb6g9ysnr6o5dtkx85nddwq X-Rspamd-Server: rspam01 X-HE-Tag: 1711051099-622181 X-HE-Meta: U2FsdGVkX19coeW8UbYzMQV4XlNBkdNE03MTegHT5Q2lFfTSA0mNYApI1MJONmP9ZOX4Oa65N3r5637cruFhhUY4e7pqS5n3hjWxCIWR7X24pn1Tvu5nzafgUDLg02HFfIyOlNLNG5o5aeHwxcLZE7hKjAxCYcNNr8i4OjmWVuIYo3+QRn8lAi7ca6UL1GEJb2U3A3A4gfM3NJypRSdVrj3fLsReVVIyh5NTdiEVDqW9XR9wXOHq3ow3Ey22JkIry9DIVNGGwuFQFHUTC/y2O0LCysldeFMN2RBqGG1zymBXXeiN7uGn8KwMKKKAS5DGJCzb9Zjbchv3MWIFX3df6IUg+NZwABTBa39m4IdoPaMaNqCv57Z2aY8X1O7QcuBVyk4AeGXeY9f87DxLIAOC9JZwKX0+iOp+6BbG+NBlk54KwxcmoGrJ21vOSilQKjSWdAh0ASrT4o/HP6dvl3Ia3OKMfDET5Bdt+cnXvouaw8Q0LLs7+GVwZFPHNM4c5rQjKdjirdFTkncS+LzVEiSGBYxs2IZY+4eaZ+vcGQCZy1+6Viacu8z0bNB0FAxApNZb86iJ6pesWaCH0HbSCk4r2BDDCNvlqHh1I8XdWK8hxXhCWp26m13IhOPbc1CTP5xTAzVo8eKcM0NHv5xrDtVOKAlXmRQGbtmNu/ZFmb4aE/VfFGXYhqKSIBxrRnykixVyTJ+5x+nwHV4pX2+S52+/2Xl2+eMWKZ5PRJ8am/TKBTujLs7epNZDkj96oJAAndF3/+RdCB+DdvlBmjLyMbhna1x8nUFGcIeB2ijJXoQBIgbNiDLZrJ9fGehUe7Lu2Bbj5kWIukomKE1MUJVZE7hz8W1Ks7wEovstHcnS+JHGjjAwUUo2RaFX+kdDJGMxRaFCIwtaRfpapnrRHDBplygW5BLYLk3RFU0c7avaKwDSKmt236ucTlEEsMEtMPqiyX7Fw1tkdTD8VLXhuXhE1Vh s+pcz6s/ /XX10gbY4t96Apg0eP/PTeFWMtwZbHKY0Tgtjxv8utA86vRoGs8BNJ8sVW9E83BG1RPnvx3T6CBw+eAN/vnKDWgf1DqWgBOyCQFOx0jTXcpeOK44ZgbR2XF9V8C8hs4lCKZk+3DfYs3lXH1HH4eqWlY+BofExbWJ7iBRCtl19sNHj8SOAMF/uw3jXqxn4MKWapsWg8o/jA8L/lbdrBaQbvzZ7xPhMmFr2rHyXudQBfFCUiOojVfQFePBX6qYrp8xvLyiUghdLP6jC4CPeAdEsI0oqeTxNZwXrmz1UwwR1VAzbFuj0NlZE9c+e59xT0kZPzG2g4OijHohVTyU= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000002, 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 12:29=E2=80=AFPM Johannes Weiner wrote: > > On Thu, Mar 21, 2024 at 06:44:54PM +0000, Yosry Ahmed wrote: > > On Thu, Mar 21, 2024 at 11:40:32AM +0800, Chengming Zhou wrote: > > > On 2024/3/21 05:31, Yosry Ahmed wrote: > > > > On Wed, Mar 20, 2024 at 2:19=E2=80=AFPM Johannes Weiner wrote: > > > >> > > > >> On Wed, Mar 20, 2024 at 05:07:21PM -0400, Johannes Weiner wrote: > > > >>> On Wed, Mar 20, 2024 at 01:49:17PM -0700, Yosry Ahmed wrote: > > > >>>> Hey folks, > > > >>>> > > > >>>> I was looking at cleaning up the same-filled handling code in zs= wap, > > > >>>> when it hit me that after the xarray conversion, the only member= of > > > >>>> struct zwap_entry that is relevant to same-filled pages is now t= he > > > >>>> objcg pointer. > > > >>>> > > > >>>> The xarray allows a pointer to be tagged by up to two tags (1 an= d 3), > > > >>>> so we can completely avoid allocating a zswap_entry for same-fil= led > > > >>>> pages by storing a tagged objcg pointer directly in the xarray > > > >>>> instead. > > > >>>> > > > >>>> Basically the xarray would then either have a pointer to struct > > > >>>> zswap_entry or struct obj_cgroup, where the latter is tagged as > > > >>>> SAME_FILLED_ONE or SAME_FILLED_ZERO. > > > >>>> > > > >>>> There are two benefits of this: > > > >>>> - Saving some memory (precisely 64 bytes per same-filled entry). > > > >>>> - Further separating handling of same-filled pages from compress= ed > > > >>>> pages, which results in some nice cleanups (especially in > > > >>>> zswap_store()). It also makes further improvements easier (e.g. > > > >>>> skipping limit checking for same-filled entries). > > > > > > I also think this is a good idea. :) Which could simplify the code to= o. > > > > > > >>> > > > >>> This sounds interesting. > > > >>> > > > >>> Where would you store the byte value it's filled with? Or would y= ou > > > >>> limit it to zero-filled only? > > > >> > > > >> The dumb thing about objcg is that for same-filled entries we real= ly > > > >> only need it for bumping ZSWPIN. Nothing else. entry->length is 0 = for > > > >> them, so even though we call the charge function, it doesn't actua= lly > > > >> do anything. > > > >> > > > >> Loading them is cheap and doesn't involve decompression. An argume= nt > > > >> could be made to exclude them from ZSWPOUT and ZSWPIN entirely. > > > >> > > > >> Or cheat a little and bump ZSWPIN for current->objcg instead - > > > >> probably good enough to make excessive thrashing discoverable by t= he > > > >> workload that's directly affected. > > > >> > > > >> Then you could get rid of the objcg pointer and use the xarray slo= t > > > >> for whatever else you'd want. > > > > > > > > Yeah it's only useful for the stats. Using current->objcg would wor= k, > > > > and should be ultimately pointing to the same memcg in *most* cases= , I > > > > > > In some cases where the current objcg is not "correct", the testcases= in > > > test_zswap.c may break? Maybe we can use swap_cgroup info to charge t= he > > > stats to the correct memcg? Not sure if this is feasible. > > > > For cgroup v1, swap_cgroup will be cleared from > > mem_cgroup_swapin_uncharge_swap() before the zswap load. > > > > I think the current objcg will remain correct as long as swapin happens > > from the same memcg as swapout (or if swapin happens from the parent > > memcg and the swapout memcg was offlined). > > Swap readahead will pull in physically adjacent entries that may > belong to somebody unrelated. Right. For those as well the current objcg would be correct if they are readahead from the same memcg as the one they were swapped out from, but I understand your point that readahead makes that more likely to not be the case. I am slightly nervous about using the current objcg tbh, even though it only affects the stats. It's just less straightforward this way. I think I prefer either: (a) Only supporting zero-filled pages and storing the objcg directly in the xarray. (b) Having a separate two-word struct to store objcg and value for same-filled pages. In both cases, we would need one tag bit to identify same-filled pages in the xarray. (a) is more tempting for me, but I am not sure everyone else agrees with dropping support for non-zero same-filled pages.