linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Dan Magenheimer <dan.magenheimer@oracle.com>,
	Konrad Rzeszutek Wilk <konrad@darnok.org>,
	Seth Jennings <sjenning@linux.vnet.ibm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nitin Gupta <ngupta@vflare.org>,
	Robert Jennings <rcj@linux.vnet.ibm.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	devel@driverdev.osuosl.org
Subject: Re: [PATCH 0/4] promote zcache from staging
Date: Tue, 31 Jul 2012 11:36:04 -0400	[thread overview]
Message-ID: <20120731153604.GO4789@phenom.dumpdata.com> (raw)
In-Reply-To: <20120729015428.GA16643@bbox>

On Sun, Jul 29, 2012 at 10:54:28AM +0900, Minchan Kim wrote:
> On Fri, Jul 27, 2012 at 02:42:14PM -0700, Dan Magenheimer wrote:
> > > From: Konrad Rzeszutek Wilk [mailto:konrad@darnok.org]
> > > Sent: Friday, July 27, 2012 3:00 PM
> > > Subject: Re: [PATCH 0/4] promote zcache from staging
> > > 
> > > On Fri, Jul 27, 2012 at 12:21:50PM -0700, Dan Magenheimer wrote:
> > > > > From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> > > > > Subject: [PATCH 0/4] promote zcache from staging
> > > > >
> > > > > zcache is the remaining piece of code required to support in-kernel
> > > > > memory compression.  The other two features, cleancache and frontswap,
> > > > > have been promoted to mainline in 3.0 and 3.5.  This patchset
> > > > > promotes zcache from the staging tree to mainline.
> > > > >
> > > > > Based on the level of activity and contributions we're seeing from a
> > > > > diverse set of people and interests, I think zcache has matured to the
> > > > > point where it makes sense to promote this out of staging.
> > > >
> > > > Hi Seth --
> > > >
> > > > Per offline communication, I'd like to see this delayed for three
> > > > reasons:
> > > >
> > > > 1) I've completely rewritten zcache and will post the rewrite soon.
> > > >    The redesigned code fixes many of the weaknesses in zcache that
> > > >    makes it (IMHO) unsuitable for an enterprise distro.  (Some of
> > > >    these previously discussed in linux-mm [1].)
> > > > 2) zcache is truly mm (memory management) code and the fact that
> > > >    it is in drivers at all was purely for logistical reasons
> > > >    (e.g. the only in-tree "staging" is in the drivers directory).
> > > >    My rewrite promotes it to (a subdirectory of) mm where IMHO it
> > > >    belongs.
> > > > 3) Ramster heavily duplicates code from zcache.  My rewrite resolves
> > > >    this.  My soon-to-be-post also places the re-factored ramster
> > > >    in mm, though with some minor work zcache could go in mm and
> > > >    ramster could stay in staging.
> > > >
> > > > Let's have this discussion, but unless the community decides
> > > > otherwise, please consider this a NACK.
> > 
> > Hi Konrad --
> >  
> > > Hold on, that is rather unfair. The zcache has been in staging
> > > for quite some time - your code has not been posted. Part of
> > > "unstaging" a driver is for folks to review the code - and you
> > > just said "No, mine is better" without showing your goods.
> > 
> > Sorry, I'm not trying to be unfair.  However, I don't see the point
> > of promoting zcache out of staging unless it is intended to be used
> > by real users in a real distro.  There's been a lot of discussion,
> > onlist and offlist, about what needs to be fixed in zcache and not
> > much visible progress on fixing it.  But fixing it is where I've spent
> > most of my time over the last couple of months.
> > 
> > If IBM or some other company or distro is eager to ship and support
> > zcache in its current form, I agree that "promote now, improve later"
> > is a fine approach.  But promoting zcache out of staging simply because
> > there is urgency to promote zsmalloc+zram out of staging doesn't
> > seem wise.  At a minimum, it distracts reviewers/effort from what IMHO
> > is required to turn zcache into an enterprise-ready kernel feature.
> > 
> > I can post my "goods" anytime.  In its current form it is better
> > than the zcache in staging (and, please remember, I wrote both so
> > I think I am in a good position to compare the two).
> > I have been waiting until I think the new zcache is feature complete
> > before asking for review, especially since the newest features
> > should demonstrate clearly why the rewrite is necessary and
> > beneficial.  But I can post* my current bits if people don't
> > believe they exist and/or don't mind reviewing non-final code.
> > (* Or I can put them in a publicly available git tree.)
> > 
> > > There is a third option - which is to continue the promotion
> > > of zcache from staging, get reviews, work on them ,etc, and
> > > alongside of that you can work on fixing up (or ripping out)
> > > zcache1 with zcache2 components as they make sense. Or even
> > > having two of them - an enterprise and an embedded version
> > > that will eventually get merged together. There is nothing
> > > wrong with modifying a driver once it has left staging.
> > 
> > Minchan and Seth can correct me if I am wrong, but I believe
> > zram+zsmalloc, not zcache, is the target solution for embedded.
> 
> NOT ture. Some embedded devices use zcache but it's not original
> zcache but modificated one.

What kind of modifications? Would it make sense to post the patches
for those modifications?

> Anyway, although embedded people use modified zcache, I am biased to Dan.
> I admit I don't spend lots of time to look zcache but as looking the
> code, it wasn't good shape and even had a bug found during code review
> and I felt strongly we should clean up it for promoting it to mm/.

Do you recall what the bugs where?

> So I would like to wait Dan's posting if you guys are not urgent.
> (And I am not sure akpm allow it with current shape of zcache code.)
> But the concern is about adding new feature. I guess there might be some
> debate for long time and it can prevent promoting again.
> I think It's not what Seth want.
> I hope Dan doesn't mix clean up series and new feature series and
> post clean up series as soon as possible so let's clean up first and
> try to promote it and later, adding new feature or changing algorithm
> is desirable.
> 
> 
> > The limitations of zsmalloc aren't an issue for zram but they are
> > for zcache, and this deficiency was one of the catalysts for the
> > rewrite.  The issues are explained in more detail in [1],
> > but if any point isn't clear, I'd be happy to explain further.
> > 
> > However, I have limited time for this right now and I'd prefer
> > to spend it finishing the code. :-}
> > 
> > So, as I said, I am still a NACK, but if there are good reasons
> > to duplicate effort and pursue the "third option", let's discuss
> > them.
> > 
> > Thanks,
> > Dan
> > 
> > [1] http://marc.info/?t=133886706700002&r=1&w=2
> > 
> > --
> > 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>

  reply	other threads:[~2012-07-31 15:46 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <<1343413117-1989-1-git-send-email-sjenning@linux.vnet.ibm.com>
2012-07-27 19:21 ` Dan Magenheimer
2012-07-27 20:59   ` Konrad Rzeszutek Wilk
2012-07-27 21:42     ` Dan Magenheimer
2012-07-29  1:54       ` Minchan Kim
2012-07-31 15:36         ` Konrad Rzeszutek Wilk [this message]
2012-08-06  4:49           ` Minchan Kim
2012-07-30 19:19       ` Seth Jennings
2012-07-30 20:48         ` Dan Magenheimer
2012-07-31 15:58           ` Konrad Rzeszutek Wilk
2012-07-31 16:19             ` Greg Kroah-Hartman
2012-07-31 17:51               ` Konrad Rzeszutek Wilk
2012-07-31 18:19                 ` Seth Jennings
2012-08-06  0:38                 ` Minchan Kim
2012-08-06 15:24                   ` Dan Magenheimer
2012-08-06 15:47                     ` Pekka Enberg
2012-08-06 16:21                       ` Dan Magenheimer
2012-08-06 16:29                         ` Greg Kroah-Hartman
2012-08-06 16:38                           ` Dan Magenheimer
2012-08-07  0:44                         ` Minchan Kim
2012-08-07 19:28                   ` Konrad Rzeszutek Wilk
2012-07-27 18:18 Seth Jennings
2012-07-27 18:18 ` [PATCH 1/4] zsmalloc: collapse internal .h into .c Seth Jennings
2012-07-27 18:18 ` [PATCH 2/4] zsmalloc: promote to mm/ Seth Jennings
2012-07-27 18:18 ` [PATCH 3/4] drivers: add memory management driver class Seth Jennings
2012-07-31 15:31   ` Konrad Rzeszutek Wilk
2012-07-27 18:18 ` [PATCH 4/4] zcache: promote to drivers/mm/ Seth Jennings
2012-07-29  2:20 ` [PATCH 0/4] promote zcache from staging Minchan Kim
2012-08-07 20:23 ` Seth Jennings
2012-08-07 21:47   ` Dan Magenheimer
2012-08-08 16:29     ` Seth Jennings
2012-08-08 17:47       ` Dan Magenheimer
2012-08-09 18:50   ` Seth Jennings
2012-08-09 20:20     ` Dan Magenheimer
2012-08-10 18:14       ` Seth Jennings
2012-08-15  9:38         ` Konrad Rzeszutek Wilk
2012-08-15 14:24           ` Seth Jennings
2012-08-17 22:21         ` Dan Magenheimer
2012-08-17 23:33           ` Seth Jennings
2012-08-18 19:09             ` Dan Magenheimer
2012-08-14 22:18 ` Seth Jennings
2012-08-14 23:29   ` Minchan Kim

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=20120731153604.GO4789@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.magenheimer@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=konrad@darnok.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=ngupta@vflare.org \
    --cc=rcj@linux.vnet.ibm.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