Re: Improving RAIDframe Parity Handling: The Diff

[Matthias Scheler <tron%zhadum.org.uk@localhost>]

On Tue, Nov 03, 2009 at 01:35:10PM -0500, Jed Davis wrote:
> So, here are new diffs:
> 
> http://www.NetBSD.org/~jld/gsoc09-1103.diff      (-current)
> http://www.NetBSD.org/~jld/gsoc09-1103-n5.diff   (netbsd-5)

I've looked at the diffs. Here are my comments:
1.) You use "u_int" and "int" inside a structure which defines on
    the on-disk data. I wonder whether "uint32_t" and "int32_t"
    would be the better choice.
2.) rf_paritymap_test() should return a "bool".
3.) Can you please comment rf_paritymap_begin_region() and
    rf_paritymap_end_region()?
4.) Could "lk_flags" be removed if you use atomic_ops(3) to update
    the "flags" field of a parity map? Your locking looks safe
    (because you stick to the defined order). But I feel somehow
    uneasy about this.

        Kind regards

-- 
Matthias Scheler                                  http://zhadum.org.uk/