On Wed, 12 Sep 2007, Moinak Ghosh wrote: > I will fix the static definition. > This code was lifted from ZFS, vdev_queue.c where they have > that pointer comparison. I think the pointer comparison is there > to provide a protection in the case there are two requests that > affect the same block. Trying to enqueue a duplicate request > in the avl tree results in a panic. Pointer comparison avoids > that problem since the memory allocated to the two duplicate > bufs will be different. > > In my hsfs changes, this also avoids a corner case problem with > the sentinel. Say one round of reads is over and we have a > sentinel sitting in the tree. At some later time another read for > the same block comes down, because the page has expired from > the cache. Doing a pointer comparison will avoid an avl_tree > panic.
Okay, thanks for the explanation. >> 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. >> > > Sometime you may want to know why something failed and > a generic error message does not convey any helpful debug > information. That is why I put different cmn_errs that print > different messages and error codes for different failures. > But I think I can make the error messages a little more > verbose. These error messages are great for someone dealing with this code for debug purposes. For a casual user, I don't see how this provides any value. >> 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? >> > > Yes these will be added. This was the feedback from Frank as > well and it appears that private debugging/observability kstats > and SDT probes do not require an ARC review. Great. Alok
