Re: [PATCH 0/4] promote zcache from staging

[Konrad Rzeszutek Wilk]

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@xxxxxxxxxx]
> > > 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@xxxxxxxxxxxxxxxxxx]
> > > > > 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@xxxxxxxxxx  For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
> 
> -- 
> Kind regards,
> Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/