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 1AD56C47DD9 for ; Wed, 27 Mar 2024 11:25:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9DE4D6B0082; Wed, 27 Mar 2024 07:25:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 98DA56B0085; Wed, 27 Mar 2024 07:25:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 855526B0087; Wed, 27 Mar 2024 07:25:29 -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 696AF6B0082 for ; Wed, 27 Mar 2024 07:25:29 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id F35D2A0C2A for ; Wed, 27 Mar 2024 11:25:28 +0000 (UTC) X-FDA: 81942588336.05.1F5A09C Received: from out-175.mta0.migadu.com (out-175.mta0.migadu.com [91.218.175.175]) by imf04.hostedemail.com (Postfix) with ESMTP id 08C7940010 for ; Wed, 27 Mar 2024 11:25:26 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b="u2KTNfo/"; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf04.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.175 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711538727; a=rsa-sha256; cv=none; b=6YG5MCAQCGcIlR8m00eX/Yraj+d6Q8r567bSR//2RsmN4Vwdm4ojzgm3GCEpBBgq4SARwT NDVLYSrm7+46Kvi1qfYfL2CReaORgpk0SFX/3oAPWpOTCrz/egRvyHEdj5TAh3gafsQsM9 4SLw+vJgfmfoZGjLAfIP8duYh+Fcx20= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b="u2KTNfo/"; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf04.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.175 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711538727; 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=HGhimz4I+pVhVYQfazvBz3KUFgUI5ZIiXgdUoL/VTWc=; b=2M64pgOk26ofP9tagyjSg+4LkVJhmelnckEvszJUNd3CZV5WNdn78WT4f7gk0p3vKJYX6F U1zaxR2OmbjPYq9LPI4QP4U4QUa/6dxMedIIRnAPZ+RpmJ6vsEtbw5GTbZWKyLuPvUvXwp l1ZlVP1CM6wEIGbQY38042Pz/nMbDJc= Message-ID: <69c6d3f2-d201-43f2-b8c0-3599fb20ac61@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1711538724; h=from:from: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; bh=HGhimz4I+pVhVYQfazvBz3KUFgUI5ZIiXgdUoL/VTWc=; b=u2KTNfo/yr5o6IFag1zRJkv6o8rDcKwPhMtE8VcPSpRKwsZof04hSXsIjPCaqyzGOE2RFM IOc0cvni6T86ZxzL6iQWfXdtu22ageBk80iiZp7MYV0erSmf2W7B0NM/y8IrkYNIwE4khR W3dEFT5XBfctkKI5QPMlDEbIsRFSof8= Date: Wed, 27 Mar 2024 19:25:18 +0800 MIME-Version: 1.0 Subject: Re: [RFC PATCH 6/9] mm: zswap: drop support for non-zero same-filled pages handling Content-Language: en-US To: Yosry Ahmed , Andrew Morton Cc: Johannes Weiner , Nhat Pham , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240325235018.2028408-1-yosryahmed@google.com> <20240325235018.2028408-7-yosryahmed@google.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: <20240325235018.2028408-7-yosryahmed@google.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 08C7940010 X-Stat-Signature: paoawhahd1p5pxebp9tzu8opf6tp7s3m X-HE-Tag: 1711538726-106097 X-HE-Meta: U2FsdGVkX1/e2un4qQ6MqUYQUEoQ6sMLLzvowEnlrNQ5O+hYXA3hk580e6BHgMHmjK4i//gmEedQnZ18y0KhkYx6v3KmMDsvffMYt4ZwSxF4kCgwv5G5S5p2PKvsIMdnRWfRu7VlTva1qgNF9HKknl76SfdwzyTxkpEWIGbmUgICGZpySZbUgo8dxjoT7GbmvKUkQXkEgKedcU3sgvuX2tlzlTTd1MLnTmfHiOyJ11jpVpm8MQuJL2UaTjCh6YZhGkckkI5e8BBc0QuDhCStQA41II8lxNo01nV/IYmfzHSySOb5B66cxqePBnuET3MzxQEQUtj9EVVeo0U/uxS+pGvpgVPRYTfwxQPY8OeaT7Dw5Ft5yhhSD+shEpg8YpEdOMi+sdOwmE/fQxoeo56aR9Uh6XEi/mJz4Xx1/W95qAwhYI6VCd8l+v5GFgrk2YL+ryKNYOl5xNIMb9S9pXAQaWjOVUpBNDTk3zjNvXYAjhk92vjKor7/Bd0nn3e0m+EHMqr3dICjRJsNNtVbQfkcOV2BUJta0BkbzISl4NkZZmPIJzVFWMuDHJ1xUIMndZz4aQaqrNmUQqiez2DWZr9arybo5IgvI03d720ihxQIstt4v4jzmYygqE1KPooOdNHvH25VJ4AvV87o2CsMT2T4eYEXkS6xdNn0qBnroFR8hrlW4fBKOf+wIEqHPJSUVu2w/oDVZtNvCz4zDnqh+TIUcoRgRkUOL8xG7yliZOKlocZ2GBLczHsI5HwCTx3z5Cm7qSq/pFqea7iH1Q9bHsrgzZVGFdKMvZxLI+Cx0SKVkBWcsIoJKeDnDjM70RVLNr5PURngBK6jSvWLfVmvwk++TPsA1bGGxlU2y0we+ExjYH6PXQnFbb13M1SNZ1Ru9s/2z5woAKID6rqo1RiQU3rgYFgtAOcCMTnWver/PJuvFKUaCreIhFnF/+DOrlTNNiHWX8EDh1tDqsrsI0bqTwY ztmmeOKc vF6eoJmVyOBehT/YIXVyKxC8YDYVKv07mNIT8w6GVnvMy6BM7LJgvXoF+lBi5iKKNiMQATgn6GVEek7hz8UbWIQg/tkgdikdLjK9TluQMlNYfYqzRY+tnLaeecJCG2lx+1cOQckbzJQ3nr2Zd4p0gUVIdDTDZsCDqB13neZdnIvQj0/gDOYOWvOLjWcK6qETkr9VqJ9R+NFTpwVxHUaMhV7djngadAXewjqtZOJBo4EW61JQ9mTObg2PI/5Z9HLOn7Ze1SmaIfVMDppw= 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 2024/3/26 07:50, 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. > > 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 > 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 The code looks good! Reviewed-by: Chengming Zhou Thanks. > --- > 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 = ATOMIC_INIT(0); > -/* The number of same-value filled pages currently stored in zswap */ > -static atomic_t zswap_same_filled_pages = ATOMIC_INIT(0); > +/* The number of zero-filled pages currently stored in zswap */ > +static atomic_t zswap_zero_filled_pages = ATOMIC_INIT(0); > > /* > * The statistics below are not protected from concurrent access for > @@ -123,9 +123,9 @@ static unsigned int zswap_accept_thr_percent = 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 default) */ > -static bool zswap_non_same_filled_pages_enabled = 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 = 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 scalability) */ > @@ -187,11 +187,10 @@ static struct shrinker *zswap_shrinker; > * > * swpentry - associated swap entry, the offset indexes into the red-black tree > * length - the length in bytes of the compressed page data. Needed during > - * 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_entry *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 long *value) > +static bool zswap_is_folio_zero_filled(struct folio *folio) > { > - unsigned long *page; > - unsigned long val; > - unsigned int pos, last_pos = PAGE_SIZE / sizeof(*page) - 1; > + unsigned long *kaddr; > bool ret; > > - page = kmap_local_folio(folio, 0); > - val = page[0]; > - > - if (val != page[last_pos]) { > - ret = false; > - goto out; > - } > - > - for (pos = 1; pos < last_pos; pos++) { > - if (val != page[pos]) { > - ret = false; > - goto out; > - } > - } > - > - *value = val; > - ret = true; > -out: > - kunmap_local(page); > + kaddr = kmap_local_folio(folio, 0); > + ret = !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 = (unsigned long *)ptr; > - memset_l(page, value, PAGE_SIZE / sizeof(unsigned long)); > -} > - > static bool zswap_check_limit(void) > { > unsigned long cur_pages = zswap_total_pages(); > @@ -1437,7 +1407,6 @@ bool zswap_store(struct folio *folio) > struct obj_cgroup *objcg = NULL; > struct mem_cgroup *memcg = 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 = 0; > - entry->value = 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 = &folio->page; > struct xarray *tree = 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 = 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_pages); > + zswap_debugfs_root, &zswap_zero_filled_pages); > > return 0; > }