Date: Fri, 5 Jul 2002 02:38:08 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Jonathan Lemon <jlemon@flugsvamp.com> Cc: bsddiy@yahoo.com, <current@freebsd.org> Subject: Re: Timeout and SMP race Message-ID: <20020705022155.L475-100000@gamplex.bde.org> In-Reply-To: <200207041530.g64FUwE48481@prism.flugsvamp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 4 Jul 2002, Jonathan Lemon wrote:
> In article <local.mail.freebsd-current/20020704132044.7338.qmail@web20909.mail.yahoo.com> you write:
> >in RELENG_4, when one calls callout_stop() (not nested in softclock execute
> >path
> >, I am not talking about this case), after it returns, he can believe that the
> >callout is truely stopped, however in CURRENT, this assumption is false, now we
> >
> >must care if callout_stop() truely stopped the callout when it returned, this
> >is all difference I see, we bring in this race which not exists in RELENG_4,
> >see what hacking code put in kern_condvar.c and kern_synch.c in CURRENT source,
>
> I don't believe this is true. There is a corresponding race in -stable,
> where the spl() level is dropped before performing the callout, thus
> allowing either a call to callout_stop() or callout_reset() to come in
> immediately before the callout is actually made.
I think Giant locking everything prevents problems in RELENG_4, at least
when callout_stop() is called in process context (if it is called in
interrupt context then it could easily be interrupting a callout even in
the UP case).
The race window extends from when the ipl or lock is dropped across the
whole callout until the ipl or lock is regained. (The ipl is only dropped
to splstatclock(); this prevents interruption by timeouts but not by
other interrupts. In -current there is nothing much to prevent softclock()
itself being called concurrently, but in theory softclock()'s internal
locking should prevent problems.)
> The callout function is responsible for checking to see if it has lost
> the race, and perform the appropriate action. This is done with the
> CALLOUT_PENDING and CALLOUT_ACTIVE flags:
>
> s = splnet();
> if (callout_pending(tp->tt_delack) || !callout_active(tp->tt_delack)) {
> splx(s);
> return;
> }
> callout_deactivate(tp->tt_delack);
I think David is objecting to this complicating all callers that do the
check and breaking all that don't. The callers in kern_synch.c and
kern_condvar.c have an mi_switch() and other complications to handle
this sinc they can't just return.
> If 'CALLOUT_PENDING' is set, then we lost a race with callout_reset(),
> and should not perform the callout. If 'CALLOUT_ACTIVE' is clear, then
> we lost a race with callout_stop().
>
> Either way, on both -current and -stable, you cannot assume that the
> timer callback is completely gone immediately after calling callout_stop().
tsleep() seems to assume this in RELENG_4.
[Some context lost to top posting.]
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020705022155.L475-100000>