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 B102EEB64DA for ; Wed, 19 Jul 2023 14:28:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 29C4A280068; Wed, 19 Jul 2023 10:28:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2257128004C; Wed, 19 Jul 2023 10:28:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0A060280068; Wed, 19 Jul 2023 10:28:38 -0400 (EDT) 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 E912B28004C for ; Wed, 19 Jul 2023 10:28:37 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id BA2FA1402A5 for ; Wed, 19 Jul 2023 14:28:37 +0000 (UTC) X-FDA: 81028592274.06.E2E26D8 Received: from mail-qk1-f170.google.com (mail-qk1-f170.google.com [209.85.222.170]) by imf27.hostedemail.com (Postfix) with ESMTP id CFB6140017 for ; Wed, 19 Jul 2023 14:28:34 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=LAalPu6e; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf27.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.170 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689776915; 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=Tl31fPEVZv/XSj4GJGcrVMvmw4IX33aKorlexj1wX2k=; b=WrVYbKt3IRAvsf2IRyYCrkGhskTHFq/iODd54FeG2CGz1k/8TNYOs93YRUvUtlmSAwG2kt wKei4gGSJ6tPG5ujkFSt8ztQpb50UKueNrDbI3d8ZV9En/1VThysLYkPeSGVTw73/cEdjr kiQMKaICzvz0y4nK1ZZAFUCnmUYMBmk= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=LAalPu6e; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf27.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.170 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689776915; a=rsa-sha256; cv=none; b=0g9SIxy0VHSU6mruvxqUPq19SXra3z2E3ql05R0l/tnEGxYtDLosTCLL4KXhUZy48SuX91 7n91QMFb91OWmqQ1SSW8JseuMRC/zXMhvqn7CwIuNYh4kZ2k3TcSixJJ2gAsDNBVsSe9cs eH6zab577ifOzIMjREKtZgVYAKlyBtQ= Received: by mail-qk1-f170.google.com with SMTP id af79cd13be357-7659db6339eso41754585a.1 for ; Wed, 19 Jul 2023 07:28:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20221208.gappssmtp.com; s=20221208; t=1689776914; x=1690381714; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=Tl31fPEVZv/XSj4GJGcrVMvmw4IX33aKorlexj1wX2k=; b=LAalPu6eJykTNbpvUv7GdqmiAa0NeQssVV1cu5H5/loGl4URTMZeskfrSeGtfb/S6j 7w1DIl0VTfcOFpRB7AaTNUt7n4W5PhW1Y03cLDKqEGpdLgBoN3B17yjDaOhnNRd8ECmL +eYTK2flqDOooPfT0WzfOiO1odqa9CeAZ6SFHqFdmS7GAsLgCvEWQisjAL8noyuVj+Xv mdOLFBlOne1S1gv51gLVErVX6Yh127f2D6XMxFRUIiuSU2ZvRjoUg+DJimN7qrxi10cL WKeqIymmm435NdpaWWLN1fNZN93R27UGpAR0x2n6aqIz0dbFwUpedBBESx4V3qUdrrvW yV1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689776914; x=1690381714; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Tl31fPEVZv/XSj4GJGcrVMvmw4IX33aKorlexj1wX2k=; b=UCB9UMLc4bs03OofCdrnzWctghIRghF5VbVa/lJmjXrTvEt8sIFDJXT7whnlwk59tK zDe3QAd0Sskk+SLWe+UR1A7zx+0/7eWlXiQjNTld0lJuwf3VVivfUlklLG/0noGZvNGg UWBI+PCHi4ElogGaojAJDlx8M+tNeb2yQrvaeq8JfdqUWwSR0Js+GR7Kp+WrIiUoliJJ +1bidpXVWsEBItFkMPDjqu1sMRzCqN/pDfIGqmRUXd8qgkaAkeu7tSe9dgdmvBx+UCKS +yBz8rdHbSRCsjr73aBc9P+SbSK2QPmfF6WSD+Bxn9zvlQbewU498Ik+eDJetoV8xhPq McSA== X-Gm-Message-State: ABy/qLZ+i1tBN0AAIKCZLyA+Kmya8BWF5T5lPhtvGBP4rpt6EklCEWcu /l18bkg6VxMfoovrG6+fZBhkaQ== X-Google-Smtp-Source: APBJJlEMrxHzKfhacxnycVwdc58qqKM0ubUIR2hO4njFovHpv9dpC8lvbXt6kfn1Msdilz1REn71SQ== X-Received: by 2002:a05:620a:444a:b0:762:3841:c098 with SMTP id w10-20020a05620a444a00b007623841c098mr3072588qkp.30.1689776913751; Wed, 19 Jul 2023 07:28:33 -0700 (PDT) Received: from localhost (2603-7000-0c01-2716-8f57-5681-ccd3-4a2e.res6.spectrum.com. [2603:7000:c01:2716:8f57:5681:ccd3:4a2e]) by smtp.gmail.com with ESMTPSA id g20-20020a0caad4000000b00635ef0579c2sm1510550qvb.39.2023.07.19.07.28.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Jul 2023 07:28:33 -0700 (PDT) Date: Wed, 19 Jul 2023 10:28:32 -0400 From: Johannes Weiner To: Yosry Ahmed Cc: Vlastimil Babka , Matthew Wilcox , Andrew Morton , Christoph Hellwig , Nhat Pham , Domenico Cerasuolo , konrad.wilk@oracle.com, vitaly.wool@konsulko.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: kill frontswap Message-ID: <20230719142832.GA932528@cmpxchg.org> References: <20230714194610.828210-1-hannes@cmpxchg.org> <20230717141250.GA866068@cmpxchg.org> <901409ed-504b-9500-54d8-e42f832e07b0@suse.cz> <20230717160227.GA867137@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: CFB6140017 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: kmzn3gr1p4caun3imajhyc4ezexbx1fz X-HE-Tag: 1689776914-56341 X-HE-Meta: U2FsdGVkX18SDJksAmzpp4qfsjcKfBJ9qCJmUNFwHQKCL0IQq/ccIHHWEF3W2ivD5DVk93Wag5vB+fAEel3ubbUOgO6iWz0YsIBA7bTWyFrNjOfbj0fBhPqEIVyMwO60kCFpC0Mu8bcbTKKAlRdVkWH234W5R3OzOQvHFXt0ZClyLjoNqapfm9fa9WQwgoHzpy3oHS6yQZlmHrM9GNHLja8DxUgnQwoZ1cFObejRfA7k2nuoyMeynMMz9BcywYWMqhsnI1ewvwosAvmKJ0aEH4t8r/ndfpbtEm49F+5eo1gUC18QNfQHVDlZszMk37in/kQFFyMYhD8saHE005nTZJnBXWlADhVCzKo2iAfuOGQmRbpjXCLBs9PXig6X1k3CArtu+ihfw6LokfCwaoEYDkrmnOa7jGKj0dxQX+rQ1JQLSOfpa3LuWPjcUPGyWwJ56E1/z0xyPDb1UVW0VPly6qbsZvCKE7aWFbNEe1wFLhaqrusMn7Mxy3Xt8FLm5eytTYHvY/I5Pt229VxbYMBRlLSc3KNsO4LpDQrZkKujVRADZ7m2hpCkiWPf9jryya8KvB3E1PW0ZvQhBQgP71hw8e16defB9tkDrfUXBehvyJYMB7nwpE/i0GSXNb0QMqA5cenXgUoAEumY/3K01dcBBkIzM9WE+U22oyyOwn2vkXjRcrFSDHWrQWZY0m0xfvXg0kuOXUBJbfT77iZHOffi89DGBm40+ZrW5zDyF99+zBgSNH0b8YfADG8FFlopzX3lBJQaJqZ4pxuEVjmv1K7aH8Tchy0YClv+9EBfCUR4pedbijsPQwacfCDPyjJqA/l5Umwnnk0NBxOaNxSU8qPm/o8+WiCgNL/E8tTkdDcRzOVc5ON8XiHMA/+JC15qjhDmJVC2onAm24tlxxcOw53eQiuXyWoPvFWIRTcmx6PMt8jGYscEJruGi3s9FP8fB08yxl3G6OVuWxUERKH7Auf JUccc7tj Gv8kMzQQkY/RF+f9taqoYysjpWlMIJ4YGJ5BdeiiBeLGQVPr/mIh46Ogd+pCWGAW2Tdb/xvZHFTwJJ6YCAukI4h64HpCxBmC4Gdqmxm1AEnB+Ao5Eqb60ctyhcGkJIIV5sW5LCTmG5YyGW18RpSGw9/9f2231PUAXb/7mwiaCkA+uqTDUdUGC2D+Tx/qsMOeYWOc8r5vy/r81PshqdeHG8/laxdeJYgj/3wifQ0Cjrjlbu8NVgvbnmJWO8wqnd8rWs9nLp+s9QWiPJHG78fTvlNHrZOq8kWttLCHiS2AcFDhMZoEKwSKuvRvowrADsuzL7XNud4/Nj8jXe5zJiDy/tr+8I2USVo/YGq81X9BZrlA+7AUatQ5ND/o+AdzYUxzFN2FBPGFlH+anI19ilAqS8r5PILB2mRQQXFijv1OWHEJZT3u+IV+KolcRaFMS3g1qOPHTFlNz/iClEB4ViiMxrBpQqGoUR42+VNrJ 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: Hi Yosry, thanks for the review. I hope I saw everything you commented on ;) - can you please trim your replies to the relevant hunks? On Tue, Jul 18, 2023 at 11:52:45AM -0700, Yosry Ahmed wrote: > On Mon, Jul 17, 2023 at 9:02 AM Johannes Weiner wrote: > > -/* > > - * "Get" data from frontswap associated with swaptype and offset that were > > - * specified when the data was put to frontswap and use it to fill the > > - * specified page with data. Page must be locked and in the swap cache. > > - */ > > -int __frontswap_load(struct page *page) > > -{ > > - int ret = -1; > > - swp_entry_t entry = { .val = page_private(page), }; > > - int type = swp_type(entry); > > - struct swap_info_struct *sis = swap_info[type]; > > - pgoff_t offset = swp_offset(entry); > > - bool exclusive = false; > > - > > - VM_BUG_ON(!frontswap_ops); > > - VM_BUG_ON(!PageLocked(page)); > > - VM_BUG_ON(sis == NULL); > > - > > - if (!__frontswap_test(sis, offset)) > > - return -1; > > With the removal of the above, it will be a bit slower to realize an > entry is not in zswap and read it from disk (bitmask test vs. rbtree > lookup). I guess in the swapin path (especially from disk), it would > not matter much in practice. Just a note (mostly to myself). I briefly considered moving that bitmap to zswap, but it actually seems quite backwards. It adds overhead to the fast path, where entries are in-cache, in order to optimize the cold path that requires IO. As long as compression is faster than IO, zswap is expected to see the (much) bigger share of transactions in any sane config. > > @@ -1356,15 +1342,12 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, > > > > /* map */ > > spin_lock(&tree->lock); > > - do { > > - ret = zswap_rb_insert(&tree->rbroot, entry, &dupentry); > > - if (ret == -EEXIST) { > > - zswap_duplicate_entry++; > > - /* remove from rbtree */ > > - zswap_rb_erase(&tree->rbroot, dupentry); > > - zswap_entry_put(tree, dupentry); > > - } > > - } while (ret == -EEXIST); > > + while (zswap_rb_insert(&tree->rbroot, entry, &dupentry) == -EEXIST) { > > + zswap_duplicate_entry++; > > + /* remove from rbtree */ > > + zswap_rb_erase(&tree->rbroot, dupentry); > > + zswap_entry_put(tree, dupentry); > > nit: it would be nice to replace the above two lines with > zswap_invalidate_entry(), which also keeps it clear that we maintain > the frontswap semantics of invalidating a duplicated entry. Agreed, that's better. I'll send a follow-up. > > @@ -1418,7 +1401,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > > if (!entry) { > > /* entry was written back */ > > nit: the above comment is now obsolete. We may not find the entry > because it was never stored in zswap in the first place (since we > dropped the frontswap map, we won't know before we do the lookup > here). Good catch. I'll send a delta fix to Andrew. > LGTM with a few nits above, probably they can be done in follow up > patches. Thanks for the cleanup! > > FWIW: > Acked-by: Yosry Ahmed Thanks! Andrew, could you please fold this in? --- >From 86eeba389d7478e5794877254af6cc0310c835c7 Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Wed, 19 Jul 2023 10:21:49 -0400 Subject: [PATCH] mm: kill frontswap fix Signed-off-by: Johannes Weiner --- mm/zswap.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mm/zswap.c b/mm/zswap.c index d58672f23d43..583ef7b84dc3 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1399,7 +1399,6 @@ bool zswap_load(struct page *page) spin_lock(&tree->lock); entry = zswap_entry_find_get(&tree->rbroot, offset); if (!entry) { - /* entry was written back */ spin_unlock(&tree->lock); return false; } -- 2.41.0