Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1571990538-6133-1-git-send-email-teawaterz@linux.alibaba.com>
In-Reply-To: <1571990538-6133-1-git-send-email-teawaterz@linux.alibaba.com>
From:   Dan Streetman <ddstreet@ieee.org>
Date:   Fri, 8 Nov 2019 11:04:29 -0500
Message-ID: <CALZtONCQ1YqpAXfZS6jemHuKpBXhLz440EcxSoWZbxrH0kyLHg@mail.gmail.com>
Subject: Re: [PATCH] zswap: Add shrink_enabled that can disable swap shrink to
 increase store performance
To:     Hui Zhu <teawaterz@linux.alibaba.com>
Cc:     Seth Jennings <sjenning@redhat.com>, Linux-MM <linux-mm@kvack.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Oct 25, 2019 at 4:02 AM Hui Zhu <teawaterz@linux.alibaba.com> wrote:
>
> zswap will try to shrink pool when zswap is full.
> This commit add shrink_enabled that can disable swap shrink to increase
> store performance.  User can disable swap shrink if care about the store
> performance.

I don't understand - if zswap is full it can't store any more pages
without shrinking the current pool.  This commit will just force all
pages to swap when zswap is full.  This has nothing to do with 'store
performance'.

I think it would be much better to remove any user option for this and
implement some hysteresis; store pages normally until the zpool is
full, then reject all pages going to that pool until there is some %
free, at which point allow pages to be stored into the pool again.
That will prevent (or at least reduce) the constant performance hit
when a zpool fills up, and just fallback to normal swapping to disk
until the zpool has some amount of free space again.

>
> For example in a VM with 1 CPU 1G memory 4G swap:
> echo lz4 > /sys/module/zswap/parameters/compressor
> echo z3fold > /sys/module/zswap/parameters/zpool
> echo 0 > /sys/module/zswap/parameters/same_filled_pages_enabled
> echo 1 > /sys/module/zswap/parameters/enabled
> usemem -a -n 1 $((4000 * 1024 * 1024))
> 4718592000 bytes / 114937822 usecs = 40091 KB/s
> 101700 usecs to free memory
> echo 0 > /sys/module/zswap/parameters/shrink_enabled
> usemem -a -n 1 $((4000 * 1024 * 1024))
> 4718592000 bytes / 8837320 usecs = 521425 KB/s
> 129577 usecs to free memory
>
> The store speed increased when zswap shrink disabled.
>
> Signed-off-by: Hui Zhu <teawaterz@linux.alibaba.com>
> ---
>  mm/zswap.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 46a3223..731e3d1e 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -114,6 +114,10 @@ static bool zswap_same_filled_pages_enabled = true;
>  module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled,
>                    bool, 0644);
>
> +/* Enable/disable zswap shrink (enabled by default) */
> +static bool zswap_shrink_enabled = true;
> +module_param_named(shrink_enabled, zswap_shrink_enabled, bool, 0644);
> +
>  /*********************************
>  * data structures
>  **********************************/
> @@ -947,6 +951,9 @@ static int zswap_shrink(void)
>         struct zswap_pool *pool;
>         int ret;
>
> +       if (!zswap_shrink_enabled)
> +               return -EPERM;
> +
>         pool = zswap_pool_last_get();
>         if (!pool)
>                 return -ENOENT;
> --
> 2.7.4
>