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

Garrett D'Amore garrett at damore.org
Tue Jul 19 14:14:06 PDT 2011


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