Re: [Suspend2][ 0/9] Extents support.

Greg KH
2006-06-27T07:10:05Z

On Tue, Jun 27, 2006 at 03:39:26PM +1000, Nigel Cunningham wrote:
> Hi.
>
> On Tuesday 27 June 2006 15:36, Jens Axboe wrote:
> > On Tue, Jun 27 2006, Nigel Cunningham wrote:
> > > Hi.
> > >
> > > On Tuesday 27 June 2006 07:20, Rafael J. Wysocki wrote:
> > > > On Monday 26 June 2006 18:54, Nigel Cunningham wrote:
> > > > > Add Suspend2 extent support. Extents are used for storing the lists
> > > > > of blocks to which the image will be written, and are stored in the
> > > > > image header for use at resume time.
> > > >
> > > > Could you please put all of the changes in kernel/power/extents.c into
> > > > one patch? ?It's quite difficult to review them now, at least for me.
> > >
> > > I spent a long time splitting them up because I was asked in previous
> > > iterations to break them into manageable chunks. How about if I were to
> > > email you the individual files off line, so as to not send the same
> > > amount again?
> >
> > Managable chunks means logical changes go together, one function per
> > diff is really extreme and unreviewable. Support for extents is one
> > logical change, so it's one patch. Unless of course you have to do some
> > preparatory patches, then you'd do those separately.
> >
> > I must admit I thought you were kidding when I read through this extents
> > patch series, having a single patch just adding includes!
>
> Sorry for fluffing it up. I'm pretty inexperienced, but I'm trying to follow
> CodingStyle and all the other advice. If I'd known I'd misunderstood what was
> wanted, I probably could have submitted this months ago. Oh well. Live and
> learn. What would you have me do at this point?

Please break things up into logical steps to solve the problem, and try
it again.

Oh, and as a meta-comment, why /proc? You know that's not acceptable,
right?

thanks,

greg k-h