Re: [PATCH 2/9] xfs: add support for large btree blocks

[Dave Chinner <david@fromorbit.com>]

On Fri, Feb 22, 2013 at 12:34:56PM +1100, Dave Chinner wrote:
> On Fri, Feb 15, 2013 at 03:20:15PM -0600, Ben Myers wrote:
> > On Tue, Jan 22, 2013 at 12:25:53AM +1100, Dave Chinner wrote:
> > > From: Christoph Hellwig <hch@lst.de>
> > > 
> > > Add support for larger btree blocks that contains a CRC32C checksum,
> > > a filesystem uuid and block number for detecting filesystem
> > > consistency and out of place writes.
> > > 
> > > [dchinner@redhat.com] Also include an owner field to allow reverse
> > > mappings to be implemented for improved repairability and a LSN
> > > field to so that log recovery can easily determine the last
> > > modification that made it to disk for each buffer.
> > > 
> > > [dchinner@redhat.com] Add buffer log format flags to indicate the
> > > type of buffer to recovery so that we don't have to do blind magic
> > > number tests to determine what the buffer is.
> > > 
> > > [dchinner@redhat.com] Modified to fit into the verifier structure.
> > 
> > This patch is far too large for a good review.  It needs to be split up into
> > it's various ideas which you outlined in patch 0.  If you need to add dead code
> > in each piece and then enable it at the end, that's fine with me.
> 
> You want it broken up into 7 or 8 separate patches - what does it
> gain us? It'll take a week for me to do the patch monkeying and to
> retest and validate the resulting stack (i.e. it is bisectable, each
> patch passes xfstests, etc), and in the end the code will be
> identical.
> 
> As I've said before, there's a point where the tradeoff for
> splitting patches up doesn't make sense. Asking a developer to do
> days of work to end up with exactly the same code to save the
> reviewer an hour or two is *not* a good tradeoff. Especially for the
> first patch of a much larger 15-20 patch series which contains
> several larger and more complex patches....

Ben, reading this back it comes across as unnecessarily negative and
narky. I was just about at the end of the attribute leaf changes and
it's been a mind-numbing slog making mechanical changes to lots of
code.

This is after having to do the same slog through all the directory
code. The block, leaf, node, freespace and da_btree code all had to
get the same treatment, and it's worn me down.  My wrists are
starting to hurt from the last three months of slogging through this
stuff (roughly +20,000/-10,000 lines modified according to a quick
diffstat) and that doesn't make me a happy camper.

So I'm not meaning to be narky or nasty - I just want to get this
stuff done ASAP before it burns me out. Hence the prospect of having
to go back and redo significant chunks to split out trivial pieces
of code (i.e. a mind-numbing slog making mechanical changes) hit a
bit of a raw nerve.

All I'm asking is that you take into account the extra load that the
rework you ask me to do adds and whether it is absolutely necessary
to be able to review the code. The last thing I want is be burnt out
by this stuff....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs