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 AAFB0C47077 for ; Wed, 10 Jan 2024 01:32:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 09F016B0083; Tue, 9 Jan 2024 20:32:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 04F876B0085; Tue, 9 Jan 2024 20:32:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E32306B008A; Tue, 9 Jan 2024 20:32:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id CFBE36B0083 for ; Tue, 9 Jan 2024 20:32:51 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 9BC12C03F5 for ; Wed, 10 Jan 2024 01:32:51 +0000 (UTC) X-FDA: 81661677342.25.A06431A Received: from mail-io1-f45.google.com (mail-io1-f45.google.com [209.85.166.45]) by imf19.hostedemail.com (Postfix) with ESMTP id DF4C91A000F for ; Wed, 10 Jan 2024 01:32:49 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="PSNmQL/0"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.45 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704850369; a=rsa-sha256; cv=none; b=iBKrdF7fajofswgeU5z4uNlCy9qNRdOBjQ/NImKcXdV3sjW026isrcS0q572m89f1u031/ NgS5UJwxkACI0QlN+VYfNhAECuLISc8FVljZLw+lo/q3l5M2mWQ3Uq+aSd5Za3ErwbT0AK 9AHXTkhgPDgtQIN+XKIwD8BPiVZfrQs= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="PSNmQL/0"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.45 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=1704850369; 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=oy/+8Nh5YVQL6ancyhJaCNvmncSiIldyq1TK0W/GXJI=; b=79/UPiD1JcpIvB93pMrJkybEjhOZfbe0n/zCqUJAH+Wfd9sUBXfvWikxx9XYqRdNlQeLuz e9mZS0+xsgpIYYraTQE/rWDltvGCWo7AKSNDROifrgkXEqoOUWGhgq2l6fM6asX9WCQaD8 KYmn58+iKJ2kauyI8LAMiqz6XezBPyY= Received: by mail-io1-f45.google.com with SMTP id ca18e2360f4ac-7beea7c4985so28320839f.3 for ; Tue, 09 Jan 2024 17:32:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1704850369; x=1705455169; 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=oy/+8Nh5YVQL6ancyhJaCNvmncSiIldyq1TK0W/GXJI=; b=PSNmQL/02QI/hj/UYRyoq28d7kGtR6FUM5gH7R494M4ZuMV/TgXUPx3UCrlJTab3Dd HysVwPsAQRrjlsFe44Tw17hbyn8UNv7xhgxCTNerH3nn041PU8qCYHcPvSClQBPO28Pp Ev8C4XrNsgj/ASqmc/xaphPvGgjyJqRkChx2sWtK6ilPUSEfon1DqspjFBEQXsfC02h/ WJLGYwe31cuLPUURzNse7Y/zAB6/yheE0aVmmwXl8glrFEB5carggLbnIFQ/YvUS33oK kcZYWmeTmri2UuKmVLM+DopdqLBcUc7kwTB1b26+udGxijV0m/4k5CmK8tilzkCENrsS mNXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704850369; x=1705455169; 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=oy/+8Nh5YVQL6ancyhJaCNvmncSiIldyq1TK0W/GXJI=; b=gRrF6kQ5m7x1du4lK4Qkbik6fzJL1Ilw2I6RTyv2bv3q+SbQmWiG5OQ6YTS64W84FA gQv03IzML1wfs01hMcqlSYTkREpLDU2ybspEnofqjwPUclHb1luOECYO0QTYkTLA5nyS ItrJOKehWBk2/yqRRbYyOLPeB5BEPTCnp7oXEE0T5bmy7VJgaSMAjm2K+UVDENQkWsaK Z4dpr2e7Qr5otLu46jC/MsFSshZbSZsj31MxqsCIxKpVORRfCRPkxZiQfZDjSxyVMn+Q gTa6h3BWdA3ymweLl4SLCZ2AeIAGl3q1L2U/HRcywJkNRCbgyWQtcfbLcrMXz41hoUxE jCjA== X-Gm-Message-State: AOJu0YyrPgSbaYV9OE24pBH/VqpbaHG8K4fv086yPYnJDSkkNasXThy7 73QAJwYBdUPwAGpvRNGcmyP1py5vy8Cy6Fx8e5w= X-Google-Smtp-Source: AGHT+IFvauRnV16Rr5gJqSs9omwROWYh0k1h0vtgUKuZrm/hxCDaYclvSr8h4GOJwOKMZFch4kJoXZ+iFppikIYNqXU= X-Received: by 2002:a5d:8717:0:b0:7be:e84e:9cc8 with SMTP id u23-20020a5d8717000000b007bee84e9cc8mr360272iom.31.1704850368958; Tue, 09 Jan 2024 17:32:48 -0800 (PST) MIME-Version: 1.0 References: <20231024142706.195517-1-hezhongkun.hzk@bytedance.com> In-Reply-To: From: Nhat Pham Date: Tue, 9 Jan 2024 17:32:37 -0800 Message-ID: Subject: Re: [External] Re: [PATCH] mm: zswap: fix the lack of page lru flag in zswap_writeback_entry To: Yosry Ahmed Cc: Zhongkun He , akpm@linux-foundation.org, hannes@cmpxchg.org, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Chris Li Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: DF4C91A000F X-Stat-Signature: 99es8hk65pqumtc759iadj4sgobf9o4u X-HE-Tag: 1704850369-210864 X-HE-Meta: U2FsdGVkX1/hvhtTwtVf1bU0c2J6OiZS790lIdB22a7XN/m20m2bwsr9cn24wHwsi18kmhOw8+3TB5I691IKlC54fF0PRNl/ywOfvGJ9RCVtxBSthktwUG8U84v0pqklHmKERht/MT1ZKaNxZxISwPtMMDcU2UQEmasASixtWJcbqeubhlACHd5s1vpd3PMqjhg1H/OmYnYNDXX7SfwWz1OdRmz2wmN+RXSoSEUOiwA2Uhq+BITG6AWs90e2OGN23RGhO78EbzhdB+ru1klL78Sn2syRas3VNL4UO3HQM4NIoYztywnJu+jM7QfGRPZtqR0uK/MjTnLhFNl/gGm0XHbXm1SZrQUGkgSACwNkWIaRvrmdJrDZhQowac9+1RjSusxnbB+VgRpKaBk0jrjBl0jU0Dpf7tmwhlnarNbg/UPk5qkK7ohDqP3hL+GEjvNn/OHm1sxCw4qSJP9KXgTVa3TdtHUsAS99gx0dxDNDARn9V8gM+Tos8F4wfgHEP9T4isQ/qVWH3tQTBQzWExDtXzFTtUCX1Wigmc0YwViWWZX2C7lvX24oZoYLUpKTQOq/TebfT6mJd5/w/XhG7uFfBLth4G5RPBZx8y7F4KzTUgbQ0NuKTeT7JQF5QNH1PNQMU1gMvx2oGyLPbWqhiGGc7gzP/UvfE8ueJxf4i/lxu2W4bSp7wWE9fXWl6i+9iHgWyRipsaACbv41RKC5y7sOhUey1kBF1WG3rQUHYheF+gm3xt07jJ7mN/k2k5ngIa1fDR5us8Cn03Dl0iUOn1PEpvEg0ZeA2eduTp9fbZ0rwoM8xg8nx+kTy8FamLaHJSfLoV3NYXr+cCklwg4Uc7PxW/8yaobxedGeftTJ7tgMp2s+UkcgYc4KkcQBY+QP9tfYml1jPTP2VNCyMVSt/TlAQgKZY69SaBwUvmI8gFqRV5MY9IVRKM97pj57OHXE2YEhPlwHGMvYNEUy8COLJyo TXxKO3wf Ln9n6ZvkCyUbIhgDwezYpfpRWUc3kTdqa9ts8YM97KV4N9sRbidmKybK2UUlAjaTWHbJv4GuUcVc5QHXh/rWar4yMBXS9Kp9eYky5k3szCKRzdXskAiawOo8az6koqaxglrBdSMPEurxZ2YwRb91p1YP5VCBFnKzf5FbRJtKXsI1OaSyDiO9PYLe8nmJ345HYExoV8NwJIuRFjbevi2Bawsx0L4cH5XRoMbx929Qf08QjEeJpWoFjOOiXhRaG784GuKLRepmOQnTs8hO19gP6XekcaJ2O7a5T2C2nGmDbWK7MEbDFWpNj//Az060YA4KnA3b0ZabEZjn4m++VkU1xedBAG7v5F/sSD1sRMDOaAgakwNJAwTGRL7o6kAQDUplQkToxfLU58bwoiYB1sMGuy7dq22h56ke2xMfHywPKs9hbifzUpxB0Kh3pjkcmWxSomcxfdiAKQCpAMfFDaajuHl2Ci9rFf5U0r6B89FyYOuCFWMR/UbebuyE7sbRTh8dDd3iD X-Bogosity: Ham, tests=bogofilter, spamicity=0.006801, 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 Tue, Jan 9, 2024 at 8:30=E2=80=AFAM Yosry Ahmed = wrote: > > On Mon, Jan 8, 2024 at 7:13=E2=80=AFPM Zhongkun He wrote: > > > > Hi Yosry, glad to hear from you and happy new year! > > > > > Sorry for being late to the party. It seems to me that all of this > > > hassle can be avoided if lru_add_fn() did the right thing in this cas= e > > > and added the folio to the tail of the lru directly. I am no expert i= n > > > how the page flags work here, but it seems like we can do something > > > like this in lru_add_fn(): > > > > > > if (folio_test_reclaim(folio)) > > > lruvec_add_folio_tail(lruvec, folio); > > > else > > > lruvec_add_folio(lruvec, folio); > > > > > > I think the main problem with this is that PG_reclaim is an alias to > > > PG_readahead, so readahead pages will also go to the tail of the lru, > > > which is probably not good. This sounds dangerous. This is going to introduce a rather large unexpected side effect - we're changing the readahead behavior in a seemingly small zswap optimization. In fact, I'd argue that if we do this, the readahead behavior change will be the "main effect", and the zswap-side change would be a "happy consequence". We should run a lot of benchmarking and document the change extensively if we pursue this route. > > > > Agree with you=EF=BC=8C I will try it. > > +Matthew Wilcox > > I think we need to figure out if it's okay to do this first, because > it will affect pages with PG_readahead as well. > > > > > > > > > A more intrusive alternative is to introduce a folio_lru_add_tail() > > > variant that always adds pages to the tail, and optionally call that > > > from __read_swap_cache_async() instead of folio_lru_add() based on a > > > new boolean argument. The zswap code can set that boolean argument > > > during writeback to make sure newly allocated folios are always added > > > to the tail of the lru. Unless some page flag/readahead expert can confirm that the first option is safe, my vote is on this option. I mean, it's fairly minimal codewise, no? Just a bunch of plumbing. We can also keep the other call sites intact if we just rename the old versions - something along the line of: __read_swap_cache_async_head(..., bool add_to_lru_head) { ... if (add_to_lru_head) folio_add_lru(folio) else folio_add_lru_tail(folio); } __read_swap_cache_async(...) { return __read_swap_cache_async_tail(..., true); } A bit boilerplate? Sure. But this seems safer, and I doubt it's *that* much more work. > > > > I have the same idea and also find it intrusive. I think the first solu= tion > > is very good and I will try it. If it works, I will send the next versi= on. > > One way to avoid introducing folio_lru_add_tail() and blumping a > boolean from zswap is to have a per-task context (similar to > memalloc_nofs_save()), that makes folio_add_lru() automatically add > folios to the tail of the LRU. I am not sure if this is an acceptable > approach though in terms of per-task flags and such. This seems a bit hacky and obscure, but maybe it could work.