All of lore.kernel.org
 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>

WARNING: multiple messages have this Message-ID (diff)
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>
> 

  reply	other threads:[~2012-11-02 16:14 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04 21:34 [RFC] mm: add support for zsmalloc and zcache Seth Jennings
2012-09-04 21:34 ` Seth Jennings
2012-09-21 16:12 ` Mel Gorman
2012-09-21 16:12   ` Mel Gorman
2012-09-21 18:02   ` Konrad Rzeszutek Wilk
2012-09-21 18:02     ` Konrad Rzeszutek Wilk
2012-09-21 19:02     ` Seth Jennings
2012-09-21 19:02       ` Seth Jennings
2012-09-21 20:35       ` Dan Magenheimer
2012-09-21 20:35         ` Dan Magenheimer
2012-09-22  1:07         ` Mel Gorman
2012-09-22  1:07           ` Mel Gorman
2012-09-22 21:18           ` Dan Magenheimer
2012-09-22 21:18             ` Dan Magenheimer
2012-09-24 10:31             ` Mel Gorman
2012-09-24 10:31               ` Mel Gorman
2012-09-24 20:36               ` Dan Magenheimer
2012-09-24 20:36                 ` Dan Magenheimer
2012-09-25 10:20                 ` Mel Gorman
2012-09-25 10:20                   ` Mel Gorman
2012-09-23  7:34           ` James Bottomley
2012-09-23  7:34             ` James Bottomley
2012-09-24 20:05             ` Dan Magenheimer
2012-09-24 20:05               ` Dan Magenheimer
2012-09-24 17:25         ` Seth Jennings
2012-09-24 17:25           ` Seth Jennings
2012-09-24 19:17           ` Dan Magenheimer
2012-09-24 19:17             ` Dan Magenheimer
2012-09-27 20:25             ` Seth Jennings
2012-09-27 20:25               ` Seth Jennings
2012-09-27 22:07               ` Dan Magenheimer
2012-09-27 22:07                 ` Dan Magenheimer
2012-10-02 18:02                 ` Seth Jennings
2012-10-02 18:02                   ` Seth Jennings
2012-10-02 18:17                   ` Dan Magenheimer
2012-10-02 18:17                     ` Dan Magenheimer
2012-10-04 14:36                     ` Seth Jennings
2012-10-04 14:36                       ` Seth Jennings
2012-10-26 21:45                     ` Seth Jennings
2012-10-26 21:45                       ` Seth Jennings
2012-11-02 16:14                       ` Konrad Rzeszutek Wilk [this message]
2012-11-02 16:14                         ` Konrad Rzeszutek Wilk
2012-09-25 10:33           ` James Bottomley
2012-09-25 10:33             ` James Bottomley
2012-09-21 19:14   ` Dan Magenheimer
2012-09-21 19:14     ` Dan Magenheimer
2012-09-22  0:25     ` Mel Gorman
2012-09-22  0:25       ` Mel Gorman
2012-09-22 13:31     ` Sasha Levin
2012-09-22 13:31       ` Sasha Levin
2012-09-22 13:38       ` Sasha Levin
2012-09-22 13:38         ` Sasha Levin
2012-09-25 19:22         ` Dan Magenheimer
2012-09-25 19:22           ` Dan Magenheimer
2012-09-21 19:16   ` Seth Jennings
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-06 20:37   ` Dan Magenheimer
2012-09-07 14:37   ` Konrad Rzeszutek Wilk
2012-09-07 14:37     ` Konrad Rzeszutek Wilk
2012-09-09  3:46     ` Nitin Gupta
2012-09-09  3:46       ` Nitin Gupta
2012-09-17 20:42       ` Dan Magenheimer
2012-09-17 20:42         ` Dan Magenheimer
2012-09-17 23:46         ` Nitin Gupta
2012-09-17 23:46           ` Nitin Gupta
2012-09-07 17:31   ` Seth Jennings
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.