Date: Thu, 27 Dec 2007 18:17:28 -0500 From: John Baldwin <jhb@FreeBSD.org> To: freebsd-arch@FreeBSD.org Cc: Poul-Henning Kamp <phk@phk.freebsd.dk> Subject: Re: New "timeout" api, to replace callout Message-ID: <200712271817.28789.jhb@freebsd.org> In-Reply-To: <15391.1196547545@critter.freebsd.dk> References: <15391.1196547545@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday 01 December 2007 05:19:05 pm Poul-Henning Kamp wrote:
>
> Here is my proposed new timeout API for 8.x.
>
> The primary objective is to make it possible to have multiple timeout
> "providers" of possibly different kind, so that we can have per-cpu
> or per-net-stack timeout handing.
>
> A secondary goal, is to shove the anti-race handling in destruction of
> timeouts back into the implemenation, rather than force users to spend
> 20+ lines doing that.
I don't see this anymore. Perhaps you haven't looked at updated drivers
recently? Right now it looks like this:
foo_attach/create()
{
mtx_init(&foo->lock, ...);
callout_init_mtx(&foo->callout, &foo->lock);
}
foo_something()
{
callout_reset(&foo->callout, foo_timer, ...)
}
/* Called with lock held */
foo_timer()
{
/*
* Doesn't have to check 'is detaching' or any other such crap
* anymore.
*/
}
foo_stop()
{
FOO_LOCK();
callout_stop(&foo->callout);
/* foo_timer() will no longer run after this point. */
FOO_UNLOCK();
}
foo_detach/destroy()
{
foo_stop();
/*
* This drain ensures softclock() is done frobbing with our mutex
* so we can safely destroy it. Also makes sure it has no references
* to our callout structure either.
*/
callout_drain(&foo->callout);
mtx_destroy(&foo->lock);
}
That's not 20 lines. You have to do the reset/stop anyway and those now work
intuitively. The only "extra" code is an init routine (which you will need
anyway) and a teardown routine (callout_drain()). From what I can tell,
you've basically mandated a lock and when you use callout_init_mtx() (or now
callout_init_rw()), callout_stop() == timeout_safe() and callout_drain() ==
timeout_cleanup().
Thus, as far as the MPSAFEty stuff, I think the timeout changes are just
reshuffling deck chairs. The other goals (axeing hz) I agree with, but I
don't think you've changed anything as far as MPSAFEty is concerned. Also,
I'd probably find timeout_stop() more intuitive than timeout_safe() to be
honest. Maybe timeout_disarm()?
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200712271817.28789.jhb>