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 B2E12C77B7A for ; Wed, 7 Jun 2023 09:27:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4B0486B0071; Wed, 7 Jun 2023 05:27:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 439E46B0072; Wed, 7 Jun 2023 05:27:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2DA1C900002; Wed, 7 Jun 2023 05:27:17 -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 16AEE6B0071 for ; Wed, 7 Jun 2023 05:27:17 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id BA692AE96E for ; Wed, 7 Jun 2023 09:27:16 +0000 (UTC) X-FDA: 80875423272.18.9120F28 Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by imf19.hostedemail.com (Postfix) with ESMTP id D34AE1A0029 for ; Wed, 7 Jun 2023 09:27:14 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=wCuhghmj; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686130034; 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=NeOVyyzx2aFg+HiDo8xoGuPCyLIjyDb2msJx9kNdD80=; b=Vcz0YCy6MMkjC9o5eakMCoq95dRbPmZaUEbiGZ3OWwaUiHKBzj1YQjYDDrvYSWPRG7SQAD CyNd/TCZxnPSBw/08icV2Uz8Zagfr1EnWpDZDFSptMVTSfTuJhKG2Nfq1/jZdrI1AirJTA 3axvsY3VYy/2k4dnxZLyal/78xCDnR8= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=wCuhghmj; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686130034; a=rsa-sha256; cv=none; b=cq5Qg2GTIVG7zZKcFwUYzzR3IxF9PP+cSYqhPb/Wf7V+5SzNUcRaYjbbHoaPPn6BSgkL51 mPqYHFAqvnz7YGeI+cF0kkRTFUgeAYooj6IeK1JHQutGJmEBNkL2sKdh2KjlXIwkeb9Nr+ 3XKj8Wisv8aW28MCgA3yb3eyxM/qb2I= Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-9786fc23505so252066966b.2 for ; Wed, 07 Jun 2023 02:27:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686130033; x=1688722033; 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=NeOVyyzx2aFg+HiDo8xoGuPCyLIjyDb2msJx9kNdD80=; b=wCuhghmjcdqhSltrPabDz96VMoaq3h7gnM8P3G85h5r+4H1Bg8PiV1KggTHgHUS5Ps H24m3Yehlh0P/v7kPkX2BcyJyhk/bJuQmtVT2jYC99ZX2i22T7iE97RRsROtEKoYN63v GV3oH5JXO2kHz4uc0jGOfVlgQ+HEE9QNDsAoOQxWb9L0YcZZ4xpAl/yGD+K9K3Sy6y7V lpyyCoDwPdpOlFJlgn13YVr6Tr/HLsZY0GgIF7/rL9RPpInQGdfMot0t59qmnpy+lcyQ YZWETC7zKGNfWlIHHjUCeEM891MwyVX+LHA1xWHelZn5YefVCWFYVv3K1dAGh+qLIYKA CNGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686130033; x=1688722033; 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=NeOVyyzx2aFg+HiDo8xoGuPCyLIjyDb2msJx9kNdD80=; b=YwTpyXXXESWwZbKQR5CQVBRE94GA9R56RIZnFFJsuqldW6FJ7PF6TJhYMRsVOT4+ir utB3zfyRKbDUNMG1viklv25bS/cuLhfzXYiC3WqnOU8Gh+Zwr5NtOz/hnDgYu++QsLrD 1iIt9RGi8yewI7o5a+aLY+Lg0SRiD7oKkL7jNcxaaW40lsWEfEC/ykEciHwkQx87Vs8C kzWcaLGB+RTrJj2910DRTqmijV0D3pcHAvs9Q0YFqxkgTA5YEfUDqCnMHrea9UkpHzab rOL+gAWD+hP3S4beI2QQb3T+hGDRpFZ/vhJis1bGWpYUjdE+H4LkwySAGNf7XPLsZFYX CVaQ== X-Gm-Message-State: AC+VfDxPNzt3/c3dIndDsTl41iKAi/SbJkoqKvF8KdV/6y5G9HPZ8pme 1hO4psY6jFb+23D/WzudZbSczHHSZhivZw2Zf0cvIQ== X-Google-Smtp-Source: ACHHUZ6Rb3Tred2Ry6K0iwQMFvAK9MVGZ16UUCiLX1gNCkFNCO/mOM00ePKvztpPtrOgoNhRAMGyP7KKCBZTuQN/2fc= X-Received: by 2002:a17:907:eaa:b0:973:e69d:c720 with SMTP id ho42-20020a1709070eaa00b00973e69dc720mr6689340ejc.51.1686130033219; Wed, 07 Jun 2023 02:27:13 -0700 (PDT) MIME-Version: 1.0 References: <20230606145611.704392-1-cerasuolodomenico@gmail.com> <20230606145611.704392-7-cerasuolodomenico@gmail.com> In-Reply-To: <20230606145611.704392-7-cerasuolodomenico@gmail.com> From: Yosry Ahmed Date: Wed, 7 Jun 2023 02:26:37 -0700 Message-ID: Subject: Re: [RFC PATCH v2 6/7] mm: zswap: simplify writeback function 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-Rspamd-Queue-Id: D34AE1A0029 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 5h9my1m387iagyx819uc6shyq3tiagks X-HE-Tag: 1686130034-454314 X-HE-Meta: U2FsdGVkX1+Jp9Z7Mv/xQYmXeaeWOU8rkXLoY0w/2gPxGvC1PvL1md5ZQQ6OfhtqZVIDyE3/cPjwM37R4tH0Zd2h/Q+4drZyOiZDWTdiukkB+M908vLDHQARzAoFSPcIlLr2kaORHMKal/m4phs5h941TowzhJLPVRxMdUU++yBHKD5FX8cXVM4RHOwVjhuRjsyrdA4SECBiHj5mFMutUbtgZAebDtGyGXewo7Q3pL+uhu9xNGnDbKY7CfFg70LTC/Rdq35ToeWcRRt5nW9YDNGSDn0FBtK0xNXeDxsAKSDrQ9jAWz9djGaRTddDEjHxJjz2uIWPA2cZUoeWKjBhUo0ezFriy9q0gmvNvQxoQBYCk1IPx+A22dK89LxPy22U9lvRi1FsdIcAUimBqGNA7IJQzVZgpmHaFb7buufIjts3MNZQVxqjwrw7bhX0p3aIhMlW7vSIXP472pUHlz7ax2uH3COUQWG+XbA0OSCjH2KAFtDo0byMlPu3b8NdjzRDfwjmj0QU1fXiwQ555+GK9wpfMyx7hiOoFOYdn3DkZ9nnFxEIwa1xxFvsKWRB8iqWV/+pFIEkh27RjF/vuRGUekQ+DxOQympvlC7uC3kKykngQnMGuOwXHUq/d6Ag8FQhSnaaD2sUJEZGymPCt0mvyJz19IBaZ5JKVTRdtCFIESD33LmCwS04CMQHmUdndVeGL1eai/tV4verMVq4tOSR9i2gxROUPBeKiMl17jYARA/Lz7LoksD4M6cpnYQMPpL9oo6QDkneILRo8bSqAyrVsreNqFYCGSVR+7Hjuj9FW4rDPYWVVHMr+ZGZ29yu3JDGU05Kysk+5HnrlaF5TXc+eV6NMm949qFnYe6Mgg2lUbfH7ABrhZOm6kFkmYy/fpoUoodv6ipva4rhks6SshIFPAbW1Cy5x1rTgO7qHhEKrccr7MHsRi/rgUdTj01ySOTaJGBXsulw9s5GCgLq377 9mZXo5iF FV3ZEF8ZOm5XhKGToSsQa8pjiC0shmb/MxAGOHmuXAmwL/bM+wwDU7ZySChXRPcY+e/J0E7vpEfCdSDy1h4dxM8alDppLuX6GIV9tZ5Dfqo7qlGUh1BGuJy8383uHdZdAl9231Z0nSfnrVNb34Hp0/gceimGcammIGD/h6Vpe5CNVy7SGDdWQEpA88B+I1hRxQmq3goZRvrlSOWg23ihXPl/YEUqG4+nD7j+zW/RiENn7cqvRCmRgeWPf8Q== 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, in zswap, the writeback function was passed to zpool drivers > for their usage in calling the writeback operation. However, since the > drivers did not possess references to entries and the function was > specifically designed to work with handles, the writeback process has > been modified to occur directly within zswap. Consequently, this change > allows for some simplification of the writeback function, taking into > account the updated workflow. > > Signed-off-by: Domenico Cerasuolo > --- > mm/zswap.c | 69 ++++++++++++++---------------------------------------- > 1 file changed, 17 insertions(+), 52 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 2831bf56b168..ef8604812352 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -250,7 +250,8 @@ 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 zpool *pool, unsigned long handl= e); > +static int zswap_writeback_entry(struct zswap_entry *entry, struct zswap= _header *zhdr, > + struct zswap_tree *tree); > static int zswap_pool_get(struct zswap_pool *pool); > static void zswap_pool_put(struct zswap_pool *pool); > > @@ -632,15 +633,21 @@ static int zswap_shrink(struct zswap_pool *pool) > } > spin_unlock(&tree->lock); > > - ret =3D zswap_writeback_entry(pool->zpool, lru_entry->handle); > + ret =3D zswap_writeback_entry(lru_entry, zhdr, tree); > > spin_lock(&tree->lock); > if (ret) { > spin_lock(&pool->lru_lock); > list_move(&lru_entry->lru, &pool->lru); > spin_unlock(&pool->lru_lock); > + zswap_entry_put(tree, tree_entry); > + } else { > + /* free the local reference */ > + zswap_entry_put(tree, tree_entry); > + /* free the entry if it's not been invalidated*/ > + if (lru_entry =3D=3D zswap_rb_search(&tree->rbroot, swpof= fset)) > + zswap_entry_put(tree, tree_entry); The comment that was here about the 2 possible cases was useful imo, maybe keep it? Also, I am not sure why we need to do a tree search vs. just reading the refcount here before the first put. We can even make zswap_entry_put() return the refcount after the put to know if we need the additional put or not. Can anyone think of any reason why we need to explicitly search the tree he= re? > } > - zswap_entry_put(tree, tree_entry); > spin_unlock(&tree->lock); > > return ret ? -EAGAIN : 0; > @@ -1039,16 +1046,14 @@ 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 zpool *pool, unsigned long handl= e) > +static int zswap_writeback_entry(struct zswap_entry *entry, struct zswap= _header *zhdr, > + struct zswap_tree *tree) > { > - struct zswap_header *zhdr; > - swp_entry_t swpentry; > - struct zswap_tree *tree; > - pgoff_t offset; > - struct zswap_entry *entry; > + swp_entry_t swpentry =3D zhdr->swpentry; > struct page *page; > struct scatterlist input, output; > struct crypto_acomp_ctx *acomp_ctx; > + struct zpool *pool =3D entry->pool->zpool; > > u8 *src, *tmp =3D NULL; > unsigned int dlen; > @@ -1063,25 +1068,6 @@ static int zswap_writeback_entry(struct zpool *poo= l, unsigned long handle) > return -ENOMEM; > } > > - /* extract swpentry from data */ > - zhdr =3D zpool_map_handle(pool, handle, ZPOOL_MM_RO); > - swpentry =3D zhdr->swpentry; /* here */ > - tree =3D zswap_trees[swp_type(swpentry)]; > - offset =3D swp_offset(swpentry); > - zpool_unmap_handle(pool, handle); > - > - /* find and ref zswap entry */ > - spin_lock(&tree->lock); > - entry =3D zswap_entry_find_get(&tree->rbroot, offset); > - if (!entry) { > - /* entry was invalidated */ > - spin_unlock(&tree->lock); > - kfree(tmp); > - return 0; > - } > - spin_unlock(&tree->lock); > - BUG_ON(offset !=3D entry->offset); > - > /* try to allocate swap cache page */ > switch (zswap_get_swap_cache_page(swpentry, &page)) { > case ZSWAP_SWAPCACHE_FAIL: /* no memory or invalidate happened */ > @@ -1115,12 +1101,12 @@ static int zswap_writeback_entry(struct zpool *po= ol, unsigned long handle) > acomp_ctx =3D raw_cpu_ptr(entry->pool->acomp_ctx); > dlen =3D PAGE_SIZE; > > - zhdr =3D zpool_map_handle(pool, handle, ZPOOL_MM_RO); > + zhdr =3D zpool_map_handle(pool, entry->handle, ZPOOL_MM_R= O); > src =3D (u8 *)zhdr + sizeof(struct zswap_header); > if (!zpool_can_sleep_mapped(pool)) { > memcpy(tmp, src, entry->length); > src =3D tmp; > - zpool_unmap_handle(pool, handle); > + zpool_unmap_handle(pool, entry->handle); > } > > mutex_lock(acomp_ctx->mutex); > @@ -1135,7 +1121,7 @@ static int zswap_writeback_entry(struct zpool *pool= , unsigned long handle) > if (!zpool_can_sleep_mapped(pool)) > kfree(tmp); > else > - zpool_unmap_handle(pool, handle); > + zpool_unmap_handle(pool, entry->handle); > > BUG_ON(ret); > BUG_ON(dlen !=3D PAGE_SIZE); > @@ -1152,23 +1138,7 @@ static int zswap_writeback_entry(struct zpool *poo= l, unsigned long handle) > put_page(page); > zswap_written_back_pages++; > > - spin_lock(&tree->lock); > - /* drop local reference */ > - zswap_entry_put(tree, entry); > - > - /* > - * There are two possible situations for entry here: > - * (1) refcount is 1(normal case), entry is valid and on the tree > - * (2) refcount is 0, entry is freed and not on the tree > - * because invalidate happened during writeback > - * search the tree and free the entry if find entry > - */ > - if (entry =3D=3D zswap_rb_search(&tree->rbroot, offset)) > - zswap_entry_put(tree, entry); > - spin_unlock(&tree->lock); > - > return ret; > - > fail: > if (!zpool_can_sleep_mapped(pool)) > kfree(tmp); > @@ -1177,13 +1147,8 @@ static int zswap_writeback_entry(struct zpool *poo= l, unsigned long handle) > * if we get here due to ZSWAP_SWAPCACHE_EXIST > * a load may be happening concurrently. > * it is safe and okay to not free the entry. > - * if we free the entry in the following put > * it is also okay to return !0 > */ > - spin_lock(&tree->lock); > - zswap_entry_put(tree, entry); > - spin_unlock(&tree->lock); > - > return ret; > } > > -- > 2.34.1 >