From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 32DA83D9DAC for ; Thu, 25 Jun 2026 11:28:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782386898; cv=none; b=pO9M6pwn79hUeLrbozL3mlOs5Nfwz/bZP9lpMO7buu3su3wrXcEd6rQ8HOXnHMnmwTFJOK1xs9Hz/1G0eKozPfzI3wCKy9J2e5TTevXpOs55zL6QdYtnZ9T0AFvKnOFcZREHimOEHjqvZOx7BbOSOLAeU1CyKk+FL09eDn0wqII= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782386898; c=relaxed/simple; bh=9EmqR/QU3L6zxa1jX24ROznGfhsUPbqYOcUKXhdjfK8=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=uQVzVTCLDP9I3OtAsPFZfMfJFhKlb2J6flCWY+qessNshjNZrYohOKezRAcMNLskxBbkQJ90ki6h+XxEuDLnyC6fP2Yj2VBDUs6pUGUwPRJrH8lp3Dnsqec/+znEdGhJuBUhA0V75jscoKlDwPqt+xGy/uQgG59GgR1MgtydueU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=N5u1hfud; arc=none smtp.client-ip=209.85.216.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="N5u1hfud" Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-37de8c6a2b6so1083363a91.0 for ; Thu, 25 Jun 2026 04:28:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782386896; x=1782991696; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=AizybT0CQDOCLg4ll/0xAnza6/lwZnP0ZIiS3UphQeI=; b=N5u1hfudLGlr7Ek5iCAFM7c4OcS7s5DAN9ptQGE3IEINp3JStzGWWFJrepmkg8qUmo 8yBceVjUxtstooO4uP+RVvIoJMTOpD8rLH+GMprtOQYbzK1pWAMEhHRJn2AHD2A7N28G VIU/e1CA2lYPj0sPk6DwQXvFIOqD2fqH2bZrG3to/TBXXqCyfi/Z7ErveEsQTkUlQ6qG MIlMocQuJhR591vupBywibp2/BQonbkR2dcprakKEIvYmvtJvQH23MryfpJoAfzDQBGN RQnpBUWzKNlEztvHO/ASJ3xMFnIVlDlfbF8hw0m/PDf7kYEgXKgSVZov/0FsslLkId81 e0VA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782386896; x=1782991696; h=content-transfer-encoding:in-reply-to:from:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=AizybT0CQDOCLg4ll/0xAnza6/lwZnP0ZIiS3UphQeI=; b=mCwiByCiSxEGuAVxEHbKDoMX5MewMgeZ4qO3ZV5VXEw+TyHT/VheXEFy+pSjD5TrvK yQbNVOdRj1iBM3HXMU6mWefRLEzwfQOOtJUjsaUZ0U1dfEKpCodxCqvj607NT15Q/i8w A4Jr7J0L8MH3I6lI34yf5KqOxXHkujQXcWJ36fW39i/rRPrb82GEiAWKpIEBqwhpaHTB j/zNm8xgElhM9Pbp+aAvuAs9vt2XmJlzTKZFsscAGMWcBlLry70nkk5Dlmu2quVbYsul 1kaqZKvzj5Y8P50Kh8lXsH4Fz+ENTtzOZ+xzBKFTC5vmYel6MFjBztlYqpscUSTcuKZI ueYg== X-Forwarded-Encrypted: i=1; AHgh+RptHwbEERDALQTIzHdHOC3hjFtlfXBwpXIRZ/VMsmi7aq66uC3Z1xEl9pPvzvIhiDI+79fgbPuxH9A0AzE=@vger.kernel.org X-Gm-Message-State: AOJu0YxonDUqsg7glD1vnv7G/XJ6j3rInIaEHTTMh6mq3t25jSGCjnZR DthuXXgJu48LabByGJK697Dp3aRA7bHswdDaiBlnb7Qm1zWpIMmnLquI X-Gm-Gg: AfdE7ckEkR2cqDfocRxKMpNZq2dYnpGXIASgzY9zmBAdlvvZQVxE3O0VilAGOcF6ezo ibJx5hv6X1q7KxF2GQJ597Ll6bQmw8QHyJZzZQGol8EVvYBjQhEdLYIaeBwV5naBmcPCzMNnQbe YXQehYkCKF8X0/62jE2UhP4IxTgOKAAL9kap03ih3LT0mcU+ln0u9CuFEjeIX8DJps69XkzNceD vELNc/Cxva51BGTYWZvTKcIaKKHXXeOnLN/ezdL5/U0yZ4t8WFdukJMd1rjBXU3/f2QaY/tclSD 4xTfuhhzplOQXWzzKQeXXXIPhCMK4im0+LdIdeo8Z4z/0/thwbwopvgMF3hApZI8hObWbGwATxk 6+1fcmOUXgaDVAo6UfLV4jUKN/ecrktpzQzEaxtl9VqgT8phSXUhMp7mgRZ4+qiT6lryYMUahxP 0W6R1Jru5sY5duubRDbTtjMkWDgFXXIcc8DgokOR4OIMo= X-Received: by 2002:a17:903:2b0e:b0:2ba:3e2f:6883 with SMTP id d9443c01a7336-2c7fc6f396amr21714615ad.19.1782386896284; Thu, 25 Jun 2026 04:28:16 -0700 (PDT) Received: from [10.125.192.89] ([210.184.73.204]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2c7f5afb17fsm18380775ad.26.2026.06.25.04.28.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 25 Jun 2026 04:28:15 -0700 (PDT) Message-ID: <91297bc0-268c-e9c2-57ae-6066eee5df2f@gmail.com> Date: Thu, 25 Jun 2026 19:28:06 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 Subject: Re: [PATCH v4 2/5] mm/zswap: Factor writeback loop out of shrink_worker() To: Yosry Ahmed Cc: akpm@linux-foundation.org, tj@kernel.org, hannes@cmpxchg.org, shakeel.butt@linux.dev, mhocko@kernel.org, mkoutny@suse.com, nphamcs@gmail.com, chengming.zhou@linux.dev, muchun.song@linux.dev, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, Hao Jia References: <20260618044857.69439-1-jiahao.kernel@gmail.com> <20260618044857.69439-3-jiahao.kernel@gmail.com> <0916e673-861f-b472-7417-afbffbcc98ad@gmail.com> From: Hao Jia In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2026/6/25 01:00, Yosry Ahmed wrote: > On Wed, Jun 24, 2026 at 4:55 AM Hao Jia wrote: >> >> >> >> On 2026/6/23 07:36, Yosry Ahmed wrote: >> >> >> Perhaps something like this? >> >> struct zswap_shrink_state { >> int attempts; >> int failures; >> bool stop; >> }; >> >> static bool zswap_shrink_no_candidate(struct zswap_shrink_state *s) >> { >> if (!s->attempts && ++s->failures == MAX_RECLAIM_RETRIES) >> return true; >> >> s->attempts = 0; >> return false; >> } >> >> static long zswap_shrink_one(struct mem_cgroup *memcg, >> struct zswap_shrink_state *s) >> { >> long shrunk; >> >> shrunk = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH); >> if (shrunk == -ENOENT) >> return 0; >> >> s->attempts++; >> if (shrunk <= 0 && ++s->failures == MAX_RECLAIM_RETRIES) >> s->stop = true; > > Do we need 'stop' or can we just return a value here to indicate that > we should stop (e.g. -EBUSY)? > Perhaps we could return -EAGAIN instead of -EBUSY? This would align with the semantics of the memory.reclaim interface, which returns -EAGAIN when it reclaims fewer bytes than requested. >> >> return shrunk; >> } >> >> static void shrink_worker(struct work_struct *w) >> { >> struct zswap_shrink_state s = {}; >> unsigned long thr; >> >> /* Reclaim down to the accept threshold */ >> thr = zswap_accept_thr_pages(); >> >> while (zswap_total_pages() > thr) { >> struct mem_cgroup *memcg; >> >> cond_resched(); >> >> memcg = zswap_iter_global(); >> if (!memcg) { >> if (zswap_shrink_no_candidate(&s)) >> break; >> continue; >> } >> >> zswap_shrink_one(memcg, &s); >> /* Drop the extra reference taken by the iterator. */ >> mem_cgroup_put(memcg); >> if (s.stop) >> break; >> } >> } > > I think splitting the shrink/retry logic over 2 functions makes it > more difficult to follow, so yeah I think fold > zswap_shrink_no_candidate() into zswap_shrink_one(). Then the callers > only need to iterate memcgs (depending on the context) and call > zswap_shrink_one() for each of them. So, something like this? /* Track progress of a memcg-tree writeback walk. */ struct zswap_shrink_state { int attempts; int failures; }; /* * Take one step of a memcg-tree writeback walk driven by the caller's * iterator, and fold the result into @s, the retry bookkeeping shared * across steps. @memcg is the iterator's current memcg, or NULL once * it has wrapped around after a full pass over the tree. * * The function returns -EAGAIN to signal the caller to abort the walk * after encountering the following conditions MAX_RECLAIM_RETRIES times: * - No writeback-candidate memcgs were found in a memcg tree walk. * - Shrinking a writeback-candidate memcg failed. * * Return: The number of compressed bytes written back (>= 0), or -EAGAIN * once the retry budget is exhausted and the caller should abort the walk. */ static long zswap_shrink_one(struct mem_cgroup *memcg, struct zswap_shrink_state *s) { long shrunk; /* * If the iterator has completed a full pass, update the shrink state * and check whether we should keep going. */ if (!memcg) { /* * Continue shrinking without incrementing failures if we found * candidate memcgs in the last tree walk. */ if (!s->attempts && ++s->failures == MAX_RECLAIM_RETRIES) return -EAGAIN; s->attempts = 0; return 0; } shrunk = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH); /* * 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 attempts and failures. */ if (shrunk == -ENOENT) return 0; s->attempts++; if (shrunk <= 0 && ++s->failures == MAX_RECLAIM_RETRIES) return -EAGAIN; return shrunk; } static void shrink_worker(struct work_struct *w) { struct zswap_shrink_state s = {}; unsigned long thr; /* Reclaim down to the accept threshold */ thr = zswap_accept_thr_pages(); while (zswap_total_pages() > thr) { struct mem_cgroup *memcg; long ret; cond_resched(); memcg = zswap_iter_global(); ret = zswap_shrink_one(memcg, &s); /* drop the extra reference taken by zswap_iter_global() */ mem_cgroup_put(memcg); if (ret == -EAGAIN) break; } }