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 A4A81C54E67 for ; Wed, 27 Mar 2024 16:41:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 346816B0088; Wed, 27 Mar 2024 12:41:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F6E86B008A; Wed, 27 Mar 2024 12:41:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1BEA76B0092; Wed, 27 Mar 2024 12:41:11 -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 EFB586B0088 for ; Wed, 27 Mar 2024 12:41:10 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id A780FC0E06 for ; Wed, 27 Mar 2024 16:41:10 +0000 (UTC) X-FDA: 81943383900.23.56620A9 Received: from mail-qv1-f52.google.com (mail-qv1-f52.google.com [209.85.219.52]) by imf01.hostedemail.com (Postfix) with ESMTP id 07E4E4001C for ; Wed, 27 Mar 2024 16:41:08 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=C4ZQYelm; spf=pass (imf01.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.219.52 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711557669; a=rsa-sha256; cv=none; b=FKx0SHJft9ISx4hGG3FrSMGjGcxoDyuLY1Ud7pQOSf9rrjZswqXSbDQ92fzy0CivG2Edov tuDnGFu2LPYw11Q7Na3fvPdhfP3z0BqgLeubKoJ6ExEiTq3eX4/6gIuQ1xVXdIFyMl3Mjg WbJN0Te1MHYzKgg+Fog5DRujTs+s7m4= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=C4ZQYelm; spf=pass (imf01.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.219.52 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711557669; 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=YMZWWaTaJ3lxeA3Kz3iKtD6XUKMHdnC8EWB2x35/ZWU=; b=o3ARwpwB+hpLKHaGxozd4mmb0euu24Bupvf7suHQgg59UJofC+s3s7FKc/Sx0Su8yV2qBc HxY7M49xSyzY4ATSJIz+t3NFc/ZAUSHWhbs14apOpSXeUzqg+49a6lCUlsEW/xq7ACawzj 6i7Z+KTRLzbgDSoMiBLZlG0tkI0qLGw= Received: by mail-qv1-f52.google.com with SMTP id 6a1803df08f44-696a221c53aso9538856d6.0 for ; Wed, 27 Mar 2024 09:41:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1711557668; x=1712162468; 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=YMZWWaTaJ3lxeA3Kz3iKtD6XUKMHdnC8EWB2x35/ZWU=; b=C4ZQYelmIlCaPQ7WeB29qvC3RB/IOta97T9npIN3mq2eJgSQndIez1f9X1Xs9TFhK+ GPdYhlHyZGjjC7Qb4Z6h4cb9NL/UCRk9MQAw3+WvymzZPclRlaA7YwzsgDQkvECNZYxU i5tYXtI7A4RhNlKtF/jczTN5XAfu2OPr8ceRbUPqGOxuKMiDGDtm4p5IWRSvUKwfWfEg P1bUVEPZjiKU61MjOfKDNIAqVEdwmsy5K3kSOxkMg6/v8N/acdmxDndtfTzdkgp/mmO8 dr1GbwBhZ44AdfrezKqjHNL5kS5td8z8+0b+Kst9OljI04oWlR6W1Lm3frH7AzbfeTV5 fFkg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711557668; x=1712162468; 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=YMZWWaTaJ3lxeA3Kz3iKtD6XUKMHdnC8EWB2x35/ZWU=; b=s/iphLhK/EuaemVw/Fg99jC0JSRAI6u8DOo5IHaOdrCL1f5K+pZQifHmw8ZMS/ImSG 8awTgUEXrpsjk/FBcLag1Jwukbo7I0PePdMEKMA+j7P+PR270PhwJ5Spllu3a0sU9crx mu5TiZDyvem0XNbpt3GkYfKEl1ZYbLEsjz/XPPcnMSeXXIXZP/K1ZlNqHmrTPgKWB0yx dlBoFUN6FYMSMHakfwT3r2KRWuMiEVCL58Me23K/B2q5AxnhwC3jTQdL6RTIx55c5hgX g3ErepomYtBGOnXgkb8Ki9hti6AU0uBnxla/HeoLbf4I16wdi/KSOWXn2mlsAtxhriYN bYgw== X-Forwarded-Encrypted: i=1; AJvYcCVzVK/bPJCsMr1pLZ/dAEvGjZBOU+L2rEUxv7pbqEgTKCUAr4suK0ZJdzrpRXm3o5aIlVRhJA9wZ1bGJ9n0h9nJNxI= X-Gm-Message-State: AOJu0YzhGXUiIulwd/vM8nwOb59uRG0lktyUTpjuzjjFH+TYf0SB+m3t ME0YhlCNleg/Ik7MmQjbb0gJsNV4USWKl/FwS2AqX7pLViBQg9yw58Fk752TRYIqiZt7EiuZaL0 nJCGWW+xEayZXg5fp2rOhHiNt303lpgBd6lw= X-Google-Smtp-Source: AGHT+IEfdLDNpbWNtIYAq0kAU3sG8ft7rxFt6Nf3L/NlFTwxzVHYEkr+dnSwRJ1+e1UAcljWtVgdECbC493hK/wW/9Y= X-Received: by 2002:a05:6214:b14:b0:690:dc3b:179d with SMTP id u20-20020a0562140b1400b00690dc3b179dmr203067qvj.20.1711557667999; Wed, 27 Mar 2024 09:41:07 -0700 (PDT) MIME-Version: 1.0 References: <20240325235018.2028408-1-yosryahmed@google.com> <20240325235018.2028408-7-yosryahmed@google.com> In-Reply-To: <20240325235018.2028408-7-yosryahmed@google.com> From: Nhat Pham Date: Wed, 27 Mar 2024 09:40:57 -0700 Message-ID: Subject: Re: [RFC PATCH 6/9] mm: zswap: drop support for non-zero same-filled pages handling To: Yosry Ahmed 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-Server: rspam08 X-Rspamd-Queue-Id: 07E4E4001C X-Stat-Signature: u4dnedm9ty8aoy8eu5z9fdmgmhox34re X-Rspam-User: X-HE-Tag: 1711557668-632094 X-HE-Meta: U2FsdGVkX1+KuP4QD8RNaviMZ1UdFlnEG93scTo/CK9o/IMo9PIYfm2TJGoAHTpo3UXlImaMKqiuqYu9UUAsx36CcfpDoB+iMJQl29WHoVrQW/klMWvU9Y0yhEYz7sJhIVjGVMNjGCgkt4RWUZ1wQIWU5sF5GQhfEBE+rnjeJbNWmAlGvHaStNUaZKksqMAxV5MFp3YRi0aGL9LMRvZOL7UMrZsHskVqvYXqQLzPI0aNcF7tun2l0/gS0GZOJ+C8uS7OLXQH1j4E0AUw1MheMmnxloB62qxpsIuCfM3TQ9TWH+M5uO3k1nu3g448JbBwRYdA8s+zTKuDPYK7t++kQbEJgRCAwMkGBbnIuVOZlSzcVcsA6uZVCpOF0l1IynpUiUNEmZu10B9DMn4clHDmz6Mx6HIuU5hUb9/tTxKylHhFSUYoA34Vwl+CrqSaiOPa+1pH7AQ+oSTHl56VLgh1FLI1Ah0orRqRVYQ16i4PvJSZ1YKXW6hC8Ih1IpQiwZYayiWyYuyisPzF4QMbZc0UHHyQBAh5hco7EABzSlFX8z9hEYk4f4f6zBxpNnZ7/hRvrtXTKNmnjSsnFVlpNjtLtUhN2xowMMmuYdU10Wa2ojtqP99xPH5Tg/qPHpaFSMbe7TsE5fuuVmNzcilYuAW2acqONuFi9sapoEX9ejcDzOJAkwSdirpLmSpldfo+OEOlJLVXJe3pcOxxe7QwRPC2Gxgha5dKEh/YMSQq/Tfu0Dg5xFJ9VftgEV5wCU/Of/Z/zHheRR86ZvTsqqzkJh/UmdBpoO9cAL3YFH3OKrTE6/fWuLlCOuDkq1k4DF7X6Rxh58gn5yFhn+1SqxEgazy/8eJ3xgE7rfyRBhs8rzSxATY8o9/LEK8qaQgK3pcjAYKQnclzNcsiy2Mz99ofkj8Dtx62OVGmDsX1ZI5EZ+JNN3nbew0YcdUXyS5Dop+0sE/WfTO62HKbB/R0tnzWQD+ M70aWQFZ UFK/2bYRj1chRdiKgNUvkZgpApLLI+1yDhBKU 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 25, 2024 at 4:50=E2=80=AFPM Yosry Ahmed = wrote: > > The current same-filled pages handling supports pages filled with any > repeated word-sized pattern. However, in practice, most of these should > be zero pages anyway. Other patterns should be nearly as common. It'd be nice if we can verify this somehow. Maybe hooking bpftrace, trace_printk, etc. here? That aside, my intuition is that this is correct too. It's much less likely to see a non-zero filled page. > > Drop the support for non-zero same-filled pages, but keep the names of > knobs exposed to userspace as "same_filled", which isn't entirely > inaccurate. > > This yields some nice code simplification and enables a following patch > that eliminates the need to allocate struct zswap_entry for those pages > completely. > > There is also a very small performance improvement observed over 50 runs > of kernel build test (kernbench) comparing the mean build time on a > skylake machine when building the kernel in a cgroup v1 container with a > 3G limit: > > base patched % diff > real 70.167 69.915 -0.359% > user 2953.068 2956.147 +0.104% > sys 2612.811 2594.718 -0.692% > > This probably comes from more optimized operations like memchr_inv() and > clear_highpage(). Note that the percentage of zero-filled pages during TIL clear_highpage() is a thing :) > this test was only around 1.5% on average, and was not affected by this > patch. Practical workloads could have a larger proportion of such pages > (e.g. Johannes observed around 10% [1]), so the performance improvement > should be larger. > > [1]https://lore.kernel.org/linux-mm/20240320210716.GH294822@cmpxchg.org/ > > Signed-off-by: Yosry Ahmed > --- > mm/zswap.c | 76 ++++++++++++++---------------------------------------- > 1 file changed, 20 insertions(+), 56 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 0fc27ae950c74..413d9242cf500 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -44,8 +44,8 @@ > **********************************/ > /* The number of compressed pages currently stored in zswap */ > atomic_t zswap_stored_pages =3D ATOMIC_INIT(0); > -/* The number of same-value filled pages currently stored in zswap */ > -static atomic_t zswap_same_filled_pages =3D ATOMIC_INIT(0); > +/* The number of zero-filled pages currently stored in zswap */ > +static atomic_t zswap_zero_filled_pages =3D ATOMIC_INIT(0); > > /* > * The statistics below are not protected from concurrent access for > @@ -123,9 +123,9 @@ static unsigned int zswap_accept_thr_percent =3D 90; = /* of max pool size */ > module_param_named(accept_threshold_percent, zswap_accept_thr_percent, > uint, 0644); > > -/* Enable/disable handling non-same-value filled pages (enabled by defau= lt) */ > -static bool zswap_non_same_filled_pages_enabled =3D true; > -module_param_named(non_same_filled_pages_enabled, zswap_non_same_filled_= pages_enabled, > +/* Enable/disable handling non-zero-filled pages (enabled by default) */ > +static bool zswap_non_zero_filled_pages_enabled =3D true; > +module_param_named(non_same_filled_pages_enabled, zswap_non_zero_filled_= pages_enabled, > bool, 0644); > > /* Number of zpools in zswap_pool (empirically determined for scalabilit= y) */ > @@ -187,11 +187,10 @@ static struct shrinker *zswap_shrinker; > * > * swpentry - associated swap entry, the offset indexes into the red-bla= ck tree > * length - the length in bytes of the compressed page data. Needed dur= ing > - * decompression. For a same value filled page length is 0, and= both > + * decompression. For a zero-filled page length is 0, and both > * pool and lru are invalid and must be ignored. > * pool - the zswap_pool the entry's data is in > * handle - zpool allocation handle that stores the compressed page data > - * value - value of the same-value filled pages which have same content > * objcg - the obj_cgroup that the compressed memory is charged to > * lru - handle to the pool's lru used to evict pages. > */ > @@ -199,10 +198,7 @@ struct zswap_entry { > swp_entry_t swpentry; > unsigned int length; > struct zswap_pool *pool; > - union { > - unsigned long handle; > - unsigned long value; > - }; > + unsigned long handle; > struct obj_cgroup *objcg; > struct list_head lru; > }; > @@ -805,7 +801,7 @@ static struct zpool *zswap_find_zpool(struct zswap_en= try *entry) > static void zswap_entry_free(struct zswap_entry *entry) > { > if (!entry->length) > - atomic_dec(&zswap_same_filled_pages); > + atomic_dec(&zswap_zero_filled_pages); > else { > zswap_lru_del(&zswap_list_lru, entry); > zpool_free(zswap_find_zpool(entry), entry->handle); > @@ -1377,43 +1373,17 @@ static void shrink_worker(struct work_struct *w) > } while (zswap_total_pages() > thr); > } > > -static bool zswap_is_folio_same_filled(struct folio *folio, unsigned lon= g *value) > +static bool zswap_is_folio_zero_filled(struct folio *folio) > { > - unsigned long *page; > - unsigned long val; > - unsigned int pos, last_pos =3D PAGE_SIZE / sizeof(*page) - 1; > + unsigned long *kaddr; > bool ret; > > - page =3D kmap_local_folio(folio, 0); > - val =3D page[0]; > - > - if (val !=3D page[last_pos]) { > - ret =3D false; > - goto out; > - } > - > - for (pos =3D 1; pos < last_pos; pos++) { > - if (val !=3D page[pos]) { > - ret =3D false; > - goto out; > - } > - } > - > - *value =3D val; > - ret =3D true; > -out: > - kunmap_local(page); > + kaddr =3D kmap_local_folio(folio, 0); > + ret =3D !memchr_inv(kaddr, 0, PAGE_SIZE); > + kunmap_local(kaddr); > return ret; > } > > -static void zswap_fill_page(void *ptr, unsigned long value) > -{ > - unsigned long *page; > - > - page =3D (unsigned long *)ptr; > - memset_l(page, value, PAGE_SIZE / sizeof(unsigned long)); > -} > - > static bool zswap_check_limit(void) > { > unsigned long cur_pages =3D zswap_total_pages(); > @@ -1437,7 +1407,6 @@ bool zswap_store(struct folio *folio) > struct obj_cgroup *objcg =3D NULL; > struct mem_cgroup *memcg =3D NULL; > struct zswap_entry *entry; > - unsigned long value; > > VM_WARN_ON_ONCE(!folio_test_locked(folio)); > VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); > @@ -1470,14 +1439,13 @@ bool zswap_store(struct folio *folio) > goto reject; > } > > - if (zswap_is_folio_same_filled(folio, &value)) { > + if (zswap_is_folio_zero_filled(folio)) { > entry->length =3D 0; > - entry->value =3D value; > - atomic_inc(&zswap_same_filled_pages); > + atomic_inc(&zswap_zero_filled_pages); > goto insert_entry; > } > > - if (!zswap_non_same_filled_pages_enabled) > + if (!zswap_non_zero_filled_pages_enabled) > goto freepage; > > /* if entry is successfully added, it keeps the reference */ > @@ -1532,7 +1500,7 @@ bool zswap_store(struct folio *folio) > > store_failed: > if (!entry->length) > - atomic_dec(&zswap_same_filled_pages); > + atomic_dec(&zswap_zero_filled_pages); > else { > zpool_free(zswap_find_zpool(entry), entry->handle); > put_pool: > @@ -1563,7 +1531,6 @@ bool zswap_load(struct folio *folio) > struct page *page =3D &folio->page; > struct xarray *tree =3D swap_zswap_tree(swp); > struct zswap_entry *entry; > - u8 *dst; > > VM_WARN_ON_ONCE(!folio_test_locked(folio)); > > @@ -1573,11 +1540,8 @@ bool zswap_load(struct folio *folio) > > if (entry->length) > zswap_decompress(entry, page); > - else { > - dst =3D kmap_local_page(page); > - zswap_fill_page(dst, entry->value); > - kunmap_local(dst); > - } > + else > + clear_highpage(page); > > count_vm_event(ZSWPIN); > if (entry->objcg) > @@ -1679,7 +1643,7 @@ static int zswap_debugfs_init(void) > debugfs_create_atomic_t("stored_pages", 0444, > zswap_debugfs_root, &zswap_stored_pages); > debugfs_create_atomic_t("same_filled_pages", 0444, > - zswap_debugfs_root, &zswap_same_filled_pa= ges); > + zswap_debugfs_root, &zswap_zero_filled_pa= ges); > > return 0; > } > -- > 2.44.0.396.g6e790dbe36-goog > The code itself LGTM, FWIW: Reviewed-by: Nhat Pham