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;
Sender: Minchan Kim <minchan.kim@gmail.com>
Date:   Fri, 18 Mar 2022 09:41:44 -0700
From:   Minchan Kim <minchan@kernel.org>
To:     Brian Geffon <bgeffon@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Nitin Gupta <ngupta@vflare.org>,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-block@vger.kernel.org
Subject: Re: [PATCH] zram: Add a huge_idle writeback mode
Message-ID: <YjS2SJU7VE1bGb/F@google.com>
References: <20220315172221.9522-1-bgeffon@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220315172221.9522-1-bgeffon@google.com>
Precedence: bulk

On Tue, Mar 15, 2022 at 10:22:21AM -0700, Brian Geffon wrote:
> Today it's only possible to write back as a page, idle, or huge.
> A user might want to writeback pages which are huge and idle first
> as these idle pages do not require decompression and make a good
> first pass for writeback.

Hi Brian,

I am not sure how much the decompression overhead matter for idle pages
writeback since it's already *very slow* path in zram but I agree that
it would be a good first pass since the memory saving for huge writing
would be cost efficient.

Just out of curiosity. Do you have real usecase?

> 
> Signed-off-by: Brian Geffon <bgeffon@google.com>
> ---
>  Documentation/admin-guide/blockdev/zram.rst |  6 ++++++
>  drivers/block/zram/zram_drv.c               | 10 ++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/admin-guide/blockdev/zram.rst b/Documentation/admin-guide/blockdev/zram.rst
> index 3e11926a4df9..af1123bfaf92 100644
> --- a/Documentation/admin-guide/blockdev/zram.rst
> +++ b/Documentation/admin-guide/blockdev/zram.rst
> @@ -343,6 +343,12 @@ Admin can request writeback of those idle pages at right timing via::
>  
>  With the command, zram writeback idle pages from memory to the storage.
>  
> +Additionally, if a user choose to writeback only huge and idle pages
> +this can be accomplished with::
> +
> +        echo huge_idle > /sys/block/zramX/writeback
> +
> +
>  If admin want to write a specific page in zram device to backing device,
>  they could write a page index into the interface.
>  
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index cb253d80d72b..f196902ae554 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -643,8 +643,8 @@ static int read_from_bdev_async(struct zram *zram, struct bio_vec *bvec,
>  #define PAGE_WB_SIG "page_index="
>  
>  #define PAGE_WRITEBACK 0
> -#define HUGE_WRITEBACK 1
> -#define IDLE_WRITEBACK 2
> +#define HUGE_WRITEBACK (1<<0)
> +#define IDLE_WRITEBACK (1<<1)
>  
>  
>  static ssize_t writeback_store(struct device *dev,
> @@ -664,6 +664,8 @@ static ssize_t writeback_store(struct device *dev,
>  		mode = IDLE_WRITEBACK;
>  	else if (sysfs_streq(buf, "huge"))
>  		mode = HUGE_WRITEBACK;
> +	else if (sysfs_streq(buf, "huge_idle"))
> +		mode = IDLE_WRITEBACK | HUGE_WRITEBACK;
>  	else {
>  		if (strncmp(buf, PAGE_WB_SIG, sizeof(PAGE_WB_SIG) - 1))
>  			return -EINVAL;
> @@ -725,10 +727,10 @@ static ssize_t writeback_store(struct device *dev,
>  				zram_test_flag(zram, index, ZRAM_UNDER_WB))
>  			goto next;
>  
> -		if (mode == IDLE_WRITEBACK &&
> +		if (mode & IDLE_WRITEBACK &&
>  			  !zram_test_flag(zram, index, ZRAM_IDLE))
>  			goto next;
> -		if (mode == HUGE_WRITEBACK &&
> +		if (mode & HUGE_WRITEBACK &&
>  			  !zram_test_flag(zram, index, ZRAM_HUGE))
>  			goto next;
>  		/*
> -- 
> 2.35.1.723.g4982287a31-goog
>