Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 7 Jun 2023 10:13:57 -0400
From:   Johannes Weiner <hannes@cmpxchg.org>
To:     Yosry Ahmed <yosryahmed@google.com>
Cc:     Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Seth Jennings <sjenning@redhat.com>,
        Dan Streetman <ddstreet@ieee.org>,
        Vitaly Wool <vitaly.wool@konsulko.com>,
        Nhat Pham <nphamcs@gmail.com>,
        Domenico Cerasuolo <cerasuolodomenico@gmail.com>,
        Yu Zhao <yuzhao@google.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: zswap: support exclusive loads
Message-ID: <20230607141357.GA338934@cmpxchg.org>
References: <20230530210251.493194-1-yosryahmed@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230530210251.493194-1-yosryahmed@google.com>
Precedence: bulk

On Tue, May 30, 2023 at 09:02:51PM +0000, Yosry Ahmed wrote:
> @@ -46,6 +46,19 @@ config ZSWAP_DEFAULT_ON
>  	  The selection made here can be overridden by using the kernel
>  	  command line 'zswap.enabled=' option.
>  
> +config ZSWAP_EXCLUSIVE_LOADS
> +	bool "Invalidate zswap entries when pages are loaded"
> +	depends on ZSWAP
> +	help
> +	  If selected, when a page is loaded from zswap, the zswap entry is
> +	  invalidated at once, as opposed to leaving it in zswap until the
> +	  swap entry is freed.
> +
> +	  This avoids having two copies of the same page in memory
> +	  (compressed and uncompressed) after faulting in a page from zswap.
> +	  The cost is that if the page was never dirtied and needs to be
> +	  swapped out again, it will be re-compressed.
> +
>  choice
>  	prompt "Default compressor"
>  	depends on ZSWAP
> diff --git a/mm/frontswap.c b/mm/frontswap.c
> index 279e55b4ed87..e5d6825110f4 100644
> --- a/mm/frontswap.c
> +++ b/mm/frontswap.c
> @@ -216,8 +216,13 @@ int __frontswap_load(struct page *page)
>  
>  	/* Try loading from each implementation, until one succeeds. */
>  	ret = frontswap_ops->load(type, offset, page);
> -	if (ret == 0)
> +	if (ret == 0) {
>  		inc_frontswap_loads();
> +		if (frontswap_ops->exclusive_loads) {
> +			SetPageDirty(page);
> +			__frontswap_clear(sis, offset);
> +		}
> +	}
>  	return ret;

This would be a much more accessible feature (distro kernels,
experimenting, adapting to different workloads) if it were runtime
switchable.

That should be possible, right? As long as frontswap and zswap are
coordinated, this can be done on a per-entry basis:

	exclusive = READ_ONCE(frontswap_ops->exclusive_loads);
	ret = frontswap_ops->load(type, offset, page, exclusive);
	if (ret == 0) {
		if (exclusive) {
			SetPageDirty(page);
			__frontswap_clear(sis, offset);
		}
	}