linux-kernel - Re: [PATCH 3/4] zswap: Zero-filled pages handling

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 17 Aug 2016 15:25:26 +0300
From:	Pekka Enberg <penberg@...nel.org>
To:	srividya.dr@...sung.com
Cc:	sjenning@...hat.com, "linux-mm@...ck.org" <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Dinakar Reddy Pathireddy <dinakar.p@...sung.com>,
	샤란 <sharan.allur@...sung.com>,
	SUNEEL KUMAR SURIMANI <suneel@...sung.com>,
	김주훈 <juhunkim@...sung.com>
Subject: Re: [PATCH 3/4] zswap: Zero-filled pages handling

On Wed, Aug 17, 2016 at 1:18 PM, Srividya Desireddy
<srividya.dr@...sung.com> wrote:
> This patch adds a check in zswap_frontswap_store() to identify zero-filled
> page before compression of the page. If the page is a zero-filled page, set
> zswap_entry.zeroflag and skip the compression of the page and alloction
> of memory in zpool. In zswap_frontswap_load(), check if the zeroflag is
> set for the page in zswap_entry. If the flag is set, memset the page with
> zero. This saves the decompression time during load.
>
> The overall overhead caused due to zero-filled page check is very minimal
> when compared to the time saved by avoiding compression and allocation in
> case of zero-filled pages. The load time of a zero-filled page is reduced
> by 80% when compared to baseline.

AFAICT, that's an overall improvement only if there are a lot of
zero-filled pages because it's just overhead for pages that we *need*
to compress, no? So I suppose the question is, are there a lot of
zero-filled pages that we need to swap and why is that the case?

> @@ -1314,6 +1347,13 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
>         }
>         spin_unlock(&tree->lock);
>
> +       if (entry->zeroflag) {
> +               dst = kmap_atomic(page);
> +               memset(dst, 0, PAGE_SIZE);
> +               kunmap_atomic(dst);
> +               goto freeentry;
> +       }

Don't we need the same thing in zswap_writeback_entry() for the
ZSWAP_SWAPCACHE_NEW case?

> +
>         /* decompress */
>         dlen = PAGE_SIZE;
>         src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->zhandle->handle,
> @@ -1327,6 +1367,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
>         zpool_unmap_handle(entry->pool->zpool, entry->zhandle->handle);
>         BUG_ON(ret);

- Pekka