diff mbox series

[PATCH/RFC] zswap: do not map same object twice

Message ID 20190916004640.b453167d3556c4093af4cf7d@gmail.com (mailing list archive)
State New, archived
Headers show
Series [PATCH/RFC] zswap: do not map same object twice | expand

Commit Message

Vitaly Wool Sept. 15, 2019, 9:46 p.m. UTC
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.

Signed-off-by: Vitaly Wool <vitalywool@gmail.com>
---
 mm/zswap.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Dan Streetman Sept. 18, 2019, 11:47 a.m. UTC | #1
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
diff mbox series

Patch

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;
 }