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 E6FE1C3DA5D for ; Mon, 22 Jul 2024 21:39:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 64CBB6B0085; Mon, 22 Jul 2024 17:39:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5FDC46B0088; Mon, 22 Jul 2024 17:39:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4C47A6B0089; Mon, 22 Jul 2024 17:39:40 -0400 (EDT) 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 2C51F6B0085 for ; Mon, 22 Jul 2024 17:39:40 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id A499EC19EE for ; Mon, 22 Jul 2024 21:39:39 +0000 (UTC) X-FDA: 82368705678.30.E87D6B9 Received: from mail-qk1-f181.google.com (mail-qk1-f181.google.com [209.85.222.181]) by imf29.hostedemail.com (Postfix) with ESMTP id DB934120011 for ; Mon, 22 Jul 2024 21:39:37 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AX1hfkrn; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf29.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.222.181 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721684343; a=rsa-sha256; cv=none; b=4IeCw0sh+lq94GKTkwBBXOs9HGQ2zvzhfamOFFxc9LvdQzFfnNTQRGsxUT2qKMol+d2GmT GOTlslHcyUbzkNtUEOri5C7useopaVS1zvINA7HM+dD8vkYn3Ytzyfp1/+CjUWbXocgduu P1zxwRYbQBQ5Z+wnDORaEVMwEY1uJRY= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AX1hfkrn; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf29.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.222.181 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=1721684343; 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=N1Jz2M2BG7Jm1AKSR4a1EacnCUYfmHPhctjNqd1nvGI=; b=t1Dav123HkqItiYvRZsPE8J4cDEAq5hcFfOauICh+XNr4fwGbWz81rrnclPWj8OvpUDpCb x0Kxa5Pm1HkQc4TngSvA6+QqeEAVaqmYf9CdlQKMWMnh7jS3x4E/BwxN2KYGKBWZkSWolo j4SxjWmUy9xBvTDFPPzw3wE8jER5/ig= Received: by mail-qk1-f181.google.com with SMTP id af79cd13be357-79f178e62d1so316955785a.1 for ; Mon, 22 Jul 2024 14:39:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721684377; x=1722289177; 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=N1Jz2M2BG7Jm1AKSR4a1EacnCUYfmHPhctjNqd1nvGI=; b=AX1hfkrn572hnvcrXFI73sIhqjp94/9xg2UjpjEcXv9nxB5xC8r7srE2ZOV34nK8oa 8VLWXamFr+9FmqP98iB+osUa1lmpIUGO3+lTWyF+w0jl0YebfGqstyseQRI/fCBSiX6g u17rkVop104pNZGotDKYCgGmXxiE1Qxy+2F2geDEQx+SpwCfRra++2+vHpwq7ZMUZfUr Ma2KN2oPhxT8gQMsyfJH9tnWnJ2i+5XWaU/KQ5GGMYcZmkocTKkUaipDM90Ez7swNB0t 6R3KcgcA7nUuPIWtM4rw2HFI5nyBFhMc0gOSVC4ZkSJjSigBodaXQSJmDTdLnB9Cg3jz GubQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721684377; x=1722289177; 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=N1Jz2M2BG7Jm1AKSR4a1EacnCUYfmHPhctjNqd1nvGI=; b=P0P29Z9rijtHmImboxcp/ZTk2wEGdM4PobN41O5dOUhk8WJKjkF5lpfYS4egxP1/tt YcPaLZLQg2c46kgDZ9ui3hpOvAFj4M2Eo3xX3b8I6aemr2LwuaqO/Rz0Pb1RqgejEf9A Vguj76EfYs7FAwP0if/aHrihAUa20F+mOFzRFhF1loIS44VdqJFqqwOdF/Lu8gIUNKb4 aE+h/MSeG5ZIdVfNQDcHJGlvxE0Sqlsljhhh3GaRScJM1gYK1VxJ2BGrHTX7eL+NWQv0 OoSL5Wyk108wJTPlYjisaS9oGnbHc/xU5lpc5zmiSEoJ8RV2eqAlAto4bsiN2TLcXymq ZCSw== X-Forwarded-Encrypted: i=1; AJvYcCW8weIdqFIRcM1ufPfG5rO3kYoCiklNaTVZWPRfVbYdoDc2Tm4v+s4kS1wc9YKonZHWFCE0boOImUrB/EqW7N5rLyw= X-Gm-Message-State: AOJu0YzPXgFu5OhMg40kJLvtn5PE0HENl5zj5ubjHBWOPuYGG/XKF1J4 0QHvWleLno0eWxNfReizNTgfbH0O1TBsEofk9q1KVFQtklX0er3dl1expM5l89zQ/fS2f3YHNNl HqO9S5GWC8bcYxPkXSgesIccCeng= X-Google-Smtp-Source: AGHT+IGbo31K+2oCMqlDUudtu4Lczyb0s1wh7dXqQFTh9uKWZW3oHVI/YP68W23DkQ9mI7o10wJoVxvWQ6aui+4iieQ= X-Received: by 2002:a05:6214:767:b0:6b7:a32a:2eeb with SMTP id 6a1803df08f44-6b94f12865bmr126918626d6.23.1721684376854; Mon, 22 Jul 2024 14:39:36 -0700 (PDT) MIME-Version: 1.0 References: <20240720044127.508042-1-flintglass@gmail.com> <20240720044127.508042-2-flintglass@gmail.com> In-Reply-To: <20240720044127.508042-2-flintglass@gmail.com> From: Nhat Pham Date: Mon, 22 Jul 2024 14:39:23 -0700 Message-ID: Subject: Re: [PATCH v3 1/2] mm: zswap: fix global shrinker memcg iteration To: Takero Funaki 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-Rspamd-Queue-Id: DB934120011 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: ncynf95ucgoijajaetkyip75ferbndeq X-HE-Tag: 1721684377-136135 X-HE-Meta: U2FsdGVkX1917FttbaqS0zNkcN6YzfSAVQkHLCfCc/Lq8gxywHfe3pcHYrQxpNJb9UjowolBdotDw4CJdRp4v+fuDoUPlKAiZ6IQ9mgHMj9WQDacEEqtisXdGU6NLwRud6lvnC/TSHQIeBDq442CVTLQ2ozbce0zQ0GM8EyQUIpOyonVe2qFq0LFBYlIk7xbwCJrWMJ3xbiSE15rrt2VU355pOihjfDOhxnpd9zpTrIqx6wwtER45ncDdrSYTOPaOmMRAdo+HDaT+hgwjFr2av3S6PJnbcmEDSc0j2fVNm/QMFV8uXf5l5DfG+1Mo/Soes9299t1KoU1e0JMPI8uA006kJYhCBDfgaFPkt96m9JCSV87XzFIJ8kTsrZfNA7Kp+GAuvZ5oHU9YM7vLWGpnLz2fkm94Qg9gYnSImYHInXXKgyqkuU9hz/qHrezPSOFrkTuUIqfhChPNAnTWsZXwbgBOAY+rPpE5srdW2b8o13pWY9SnooZkEe4L2c74KDMLQfyM4CXa6EGnF1mZrFNGReTeDU50xPFmgaaoKTandq07vpgAleGGFtc1wJReXu+6tgEvttFsfKicBhYe/N1Nzp79CWKYbGkTBByD3O++7mtzxu/BlAuCjGL8bEcUUJzvI6QAQXLT8dN8dWpZAE7biv4BsjG6f7q1q5Qm0iq+GRPKtaf9L4tNvguyPwE72sApSLAQQ4hyHL6FGcP2qch8FW8ojmJdXTZNCB7zvBNbAQzmacWZMprSRZZxjMi4EOVsrWwcdPMK9hrWzucdGLagsVLtL1dM9HjlvgG8h89zG60qhoc6QzcTC4uJWG01dAA+B6JoOgKFWjEkLii+zm7uA2nhRwlA7L1m3NS36CXjbn8OS7C7nhv/UBcryWEV8n92brjecho7wfFSIzSpp7aIwpgENwy8Cb0EQIPx/cgOUCJQHWzrhTNrC4W+fo/KJqzkErNafToJp872uvYHl0 5Ggc93Eh YQSkVI9I3Gdpulozy/nZhosBmK0VmgJ3gaAOPXokZXWOvbbAlUZmpZSu59PMPDyLXnWDYBm5b52ETeUAaiKiivpGOL5Pwbf16+lAFihSYHYyTueMDQnPOK9XCN+Ao+eZr3AKJ6H3GFwuZrhvp+yszfU38SuJlXNxCMHhuFo/UAwirW+TRAxlxRX6tcuCZV4IFzOYQbJ51KAqFIa5YJnm3d78571Hlg+un7Y2WYR3huuTCEauKeE/bYzhIY9tIGlUN6/Kjh/OSYtlMsFcptbV7QRrw/M4crdJqC0Ac3xf3xF/z8r23dsJWoL8GaL4D6jyGErFl 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 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 charac= ters. [...] > + 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 memcg > + * reference. Our memcg might be offlined concurrently b= ut the > + * respective offline cleaner must be waiting for our loc= k. > + */ > 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