On Wed, 5 Sep 2007, Moinak Ghosh wrote: > Updated webrevs are put up at: > > http://www.genunix.org/distributions/belenix_site//binfiles/hsfswebrev/index.htm
I've got a few comments. hsfs_vnops.c: the two avl comparison helpers hsched_deadline_compare() and hsched_offset_compare() are declared as static but not defined as static. Needs to be changed. Also both the functions resort to checking 'hio' pointers if timestamp/blkno is equal - I don't think this accomplishes anything and should be removed. hsfs_vnops.c: line 1933: this allows for an fsid bigger than the maxsize of an int (which is what is passed in). probably reduce the size to 23 instead and add a comment noting the reasoning behind it. hsfs_vnops.c: hsched_init() - the cmn_errs don't convey any useful information here. Perhaps emit a single error message indicating quering ldi for maxtransfer failed, resorting to 16k default. hsfs_vnops.c: hsched_enqueue_io() - make it static hsfs_vnops.c: It doesn't look like hsfs keeps any kstats around, it would have been useful to keep stats around for things like readahead and coalesced reads; or, even deadline scheduling. Does it make any sense to add static dtrace probes to sections of code that do read coalescing and deadline reads? Alok
