Date: Wed, 7 Nov 2007 10:57:18 -0800 (PST) From: Arne "Wörner" <arne_woerner@yahoo.com> To: Ulf Lilleengen <lulf@stud.ntnu.no> Cc: John Nielsen <john@jnielsen.net>, freebsd-current@freebsd.org, "fluffles.net" <bsd@fluffles.net> Subject: Re: geom_raid5 inclusion in HEAD? Message-ID: <18792.71957.qm@web30311.mail.mud.yahoo.com> In-Reply-To: <20071107170941.GA21274@stud.ntnu.no>
next in thread | previous in thread | raw e-mail | index | archive | help
--- Ulf Lilleengen <lulf@stud.ntnu.no> wrote: > - Many style(9) issues. > Hmm... Would "cb" help? R some function too long? I tried to comply to Pawel's style, but obviously I deviated from it after some weeks... :) Could give me an example of the worst issue? > - Lack of documentation. There are many small comments, but there is little > description on top of functions describing their purpose and what they do. > This makes it hard to get into it for reviewers and other developers. > Hmm... Yup... There r interface functions to the GEOM system: ..._start(), ..._done(), ..._create(), and so on... Then there r 2 worker threads (one for the graid5 start queue (...worker()) and one for the graid5 done queue (...workerD()). The other functions r helper functions... I could add the function-purpose-comment in PP and then try to merge it to TNG and TOS... > - As to the code logic itself I was a bit sceptic about having the malloc > saving > queue. Does it really improve performance that much? It's just the sort of > thing that could easily lead to bugs. > Hmm... if I understood correctly, FreeBSD's kernel memory suffers under fragmentation, if many big mem areas r needed... There might be even a dead lock, if UFS uses 64kb block size... So I thought it would be a good idea to avoid those sleeps but "hamster-ing" the big chunks... :) But I am not sure anymore, that it improved performance (but performance was the reason for it)... > - I also wonder a bit why you use two worker threads, as this also increases > complexity (but again, does it improve performance to the point that it's > worth it?). > Hmm... I think so... At least on MP boxes, since both threads do some XOR-ing (worker() uses XOR for writing "full-stripes" (where no read is necessary) and bcopy; and workerD() uses XOR after the old data/parity has been read)... > And last but not least: All of this have to be reviewed before going into the > tree, and there are not many people who can do that right now. However, I > really like your work and would gladly help improving it. > OK... review sounds good... maybe we should concentrate on PP then (it is quite space (in comparison to TNG but not TOS)+time (in comparison to TOS; maybe in comparison to TNG, too?) efficient and has a read cache)? Although fluffles favors TNG, although it is quite nasty (a write request of size 4KB costs 3 full stripes ((<number of disks>-1)*<stripe size>) plus 2*128KB... *giggle*)... -Arne __________________________________________________ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?18792.71957.qm>