From: Minchan Kim <minchan@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Dan Streetman <ddstreet@ieee.org>,
Seth Jennings <sjennings@variantweb.net>,
Linux-MM <linux-mm@kvack.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
Bob Liu <bob.liu@oracle.com>,
Weijie Yang <weijie.yang@samsung.com>,
Shirish Pargaonkar <spargaonkar@suse.com>,
Mel Gorman <mgorman@suse.de>
Subject: Re: [PATCH] mm/zswap: add writethrough option
Date: Thu, 23 Jan 2014 09:18:06 +0900 [thread overview]
Message-ID: <20140123001806.GF31230@bbox> (raw)
In-Reply-To: <20140122123358.a65c42605513fc8466152801@linux-foundation.org>
Hello all,
On Wed, Jan 22, 2014 at 12:33:58PM -0800, Andrew Morton wrote:
> On Wed, 22 Jan 2014 09:19:58 -0500 Dan Streetman <ddstreet@ieee.org> wrote:
>
> > >>> > Acutally, I really don't know how much benefit we have that in-memory
> > >>> > swap overcomming to the real storage but if you want, zRAM with dm-cache
> > >>> > is another option rather than invent new wheel by "just having is better".
> > >>>
> > >>> I'm not sure if this patch is related to the zswap vs. zram discussions. This
> > >>> only adds the option of using writethrough to zswap. It's a first
> > >>> step to possibly
> > >>> making zswap work more efficiently using writeback and/or writethrough
> > >>> depending on
> > >>> the system and conditions.
> > >>
> > >> The patch size is small. Okay I don't want to be a party-pooper
> > >> but at least, I should say my thought for Andrew to help judging.
> > >
> > > Sure, I'm glad to have your suggestions.
> >
> > To give this a bump - Andrew do you have any concerns about this
> > patch? Or can you pick this up?
>
> I don't pay much attention to new features during the merge window,
> preferring to shove them into a folder to look at later. Often they
> have bitrotted by the time -rc1 comes around.
>
> I'm not sure that this review discussion has played out yet - is
> Minchan happy?
>From the beginning, zswap is for reducing swap I/O but if workingset
overflows, it should write back rather than OOM with expecting a small
number of writeback would make the system happy because the high memory
pressure is temporal so soon most of workload would be hit in zswap
without further writeback.
If memory pressure continues and writeback steadily, it means zswap's
benefit would be mitigated, even worse by addding comp/decomp overhead.
In that case, it would be better to disable zswap, even.
Dan said writethrough supporting is first step to make zswap smart
but anybody didn't say further words to step into the smart and
what's the *real* workload want it and what's the *real* number from
that because dm-cache/zram might be a good fit.
(I don't intend to argue zram VS zswap. If the concern is solved by
existing solution, why should we invent new function and
have maintenace cost?) so it's very hard for me to judge that we should
accept and maintain it.
We need blueprint for the future and make an agreement on the
direction before merging this patch.
But code size is not much and Seth already gave an his Ack so I don't
want to hurt Dan any more(Sorry for Dan) and wasting my time so pass
the decision to others(ex, Seth and Bob).
If they insist on, I don't object any more.
Sorry for bothering Dan.
Thanks.
>
> Please update the changelog so that it reflects the questions Minchan
> asked (any reviewer question should be regarded as an inadequacy in
> either the code commenting or the changelog - people shouldn't need to
> ask the programmer why he did something!) and resend for -rc1?
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Kind regards,
Minchan Kim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2014-01-23 0:16 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-19 13:23 [PATCH] mm/zswap: add writethrough option Dan Streetman
2014-01-02 15:38 ` Dan Streetman
2014-01-03 2:21 ` Weijie Yang
2014-01-03 15:11 ` Seth Jennings
2014-01-13 17:03 ` Dan Streetman
2014-01-14 0:11 ` Minchan Kim
2014-01-14 15:10 ` Dan Streetman
2014-01-15 5:42 ` Minchan Kim
2014-01-17 5:41 ` Dan Streetman
2014-01-22 14:19 ` Dan Streetman
2014-01-22 20:33 ` Andrew Morton
2014-01-23 0:18 ` Minchan Kim [this message]
2014-01-23 1:08 ` Bob Liu
2014-01-23 12:46 ` Austin S Hemmelgarn
2014-01-23 19:18 ` Dan Streetman
2014-01-23 20:43 ` Dan Streetman
2014-01-27 14:01 ` [PATCH v2] " Dan Streetman
2014-02-03 23:08 ` Andrew Morton
2014-02-04 2:47 ` Minchan Kim
2014-02-10 19:05 ` Dan Streetman
2014-02-10 23:06 ` Andrew Morton
2014-02-11 22:49 ` Dan Streetman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140123001806.GF31230@bbox \
--to=minchan@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=bob.liu@oracle.com \
--cc=ddstreet@ieee.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=sjennings@variantweb.net \
--cc=spargaonkar@suse.com \
--cc=weijie.yang@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).