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 BA3D7C27C6E for ; Thu, 13 Jun 2024 02:58:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C7A996B00A9; Wed, 12 Jun 2024 22:58:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C2A4D6B00AB; Wed, 12 Jun 2024 22:58:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ACA746B00AD; Wed, 12 Jun 2024 22:58:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 8C2FD6B00A9 for ; Wed, 12 Jun 2024 22:58:19 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 39D9F4056B for ; Thu, 13 Jun 2024 02:58:19 +0000 (UTC) X-FDA: 82224356718.30.C00232A Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) by imf27.hostedemail.com (Postfix) with ESMTP id 689104000D for ; Thu, 13 Jun 2024 02:58:17 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=H9DoTou5; spf=pass (imf27.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.54 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=1718247497; 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=cYlALMaSt5bKOLPpkzaQGZT0pfqD9pv8OI0qHYxUheA=; b=71jsJpB4p5JqUyCFd73sUWacOqObtM0GAtABNsn8uMCF9jw53n8hGnD+5uWwDmp09nk1fh ws35dc1Bcv/2iraPyDPO6zMiVJmXbhQc5HSst6IrhnmDMaY+VSD6ABqg0c1bdiH8n562OZ E8XrQVUc58UeiBYkqHhHU8v/rwkV/ls= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=H9DoTou5; spf=pass (imf27.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.54 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718247497; a=rsa-sha256; cv=none; b=1Vuw6oKtGZ+VdXIY4mNKD4DbNnZyGXhc7f6rlVg9IytL731it33dbKZbnNNAsTB4Jun0qh G19ilYlVENUlyy8Wd71mHFTTrUttowYNtpxSrazkqPOPxH8Tex12R2UKmSAvT7+x0EJ6QE yzXY9WQ+hKv1P4E+T/jJmuYF5/yc+q0= Received: by mail-ej1-f54.google.com with SMTP id a640c23a62f3a-a6ef64b092cso59962466b.1 for ; Wed, 12 Jun 2024 19:58:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1718247496; x=1718852296; 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=cYlALMaSt5bKOLPpkzaQGZT0pfqD9pv8OI0qHYxUheA=; b=H9DoTou5AZ8ybKMHoOFFLLsxt2n9CzuFNpQ76F62HqQ94vgdOf4HCOYHtc1uXm5eZ9 snnw+g1ZHK5Lu6OwOL+0b3HaQW+/b7ACjq965wUwPoLwcxotFMi0mDzPBgWjTeiIwlS9 iFwRI6YWmj8lcU5WZyzKI6y9mpMQaJcp0hq7ZnJxECBnxiCpB6h43jJ5x5ZYv24xiRMB mQlLkfGoU5c0QLVGSRAbDgwK+KTg4i14niI6L6B7iNDdtnhAw3d7YZQZ4oENtg3ChsM/ lhduX4QLodWlAvob+oa6u/3qC7ZC6KId1FMLtJDwrkOioii1bQxYEQKxIzp0dfZxpBdA WY4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718247496; x=1718852296; 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=cYlALMaSt5bKOLPpkzaQGZT0pfqD9pv8OI0qHYxUheA=; b=ExJ36JN7NmJexIgQ9RYTTwzZzGyPaT+7/40fu3c8lbpMN8PnQ65q23NH5O7pdRxyp9 Jlrhy19Qvlbanu7qn66s0itvOS6BjHce9t55nX6aCUbNWRU0Fy9pNQqtEH6g7AiPPnbQ vgeAW3sNyX9N99vZKsN1hDRV8zZKy9t+T1zaDumw5aHGeaeU4uH8oPZOB1lDhkoNCFLk MjYZtRhwfyJYtgTeUeP/0506HltfHS4h3uD/OQUG9KatXYykHB/LfhYpMaQkm046tvbj EA1wy4Ia2KaHY0y+q1dulhAIg2w8ilGyKuXtkYxRE2m+DiLrEE/+eWa5S0GM5uEDuo/l 7lxQ== X-Forwarded-Encrypted: i=1; AJvYcCW7G6qiq6oW6jiZin/kbdUit3GTc8DCsN/bUUGH4U1o4qD32PYm8b91LcjUfxsLBLuUtfWzukJh0E7J1e3W8Qsnt/A= X-Gm-Message-State: AOJu0YxpomU+poM4s2zAI5eNVQcF3umrbveoYcGk5/Rp9uhv7yDwknDf urPbs5FddumNGM8fJ9nDCPHhRd3XL1GSfCENBgEbaFLii8iTbZbxDuyWBF7/cfEbM4px4EqO4Wl rYC+E4ImlAfQQ+o/G3d1gl1+1kqwD3XIaKqU1ZsmMRhJ71mBE9X5118s= X-Google-Smtp-Source: AGHT+IF9CciRYK/S+Ij1+KuSCnynD6lav8vXAPrkXMbU5iqPDXNWqrABMnPq8TuNELzKnGWLPQ19ZjeHQ37ywUEgQSE= X-Received: by 2002:a17:906:c149:b0:a6f:4d38:f40c with SMTP id a640c23a62f3a-a6f4d391d2amr193732366b.62.1718247495476; Wed, 12 Jun 2024 19:58:15 -0700 (PDT) MIME-Version: 1.0 References: <20240608155316.451600-1-flintglass@gmail.com> <20240608155316.451600-2-flintglass@gmail.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 12 Jun 2024 19:57:37 -0700 Message-ID: Subject: Re: [PATCH v1 1/3] mm: zswap: fix global shrinker memcg iteration To: Takero Funaki Cc: Nhat Pham , Johannes Weiner , Chengming Zhou , Jonathan Corbet , Andrew Morton , Domenico Cerasuolo , linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, Shakeel Butt Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 689104000D X-Stat-Signature: sqctwimgae9mkfbdpzioxnjfy56spsux X-HE-Tag: 1718247497-350915 X-HE-Meta: U2FsdGVkX19RHNFy5/NYoLZxe5T19FEtd5RsYU9JNvHnpTlK7MpnO8JsquDyfKkhCuvy5H78+xb1PiX2Xob1YHKKTEpPHwRMYOdVn+pxZX5YVCikl0OhiYqhKEM2tp6xFYvnyc+q8aX7TXFIeX4JFj08ZNwyuggM4k9VuMTthazjTcJNaScfvg1uZRckTE7xdBWR9jQLfWG8Hj4NBzsnPwH8JvEf6CUnSgNwhy8mZpevpZ2ializXPVIK8dv4/Kye+Ue5qusrhIFrxUlYp/Gnh/3ajx7ONoMAXm0L1vffPfGa5C5ACI0WoXuw6a0GyqIr5sD4e3tBNWGOsv9AbXalM5PcGxpDUCjTY/PnmNt+Wu1vtPtML+NjK68xsOphSW9VWWZ/qGQQnBcsd6TKAmLQQLYyVLDW1iDI9a6BOhoWzEIPNgHzVs9pbNMt0281WnFyUx4/SgEmwvU5rMToNRUMDu4oIn/D15ngTNhncSV398/t60FPht306z4f+NYsgiw4ir0HDOzVvOBmo1S1io2i3hgYX6lKC3RsIxKpY1YiQAPFHMH5YMVMwvZyFENC31v3C79liY/SeAPcpAGY1nqw45fs50rf0oKTXs5nxJa1JyMGZcetJoE1dkgNdnMTpV00hxDQ5/PdGJheMJSLA239CA1h8sQUP/jmFDTHdSf9qPKdQ1DbWmL9je3Pd8QxRX5CJ/JuS6q9Ni86QUglmVCnf28HrzLqMG5cYBDENjOpeu0FOxZOGNLVL7wY/xMOvO8hpnwkCeIcved4f6bW8oYdcNaj9wLCedMVRjTbR5ytW2DmS/LPdEKNPdsuMKlXoOprfwU43d6Anettdfc+ODK0o+z3CrHkZVtTOPaTGMH4YMUnzJNwdiELlu3VYa4yCgmXB7OQIoenn6NBzuT7fCP0uz16/xUPUX/0H9dLzumPCc9TwiXvuBQ7TwJ8ccfW2uLeC5jdcfZON2MTP8e59P teynyN6a fLnIQA3w+YiueqkIP1V5cVMRK21lB2uHd+4YRh+83SJEjAh4anKo2X1x3RxS7PaqQVuLRWfvjZhR1RWcd1xQH5P/TT1egwFWhAOPY6AOlmefRRZT989VYRe7IrcRu1LyE9voDoifceZ3lE/ZXa8ztGGWeTnfgxALU5dOL5TUBWZvhSsEGLGtB6uMTtpysnw8zfFlS+nlaUEUGMnXnUMAdPmUIJg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000017, 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 Wed, Jun 12, 2024 at 7:36=E2=80=AFPM Takero Funaki wrote: > > 2024=E5=B9=B46=E6=9C=8813=E6=97=A5(=E6=9C=A8) 11:18 Yosry Ahmed : > > > > The corrected version of the cleaner should be: > > > ```c > > > void zswap_memcg_offline_cleanup(struct mem_cgroup *memcg) > > > { > > > /* lock out zswap shrinker walking memcg tree */ > > > spin_lock(&zswap_shrink_lock); > > > if (zswap_next_shrink =3D=3D memcg) { > > > do { > > > zswap_next_shrink =3D mem_cgroup_iter(NULL, > > > zswap_next_shrink, NULL); > > > spin_unlock(&zswap_shrink_lock); > > > spin_lock(&zswap_shrink_lock); > > > if (!zswap_next_shrink) > > > break; > > > } while (!mem_cgroup_online(zswap_next_shrink)); > > > } > > > spin_unlock(&zswap_shrink_lock); > > > } > > > ``` > > > > Is the idea here to avoid moving the iterator to another offline memcg > > that zswap_memcg_offline_cleanup() was already called for, to avoid > > holding a ref on that memcg until the next run of zswap shrinking? > > > > If yes, I think it's probably worth doing. But why do we need to > > release and reacquire the lock in the loop above? > > Yes, the existing cleaner might leave the offline, already-cleaned memcg. > > The reacquiring lock is to not loop inside the critical section. > In shrink_worker of v0 patch, the loop was restarted on offline memcg > without releasing the lock. Nhat pointed out that we should drop the > lock after every mem_cgroup_iter() call. v1 was changed to reacquire > once per iteration like the cleaner code above. I am not sure how often we'll run into a situation where we'll be holding the lock for too long tbh. It should be unlikely to keep encountering offline memcgs for a long time. Nhat, do you think this could cause a problem in practice?