From: Dan Magenheimer <dan.magenheimer@oracle.com>
To: Seth Jennings <sjenning@linux.vnet.ibm.com>
Cc: Bob Liu <bob.liu@oracle.com>, Mel Gorman <mgorman@suse.de>,
Andrew Morton <akpm@linux-foundation.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Nitin Gupta <ngupta@vflare.org>, Minchan Kim <minchan@kernel.org>,
Konrad Wilk <konrad.wilk@oracle.com>,
Robert Jennings <rcj@linux.vnet.ibm.com>,
Jenifer Hopper <jhopper@us.ibm.com>,
Johannes Weiner <jweiner@redhat.com>,
Rik van Riel <riel@redhat.com>,
Larry Woodman <lwoodman@redhat.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Dave Hansen <dave@sr71.net>, Joe Perches <joe@perches.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Cody P Schafer <cody@linux.vnet.ibm.com>,
Hugh Dickens <hughd@google.com>,
Paul Mackerras <paulus@samba.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org
Subject: RE: [PATCHv11 3/4] zswap: add to mm/
Date: Tue, 14 May 2013 13:54:41 -0700 (PDT) [thread overview]
Message-ID: <370eb593-1a2f-41a6-8b16-163f54634f19@default> (raw)
In-Reply-To: <20130514172827.GE4024@medulla>
> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On Tue, May 14, 2013 at 09:37:08AM -0700, Dan Magenheimer wrote:
> > > From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> > > Subject: Re: [PATCHv11 3/4] zswap: add to mm/
> > >
> > > On Tue, May 14, 2013 at 05:19:19PM +0800, Bob Liu wrote:
> > > > Hi Seth,
> > >
> > > Hi Bob, thanks for the review!
> > >
> > > >
> > > > > + /* reclaim space if needed */
> > > > > + if (zswap_is_full()) {
> > > > > + zswap_pool_limit_hit++;
> > > > > + if (zbud_reclaim_page(tree->pool, 8)) {
> > > >
> > > > My idea is to wake up a kernel thread here to do the reclaim.
> > > > Once zswap is full(20% percent of total mem currently), the kernel
> > > > thread should reclaim pages from it. Not only reclaim one page, it
> > > > should depend on the current memory pressure.
> > > > And then the API in zbud may like this:
> > > > zbud_reclaim_page(pool, nr_pages_to_reclaim, nr_retry);
> > >
> > > So kswapd for zswap. I'm not opposed to the idea if a case can be
> > > made for the complexity. I must say, I don't see that case though.
> > >
> > > The policy can evolve as deficiencies are demonstrated and solutions are
> > > found.
> >
> > Hmmm... it is fairly easy to demonstrate the deficiency if
> > one tries. I actually first saw it occur on a real (though
> > early) EL6 system which started some graphics-related service
> > that caused a very brief swapstorm that was invisible during
> > normal boot but clogged up RAM with compressed pages which
> > later caused reduced weird benchmarking performance.
>
> Without any specifics, I'm not sure what I can do with this.
Well, I think its customary for the author of a patch to know
the limitations of the patch. I suggest you synthesize a
workload that attempts to measure worst case. That's exactly
what I did a year ago that led me to the realization that
zcache needed to solve some issues before it was ready to
promote out of staging.
> I'm hearing you say that the source of the benchmark degradation
> are the idle pages in zswap. In that case, the periodic writeback
> patches I have in the wings should address this.
>
> I think we are on the same page without realizing it. Right now
> zswap supports a kind of "direct reclaim" model at allocation time.
> The periodic writeback patches will handle the proactive writeback
> part to free up the zswap pool when it has idle pages in it.
I don't think we are on the same page though maybe you are heading
in the same direction now. I won't repeat the comments from the
previous email.
> > I think Mel's unpredictability concern applies equally here...
> > this may be a "long-term source of bugs and strange memory
> > management behavior."
> >
> > > Can I get your ack on this pending the other changes?
> >
> > I'd like to hear Mel's feedback about this, but perhaps
> > a compromise to allow for zswap merging would be to add
> > something like the following to zswap's Kconfig comment:
> >
> > "Zswap reclaim policy is still primitive. Until it improves,
> > zswap should be considered experimental and is not recommended
> > for production use."
>
> Just for the record, an "experimental" tag in the Kconfig won't
> work for me.
>
> The reclaim policy for zswap is not primitive, it's simple. There
> is a difference. Plus zswap is already runtime disabled by default.
> If distros/customers enabled it, it is because they purposely
> enabled it.
Hmmm... I think you are proposing to users/distros the following
use model: "If zswap works for you, turn it on. If it sucks,
turn it off. I can't tell you in advance whether it will work
or suck for your distro/workload, but it will probably work so
please try it."
That sounds awfully experimental to me.
The problem is not simple. Your solution is simple because
you are simply pretending that the harder parts of the problem
don't exist.
Dan
--
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:[~2013-05-14 20:55 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-13 12:39 [PATCHv11 0/4] zswap: compressed swap caching Seth Jennings
2013-05-13 12:40 ` [PATCHv11 1/4] debugfs: add get/set for atomic types Seth Jennings
2013-05-16 14:58 ` Rik van Riel
2013-05-13 12:40 ` [PATCHv11 2/4] zbud: add to mm/ Seth Jennings
2013-05-14 8:47 ` Bob Liu
2013-05-14 17:03 ` Seth Jennings
2013-05-16 15:30 ` Rik van Riel
2013-05-17 15:48 ` Mel Gorman
2013-05-19 20:52 ` Seth Jennings
2013-05-20 13:54 ` Mel Gorman
2013-05-20 15:42 ` Seth Jennings
2013-05-21 8:10 ` Mel Gorman
2013-05-23 2:00 ` Bob Liu
2013-05-23 9:52 ` Mel Gorman
2013-05-13 12:40 ` [PATCHv11 3/4] zswap: " Seth Jennings
2013-05-14 9:19 ` Bob Liu
2013-05-14 16:00 ` Seth Jennings
2013-05-14 16:37 ` Dan Magenheimer
2013-05-14 17:28 ` Seth Jennings
2013-05-14 20:54 ` Dan Magenheimer [this message]
2013-05-17 17:00 ` Mel Gorman
2013-05-16 17:16 ` Rik van Riel
2013-05-17 16:54 ` Mel Gorman
2013-05-19 23:33 ` Seth Jennings
2013-05-13 12:40 ` [PATCHv11 4/4] zswap: add documentation Seth Jennings
2013-05-16 17:06 ` Rik van Riel
2013-05-17 16:04 ` Mel Gorman
[not found] <<1368448803-2089-1-git-send-email-sjenning@linux.vnet.ibm.com>
[not found] ` <<1368448803-2089-3-git-send-email-sjenning@linux.vnet.ibm.com>
2013-05-13 15:43 ` [PATCHv11 2/4] zbud: add to mm/ Dan Magenheimer
2013-05-13 20:59 ` Seth Jennings
2013-05-16 15:30 ` Rik van Riel
[not found] ` <<1368448803-2089-4-git-send-email-sjenning@linux.vnet.ibm.com>
2013-05-13 22:31 ` [PATCHv11 3/4] zswap: " Dan Magenheimer
2013-05-14 16:35 ` Seth Jennings
2013-05-14 20:18 ` Dan Magenheimer
2013-05-14 22:55 ` Seth Jennings
2013-05-15 17:09 ` Dan Magenheimer
2013-05-15 18:55 ` Konrad Rzeszutek Wilk
2013-05-15 19:35 ` Dan Magenheimer
2013-05-15 20:45 ` Rik van Riel
2013-05-15 21:36 ` Dan Magenheimer
2013-05-15 22:01 ` Rik van Riel
2013-05-15 20:09 ` Seth Jennings
2013-05-15 20:24 ` Dave Hansen
2013-05-15 20:55 ` Dan Magenheimer
2013-05-15 20:45 ` Konrad Rzeszutek Wilk
2013-05-15 20:52 ` Dan Magenheimer
2013-05-15 22:14 ` Rik van Riel
2013-05-16 16:45 ` Dan Magenheimer
2013-05-16 17:06 ` Rik van Riel
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=370eb593-1a2f-41a6-8b16-163f54634f19@default \
--to=dan.magenheimer@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=bob.liu@oracle.com \
--cc=cody@linux.vnet.ibm.com \
--cc=dave@sr71.net \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=hughd@google.com \
--cc=iamjoonsoo.kim@lge.com \
--cc=jhopper@us.ibm.com \
--cc=joe@perches.com \
--cc=jweiner@redhat.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lwoodman@redhat.com \
--cc=mgorman@suse.de \
--cc=minchan@kernel.org \
--cc=ngupta@vflare.org \
--cc=paulus@samba.org \
--cc=rcj@linux.vnet.ibm.com \
--cc=riel@redhat.com \
--cc=sjenning@linux.vnet.ibm.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).