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 82095EB64DA for ; Wed, 19 Jul 2023 15:23:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 01A6828006C; Wed, 19 Jul 2023 11:23:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F0CB528004C; Wed, 19 Jul 2023 11:23:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DD48228006C; Wed, 19 Jul 2023 11:23:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id CCEC428004C for ; Wed, 19 Jul 2023 11:23:33 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 87FD31A03AC for ; Wed, 19 Jul 2023 15:23:33 +0000 (UTC) X-FDA: 81028730706.27.89D9C7C Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by imf04.hostedemail.com (Postfix) with ESMTP id A37B240010 for ; Wed, 19 Jul 2023 15:23:31 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=rdSZMmCB; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.45 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=1689780211; 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=pYTI3rxc07upa7orcKEjaXXi8vFg3GHcpbX5fCjL0zo=; b=HBnEAk3/ag0LJhtgl3Ps4V1O30o/7AXiubXkAdBeze1ABydPJIQgeGTrrtgd46tETwbab3 LDBSNx8rK37MbaGbyp/DHcNqLjBceM6E347St0XZDMnsxeLMUjT59w9TO/vFzBQL+BXoL5 pP80aXqNWMElYWWSrTnfG4DgP7ucnw8= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689780211; a=rsa-sha256; cv=none; b=vF4//t1V7MFk5HF2IhACoZDkToqFlM8s2+9FYC3EuQvaPSadnDh6AYp9U8U2kmYJuu7KJ1 0XmKIBjh8krM+/OX5mjz/EFv5wl/+niqlCfwC1A/TS81gfjpt62jzlOyoI+7z0f2vQFtLZ 4urryljhvT+E0sIDCcmxJxNTZqzS+G4= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=rdSZMmCB; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.45 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-991fe70f21bso938108166b.3 for ; Wed, 19 Jul 2023 08:23:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689780210; x=1692372210; 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=pYTI3rxc07upa7orcKEjaXXi8vFg3GHcpbX5fCjL0zo=; b=rdSZMmCBAzdoLWy2/2UJlGsstH/QYihCudnOBWEcFDbhLbNqgAM44QGcCaXrsL0MZ5 pjR3A+Zn/i9gUApwqFh6kQEXOLtwNztvHcB7lKRze6OnKeCJN6xMbEu66yJOHs4LJd5G EVnjHe5A+sPq0DPYxhnQWgZ7K1Cc59nBV3CtMGphzgzfOXaerrWxiU8ZHPOB019cIG3R yzpLrXd5SAGoryzS8IFtSkQGY03lxtpJgn4MSTDt+3/vNhCuPLJNgg45MBDntjpN6h31 N0uGKP8KYQA8QBYEJ6XIX3jpnCnhOX/Poa80m4YFWkTK744ncQw9xIiAqblLdeYW4uEm XHUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689780210; x=1692372210; 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=pYTI3rxc07upa7orcKEjaXXi8vFg3GHcpbX5fCjL0zo=; b=BsG7Uu8xhkW2JrUMvAzD4gC8vP5YXxtPRz+9gCZFAnBjWwdsi9gHt3JfWTmdHHkjIi pA97uf8hXcOXdoxO7ECbucrtH1TaY79RVjzK2KRUb4LpHrsOk96emYuymFNW80uZTMt7 TlZ4LJRWyjaQTe3bpYCP1+P+xDeO2fNQDDtcPW9Y6ELe+43rOrKAe42ZlW08hP3cPQP4 yJXjxze+BK7uRX2NJlt/KqSxCuoDIY1OLxEV7GbaRxHtznUJP/R9umqbxMUN5WzqNtwk bOYmZv0qQtsqcabxDq7ltWRBjs3HYt0MgjU0gKWgQOg9JQeG8cI89Y/6702ZfkTMI2nX It8w== X-Gm-Message-State: ABy/qLYCFfvt+PjJ72gc3dkuRHPWBO+ESIHiKafgcfju5CY49czTvpLG CqLzKkNR55GW41qrb64/BOcB5PZqrMxVN78GvFX46g== X-Google-Smtp-Source: APBJJlGe6ARv3ysgZg/JX60UPzKzjCRWAJishv5hbT41vqoQjAhbq5kvYF4ZBIk9ocMe7qHigXOLNSRSfN3CRgzS3pg= X-Received: by 2002:a17:907:8d11:b0:992:a838:a564 with SMTP id tc17-20020a1709078d1100b00992a838a564mr2251707ejc.28.1689780209717; Wed, 19 Jul 2023 08:23:29 -0700 (PDT) MIME-Version: 1.0 References: <20230714194610.828210-1-hannes@cmpxchg.org> <20230717141250.GA866068@cmpxchg.org> <901409ed-504b-9500-54d8-e42f832e07b0@suse.cz> <20230717160227.GA867137@cmpxchg.org> <20230719142832.GA932528@cmpxchg.org> In-Reply-To: <20230719142832.GA932528@cmpxchg.org> From: Yosry Ahmed Date: Wed, 19 Jul 2023 08:22:52 -0700 Message-ID: Subject: Re: [PATCH] mm: kill frontswap To: Johannes Weiner 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: sw3u7rf9bbsi6xhgrqqccaxqohxa8xi1 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: A37B240010 X-Rspam-User: X-HE-Tag: 1689780211-325280 X-HE-Meta: U2FsdGVkX19Y/l3x7leSBlib/prYAjDEBZdG1Vfwi4NzqQMnpI++pAXf4+HNCiFJLv9rabxo8rYFG2PSPl6plDDAehuYS1L0zzSFMETQyNLSuW9d54l60oL8RUm8LC2yAdB8wXcb2iFg8FZnlXu36gnrNzpbQZsONWyWT65sn9vSc8uAMT849NWmsPFut6a1+8rT+JlHi3I4Q+hasXo87caswoCZ0tvniAjEva9lrpZMrD3H06d+P+Xw9t9w/eN0a11Y60GLCSQ19/IdjjWaHPSrf3gHxFbdhzgbrm6fJ3x51fPw/0DaSR8zlDi7OUmjqs6qyfhCdky0wSHl9T6VsFA6/S/tSB+HVKswDKeiVDm4pFpOTDM1DYBXzt5WHjSwfumOolD9vfkrK+vyTyNCfhda7RDdUaddqErtf1XNaA2zvWGc3c8gauE2dcUfoxmU76a309tsSV4O0gXkVJeDisO96Lm/FqaHXNWZmPLUsm7eryxzLtNNcnlh+QlDBgK+eU9BBlyUrENo1tICUnpJRQh4zHp05j2W4Bwg9duyRnRK1Vk5x0QVrPqd97SyEQpX6RKRrS4Qw++BZdjE5O8aesUWogDEYq8DA/NppaTJhVcAqIzQiN/FHCsTt10JaTqPBadK0vCw0V7AsvoruJpCxPWRXBGztwFXHjkie7cPKwZapN1f0QAmg73c/UCdOvJpZbNQ1fmIuN7sVTZWHBtObpdRToC+bCEhkqQ1VSwmNiHVDtB9IpluI8zg2ksuEHp1JTp/aBLhq+OIiA2+sCj05QKO51LsYgtPz5B31Q9yCUlIVthK/0ElgdZ4KlnMbxl3D7R78LVg+QHlnSSnI0/usVFY9R/9fTVBw4VlcltmmM1fcuLKPW6iGQFcYijJjqtQL6YNV0M3JvUHjpYWRBtksesb58N5oGV/ScfzgC7uERwx4+mQf+Njvolie41jyU9tC5qxmfakrYVIrULIL9e z+dQqYOk A0B3ZFAXZmW1/KwJ9RCA91B82kxrL+n9ia9vEcIVixwZqlndyNpEjdi/Fm/U4iX3OZ2budLkevpPQvA2UXwJ2fh8abrx7hAWKUx4WpgihEkVlu/6qblQMI7W9ujYU01V7h9Pu2ty/+GyE3XCp3J2jRCWRu8Sd5SElcGcISIEhCky3MhHSBTXd0fP7rYurI8O7BfCyQdn0TbHRQfUuxQRLuK5RUDsLV4o/Sk373obQyuoK7cPVZoJJq4fTsbeve/Ns12UcLOkG8391yuak3zSxVop1NEU5OaI5b76oeboSYD7ZXmaI+jP+xJulNg== 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 Wed, Jul 19, 2023 at 7:28=E2=80=AFAM Johannes Weiner wrote: > > Hi Yosry, > > thanks for the review. I hope I saw everything you commented on ;) - > can you please trim your replies to the relevant hunks? You did :) Sorry for that, my client automatically trims the quotes so I couldn't perceive the size of the problem :p > > On Tue, Jul 18, 2023 at 11:52:45AM -0700, Yosry Ahmed wrote: > > On Mon, Jul 17, 2023 at 9:02=E2=80=AFAM Johannes Weiner wrote: > > > -/* > > > - * "Get" data from frontswap associated with swaptype and offset tha= t were > > > - * specified when the data was put to frontswap and use it to fill t= he > > > - * specified page with data. Page must be locked and in the swap cac= he. > > > - */ > > > -int __frontswap_load(struct page *page) > > > -{ > > > - int ret =3D -1; > > > - swp_entry_t entry =3D { .val =3D page_private(page), }; > > > - int type =3D swp_type(entry); > > > - struct swap_info_struct *sis =3D swap_info[type]; > > > - pgoff_t offset =3D swp_offset(entry); > > > - bool exclusive =3D false; > > > - > > > - VM_BUG_ON(!frontswap_ops); > > > - VM_BUG_ON(!PageLocked(page)); > > > - VM_BUG_ON(sis =3D=3D 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. Makes sense to me, thanks for the clarification. Maybe we should put this in the commit log as well? > > > > @@ -1356,15 +1342,12 @@ static int zswap_frontswap_store(unsigned typ= e, pgoff_t offset, > > > > > > /* map */ > > > spin_lock(&tree->lock); > > > - do { > > > - ret =3D zswap_rb_insert(&tree->rbroot, entry, &dupent= ry); > > > - if (ret =3D=3D -EEXIST) { > > > - zswap_duplicate_entry++; > > > - /* remove from rbtree */ > > > - zswap_rb_erase(&tree->rbroot, dupentry); > > > - zswap_entry_put(tree, dupentry); > > > - } > > > - } while (ret =3D=3D -EEXIST); > > > + while (zswap_rb_insert(&tree->rbroot, entry, &dupentry) =3D= =3D -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. Thanks! > > > > @@ -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 =3D zswap_entry_find_get(&tree->rbroot, offset); > if (!entry) { > - /* entry was written back */ > spin_unlock(&tree->lock); > return false; > } > -- > 2.41.0