linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad@kernel.org>
To: Seth Jennings <sjenning@linux.vnet.ibm.com>
Cc: Dan Magenheimer <dan.magenheimer@oracle.com>,
	Konrad Wilk <konrad.wilk@oracle.com>,
	Mel Gorman <mgorman@suse.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nitin Gupta <ngupta@vflare.org>, Minchan Kim <minchan@kernel.org>,
	Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>,
	Robert Jennings <rcj@linux.vnet.ibm.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	devel@driverdev.osuosl.org,
	James Bottomley <James.Bottomley@HansenPartnership.com>
Subject: Re: [RFC] mm: add support for zsmalloc and zcache
Date: Fri, 2 Nov 2012 12:14:47 -0400	[thread overview]
Message-ID: <20121102161444.GB4633@konrad-lan.dumpdata.com> (raw)
In-Reply-To: <508B046A.6050006@linux.vnet.ibm.com>

On Fri, Oct 26, 2012 at 04:45:14PM -0500, Seth Jennings wrote:
> On 10/02/2012 01:17 PM, Dan Magenheimer wrote:
> > If so, <shake hands> and move forward?  What do you see as next steps?
> 
> I've been reviewing the changes between zcache and zcache2 and getting
> a feel for the scope and direction of those changes.
> 
> - Getting the community engaged to review zcache1 at ~2300SLOC was
>   difficult.
> - Adding RAMSter has meant adding RAMSter-specific code broadly across
>   zcache and increases the size of code to review to ~7600SLOC.

One can ignore the drivers/staging/ramster/ramster* directory.

> - The changes have blurred zcache's internal layering and increased
>   complexity beyond what a simple SLOC metric can reflect.

Not sure I see a problem.
> - Getting the community engaged in reviewing zcache2 will be difficult
>   and will require an exceptional amount of effort for maintainer and
>   reviewer.

Exceptional? I think if we start trimming the code down and moving it
around - and moving the 'ramster' specific calls to header files to
not be compiled - that should make it easier to read.

I mean the goal of any review is to address all of the concern you saw
when you were looking over the code. You probably have a page of
questions you asked yourself - and in all likehood the other reviewers
would ask the same questions. So if you address them - either by
giving comments or making the code easier to read - that would do it.

> 
> It is difficult for me to know when it could be ready for mainline and
> production use.  While zcache2 isn't getting broad code reviews yet,
> how do suggest managing that complexity to make the code maintainable
> and get it reviewed?

There are Mel's feedback that is also applicable to zcache2.

Thanks for looking at the code!
> 
> 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>
> 

--
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-11-02 16:14 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04 21:34 Seth Jennings
2012-09-21 16:12 ` Mel Gorman
2012-09-21 18:02   ` Konrad Rzeszutek Wilk
2012-09-21 19:02     ` Seth Jennings
2012-09-21 20:35       ` Dan Magenheimer
2012-09-22  1:07         ` Mel Gorman
2012-09-22 21:18           ` Dan Magenheimer
2012-09-24 10:31             ` Mel Gorman
2012-09-24 20:36               ` Dan Magenheimer
2012-09-25 10:20                 ` Mel Gorman
2012-09-23  7:34           ` James Bottomley
2012-09-24 20:05             ` Dan Magenheimer
2012-09-24 17:25         ` Seth Jennings
2012-09-24 19:17           ` Dan Magenheimer
2012-09-27 20:25             ` Seth Jennings
2012-09-27 22:07               ` Dan Magenheimer
2012-10-02 18:02                 ` Seth Jennings
2012-10-02 18:17                   ` Dan Magenheimer
2012-10-04 14:36                     ` Seth Jennings
2012-10-26 21:45                     ` Seth Jennings
2012-11-02 16:14                       ` Konrad Rzeszutek Wilk [this message]
2012-09-25 10:33           ` James Bottomley
2012-09-21 19:14   ` Dan Magenheimer
2012-09-22  0:25     ` Mel Gorman
2012-09-22 13:31     ` Sasha Levin
2012-09-22 13:38       ` Sasha Levin
2012-09-25 19:22         ` Dan Magenheimer
2012-09-21 19:16   ` Seth Jennings
     [not found] <<1346794486-12107-1-git-send-email-sjenning@linux.vnet.ibm.com>
2012-09-06 20:37 ` Dan Magenheimer
2012-09-07 14:37   ` Konrad Rzeszutek Wilk
2012-09-09  3:46     ` Nitin Gupta
2012-09-17 20:42       ` Dan Magenheimer
2012-09-17 23:46         ` Nitin Gupta
2012-09-07 17:31   ` Seth Jennings

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=20121102161444.GB4633@konrad-lan.dumpdata.com \
    --to=konrad@kernel.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.magenheimer@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=ngupta@vflare.org \
    --cc=rcj@linux.vnet.ibm.com \
    --cc=sjenning@linux.vnet.ibm.com \
    --cc=xiaoguangrong@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