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: <20190916004640.b453167d3556c4093af4cf7d@gmail.com>
In-Reply-To: <20190916004640.b453167d3556c4093af4cf7d@gmail.com>
From:   Dan Streetman <ddstreet@ieee.org>
Date:   Wed, 18 Sep 2019 07:47:36 -0400
Message-ID: <CALZtONDROMJoyxgRSG2+xNVs2B0q+vQQOGG09fH0QCSzgRi5CA@mail.gmail.com>
Subject: Re: [PATCH/RFC] zswap: do not map same object twice
To:     Vitaly Wool <vitalywool@gmail.com>
Cc:     Linux-MM <linux-mm@kvack.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Seth Jennings <sjenning@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, Sep 15, 2019 at 5:46 PM Vitaly Wool <vitalywool@gmail.com> wrote:
>
> zswap_writeback_entry() maps a handle to read swpentry first, and
> then in the most common case it would map the same handle again.
> This is ok when zbud is the backend since its mapping callback is
> plain and simple, but it slows things down for z3fold.
>
> Since there's hardly a point in unmapping a handle _that_ fast as
> zswap_writeback_entry() does when it reads swpentry, the
> suggestion is to keep the handle mapped till the end.

LGTM

>
> Signed-off-by: Vitaly Wool <vitalywool@gmail.com>

Reviewed-by: Dan Streetman <ddstreet@ieee.org>

> ---
>  mm/zswap.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 0e22744a76cb..b35464bc7315 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -856,7 +856,6 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>         /* extract swpentry from data */
>         zhdr = zpool_map_handle(pool, handle, ZPOOL_MM_RO);
>         swpentry = zhdr->swpentry; /* here */
> -       zpool_unmap_handle(pool, handle);
>         tree = zswap_trees[swp_type(swpentry)];
>         offset = swp_offset(swpentry);
>
> @@ -866,6 +865,7 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>         if (!entry) {
>                 /* entry was invalidated */
>                 spin_unlock(&tree->lock);
> +               zpool_unmap_handle(pool, handle);
>                 return 0;
>         }
>         spin_unlock(&tree->lock);
> @@ -886,15 +886,13 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>         case ZSWAP_SWAPCACHE_NEW: /* page is locked */
>                 /* decompress */
>                 dlen = PAGE_SIZE;
> -               src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
> -                               ZPOOL_MM_RO) + sizeof(struct zswap_header);
> +               src = (u8 *)zhdr + sizeof(struct zswap_header);
>                 dst = kmap_atomic(page);
>                 tfm = *get_cpu_ptr(entry->pool->tfm);
>                 ret = crypto_comp_decompress(tfm, src, entry->length,
>                                              dst, &dlen);
>                 put_cpu_ptr(entry->pool->tfm);
>                 kunmap_atomic(dst);
> -               zpool_unmap_handle(entry->pool->zpool, entry->handle);
>                 BUG_ON(ret);
>                 BUG_ON(dlen != PAGE_SIZE);
>
> @@ -940,6 +938,7 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>         spin_unlock(&tree->lock);
>
>  end:
> +       zpool_unmap_handle(pool, handle);
>         return ret;
>  }
>
> --
> 2.17.1