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 5EB64C3DA6E for ; Mon, 8 Jan 2024 23:13:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D40456B0082; Mon, 8 Jan 2024 18:13:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CEFEB6B0083; Mon, 8 Jan 2024 18:13:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BB8346B0085; Mon, 8 Jan 2024 18:13:25 -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 A9D0C6B0082 for ; Mon, 8 Jan 2024 18:13:25 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 74A0B801A8 for ; Mon, 8 Jan 2024 23:13:25 +0000 (UTC) X-FDA: 81657697170.15.E684CB2 Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com [209.85.218.49]) by imf18.hostedemail.com (Postfix) with ESMTP id 9E9E71C0004 for ; Mon, 8 Jan 2024 23:13:23 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iWmSpA+B; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.49 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=1704755603; 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=fecCiAR481SWOqpFkPJSiF/c70aAk78xZ2uswrvKLww=; b=fFq+/9Koeo7inYtR7tvj3g31sakaEug5OrjNJDn7eBEFhwiFkFShVS3dB4OwdeJB/BroTq M7BTZg+Lq39uU+3QmSrt+/1/HDiabuR1voKq1LYFXb6jAbhOGzOjxzofNjiMbV73YyGTyu +AxxbAdSHX+nMWA7YGvWYhaxu4dsKeY= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iWmSpA+B; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.49 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704755603; a=rsa-sha256; cv=none; b=mIazYrUUoV+cuAM2FVN9dYBeldzxTwmpWurmaV0+mQxLcmfXqubQ5OD/3hZ5suJ8EQy2DB TSCPJUPH/6lWewlGxRzVO/x0k+26oArAK8Uklnd+GJygn1+G/UQzOwTjayQgSENcx3SaPn 769Ad6KsxKwq4l78PXVrOr2sRegFmrM= Received: by mail-ej1-f49.google.com with SMTP id a640c23a62f3a-a28bf46ea11so415176866b.1 for ; Mon, 08 Jan 2024 15:13:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1704755602; x=1705360402; 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=fecCiAR481SWOqpFkPJSiF/c70aAk78xZ2uswrvKLww=; b=iWmSpA+BhwRfbtO36q0QQ/rdf6mHPIxtdsva6XPCYJNmonLySxTK+GAD6kMOnCl3ZO NVXNdmBxVvme1DIpiqMzITKyBbDF+mOBrlDZw+EC85ufw8oM6Aq0mnnzQtQcYvwnyjyg znVnB5qepp87Hjxi7hX1KyHhUmng2zijBTeHkeYr8kcPIBR8o9I6oQuYYddCR/LjYAjs l8hjo6OeP3PIP2Qj0o0acKuI2YANqvJhKy2xKqAHz/GSa+vjQmP+oZElEUl+Cy6Idzx8 +GvAPJg3VgL6maLAhx2IaAoFQYJ7lplwfCsFE0tweLwrhqvC8tA0FJjmHKNyxtsig0NX Qw8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704755602; x=1705360402; 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=fecCiAR481SWOqpFkPJSiF/c70aAk78xZ2uswrvKLww=; b=CwfU1/GyAW03D7cGgwX42BdZtbiLDwkCTbIQB5CVWZT9NgD+Ymehbt2J7Nmf6JZhCv PrESnsI/4CSZbV3zZXuCWCA3y1QL89jocpyDY5Y/3tPkhAHb9wBfINefC6418mSb+3jB WWxiGfVHd03aghlJySMMmjLHXW+2kcuZrVYLayyWdcdjgwsGLHzVcAU4fH1UnqPYfFgq pr1BbYqXU6r3MOBszLbvJVSdnAHJGL4qBys8LWKR4D753gjuVHffH4h7pcVPC34UsQS2 +LomneN205LNOY3eNxHwh+w+PZopY2bhqLxJdKvGSCg0QZQtkI/HQomwR+pAwGs9KNcA te4Q== X-Gm-Message-State: AOJu0Ywb9Z+ilFbN81S1RsNFFljwnY+JXQGpqgorE/lqucD6fYHTGw7R /r3N50t5qH9bdjE0AoNwF3Iz81H3Km4pJAHwIlOAatSeRIq4 X-Google-Smtp-Source: AGHT+IG9FZliAE8ycwaVIJMrC1FRCDl1rIILVQHoDAQ2lQ0DPsbMkwQgzFpd6F5F+y2lmeGdKKadoIEf1q5ihv/vjaY= X-Received: by 2002:a17:906:1dd:b0:a28:c04e:315b with SMTP id 29-20020a17090601dd00b00a28c04e315bmr131634ejj.13.1704755602122; Mon, 08 Jan 2024 15:13:22 -0800 (PST) MIME-Version: 1.0 References: <20231024142706.195517-1-hezhongkun.hzk@bytedance.com> In-Reply-To: From: Yosry Ahmed Date: Mon, 8 Jan 2024 15:12:45 -0800 Message-ID: Subject: Re: [External] Re: [PATCH] mm: zswap: fix the lack of page lru flag in zswap_writeback_entry To: Nhat Pham 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 , weijie.yang@samsung.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 9E9E71C0004 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 8af55f87dgyspg7muq5krboru8c4swqw X-HE-Tag: 1704755603-915466 X-HE-Meta: U2FsdGVkX1+6EsRcI/gtfHZc1+auI2dAJKaSEmsfXK/wG0pBvb8eiAM1scZwlxJOINVJkXbwik7zt/rEKaZd48HNAnORFGibYe3lqoRFwIyv5PJeAUdWyY+pq9jqDWtocUpoNV21EFoaOIEnK3JiHFNBGoy8Yz0NsHs1+vq167bAZIgHSfZdBpVwKzVBxqbqEOe5Aq+xPimbeYV72dFUstyl17di54nTEydSYxZ5dYVA0Axw6QMlhyCI34C3grjsR7rJLwvRmaiXsaNsem3HAawI4EfvYxRBNgt7gITZ7OoR9++b6mFpaaxwfB5QtjQWb98SWmo1YaYTgLn3fy6460yMf0LDC+B2HV7/+GAHiY0tSfhnstJSyGskBQp1rvTImPv2ymIZC4IAB/g81jMrOrInHd7Esy+7QqFXuYJPp6pJdxt3Ns5UYGmfTwaydTNUmydIdpI/YzvAFGNJXqAJ2wjc8V6AcXJjwrRQ8J9zfmg5gmPUjGq6is/tp5DTcYhCqRJi/vJQSahP+AUTgbatgo1vENy/r/cte0i2o3IBsvYk6WWr0fKQhW7638nOYetDXqRsSTI9abGOXcNdMBW+/3y/8RschaByIIBLhj2b+Gm+f/EzN0YNCy9UAeBCZ8gnaR7hqJ4iNAYX7RqeBzlU9saRV/c8iHTx0kxDsoXsNBZ3vMS4QRRWQIV0+3d9W01H6ij0MGP8bGLUnNWkXZZWTieRH+RCuKkttE2NtcTGsh1s4yKZXoH0E95rCd825Plbpt78ZFdvzHcsdntMAHK+lw7CGuYIGlSgpPQXZEGTTz8GThKcKtYIwhNDF6V39Mj50oxNyOGlVb1lw4LE5J2ELCxZvXQMAeQ3W/U/IOKAjaYzV7Ti2/mK0FBQDl2dGKINqJU8kRmXPewrNYxC0eD3CkOB3QNSktLyiXDfnCOIY+Z2kTbwhu0bfgm8aR77UpIX8eeGv06aaA3X/Xi6+3p 2wKM7TcL +ygsAh2Acfb3Oepu7KMC7OTqdqFYan1yeZxNyVyBO8wKUfkTWS85dXxKn3RiZohm79C9kqNjDsifIDJJDXBqh2sHoB3EUhClKJKPCh1pqMfu2WOfLI3sIa4nwXP9mala3NCm04m3dtB1OzBcoiGkIv9SUuh15/STJRuLcEqJh7ygA0N8O6YOBr58L3p+GXR0DoT42WFt6ancG5uYmJSj96n5QDej5dwWC22/IH5XS3oZTbJFRDmzcvZoix7nzmweXmiK6P7fx5Y8s2fhD+6qc9HQl8raGt2wwWrXipF1+kwgETH7ZUQPAgD+XP/JXGtbnB00mAc0VcZJ19eMpRzfmquZuNdA5Ml0EQlLzsu/vXh85PMKDRfeiWsWUaMOe6NFlr7OvuNCoNpO0hpOSnq/Qu/fONw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000005, 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 Sun, Jan 7, 2024 at 1:59=E2=80=AFPM Nhat Pham wrote: > > On Sun, Jan 7, 2024 at 1:29=E2=80=AFPM Nhat Pham wrot= e: > > > > On Fri, Jan 5, 2024 at 6:10=E2=80=AFAM Zhongkun He wrote: > > > > > > > > There is another option here, which is not to move the page to th= e > > > > > tail of the inactive > > > > > list after end_writeback and delete the following code in > > > > > zswap_writeback_entry(), > > > > > which did not work properly. But the pages will not be released f= irst. > > > > > > > > > > /* move it to the tail of the inactive list after end_writeback *= / > > > > > SetPageReclaim(page); > > > > > > Ok, so I took a look at the patch that originally introduced this > > piece of logic: > > > > https://github.com/torvalds/linux/commit/b349acc76b7f65400b85abd09a5379= ddd6fa5a97 > > > > Looks like it's not for the sake of correctness, but only as a > > best-effort optimization (reducing page scanning). If it doesn't bring > > any benefit (i.e due to the newly allocated page still on the cpu > > batch), then we can consider removing it. After all, if you're right > > and it's not really doing anything here - why bother. Perhaps we can > > replace this with some other mechanism to avoid it being scanned for > > reclaim. > > For instance, we can grab the local lock, look for the folio in the > add batch and take the folio off it, then add it to the rotate batch > instead? Not sure if this is doable within folio_rotate_reclaimable(), > or you'll have to manually perform this yourself (and remove the > PG_reclaim flag set here so that folio_end_writeback() doesn't try to > handle it). > > There is still some overhead with this, but at least we don't have to > *drain everything* (which looks like what's lru_add_drain() -> > lru_add_drain_cpu() is doing). The latter sounds expensive and > unnecessary, whereas this is just one element addition and one element > removal - and if IIUC the size of the per-cpu add batch is capped at > 15, so lookup + removal (if possible) shouldn't be too expensive? > > Just throwing ideas out there :) 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 case and added the folio to the tail of the lru directly. I am no expert in 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. 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.