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:   Thu, 3 Nov 2022 15:03:48 +0900
From:   Sergey Senozhatsky <senozhatsky@chromium.org>
To:     Minchan Kim <minchan@kernel.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Nitin Gupta <ngupta@vflare.org>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, Sergey Senozhatsky <senozhatsky@chromium.org>
Subject: Re: [PATCHv4 4/9] zram: Introduce recompress sysfs knob
Message-ID: <Y2NZxD5SpHEObPaE@google.com>
References: <20221018045533.2396670-1-senozhatsky@chromium.org>
 <20221018045533.2396670-5-senozhatsky@chromium.org>
 <Y2Lbxp6to4QNYyGe@google.com>
 <Y2M0t5etyJiUfeQi@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y2M0t5etyJiUfeQi@google.com>
Precedence: bulk

On (22/11/03 12:25), Sergey Senozhatsky wrote:
> > or we can introduce the threshold with action item.
> >   
> >   echo "idle 888" > /sys/block/zram0/recompress
> >   echo "huge 888" > /sys/block/zram0/recompress
> >   echo "normal 888" > /sys/block/zram0/recompress
> 
> I like the latter one, when threshold is an optional argument.
> I probably would even go a bit further and add keywords:
> 
> 	type=STRING threshold=INT

E.g. recompress support for type= and optional threshold=

We kind of don't have a use case of type=normal, as it is an equivalent
of no type. So we have huge, idle, huge_idle and no param means all
pages (which is sort of logical). threshold is optional.

---
 drivers/block/zram/zram_drv.c | 55 ++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 9a614253de07..12f03745baf9 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1688,7 +1688,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
  * Corresponding ZRAM slot should be locked.
  */
 static int zram_recompress(struct zram *zram, u32 index, struct page *page,
-			   int size_watermark)
+			   int size_threshold)
 {
 	unsigned long handle_prev;
 	unsigned long handle_next;
@@ -1708,7 +1708,7 @@ static int zram_recompress(struct zram *zram, u32 index, struct page *page,
 	/*
 	 * Do not recompress objects that are already "small enough".
 	 */
-	if (comp_len_prev < size_watermark)
+	if (comp_len_prev < size_threshold)
 		return 0;
 
 	ret = zram_read_from_zspool(zram, page, index);
@@ -1780,29 +1780,42 @@ static ssize_t recompress_store(struct device *dev,
 {
 	struct zram *zram = dev_to_zram(dev);
 	unsigned long nr_pages = zram->disksize >> PAGE_SHIFT;
+	char *args, *param, *val;
 	unsigned long index;
 	struct page *page;
 	ssize_t ret;
-	int mode, size_watermark = 0;
-
-	if (sysfs_streq(buf, "idle")) {
-		mode = RECOMPRESS_IDLE;
-	} else if (sysfs_streq(buf, "huge")) {
-		mode = RECOMPRESS_HUGE;
-	} else if (sysfs_streq(buf, "huge_idle")) {
-		mode = RECOMPRESS_IDLE | RECOMPRESS_HUGE;
-	} else {
-		/*
-		 * We will re-compress only idle objects equal or greater
-		 * in size than watermark.
-		 */
-		ret = kstrtoint(buf, 10, &size_watermark);
-		if (ret)
-			return ret;
-		mode = RECOMPRESS_IDLE;
+	int mode = 0, size_threshold = 0;
+
+	args = skip_spaces(buf);
+	while (*args) {
+		args = next_arg(args, &param, &val);
+
+		if (!*val)
+			return -EINVAL;
+
+		if (!strcmp(param, "type")) {
+			if (!strcmp(val, "idle"))
+				mode = RECOMPRESS_IDLE;
+			if (!strcmp(val, "huge"))
+				mode = RECOMPRESS_HUGE;
+			if (!strcmp(val, "huge_idle"))
+				mode = RECOMPRESS_IDLE | RECOMPRESS_HUGE;
+			continue;
+		}
+
+		if (!strcmp(param, "threshold")) {
+			/*
+			 * We will re-compress only idle objects equal or
+			 * greater in size than watermark.
+			 */
+			ret = kstrtoint(val, 10, &size_threshold);
+			if (ret)
+				return ret;
+			continue;
+		}
 	}
 
-	if (size_watermark > PAGE_SIZE)
+	if (size_threshold > PAGE_SIZE)
 		return -EINVAL;
 
 	down_read(&zram->init_lock);
@@ -1841,7 +1854,7 @@ static ssize_t recompress_store(struct device *dev,
 		    zram_test_flag(zram, index, ZRAM_RECOMP_SKIP))
 			goto next;
 
-		err = zram_recompress(zram, index, page, size_watermark);
+		err = zram_recompress(zram, index, page, size_threshold);
 next:
 		zram_slot_unlock(zram, index);
 		if (err) {
-- 
2.38.1.273.g43a17bfeac-goog