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

[George Wilson]

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
>