Received-SPF: pass (google.com: domain of linux-kernel+bounces-126686-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240329094050.2815699-1-senozhatsky@chromium.org>
In-Reply-To: <20240329094050.2815699-1-senozhatsky@chromium.org>
From: Brian Geffon <bgeffon@google.com>
Date: Mon, 1 Apr 2024 09:28:42 -0400
Message-ID: <CADyq12weZHtQaV_muEOe-gggu07hiun+xDuMJmqaxv8oyz9pLA@mail.gmail.com>
Subject: Re: [PATCHv2] zram: add max_pages param to recompression
To: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>, Minchan Kim <minchan@kernel.org>, 
	linux-kernel@vger.kernel.org, linux-block@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 29, 2024 at 5:40=E2=80=AFAM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> Introduce "max_pages" param to recompress device attribute
> which sets an upper limit on the number of entries (pages)
> zram attempts to recompress (in this particular recompression
> call). S/W recompression can be quite expensive so limiting
> the number of pages recompress touches can be quite helpful.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Acked-by: Brian Geffon <bgeffon@google.com>

>
> ---
>  Documentation/admin-guide/blockdev/zram.rst |  5 ++++
>  drivers/block/zram/zram_drv.c               | 31 +++++++++++++++++++--
>  2 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/blockdev/zram.rst b/Documentation/=
admin-guide/blockdev/zram.rst
> index ee2b0030d416..091e8bb38887 100644
> --- a/Documentation/admin-guide/blockdev/zram.rst
> +++ b/Documentation/admin-guide/blockdev/zram.rst
> @@ -466,6 +466,11 @@ of equal or greater size:::
>         #recompress idle pages larger than 2000 bytes
>         echo "type=3Didle threshold=3D2000" > /sys/block/zramX/recompress
>
> +It is also possible to limit the number of pages zram re-compression wil=
l
> +attempt to recompress:::
> +
> +       echo "type=3Dhuge_idle max_pages=3D42" > /sys/block/zramX/recompr=
ess
> +
>  Recompression of idle pages requires memory tracking.
>
>  During re-compression for every page, that matches re-compression criter=
ia,
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.=
c
> index f0639df6cd18..4cf38f7d3e0a 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1568,7 +1568,8 @@ static int zram_bvec_write(struct zram *zram, struc=
t bio_vec *bvec,
>   * Corresponding ZRAM slot should be locked.
>   */
>  static int zram_recompress(struct zram *zram, u32 index, struct page *pa=
ge,
> -                          u32 threshold, u32 prio, u32 prio_max)
> +                          u64 *num_recomp_pages, u32 threshold, u32 prio=
,
> +                          u32 prio_max)
>  {
>         struct zcomp_strm *zstrm =3D NULL;
>         unsigned long handle_old;
> @@ -1645,6 +1646,15 @@ static int zram_recompress(struct zram *zram, u32 =
index, struct page *page,
>         if (!zstrm)
>                 return 0;
>
> +       /*
> +        * Decrement the limit (if set) on pages we can recompress, even
> +        * when current recompression was unsuccessful or did not compres=
s
> +        * the page below the threshold, because we still spent resources
> +        * on it.
> +        */
> +       if (*num_recomp_pages)
> +               *num_recomp_pages -=3D 1;
> +
>         if (class_index_new >=3D class_index_old) {
>                 /*
>                  * Secondary algorithms failed to re-compress the page
> @@ -1710,6 +1720,7 @@ static ssize_t recompress_store(struct device *dev,
>         struct zram *zram =3D dev_to_zram(dev);
>         unsigned long nr_pages =3D zram->disksize >> PAGE_SHIFT;
>         char *args, *param, *val, *algo =3D NULL;
> +       u64 num_recomp_pages =3D ULLONG_MAX;
>         u32 mode =3D 0, threshold =3D 0;
>         unsigned long index;
>         struct page *page;
> @@ -1732,6 +1743,17 @@ static ssize_t recompress_store(struct device *dev=
,
>                         continue;
>                 }
>
> +               if (!strcmp(param, "max_pages")) {
> +                       /*
> +                        * Limit the number of entries (pages) we attempt=
 to
> +                        * recompress.
> +                        */
> +                       ret =3D kstrtoull(val, 10, &num_recomp_pages);
> +                       if (ret)
> +                               return ret;
> +                       continue;
> +               }
> +
>                 if (!strcmp(param, "threshold")) {
>                         /*
>                          * We will re-compress only idle objects equal or
> @@ -1788,6 +1810,9 @@ static ssize_t recompress_store(struct device *dev,
>         for (index =3D 0; index < nr_pages; index++) {
>                 int err =3D 0;
>
> +               if (!num_recomp_pages)
> +                       break;
> +
>                 zram_slot_lock(zram, index);
>
>                 if (!zram_allocated(zram, index))
> @@ -1807,8 +1832,8 @@ static ssize_t recompress_store(struct device *dev,
>                     zram_test_flag(zram, index, ZRAM_INCOMPRESSIBLE))
>                         goto next;
>
> -               err =3D zram_recompress(zram, index, page, threshold,
> -                                     prio, prio_max);
> +               err =3D zram_recompress(zram, index, page, &num_recomp_pa=
ges,
> +                                     threshold, prio, prio_max);
>  next:
>                 zram_slot_unlock(zram, index);
>                 if (err) {
> --
> 2.44.0.478.gd926399ef9-goog
>