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 ADE68C3DA63 for ; Tue, 23 Jul 2024 15:35:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 53AA86B00BB; Tue, 23 Jul 2024 11:35:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4B3646B00BC; Tue, 23 Jul 2024 11:35:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 26B286B00BD; Tue, 23 Jul 2024 11:35:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 081896B00BB for ; Tue, 23 Jul 2024 11:35:18 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id A89D41201AA for ; Tue, 23 Jul 2024 15:35:17 +0000 (UTC) X-FDA: 82371416274.19.63BC652 Received: from mail-yb1-f182.google.com (mail-yb1-f182.google.com [209.85.219.182]) by imf27.hostedemail.com (Postfix) with ESMTP id 9032240025 for ; Tue, 23 Jul 2024 15:35:15 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MAvL1ql3; spf=pass (imf27.hostedemail.com: domain of flintglass@gmail.com designates 209.85.219.182 as permitted sender) smtp.mailfrom=flintglass@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721748893; 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=EEWtktvlhhKGtS7ypqK5XQEDZZZuM3sheGLw0Jcss8g=; b=IXzUgrbqlFf8LeC6dWXHZ3HO2XhmvZ3B84Vx898keP+EiaAY3m2OIHb28iZlfPsOvqx6B/ 8NMt796eaqthG6QkAUxtY1Fqt2gWw/RnXr/+EBTpJ3b54Mxk8MOlNSt6WTJ0PDIRd/x0YE 5L5J3Gza41t+V4P1EHy+YlijWNKu1/c= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MAvL1ql3; spf=pass (imf27.hostedemail.com: domain of flintglass@gmail.com designates 209.85.219.182 as permitted sender) smtp.mailfrom=flintglass@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721748893; a=rsa-sha256; cv=none; b=DU8ulWtgCGcO+dbK1ZLDO8c8nMlCGTK3/Y3f0GH0Te/jhjJhSvnEuHik02EVXt07Xdwbje cB2lNjUNyaitotmmaWvaDr1Efl3EQhJ6+08fyqXLJmNltTlNM6xuhL84ZB0q6JghwMeSTv N5Es03CQstCV28qOgkhVAtiFDMDX6RQ= Received: by mail-yb1-f182.google.com with SMTP id 3f1490d57ef6-e087264e297so3227121276.3 for ; Tue, 23 Jul 2024 08:35:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721748914; x=1722353714; 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=EEWtktvlhhKGtS7ypqK5XQEDZZZuM3sheGLw0Jcss8g=; b=MAvL1ql3EVkZLiu0Mko4fYI46sk/8BeCnFAyFuq2hh9Lad4RX7wCWIx7jKnRunlmQH BWH0EcvndnnFa57n+GgQLbDUeK6hnHjqjFs6gMSg5ZIiGqck2lUEABsBb9DKddOjFJfW Uutz7LqevlWx2mr5adRv/1xEUh8qxCe5M3k7NjogrIzj4ZQD4weCI+WRm+Q+d1+9Epu1 Ylcv6LScYmTtQEsNuke2Q1k530k65ontLX84S8K2MeG4Z6c4wCF0g+42ct2r61kOk+Sv +FRGkUUM0K6QQm+4uCRySIiEuzPPYiJVAmnJJOZKKZ8QDZrCOEFlzuUOZW0KBinnHpf1 APCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721748914; x=1722353714; 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=EEWtktvlhhKGtS7ypqK5XQEDZZZuM3sheGLw0Jcss8g=; b=X4DTWKWO+h6FLSkPtXolLd6CfxkgIKG8vyHKKeSmTQgYXjA5S2/XW+DzldAFa7C4AG nzNWMavIcP1O/DBN18ayMtf3BFw/ISU2l1uizWJnuTntYwlgMNX1oZ32YVQMmrzrzp1M /6gqxOSORqNjxiv7pxosmEUG/F/S1kCWAn6jRsighgbS2zPNt+0s9X860Q0bCOb5uZC2 GE9wV9NVuFZjWh4upKNdaCUyCz5pXnxb0D9Md+b6KeN4U40XqyPJ0s/R+HjCiOjdqQS5 7NY6edP/FeDUb3mcHSPUu/2YulB7lPkI+PgFP1TpJOc/8rVO2TTqjCbjMJRk7l2MHhc1 yMeA== X-Forwarded-Encrypted: i=1; AJvYcCXpOTmdc9n5HH8u1xcsARwxNFQnj7EVIxAkzBJXgNNl528NjSeU7xR4hOgecP+hagu4aqI0YB3hdHsKyVQ2aqK2Prc= X-Gm-Message-State: AOJu0YwYrKu3Ye7DK5wkjQOl4GWGEQ3W15TNW05dJilncamH5QMkIM89 I4YVI5i6PCpwaE627ESHx+6NXQiE07dJjAEF6tKuzp4VNsAtZEohpOawPDztsfJBMLDUxU51XjW 3UZRknUAyAfZCmiFIfCqH2z9mRLQ= X-Google-Smtp-Source: AGHT+IGKMH+dcaVNTsXNK8AYPostkHDBYgQ96LvIvNO+U+bChlyoYXIqNBF1V99PHrygv64yqroC9k3m7sxxmR+hgYI= X-Received: by 2002:a05:6902:703:b0:e03:ab31:9aa6 with SMTP id 3f1490d57ef6-e08701898c5mr14273091276.24.1721748914582; Tue, 23 Jul 2024 08:35:14 -0700 (PDT) MIME-Version: 1.0 References: <20240720044127.508042-1-flintglass@gmail.com> <20240720044127.508042-2-flintglass@gmail.com> In-Reply-To: From: Takero Funaki Date: Wed, 24 Jul 2024 00:35:04 +0900 Message-ID: Subject: Re: [PATCH v3 1/2] mm: zswap: fix global shrinker memcg iteration To: Nhat Pham Cc: Johannes Weiner , Yosry Ahmed , Chengming Zhou , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 9032240025 X-Stat-Signature: c84xcy1ei45cpjabk8dayhc3onfxgu9f X-HE-Tag: 1721748915-925618 X-HE-Meta: U2FsdGVkX1+oiyd8M6PQb4d/zCnC2JvSiYzrXckBj/v0kuyRutAjFbdB8If8dA8mU8QzkfBlT8OcPTJylK00NgxffrIIViByM84GuYM54SVHv/Gx2ktWthWfKOlRj99i9pmoXPautNsROd4+aup40INzV9iJLolio13kwnRM+N/BpNWZl/wztFrxqrkoFgBoULW4gJUdx5t7lRHbQGJmhk/Gb1UT6BI+h8i0VvpF5jn141B5c08pHZcS/axP2CHMgZ5g3JJccMEKtEhVwaW3fFEv3uz3gYQ4whFYIYtjmXpDMCrGZdZ+TIbYQqlCIcX5Dda7W1sf1wtsqq/LiC6TpRPHEYyesxnAKpZvo8/tFd4+ZkoS/5bLOrkbtvSNBdqb6xiHWI4SQG8qDsYitI3BC9yQ2/DlRR/q/vj/so2O5yWp5OLYJJJTFeGxi5AJWSsaNwHInt1vKLvOq7KnVptsx3Weo6/czJ95Mos03WFlrwbLGV1aJI9nDM3gPXX/3k/O/WNzQONyAx5e7OhtB7roTBo6mx5JM/bSvamDFTTKDasllp4gRG3cHgjpaGFYOtwGnocjsvuLbr8u2aE2IEm5g3AcEmgPpO0Fsrj7C1AZ0r750Pp6SeOmL4HOO5Jq89FahqpR3LhOyRfTvqWMejRDHn9M4zD7m2gxexNQxpUXyJi1dM3biTSUBgugmLVBICtv1Xj5lOKokhQcKdV3BDk1C8uSf6sMe9cNwRxDf2jj7UZrqV+lLftcPok75iNRYo+lA/L1gBJj0wGh0cbrkS9IJIry8WzNf11UMLuSXBLBzpl2P1mSiyhk7gNPYuxwi8X81oEAOuisdxix21Qf+DNTDBP3RU0LquXFIxCV1igK3p4jsXgQR+aST393fC7lV0Uf31AYBD+n4y0KpD97DdT2NixbIJj+Hi5aGvGdf8UWTWUcI6dixagCsgDbiKgDD5KoDL1XaU4R7IdxsMEDf9s rbvdMI1Y AVweCHWawG1l+1HB6ubi8NXCxUXIARWXHFEP6+Yq4y058DQKoephYlh6b0rCikS5VkcPtcI3VL2xp6B5sFZQ/xIWP6ATpKnb6PEpHEBWLbHt0g1QvPJA92BHe9UrwvM3kQ4SLioUJQoHY68ShplQA4JMMU7HJcgy7GJG3EcvlcrqHfbfD3FoY319jgodVbjrEPPhyBrrbTpKTQ6XLPez78a7+ibM8d5O7NntRxRptzRNuTpgmGipGvsrph5SRCe39rC2L/2wGhyDDcS8Sw7fJLAk4hEsAdj05WQx+Nf49VOawXogV1Sm5/eca2sX899oIOMkg+ZxJIXY0BHSsSR6FPKjfX3i8CjGL1qZP 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: 2024=E5=B9=B47=E6=9C=8823=E6=97=A5(=E7=81=AB) 6:39 Nhat Pham : > > On Fri, Jul 19, 2024 at 9:41=E2=80=AFPM Takero Funaki wrote: > > > > This patch fixes an issue where the zswap global shrinker stopped > > iterating through the memcg tree. > > > > The problem was that shrink_worker() would stop iterating when a memcg > > was being offlined and restart from the tree root. Now, it properly > > handles the offline memcg and continues shrinking with the next memcg. > > > > To avoid holding refcount of offline memcg encountered during the memcg > > tree walking, shrink_worker() must continue iterating to release the > > offline memcg to ensure the next memcg stored in the cursor is online. > > > > The offline memcg cleaner has also been changed to avoid the same issue= . > > When the next memcg of the offlined memcg is also offline, the refcount > > stored in the iteration cursor was held until the next shrink_worker() > > run. The cleaner must release the offline memcg recursively. > > > > Fixes: a65b0e7607cc ("zswap: make shrinking memcg-aware") > > Signed-off-by: Takero Funaki > Hmm LGTM for the most part - a couple nits > [...] > > + zswap_next_shrink =3D mem_cgroup_iter(NULL, > > + zswap_next_shrink, NULL); > nit: this can fit in a single line right? Looks like it's exactly 80 char= acters. Isn't that over 90 chars? But yes, we can reduce line breaks using memcg as temporary, like: - if (zswap_next_shrink =3D=3D memcg) - zswap_next_shrink =3D mem_cgroup_iter(NULL, zswap_next_shrink, NULL); + if (zswap_next_shrink =3D=3D memcg) { + do { + memcg =3D mem_cgroup_iter(NULL, zswap_next_shrink, = NULL); + zswap_next_shrink =3D memcg; + } while (memcg && !mem_cgroup_online(memcg)); > [...] > > + zswap_next_shrink =3D mem_cgroup_iter(NULL, > > + zswap_next_shrink, NULL= ); > Same with this. > [...] > > + /* > > + * We verified the memcg is online and got an extra mem= cg > > + * reference. Our memcg might be offlined concurrently= but the > > + * respective offline cleaner must be waiting for our l= ock. > > + */ > > spin_unlock(&zswap_shrink_lock); > nit: can we remove this spin_unlock() call + the one within the `if > (!memcg)` block, and just do it unconditionally outside of if > (!memcg)? Looks like we are unlocking regardless of whether memcg is > null or not. > > memcg is a local variable, not protected by zswap_shrink_lock, so this > should be fine right? > > Otherwise: > Reviewed-by: Nhat Pham Ah that's right. We no longer modify zswap_next_shrink in the if branches. Merging the two spin_unlock.