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 460FEC7EE29 for ; Wed, 7 Jun 2023 09:30:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D146F8E0001; Wed, 7 Jun 2023 05:30:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CC3256B0072; Wed, 7 Jun 2023 05:30:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B8B318E0001; Wed, 7 Jun 2023 05:30:44 -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 A47376B0071 for ; Wed, 7 Jun 2023 05:30:44 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 5DB951601C4 for ; Wed, 7 Jun 2023 09:30:44 +0000 (UTC) X-FDA: 80875432008.13.617419C Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by imf05.hostedemail.com (Postfix) with ESMTP id 60128100006 for ; Wed, 7 Jun 2023 09:30:42 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=dd7szx7k; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.53 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=1686130242; 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=N0zgFaNuQcABlLa+CTwGXRDPJZJRvb9S50Rqfd30V94=; b=LarJY9jg5NdVhtMfXzBU5IVl9qHOQTXZSEaVaRiOkDUupKlGQstw1ODjEeum0x7K1F2+3/ KHUYE3fALaWWfOM178Pf/TAuO4M6L/Qt6WIIM9yWEgknJAmQ44GwLSOwfIGz8vbJoBblex OQPoaH1OdpQYl/OW61h+ccHiW69Jr2o= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686130242; a=rsa-sha256; cv=none; b=LEVKBUe4++nvIHus7RGVYY/FoFSiDdpmFknc3P3wr1IH9oHesv0e2ho/oxzi1y37xpOQK2 Jt3MIk8DnXrSP9/LdUxpfJHPsAVcUxaSuhZT2zQi5IamrzCm3zGMSdMhjmNU5WkxruXJzP Fb783Qu+1JPljXB/urhiCnQW2RbNkFk= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=dd7szx7k; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-977c8423dccso81281466b.1 for ; Wed, 07 Jun 2023 02:30:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686130241; x=1688722241; 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=N0zgFaNuQcABlLa+CTwGXRDPJZJRvb9S50Rqfd30V94=; b=dd7szx7k41G43KBm/a86dAGJAaPQLmdJdamkHxlEJzb0d0kNobqsdlaxAnL5DcK2TP B9mzPxvBcNE1fycquQg0sC/KmTkguVAAv7fBxhZIvTs89TehT7e8jzOhIVYx2gwCibIO ZOR1a/Kln82tfx5u4wEGSV715+5cA6nldnUNeJ2QJiIWPSC0+D3Vuk545GtzZMsblqBc VY+c4C5/A/KAgMhiTMNVIwyV0R8udwbxHzs37hsHxY4rWb+TXyj+NXqL/5uGQReVkE0P c0XnnWFHFDF/axno6XSYA8wEhJ5dLCXe74GqH2Ca8IU+f1UyNydlqvOaNIQtCf0ZTcim +wqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686130241; x=1688722241; 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=N0zgFaNuQcABlLa+CTwGXRDPJZJRvb9S50Rqfd30V94=; b=WAVJUpSRkxR3RG5q9wHmOZoPXpXWhKeGiCMHkTEqdDFa85HdZTwLoOcV3D3ZPjbL3i rYT0SMV0/3Kp5Yg0STat4zOqtu8g8GiAQBByeFajWDF662BDrzxv0qePlHT+4nbPVA2O gMfLu/p1bhy+X09kqOwWcR1gyIQ9bn7WrJ7ipRGEv5QglQOxgeYYgC5sbUqAMJvrj2WJ vdNaqPBH5B5jNheI50i5ZX6kkNKk11jyqncZfXkQKdu9DdGQbwhQfMVVJBwmzYPeoiUZ M0GSA8v+dcjXo1sEvfzx1uWxLD/HS9Fs6mMyZNJejw8aDBBk0IBOzvU6DCNTiRe+j1SR unfA== X-Gm-Message-State: AC+VfDxS1apBReUAzhW4ufjgBM1vqj7odfJB3ZKxdoigBZuHWOwupiCL BzhQkg4bDluGic+yc5D/ZVYTO2v6yMC5By2d/Hmfag== X-Google-Smtp-Source: ACHHUZ6dP0+GRDVU3RS0pq3FfZaKfF9ok/JvpmsJGnYI4zVk5QLA0qcIdaA2ON/hto583ordiiISon8g+1gfeCvCkeQ= X-Received: by 2002:a17:907:9810:b0:94e:4285:390c with SMTP id ji16-20020a170907981000b0094e4285390cmr5445077ejc.10.1686130240644; Wed, 07 Jun 2023 02:30:40 -0700 (PDT) MIME-Version: 1.0 References: <20230606145611.704392-1-cerasuolodomenico@gmail.com> <20230606145611.704392-8-cerasuolodomenico@gmail.com> In-Reply-To: <20230606145611.704392-8-cerasuolodomenico@gmail.com> From: Yosry Ahmed Date: Wed, 7 Jun 2023 02:30:04 -0700 Message-ID: Subject: Re: [RFC PATCH v2 7/7] mm: zswap: remove zswap_header To: Domenico Cerasuolo Cc: vitaly.wool@konsulko.com, minchan@kernel.org, senozhatsky@chromium.org, linux-mm@kvack.org, ddstreet@ieee.org, sjenning@redhat.com, nphamcs@gmail.com, hannes@cmpxchg.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: mnogb1zb55hyh4jk5nmq8637arzz961x X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 60128100006 X-Rspam-User: X-HE-Tag: 1686130242-888001 X-HE-Meta: U2FsdGVkX18oETm5F1g9boanPGPupydw1Zs02x9794IqR/+Ol3Nv+1I7ow0U2620FX4Bd5NwgdxP+p+EQPA4UxGf0ACnk4V70e7m+Yx5cvkSSO+qrKJeY5l+yzU3WCwGcYa3iDTgy3ZVgNsaEs9IPyAbazWzbKHv4+U+37nuu4V4wSNP7NETTsXAYdR3yOhxnVFzuD4yRm/qr3zCssTDQHzZwu6qJVQiP5QzWtE+72VuSL5Cr3Sn7tN5OpNWs5VLQudZLIvNNE6uqr6u++i/L7A/L8ZAa0Fs3nAFu9vXKQ7QQwGFoSxMlB7s786BHbzFRoSJmIwiS/ZZBMDp6wYY2OJd3pI2BVqGddVCxVgeo3nzTaSkkzhSDFoRimXKa0lm3QLvcrnv9GV/Ren77LT+Vjqp0VK4/IzmA+YTDCEfyxG1hVmnmSdpUR9iZj2H/hdfIeR+165tRgnKlw7v1lAZnYfqmZnrwWjNla+FgKtId+kDNUClm8J/QduCdhkX6WiTe4Z0wQ+Dd6pw10zRW4jqklSRVcaFSZ2aYgZYxf/xeosq8jBbgjoRhxVCVZ6J95zLSnTyI8+EzMrZWRKvAyrzfEzkj7N8pDTC+6PBBgNS22XhA5xCHRSzUFFp0wCYV/SeAZNtnv8EE1SUBYyNNlZvgoytSFYDRURv94rdZt3NDuG/f+gzJ6w1jQ298jmp6owKVfzk4Gig2SH03Y9bDbs6AB1vxv8SeN7Jz/mw1Zo1jmZxDelZhH47pm0VeFc1yo7xNr4fjZYxX33khC+NxKm9x75/0Jrusl+8d7oZzNeE+bOwK54TUO9KAzRzOyHqAV5Z+X8q0ZuFZCsMolwgkvmoJFa99tHzQ6atL/VyUG22KYa4t3veGhklCzI83lXJZn9Yvdh9pMh7S582ls28fRThMZVEt4+hqZey/Jeefqk1mmtPTWHgIUlQ04i2pCrmw33OVtTbAFw1Iwi5LpsQQK/ +QvmFBUh K4r7CoIZaHr4tcn9PGbunhh4Zjp+CrvnVYPlvkU98H9CA7i96yrjoOAZp6em9K/6t0Pa1gZBTsxOrBi/WJlghen3I/WZC+GFEey4CUX/y17W+Wgmsf5vSHQm/oiYsGXKcDX5i5oXYLXW2P+/E/a0UwS+UxeavBV59XFqSRtOtXm06D/ymv1ORJcfrEkaz6yKj09vR4Kd16pHDi5f3d1wxKOlldUBFfiOiHnrX79mCoGp0d5h5PwrInp8uRkahaSUgEAMkIZwFR9ihdxEuk/4ajcMdVw5tqBkEN19z1F0hZCBOH8aBmAMsQttS2x8SaZNzGgxq 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: On Tue, Jun 6, 2023 at 7:56=E2=80=AFAM Domenico Cerasuolo wrote: > > Previously, zswap_header served the purpose of storing the swpentry > within zpool pages. This allowed zpool implementations to pass relevant > information to the writeback function. However, with the current > implementation, writeback is directly handled within zswap. > Consequently, there is no longer a necessity for zswap_header, as the > swp_entry_t can be stored directly in zswap_entry. > > Suggested-by: Yosry Ahmed > Signed-off-by: Domenico Cerasuolo Thanks for this cleanup. It gives us back some of the memory used by list_head in zswap entry, and remove an unnecessary zpool map operation. > --- > mm/zswap.c | 52 ++++++++++++++++++++++------------------------------ > 1 file changed, 22 insertions(+), 30 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index ef8604812352..f689444dd5a7 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -193,7 +193,7 @@ struct zswap_pool { > */ > struct zswap_entry { > struct rb_node rbnode; > - pgoff_t offset; > + swp_entry_t swpentry; > int refcount; > unsigned int length; > struct zswap_pool *pool; > @@ -205,10 +205,6 @@ struct zswap_entry { > struct list_head lru; > }; > > -struct zswap_header { > - swp_entry_t swpentry; > -}; > - > /* > * The tree lock in the zswap_tree struct protects a few things: > * - the rbtree > @@ -250,7 +246,7 @@ static bool zswap_has_pool; > pr_debug("%s pool %s/%s\n", msg, (p)->tfm_name, \ > zpool_get_type((p)->zpool)) > > -static int zswap_writeback_entry(struct zswap_entry *entry, struct zswap= _header *zhdr, > +static int zswap_writeback_entry(struct zswap_entry *entry, > struct zswap_tree *tree); > static int zswap_pool_get(struct zswap_pool *pool); > static void zswap_pool_put(struct zswap_pool *pool); > @@ -311,12 +307,14 @@ static struct zswap_entry *zswap_rb_search(struct r= b_root *root, pgoff_t offset) > { > struct rb_node *node =3D root->rb_node; > struct zswap_entry *entry; > + pgoff_t entry_offset; > > while (node) { > entry =3D rb_entry(node, struct zswap_entry, rbnode); > - if (entry->offset > offset) > + entry_offset =3D swp_offset(entry->swpentry); > + if (entry_offset > offset) > node =3D node->rb_left; > - else if (entry->offset < offset) > + else if (entry_offset < offset) > node =3D node->rb_right; > else > return entry; > @@ -333,13 +331,15 @@ static int zswap_rb_insert(struct rb_root *root, st= ruct zswap_entry *entry, > { > struct rb_node **link =3D &root->rb_node, *parent =3D NULL; > struct zswap_entry *myentry; > + pgoff_t myentry_offset, entry_offset =3D swp_offset(entry->swpent= ry); > > while (*link) { > parent =3D *link; > myentry =3D rb_entry(parent, struct zswap_entry, rbnode); > - if (myentry->offset > entry->offset) > + myentry_offset =3D swp_offset(myentry->swpentry); > + if (myentry_offset > entry_offset) > link =3D &(*link)->rb_left; > - else if (myentry->offset < entry->offset) > + else if (myentry_offset < entry_offset) This whole myentry thing is very confusing to me. I initially thought myentry would be the entry passed in as an argument. Can we change the naming here to make it more consistent with zswap_rb_search() naming? > link =3D &(*link)->rb_right; > else { > *dupentry =3D myentry; > @@ -598,7 +598,6 @@ static struct zswap_pool *zswap_pool_find_get(char *t= ype, char *compressor) > static int zswap_shrink(struct zswap_pool *pool) > { > struct zswap_entry *lru_entry, *tree_entry =3D NULL; > - struct zswap_header *zhdr; > struct zswap_tree *tree; > int swpoffset; > int ret; > @@ -611,15 +610,13 @@ static int zswap_shrink(struct zswap_pool *pool) > } > lru_entry =3D list_last_entry(&pool->lru, struct zswap_entry, lru= ); > list_del_init(&lru_entry->lru); > - zhdr =3D zpool_map_handle(pool->zpool, lru_entry->handle, ZPOOL_M= M_RO); > - tree =3D zswap_trees[swp_type(zhdr->swpentry)]; > - zpool_unmap_handle(pool->zpool, lru_entry->handle); > /* > * Once the pool lock is dropped, the lru_entry might get freed. = The > * swpoffset is copied to the stack, and lru_entry isn't deref'd = again > * until the entry is verified to still be alive in the tree. > */ > - swpoffset =3D swp_offset(zhdr->swpentry); > + swpoffset =3D swp_offset(lru_entry->swpentry); > + tree =3D zswap_trees[swp_type(lru_entry->swpentry)]; > spin_unlock(&pool->lru_lock); > > /* hold a reference from tree so it won't be freed during writeba= ck */ > @@ -633,7 +630,7 @@ static int zswap_shrink(struct zswap_pool *pool) > } > spin_unlock(&tree->lock); > > - ret =3D zswap_writeback_entry(lru_entry, zhdr, tree); > + ret =3D zswap_writeback_entry(lru_entry, tree); > > spin_lock(&tree->lock); > if (ret) { > @@ -1046,10 +1043,10 @@ static int zswap_get_swap_cache_page(swp_entry_t = entry, > * the swap cache, the compressed version stored by zswap can be > * freed. > */ > -static int zswap_writeback_entry(struct zswap_entry *entry, struct zswap= _header *zhdr, > +static int zswap_writeback_entry(struct zswap_entry *entry, > struct zswap_tree *tree) > { > - swp_entry_t swpentry =3D zhdr->swpentry; > + swp_entry_t swpentry =3D entry->swpentry; > struct page *page; > struct scatterlist input, output; > struct crypto_acomp_ctx *acomp_ctx; > @@ -1089,7 +1086,7 @@ static int zswap_writeback_entry(struct zswap_entry= *entry, struct zswap_header > * writing. > */ > spin_lock(&tree->lock); > - if (zswap_rb_search(&tree->rbroot, entry->offset) !=3D en= try) { > + if (zswap_rb_search(&tree->rbroot, swp_offset(entry->swpe= ntry)) !=3D entry) { > spin_unlock(&tree->lock); > delete_from_swap_cache(page_folio(page)); > ret =3D -ENOMEM; > @@ -1101,8 +1098,7 @@ static int zswap_writeback_entry(struct zswap_entry= *entry, struct zswap_header > acomp_ctx =3D raw_cpu_ptr(entry->pool->acomp_ctx); > dlen =3D PAGE_SIZE; > > - zhdr =3D zpool_map_handle(pool, entry->handle, ZPOOL_MM_R= O); > - src =3D (u8 *)zhdr + sizeof(struct zswap_header); > + src =3D zpool_map_handle(pool, entry->handle, ZPOOL_MM_RO= ); > if (!zpool_can_sleep_mapped(pool)) { > memcpy(tmp, src, entry->length); > src =3D tmp; > @@ -1196,11 +1192,10 @@ static int zswap_frontswap_store(unsigned type, p= goff_t offset, > struct obj_cgroup *objcg =3D NULL; > struct zswap_pool *pool; > int ret; > - unsigned int hlen, dlen =3D PAGE_SIZE; > + unsigned int dlen =3D PAGE_SIZE; > unsigned long handle, value; > char *buf; > u8 *src, *dst; > - struct zswap_header zhdr =3D { .swpentry =3D swp_entry(type, offs= et) }; > gfp_t gfp; > > /* THP isn't supported */ > @@ -1245,7 +1240,7 @@ static int zswap_frontswap_store(unsigned type, pgo= ff_t offset, > src =3D kmap_atomic(page); > if (zswap_is_page_same_filled(src, &value)) { > kunmap_atomic(src); > - entry->offset =3D offset; > + entry->swpentry =3D swp_entry(type, offset); > entry->length =3D 0; > entry->value =3D value; > atomic_inc(&zswap_same_filled_pages); > @@ -1299,11 +1294,10 @@ static int zswap_frontswap_store(unsigned type, p= goff_t offset, > } > > /* store */ > - hlen =3D sizeof(zhdr); > gfp =3D __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM; > if (zpool_malloc_support_movable(entry->pool->zpool)) > gfp |=3D __GFP_HIGHMEM | __GFP_MOVABLE; > - ret =3D zpool_malloc(entry->pool->zpool, hlen + dlen, gfp, &handl= e); > + ret =3D zpool_malloc(entry->pool->zpool, dlen, gfp, &handle); > if (ret =3D=3D -ENOSPC) { > zswap_reject_compress_poor++; > goto put_dstmem; > @@ -1313,13 +1307,12 @@ static int zswap_frontswap_store(unsigned type, p= goff_t offset, > goto put_dstmem; > } > buf =3D zpool_map_handle(entry->pool->zpool, handle, ZPOOL_MM_WO)= ; > - memcpy(buf, &zhdr, hlen); > - memcpy(buf + hlen, dst, dlen); > + memcpy(buf, dst, dlen); > zpool_unmap_handle(entry->pool->zpool, handle); > mutex_unlock(acomp_ctx->mutex); > > /* populate entry */ > - entry->offset =3D offset; > + entry->swpentry =3D swp_entry(type, offset); > entry->handle =3D handle; > entry->length =3D dlen; > > @@ -1418,7 +1411,6 @@ static int zswap_frontswap_load(unsigned type, pgof= f_t offset, > /* decompress */ > dlen =3D PAGE_SIZE; > src =3D zpool_map_handle(entry->pool->zpool, entry->handle, ZPOOL= _MM_RO); > - src +=3D sizeof(struct zswap_header); > > if (!zpool_can_sleep_mapped(entry->pool->zpool)) { > memcpy(tmp, src, entry->length); > -- > 2.34.1 >