linux-kernel - Re: [PATCHv5 7/8] zswap: add swap page writeback support

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: <512BA147.2090308@linux.vnet.ibm.com>
Date:	Mon, 25 Feb 2013 11:37:11 -0600
From:	Seth Jennings <sjenning@...ux.vnet.ibm.com>
To:	Minchan Kim <minchan@...nel.org>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Nitin Gupta <ngupta@...are.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Dan Magenheimer <dan.magenheimer@...cle.com>,
	Robert Jennings <rcj@...ux.vnet.ibm.com>,
	Jenifer Hopper <jhopper@...ibm.com>,
	Mel Gorman <mgorman@...e.de>,
	Johannes Weiner <jweiner@...hat.com>,
	Rik van Riel <riel@...hat.com>,
	Larry Woodman <lwoodman@...hat.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Dave Hansen <dave@...ux.vnet.ibm.com>,
	Joe Perches <joe@...ches.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org
Subject: Re: [PATCHv5 7/8] zswap: add swap page writeback support

On 02/24/2013 08:54 PM, Minchan Kim wrote:
> Hi Seth,
> 
> On Wed, Feb 13, 2013 at 12:38:50PM -0600, Seth Jennings wrote:
>> This patch adds support for evicting swap pages that are currently
>> compressed in zswap to the swap device.  This functionality is very
>> important and make zswap a true cache in that, once the cache is full
>> or can't grow due to memory pressure, the oldest pages can be moved
>> out of zswap to the swap device so newer pages can be compressed and
>> stored in zswap.
>>
>> This introduces a good amount of new code to guarantee coherency.
>> Most notably, and LRU list is added to the zswap_tree structure,
>> and refcounts are added to each entry to ensure that one code path
>> doesn't free then entry while another code path is operating on it.
>>
>> Signed-off-by: Seth Jennings <sjenning@...ux.vnet.ibm.com>
> 
> In this time, I didn't review the code in detail yet but it seems
> resolve of all review point in previous interation. Thanks!
> But unfortunately, I couldn't find anything related to tmppage handling
> so I'd like to ask.
> 
> The reason of tmppage is temporal buffer to keep compressed data during
> writeback to avoid unnecessary compressing again when we retry?

Yes.

> Is it really critical about performance?

It's hard to measure.  There is no guarantee that
zswap_flush_entries() has made room for the allocation so if we fail
again, we've compressed the page twice and still fail

So my motivation was to prevent the second compression.  It does add
significant complexity though without a completely clear (i.e.
measurable) benefit.


What's the wrong if we remove
> tmppage handling?
> 
> zswap_frontswap_store
> retry:
>         get_cpu_var(zswap_dstmem);
>         zswap_com_op(COMPRESS)
>         zs_malloc()
>         if (!handle) {
>                 put_cpu_var(zswap_dstmem);
>                 if (retry > MAX_RETRY)
>                         goto error_nomem;
>                 zswap_flush_entries()
>                 goto retry;
>         }

I dislike "jump up" labels, but yes, something like this could be done.

Thanks,
Seth

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/