[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Improving RAIDframe Parity Handling: The Diff
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)))))
Main Index |
Thread Index |