From: Seth Jennings <sjenning@linux.vnet.ibm.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Dan Magenheimer <dan.magenheimer@oracle.com>,
devel@linuxdriverproject.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, ngupta@vflare.org,
Konrad Wilk <konrad.wilk@oracle.com>,
minchan@kernel.org
Subject: Re: [PATCH 0/3] staging: zcache+ramster: move to new code base and re-merge
Date: Fri, 17 Aug 2012 17:32:31 -0500 [thread overview]
Message-ID: <502EC67F.4070603@linux.vnet.ibm.com> (raw)
In-Reply-To: <20120816230817.GA14757@kroah.com>
On 08/16/2012 06:08 PM, Greg KH wrote:
> On a larger note, I _really_ don't want a set of 'delete and then add it
> back' set of patches. That destroys all of the work that people had
> done up until now on the code base.
>
> I understand your need, and want, to start fresh, but you still need to
> abide with the "evolve over time" model here. Surely there is some path
> from the old to the new codebase that you can find?
I very much agree that this is the wrong way to do this.
I can't possibly inspect the code changes in this format, so
I'll just comment on some high level changes and mention
some performance results.
I like frontswap reclaiming memory from cleancache. I think
that would work better than having the pages go back to the
kernel-wide page pool using the shrinker interface.
That being said, I can't test the impact of this alone
because all these changes are being submitted together.
I also like the sysfs->debugfs cleanup and zbud being moved
into its own file.
I do _not_ support replacing zsmalloc with zbud:
https://lkml.org/lkml/2012/8/14/347
I do not support the integration of ramster mixed in with
all the rest of these changes. I have no way to see or
measure the impact of the ramster code.
I ran my kernel building benchmark twice on an unmodified
v3.5 kernel with zcache and then with these changes. On
none-low memory pressure, <16 threads, they worked roughly
the same with low swap volume. However, in mid-high
pressure, >20 threads, these changes degraded zcache runtime
and I/O savings by 30-80%.
I would suspect the low-density storage of zbud as the
culprit; however I can't confirm this because, again, it all
one huge change.
Some smaller issues:
1. This patchset breaks the build when CONFIG_SWAP in not
set. FRONTSWAP depends on SWAP, but ZCACHE _selects_
FRONTSWAP. If ZCACHE is selected and FRONTSWAP can't be
selected because SWAP isn't selected, then there is a break.
2. I get about 8 unsued/uninit'ed variable warnings at
compile time.
So I can't support this patchset, citing the performance
degradation and the fact that this submission is
unreviewable due to it being one huge monolithic patchset on
top of an existing codebase.
Seth
--
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:[~2012-08-17 22:32 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-16 22:31 [PATCH 0/3] staging: zcache+ramster: move to new code base and re-merge Dan Magenheimer
2012-08-16 22:31 ` [PATCH 1/3] Remove old zcache and ramster staging drivers to prep for new code base Dan Magenheimer
2012-08-16 22:31 ` [PATCH 2/3] Move to new zcache codebase Dan Magenheimer
2012-08-16 22:31 ` [PATCH 3/3] Move to new codebase for ramster, re-merged with " Dan Magenheimer
2012-08-16 22:48 ` [PATCH 0/3] staging: zcache+ramster: move to new code base and re-merge Greg KH
2012-08-16 22:53 ` Dan Magenheimer
2012-08-16 23:08 ` Greg KH
2012-08-17 0:19 ` Dan Magenheimer
2012-08-17 22:32 ` Seth Jennings [this message]
2012-08-18 19:10 ` Dan Magenheimer
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=502EC67F.4070603@linux.vnet.ibm.com \
--to=sjenning@linux.vnet.ibm.com \
--cc=dan.magenheimer@oracle.com \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@linuxfoundation.org \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=ngupta@vflare.org \
/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).