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 7BFAEC4332F for ; Mon, 13 Nov 2023 15:12:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 090EB6B019D; Mon, 13 Nov 2023 10:12:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 018936B019F; Mon, 13 Nov 2023 10:12:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DD4266B01A1; Mon, 13 Nov 2023 10:12:12 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id C4DC16B019D for ; Mon, 13 Nov 2023 10:12:12 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 6AE99C0864 for ; Mon, 13 Nov 2023 15:12:12 +0000 (UTC) X-FDA: 81453271704.22.2B98F15 Received: from mail-il1-f169.google.com (mail-il1-f169.google.com [209.85.166.169]) by imf19.hostedemail.com (Postfix) with ESMTP id 621D11A0029 for ; Mon, 13 Nov 2023 15:12:10 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="WBtq/a+h"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.169 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1699888330; 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=5VOhXq0ReQvgC7nwSQo9GnYYnCnNRcspqiKABN5rr6I=; b=RpjwTKEc+Fd9sUJWHnHUseD0eU56f9JmZXmXNat8kUPbd5HKb3w8c7pkwAE/lOs1O9BEdi XHnNzCys462X0asrmEG3plV3c0qZ8WOfqEwwR8lMcrRVCX8xdpnEQRnQodZgXIRU6gHiMc TTWnV198rl+RCliBDrfmMXF8aBIfIUw= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="WBtq/a+h"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.169 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699888330; a=rsa-sha256; cv=none; b=2G41184PeKIOka5r8uah0B/jHvk8tt9Oa4mohzsZMN7JNvRKNjrC2IOWxMMwkmE+qE9wyF RB1o6LBRxCl4JA5Lnuj6fGI4XrGn+CMlBSiWRN2ApgiOyR0UjZQI7jfeQWfWH935gsOnXi goQXLJRsMidF1CDa4UilkIqoOY9NhX4= Received: by mail-il1-f169.google.com with SMTP id e9e14a558f8ab-3594c100735so11160555ab.1 for ; Mon, 13 Nov 2023 07:12:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699888329; x=1700493129; 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=5VOhXq0ReQvgC7nwSQo9GnYYnCnNRcspqiKABN5rr6I=; b=WBtq/a+hRLiWzK3uJOI6Fh1RDnWNJby8IxyCqg/m7sUPrxlqeGuee0eFH/Gfk+JBhm V2W9ef7ocxQd1uysFTZKopAZkNeB9If4bx5WCLgS45N4TLQyKX2Uk4IuI7+o1fTMK0O3 F2ZnDfIo13YHDdg077rEX3y1CroFTqg4JlzvYz9aCSaW61egVDtkSz6e48B1uDXhNk7D eA7pStZ2VcTWJnwdwgz2kDhwBE6b05VrPgQNuZ0+AjrBxA7ZkxM67bYUbY+CwCL6l5kY mnSCGz9L4JZ3jAjn0IMecCz97dfE/mUUZ5+jdG1QzT8Ke/Hfhpmaf3L+ZAqHbEnV0C9d ObLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699888329; x=1700493129; 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=5VOhXq0ReQvgC7nwSQo9GnYYnCnNRcspqiKABN5rr6I=; b=UCFCxoyIQi9SjqnMP0sKl3HQPf5lrLHJa1uSmVdn0ErX9EschWQ3Cj2kIAIq5iWyb/ fW/bBojoDXsQ9N3V4o3ZMoZf8WvepsHr0Cb+pWG691tEhux4PnpCMbtOTarvJX1Zxpoq 5Ljjo8ESeFbr74LQ8BVp7uKtjUaEQ8asNz+LKho/G0q+DpMrR4enlCJrnTBY/bHj9J6i Vi0Z15g7RFzxT5Nwl40cZWe/yeGvDYdIqvdjibCtUYAwVDkR8paaXkCpdkFeE22dhh3Q amClYF0o4CmPB/dE0m9bDWNm7JolYDQLRQ/VZ6smPFf0O6eXTLbQ9tSQ7um4Ri0nm/aw wvQQ== X-Gm-Message-State: AOJu0YzCcWTwO6DxcRKr1oz2qM9MxtkjBs+5t2ywEoBa9DNdRiioAzRt KkUDdzUlue/Takcup9iS9NiQ/QaB4q2XD8cNYm0= X-Google-Smtp-Source: AGHT+IF+/SKc67vBQ3OpVYsnnkI9yBZxnoAtrYD46UcMZ9psHUspIiI4gea9lgdyl/HeSdMNrk4Wz4eVqT1dOSVeeM0= X-Received: by 2002:a05:6e02:3b02:b0:359:3491:9042 with SMTP id ct2-20020a056e023b0200b0035934919042mr8100129ilb.9.1699888329330; Mon, 13 Nov 2023 07:12:09 -0800 (PST) MIME-Version: 1.0 References: <20231113130601.3350915-1-hezhongkun.hzk@bytedance.com> In-Reply-To: <20231113130601.3350915-1-hezhongkun.hzk@bytedance.com> From: Nhat Pham Date: Mon, 13 Nov 2023 10:11:58 -0500 Message-ID: Subject: Re: [PATCH] mm:zswap: fix zswap entry reclamation failure in two scenarios To: Zhongkun He Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, yosryahmed@google.com, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 621D11A0029 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: keihw4cokj9ps6xh7oqf9p4fxpwtb6g5 X-HE-Tag: 1699888330-179900 X-HE-Meta: U2FsdGVkX18aFnQGJh1dFMtjQzr81IcvQmVLYOJahdl8USiUh2t0sNiYzkNraZcDT1hpIdmO8WuE6ELkmg4rdW4TZCOnehJmwLRov4YK+5DhMCxI9zslp+CF4JSjm3lHY5qNEQnpExm+RAXIgw1Sg9Txdi4tfHM/wcGjNaPfjPxZTjYFWY0npp9kRdyHD/XElL8S9es5amc7s+5w0ZDLwCyvMsMK7MS318+GSjsBsCqPUkmkZwZdOo025y9Gfpan1r48pH1PXXAJN10OpMFxcMHLZHl+GQJUj0D9V2pOcpImfsLWa/Sq7d9EVNd0Th3WfSRpBiO0IhkbUCU82/4PJrTiJaNwkibn2vNR6vu+GgUxAp/SqeHc1xKaEhiCmWWrtGCyifwL8cJYW2q6WZHCB8mvTanM43eeuAuOpa4ykntbsH4J4NecX82ilA/unIrkP/UcwrR4kf3s4IHfhrO66AmH+3kAID3qV477vcPPwB3XF0Z1BMyNSvg1BVy+NAbbVcouOho1cyEzeAAEj94C1Th18a2mSuLow6v5fN0VeE+H4deV0RvvhveZ9fcn0pQD9GS6wuNqDcrW3Djfpc//k3aJP29CzRYkMEKMbm9QxNnrD5hKM9Qk1C1VoARfPbjSwdQ0zbKnv5ML8GJUHeTQuvRBw3pkwzwooKZViGdvZ3anzEX7Yj5aGCAtocou4mHUJBgvvS2E7nOQCl+vtbNFrL3ttf233kUDvz3kSsZS237ymh/zylKdadlyHLywT2fCRMGQsPkeme39M8COPl3+t90F3DCqiX+pWkFBy2sUXqPZyVRr4WHwruyhbv5LJRFo4BzrUwMN/vu9MCU8swMA27AfY+ElWTW6NbRA5d2pIAuacdsMqIgk15JbdSBG3t9rCzfcNajuVCzQZv9N3JvR8r1M2qNnqkqv/X1LjxMTbFkkzyWFwCACKXcjn9Sgny+kAr2E4DmbkFI2L1Dvj4x m1t2S1VQ tZY6+ibfVIb/a2CnEwPDmpcLNMR11c8gEfQxlTkykYmHp/Zlr3jCciiidQjdrciR3qGirVL0lifpTaC9nVOpZNyXmCqcMnEj2simf9P2ckrPL59jK91va87bkhOEW4NPLYmn8qfS2FWrlWBuR15SkcasU/Z0HoQSGFlRjr3NeohT1bHU6ap3+yjJGtKBAPWysvQoge78Ct2MfyeAyC0r3AUTRVIZOXis/wxSLjZPiax2LQ0FoHeYeMf6PDGkwG3tTXZKdADp09ER9/P8VzuMlrN3Ntaf8VV41sVWFvpM7NLKlCD+JCKy5LqUXPudCDd9JLQxjdc3oKAB8FLLcxuzl2DA11jt7gysY7sXajfs0kDzxvcwEWbutiyWkAB0hBZ0X0hzIQozIPjpW2GGXA3oJL5xhwEO+a5XqjLee 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, Nov 13, 2023 at 8:06=E2=80=AFAM Zhongkun He wrote: > > I recently found two scenarios where zswap entry could not be > released, which will cause shrink_worker and active recycling > to fail. > 1)The swap entry has been freed, but cached in swap_slots_cache, > no swap cache and swapcount=3D0. > 2)When the option zswap_exclusive_loads_enabled disabled and > zswap_load completed(page in swap_cache and swapcount =3D 0). > > The above two cases need to be determined by swapcount=3D0, > fix it. > > Signed-off-by: Zhongkun He > --- > mm/zswap.c | 35 +++++++++++++++++++++++++---------- > 1 file changed, 25 insertions(+), 10 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 74411dfdad92..db95491bcdd5 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1063,11 +1063,12 @@ static int zswap_writeback_entry(struct zswap_ent= ry *entry, > struct mempolicy *mpol; > struct scatterlist input, output; > struct crypto_acomp_ctx *acomp_ctx; > + struct swap_info_struct *si; > struct zpool *pool =3D zswap_find_zpool(entry); > bool page_was_allocated; > u8 *src, *tmp =3D NULL; > unsigned int dlen; > - int ret; > + int ret =3D 0; > struct writeback_control wbc =3D { > .sync_mode =3D WB_SYNC_NONE, > }; > @@ -1082,16 +1083,30 @@ static int zswap_writeback_entry(struct zswap_ent= ry *entry, > mpol =3D get_task_policy(current); > page =3D __read_swap_cache_async(swpentry, GFP_KERNEL, mpol, > NO_INTERLEAVE_INDEX, &page_was_allocated)= ; > - if (!page) { > + if (!page) > ret =3D -ENOMEM; > - goto fail; > - } > - > - /* Found an existing page, we raced with load/swapin */ > - if (!page_was_allocated) { > + else if (!page_was_allocated) { > + /* Found an existing page, we raced with load/swapin */ > put_page(page); > ret =3D -EEXIST; > - goto fail; > + } > + > + if (ret) { > + si =3D get_swap_device(swpentry); > + if (!si) > + goto out; > + > + /* Two cases to directly release zswap_entry. > + * 1) -ENOMEM,if the swpentry has been freed, but cached = in > + * swap_slots_cache(no page and swapcount =3D 0). > + * 2) -EEXIST, option zswap_exclusive_loads_enabled disab= led and > + * zswap_load completed(page in swap_cache and swapcount = =3D 0). > + */ These two cases should not count as "successful writeback" right? I'm slightly biased of course, since my zswap shrinker depends on this as one of the potential signals for over-shrinking - but that aside, I thin= k that this constitutes a failed writeback (i.e should not increment writebac= k counter, and the limit-based reclaim should try again etc.). If anything, it will make it incredibly confusing for users. For instance, we were trying to estimate the number of zswap store fails by subtracting the writeback count from the overall pswpout, and this could throw us off by inflating the writeback count, and deflating the zswap store failure count as a result. Regarding the second case specifically, I thought that was the point of having zswap_exclusive_loads_enabled disabled - i.e still keeps a copy around in the zswap pool even after a completed zswap_load? Based on the Kconfig documentation: "This avoids having two copies of the same page in memory (compressed and uncompressed) after faulting in a page from zswap. The cost is that if the page was never dirtied and needs to be swapped out again, it will be re-compressed." > + if (!swap_swapcount(si, swpentry)) > + ret =3D 0; > + > + put_swap_device(si); > + goto out; > } > > /* > @@ -1106,7 +1121,7 @@ static int zswap_writeback_entry(struct zswap_entry= *entry, > spin_unlock(&tree->lock); > delete_from_swap_cache(page_folio(page)); > ret =3D -ENOMEM; > - goto fail; > + goto out; > } > spin_unlock(&tree->lock); > > @@ -1151,7 +1166,7 @@ static int zswap_writeback_entry(struct zswap_entry= *entry, > > return ret; > > -fail: > +out: > if (!zpool_can_sleep_mapped(pool)) > kfree(tmp); > > -- > 2.25.1 >