On Friday, November 14, 2014 04:26:26 AM Adrian Chadd wrote:
> Author: adrian
> Date: Fri Nov 14 04:26:26 2014
> New Revision: 274493
> URL: https://svnweb.freebsd.org/changeset/base/274493
>
> Log:
> Migrate the callouts from using mutex locks to being mpsafe with
> the locks being held by the callers.
>
> Kill callout_drain() and use callout_stop().
>
> Modified:
> head/sys/dev/ath/if_ath.c
>
> Modified: head/sys/dev/ath/if_ath.c
> ============================================================================
> == --- head/sys/dev/ath/if_ath.c Fri Nov 14 00:25:10 2014
> (r274492)
> +++ head/sys/dev/ath/if_ath.c Fri Nov 14 04:26:26 2014 (r274493)
> @@ -669,8 +669,8 @@ ath_attach(u_int16_t devid, struct ath_s
> goto bad;
> }
>
> - callout_init_mtx(&sc->sc_cal_ch, &sc->sc_mtx, 0);
> - callout_init_mtx(&sc->sc_wd_ch, &sc->sc_mtx, 0);
> + callout_init(&sc->sc_cal_ch, 1); /* MPSAFE */
> + callout_init(&sc->sc_wd_ch, 1); /* MPSAFE */
Why did you do this? You probably don't want this. You can use
callout_init_mtx() with callout_stop(), and if you do it closes races
between your callout routine and callout_stop(). If you use callout_init()
instead, then you need to patch your callout routines to now explicitly
check your own private flag after doing ATH_LOCK(). That is, you can
either do:
foo_attach()
{
callout_init_mtx(&sc->timer, &sc->mtx, 0);
}
foo_timeout(void *arg)
{
sc = arg;
mtx_assert(&sc->mtx, MA_OWNED);
/* periodic actions */
callout_schedule() /* or callout_reset */
}
foo_start()
{
mtx_lock(&sc->mtx);
callout_reset(&sc->timer, ...);
}
foo_stop()
{
mtx_lock(&sc->mtx);
callout_stop(&sc->timer);
/* foo_timeout will not execute after this point */
mtx_unlock(&sc->mtx);
}
or you can do this:
foo_attach()
{
callout_init(&sc->timer, CALLOUT_MPSAFE);
}
foo_timeout(void *arg)
{
sc = arg;
mtx_lock(&sc->mtx);
if (sc->stopped) {
mtx_unlock(&sc->mtx);
return;
}
/* periodic actions */
callout_schedule()
mtx_unlock(&sc->mtx);
}
foo_start()
{
mtx_lock(&sc->mtx);
foo->stopped = 0;
callout_reset(&sc->timer, ...);
}
foo_stop()
{
mtx_lock(&sc->mtx);
foo->stopped = 1;
callout_stop(&sc->timer);
/*
* foo_timeout might still execute, but it will see the 'stopped'
* flag and return
*/
mtx_unlock(&sc->mtx);
}
Personally, I find the former (using callout_init_mtx()) a lot simpler to read
and work with. As it is you can now either switch back to using
callout_init_mtx() and undo the changes to ath_calibrate() (but leave all your
other changes in place), or you now need to add a new stopped flag that you
set / clear in the places you do callout_stop() / callout_reset() and add a
new check for the new stopped flag in ath_calibrate().
--
John Baldwin
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
