Re: Improving RAIDframe Parity Handling: The Diff

[Jed Davis <jld%panix.com@localhost>]

Matthias Scheler <tron%zhadum.org.uk@localhost> writes:

> 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.

I wondered that as well.  Note that all the existing label fields are
the same way (and, in particular, the int array that I took my fields
out of).  Do we have any platforms with non-32-bit "int" that would
break from changing one to the other?

> 2.) rf_paritymap_test() should return a "bool".

I didn't realize at the time that we had a "bool"; that can be done (and
there are probably some variables that can be altered likewise).

> 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.

I think it could; at the time I might not have known I wasn't going to
wind up putting more fields under it, or something along those lines,
but at this point I think the change can be made.

-- 
(let ((C call-with-current-continuation)) (apply (lambda (x y) (x y)) (map
((lambda (r) ((C C) (lambda (s) (r (lambda l (apply (s s) l))))))  (lambda
(f) (lambda (l) (if (null? l) C (lambda (k) (display (car l)) ((f (cdr l))
(C k)))))))    '((#\J #\d #\D #\v #\s) (#\e #\space #\a #\i #\newline)))))