dtrace discuss - Mar 2007 - What is lockstat_active_threads() guarding?

I''ve been reading through lockstat.c, trying to understand how all
the pieces fit together.

	Inside the lockstat_disable() function, the final work before returning
is this:

> 	/*
> 	 * The delay() here isn''t as cheesy as you might think.  We
don''t
> 	 * want to busy-loop in the kernel, so we have to give up the
> 	 * CPU between calls to lockstat_active_threads(); that much is
> 	 * obvious.  But the reason it''s a do..while loop rather than a
> 	 * while loop is subtle.  The memory barrier above guarantees that
> 	 * no threads will enter the lockstat code from this point forward.
> 	 * However, another thread could already be executing lockstat code
> 	 * without our knowledge if the update to its t_lockstat field
hasn''t
> 	 * cleared its CPU''s store buffer.  Delaying for one clock tick
> 	 * guarantees that either (1) the thread will have *ample* time to
> 	 * complete its work, or (2) the thread will be preempted, in which
> 	 * case it will have to grab and release a dispatcher lock, which
> 	 * will flush that CPU''s store buffer.  Either way we''re
covered.
> 	 */
> 	do {
> 		delay(1);
> 	} while (lockstat_active_threads());
>
	I follow how this works to prevent the method from returning before
all threads have exited the lockstat code.

	I''m not following why it matters :-).

	It seems like all teardown that is going to be done has already  
occurred
at the point this waits for lockstat_active_threads() == FALSE.  
Nothing else
is cleared or set after the last thread exits lockstat code.

	What goes wrong if the lockstat_disable() function returns without  
waiting?

	James M

On 3/27/07, James McIlree <jmcilree at apple.com>
wrote:
>
>         I''ve been reading through lockstat.c, trying to understand
how all
> the pieces fit together.
>
>         Inside the lockstat_disable() function, the final work before
returning
> is this:
>
>
> >       /*
> >        * The delay() here isn''t as cheesy as you might think.
We don''t
> >        * want to busy-loop in the kernel, so we have to give up the
> >        * CPU between calls to lockstat_active_threads(); that much is
> >        * obvious.  But the reason it''s a do..while loop
rather than a
> >        * while loop is subtle.  The memory barrier above guarantees
that
> >        * no threads will enter the lockstat code from this point
forward.
> >        * However, another thread could already be executing lockstat
code
> >        * without our knowledge if the update to its t_lockstat field
hasn''t
> >        * cleared its CPU''s store buffer.  Delaying for one
clock tick
> >        * guarantees that either (1) the thread will have *ample* time
to
> >        * complete its work, or (2) the thread will be preempted, in
which
> >        * case it will have to grab and release a dispatcher lock,
which
> >        * will flush that CPU''s store buffer.  Either way
we''re covered.
> >        */
> >       do {
> >               delay(1);
> >       } while (lockstat_active_threads());
> >
>
>         I follow how this works to prevent the method from returning before
> all threads have exited the lockstat code.
>
>         I''m not following why it matters :-).
>
>         It seems like all teardown that is going to be done has already
> occurred
> at the point this waits for lockstat_active_threads() == FALSE.
> Nothing else
> is cleared or set after the last thread exits lockstat code.
>
>         What goes wrong if the lockstat_disable() function returns without
> waiting?
The module can then be unloaded, unmapping the kernel text some other
thread is currently executing.  *boom*

Cheers,
- jonathan