[illumos-Developer] Final (?) review for light-weight task dispatch

Garrett D'Amore garrett at damore.org
Mon Jul 25 16:19:56 PDT 2011


Thanks!  Once Adam confirms my reply, I'll go ahead and file an RTI.

	- Garrett

On Mon, 2011-07-25 at 14:58 -0700, George Wilson wrote:
> I've gone through the latest changes and they seems fine.
> 
> - George
> 
> On 7/19/11 2:14 PM, Garrett D'Amore wrote:
> > The review is here:
> > http://mexico.purplecow.org/gdamore/webrev/taskq-redux/
> >
> > This is the fixes for
> >
> > 734 taskq_dispatch_prealloc() desired
> > 943 zio_interrupt ends up calling taskq_dispatch with TQ_SLEEP
> >
> >
> > The main difference with earlier reviews is that I've considered
> > George's feedback, and have implemented "userland" equivalent support
> > taskq_dispatch_ent().  (Sorry it took so long -- I was hitting an
> > assertion in zfstest, which I thought was an indication that my
> > assumptions about zio scheduling were wrong.  Turns out that no, it was
> > just an annoying use-after-free bug in my userland implementation of
> > taskq_dispatch_ent.)
> >
> > So now this code has also been tested with ztest.  Passes beautifully.
> > And it has fewer #ifdef KERNEL hacks than the prior version.  Yay.
> >
> > I did not try to change the API to support dynamic allocation of
> > taskq_ent_t's; I view that as a potential future enhancement, and
> > probably something that certainly ought to be done as part of any future
> > work to raise this to a public API.  I also kept the TQENT_FLAG_PREALLOC
> > name (it turns out the supposed collision was with TASKQ_PREPOPULATE),
> > and I left it in the union.  Mostly because I did not want to have to
> > redo the performance tests -- which I would have if I had increased the
> > size of the taskq_ent_t structure. (There has been significant
> > performance testing done on this code, already.)
> >
> > Anyway, I'm looking forward to feedback... especially George, if you can
> > review the new changes, that would be awesome.  I'd like a very short
> > timer on this if possible, because I have another change that I'd like
> > to integrate which depends on these changes.  And trust me, you *want*
> > that fix if you use COMSTAR. :-)
> >
> > 	- Garrett
> >
> 





More information about the Developer mailing list