On Tue, May 5, 2009 at 6:04 PM, Juergen Keil <jrgn.keil at googlemail.com> wrote: > 2009/5/4 Alok Aggarwal <Alok.Aggarwal at sun.com>: >> I am sending this code review request on behalf >> of Moinak Ghosh (CC'd). Please review the changes >> for - >> >> 6802997 Clofi memory allocation behavior can be improved >> >> Webrev location - >> >> http://cr.opensolaris.org/~aalok/sponsor-6802997/ >> >> Moinak's testing has showed that these changes result >> in a performance improvement during installation. > > In lofi.c, lines 1123 .. 1133: > > 1123 ? ? ? ? ? ? ? ? mutex_enter(&lsp->ls_comp_bufs_lock); > 1124 ? ? ? ? ? ? ? ? for (i = 0; i < lofi_taskq_nthreads; i++) { > 1125 ? ? ? ? ? ? ? ? ? ? ? ? if (lsp->ls_comp_bufs[i].inuse == 0) { > 1126 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? lsp->ls_comp_bufs[i].inuse = 1; > 1127 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? j = i; > 1128 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break; > 1129 ? ? ? ? ? ? ? ? ? ? ? ? } > 1130 ? ? ? ? ? ? ? ? } > 1131 > 1132 ? ? ? ? ? ? ? ? mutex_exit(&lsp->ls_comp_bufs_lock); > 1133 ? ? ? ? ? ? ? ? ASSERT(j < lofi_taskq_nthreads); > > I think this needs to handle the case when all > buffers are in use. ?With the current code we > exit from the loop with 'j' uninitialized, and could > panic a debug kernel with the > ASSERT(j < lofi_taskq_nthreads) >
The number of buffers == number of lofi threads and each thread uses exactly one buffer at any given point of time. So this scenario will not occur. Regards, Moinak. -- ================================ http://www.belenix.org/ http://moinakg.wordpress.com/
