[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