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 6642BC3DA64 for ; Fri, 26 Jul 2024 03:21:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 87EE16B008C; Thu, 25 Jul 2024 23:21:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 82E926B0092; Thu, 25 Jul 2024 23:21:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6F6426B0098; Thu, 25 Jul 2024 23:21:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5023C6B008C for ; Thu, 25 Jul 2024 23:21:37 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id AD4EA161504 for ; Fri, 26 Jul 2024 03:21:36 +0000 (UTC) X-FDA: 82380453792.05.772C877 Received: from out-177.mta1.migadu.com (out-177.mta1.migadu.com [95.215.58.177]) by imf28.hostedemail.com (Postfix) with ESMTP id A0194C0007 for ; Fri, 26 Jul 2024 03:21:34 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=NBa2KAzd; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf28.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.177 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721964093; a=rsa-sha256; cv=none; b=6f2OIK3Roc7ytNmMUDnq2L1QVQsO+udQNKQ58/itLqgzFaZTatdv17mBNabbyPHCDYiAoq Gg9a0hanwIj64mSnxEsTzUJYP6yJ5/ExMY+MZhrKeFe+n6KIAjBrkNZ1csWLR3hpfNJYra Dm5+JRdhzo30E9lOpgtrqLHTlOWOXKo= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=NBa2KAzd; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf28.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.177 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721964093; 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=1DzQcvsVu423D9gDp/sw1eetVDLB1O9SBjw/8CnRb34=; b=o2v8mjGBKfQXaK6CM8ZkSGwIXr0PTb65ctJC2makSmviXDeMoJHZNTtFkOs86Xednky5ar d6oDVDVwaQ/oWe2adgX0TLiAF8idtzD/Nsss03LAPaz0M2uaGOqIp/TrARPPamiRXw6Cxh VB6Ze2FLmkxu+Sx0Rse7k3YpyVMMGIQ= Message-ID: <9ac88791-54e2-4f7c-9171-5a1d44e01657@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1721964092; h=from:from: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; bh=1DzQcvsVu423D9gDp/sw1eetVDLB1O9SBjw/8CnRb34=; b=NBa2KAzdfVwyCEfL+ExoNbhrTYMLE61UWRwpn1jNTAa16ONCwEj+FqDvDFhJreH2DB2v3Y b0ahqXH0lkUx4AiH3r7Pd9LdkTOX6xNfrL9ewVWtk/QkUi25VTeSMwBFx7HU9fvoAQ3ieQ zH5yqdj5U0z6g5pFURWqT4P9q+lHf3U= Date: Fri, 26 Jul 2024 11:21:23 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v3 2/2] mm: zswap: fix global shrinker error handling logic To: Takero Funaki , Nhat Pham Cc: Johannes Weiner , Yosry Ahmed , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240720044127.508042-1-flintglass@gmail.com> <20240720044127.508042-3-flintglass@gmail.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Queue-Id: A0194C0007 X-Rspamd-Server: rspam01 X-Stat-Signature: cw7d3igninxxayadugkoas8643zb1dp3 X-HE-Tag: 1721964094-936253 X-HE-Meta: U2FsdGVkX18/lTFWqAiAgeM/NYxcxlnkPCiE+f13t+qil7KOTqsSpl62GzTcHqZn5ZdPWxkiitDMu+/UkMaHpXvYVPp6SW85VB5aNnVTw7WVat61UZoXVV1kBv1OpSh8ZpBJMsS5wM6eaF7UnDDac2Trj/8VhHjh8bCO/hhiSzbRg2b/j/vVFGiNh6mh4dHM2xPj7Im4O6YpW0LPbRwKQP0dlk+xqkURjvYUocz4qLYMSHoqdDnXHn5LRLVLAK1J1Eda/uJm1suzFXtMKUcPVd+QCnK7zz+XBSr8cIGS/mnudsqpRJzO63DIdQ7/NszKgtsAhCP9LYtNX9yI4reVc1BHb5mWVludb3guIW20WEKGiAFfvBNFZcHBa5voHJmQrKvOXLM6BCUmUKU7g/9NtPlHS8J2F6LkPfxfTWvneZuBJJesESpYdKx+lOKbXQ+TTpVBQzNUfoHXK34JjK+zyzYB9n86Yj35I1inJaEWiojGtW//wPLUpUY9kODROdRp8EB5Vo+fyXnI89ba4I0MuTUl18SttXCUyCwzn9YUs6JNMsA7TBecj9mi/frxOL7UGjxkBLzOHIZCt4rK7ioxK2tGLioVNnw08oB6tiTLNPQFCCdpXSC8q/YTcfp3MwqtOA+w3PM6qYMBm4rUE1mYl/kMJ4zClHmp2ARnwYNkNEAPbIcCGLnE6d+q797fBind4LxMI/ID5G21kurtRKNXs16ploO43ID6DW7m0hdFB+oPnBaFx5kbyvANa0GLwzamkZgoF/UuF9E62kn4Cd5bzO39VD61If/PhBxocUCvDsB9KkJA7YAp96TL2nqULE1iEe2AZ/kIOxxYts+jubPKHRR+IU+LnJaGFMY/MNfjkUzfBWjF74LIfA9ux8sPfZSfubo0JjWkoXLeYcXevi7AAa8eEI5tuD4KoElWIhes0FhZFltfaKbns89ivpuY0HVCIOd7H2dcTnOlnEMebYV wKiZ4eDW WJDqJecXAbhg7fX9zBilz6gODesGes79FZiXfNKcbK9DULo1V/ZPRK5LbdKlEX1NrUMJaNsOQnS1BeZ4Kk0hWMAKYHAeK1gcWuwc7x9qRtelk3UN+Mgb01hMTL7GQGxEc0SAcOoednyRNjvMA+tPvhZzppuRU0RYTU5NSsSXG/lxB/Ak= 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: On 2024/7/24 00:44, Takero Funaki wrote: > 2024年7月23日(火) 6:51 Nhat Pham : >> >> On Fri, Jul 19, 2024 at 9:41 PM Takero Funaki wrote: >>> >>> This patch fixes zswap global shrinker that did not shrink zpool as >>> expected. >>> >>> The issue it addresses is that `shrink_worker()` did not distinguish >>> between unexpected errors and expected error codes that should be >>> skipped, such as when there is no stored page in a memcg. This led to >>> the shrinking process being aborted on the expected error codes. >> >> The code itself seems reasonable to me, but may I ask you to document >> (as a comment) all the expected v.s unexpected cases? i.e when do we >> increment (or not increment) the failure counter? >> > > In addition to changes in the commit log suggested by Yosry, > adding some comments specifying what memcg is (not) candidates for > writeback, and what should be a failure. > > - /* global reclaim will select cgroup in a round-robin fashion. > + /* > + * Global reclaim will select cgroup in a round-robin fashion from all > + * online memcgs, but memcgs that have no pages in zswap and > + * writeback-disabled memcgs (memory.zswap.writeback=0) are not > + * candidates for shrinking. > + * > + * Shrinking will be aborted if we encounter the following > + * MAX_RECLAIM_RETRIES times: > + * - No writeback-candidate memcgs found in a memcg tree walk. > + * - Shrinking a writeback-candidate memcg failed. > * > * We save iteration cursor memcg into zswap_next_shrink, > * which can be modified by the offline memcg cleaner > > and, the reasons to (not) increment the progress: > > @@ -1387,10 +1407,20 @@ static void shrink_worker(struct work_struct *w) > /* drop the extra reference */ > mem_cgroup_put(memcg); > > - if (ret == -EINVAL) > - break; > + /* > + * There are no writeback-candidate pages in the memcg. > + * This is not an issue as long as we can find another memcg > + * with pages in zswap. Skip this without incrementing progress > + * and failures. > + */ > + if (ret == -ENOENT) > + continue; > + > if (ret && ++failures == MAX_RECLAIM_RETRIES) > break; > + > + /* completed writeback or incremented failures */ > + ++progress; Maybe the name "progress" is a little confusing here? "progress" sounds to me that we have some writeback completed. But actually it just means we have encountered some candidates, right? Thanks. > resched: > > >> My understanding is, we only increment the failure counter if we fail >> to reclaim from a selected memcg that is non-empty and >> writeback-enabled, or if we go a full tree walk without making any >> progress. Is this correct? >> > > Yes, that's the expected behavior. > Please let me know if there is more appropriate wording. > > Thanks.