Received-SPF: pass (google.com: domain of linux-kernel+bounces-108716-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 20 Mar 2024 05:50:53 -0400
From: Johannes Weiner <hannes@cmpxchg.org>
To: Yosry Ahmed <yosryahmed@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Nhat Pham <nphamcs@gmail.com>,
	Chengming Zhou <chengming.zhou@linux.dev>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] mm: zswap: increase shrinking protection for zswap
 swapins only
Message-ID: <20240320095053.GA294822@cmpxchg.org>
References: <20240320020823.337644-1-yosryahmed@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240320020823.337644-1-yosryahmed@google.com>

On Wed, Mar 20, 2024 at 02:08:22AM +0000, Yosry Ahmed wrote:
> Currently, the number of protected zswap entries corresponding to an
> lruvec are incremented every time we swapin a page.

Correct. This is the primary signal that the shrinker is being too
aggressive in moving entries to disk and should slow down...?

> This happens regardless of whether or not the page originated in
> zswap. Hence, swapins from disk will lead to increasing protection
> on potentially stale zswap entries. Furthermore, the increased
> shrinking protection can lead to more pages skipping zswap and going
> to disk, eventually leading to even more swapins from disk and
> starting a vicious circle.

How does shrinker protection affect zswap stores?

On the contrary, I would expect this patch to create a runaway
shrinker. The more aggressively it moves entries out to disk, the
lower the rate of zswap loads, the more aggressively it moves more
entries out to disk.

> Instead, only increase the protection when pages are loaded from zswap.
> This also has a nice side effect of removing zswap_folio_swapin() and
> replacing it with a static helper that is only called from zswap_load().
> 
> No problems were observed in practice, this was found through code
> inspection.

This is missing test results :)