linux-kernel - Re: [PATCH/RFC] zswap: do not map same object twice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CALZtONDROMJoyxgRSG2+xNVs2B0q+vQQOGG09fH0QCSzgRi5CA@mail.gmail.com>
Date:   Wed, 18 Sep 2019 07:47:36 -0400
From:   Dan Streetman <ddstreet@...e.org>
To:     Vitaly Wool <vitalywool@...il.com>
Cc:     Linux-MM <linux-mm@...ck.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Seth Jennings <sjenning@...hat.com>
Subject: Re: [PATCH/RFC] zswap: do not map same object twice

On Sun, Sep 15, 2019 at 5:46 PM Vitaly Wool <vitalywool@...il.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@...il.com>

Reviewed-by: Dan Streetman <ddstreet@...e.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