[illumos-Developer] missing lwp_exit() in kcfpool_svc()

[Albert Lee]

On Sun, Apr 3, 2011 at 12:57 AM, Garrett D'Amore <garrett at nexenta.com> wrote:
> I've added this comment (but not updated the webrev):
>
>                         /*
>                          * lwp_exit() assumes it is called
>                          * with the proc lock held.  But the
>                          * first thing it does is drop it.
>                          * This ensures that lwp does not
>                          * exit before lwp_create is done
>                          * with it.
>                          */
>                          mutex_enter(&curproc->p_lock);
>                          lwp_exit();   /* does not return */
>
>
> Hopefully that is what you were looking for.
>
>        - Garrett
>


The fix looks correct to me. However, I'm concerned about the workers'
accesses to of kcfpool->kp_threads. While chunks of kcfpool_svc() are
synchronised between workers with gswq->gs_lock, kcfpoold's use of
kp_threads and kp_blockedthreads is only guarded by kp_lock so it is
possible to race with workers. Inconsistent conditions like
kp_blockedthreads > kp_threads can be visible in kcfpoold, and may
cause too few or too many threads to be spawned (although the latter
case unlikely since it is limited to racing with increments).

-Albert

> On Fri, 2011-04-01 at 15:26 -0400, Dan McDonald wrote:
>> On Fri, Apr 01, 2011 at 11:51:18AM -0700, Garrett D'Amore wrote:
>> >
>> >  Webrev with the fix is at
>> > http://mexico.purplecow.org/gdamore/webrev/kcf-fix/
>> >
>>
>> It might be instructive to have a small comment about why you're acquiring
>> curproc->p_lock prior to calling lwp_exit().
>>
>> Otherwise, ship it!
>>
>> Dan
>>