Add a mechanism for associating a mutex with a callout when the
callout is first initialised, using a new function callout_init_mtx(). The callout system will acquire this mutex before calling the callout function and release it on return. In addition, the callout system uses the mutex to avoid most of the complications and race conditions inherent in asynchronous timer facilities, so mutex-protected callouts have much simpler semantics. As long as the mutex is held when invoking callout_stop() or callout_reset(), then these functions will guarantee that the callout will be stopped, even if softclock() had already begun to process the callout. Existing Giant-locked callouts will automatically pick up the new race-free semantics. This should close a number of race conditions in the USB code and probably other areas of the kernel too. There should be no change in behaviour for "MP-safe" callouts; these still need to use the techniques mentioned in timeout(9) to avoid race conditions.
This commit is contained in:
Ian Dowse
parent
b542a023e5
commit
98c926b20f
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=141428
@ -919,9 +919,14 @@ MLINKS+=taskqueue.9 TASK_INIT.9 \
|
||||
MLINKS+=time.9 boottime.9 \
|
||||
time.9 time_second.9 \
|
||||
time.9 time_uptime.9
|
||||
MLINKS+=timeout.9 callout_drain.9 \
|
||||
MLINKS+=timeout.9 callout.9 \
|
||||
timeout.9 callout_active.9 \
|
||||
timeout.9 callout_deactivate.9 \
|
||||
timeout.9 callout_drain.9 \
|
||||
timeout.9 callout_handle_init.9 \
|
||||
timeout.9 callout_init.9 \
|
||||
timeout.9 callout_init_mtx.9 \
|
||||
timeout.9 callout_pending.9 \
|
||||
timeout.9 callout_reset.9 \
|
||||
timeout.9 callout_stop.9 \
|
||||
timeout.9 untimeout.9
|
||||
|
||||
@ -36,7 +36,7 @@
|
||||
.\"
|
||||
.\" $FreeBSD$
|
||||
.\"
|
||||
.Dd January 23, 2005
|
||||
.Dd February 6, 2005
|
||||
.Dt TIMEOUT 9
|
||||
.Os
|
||||
.Sh NAME
|
||||
@ -44,6 +44,7 @@
|
||||
.Nm untimeout ,
|
||||
.Nm callout_handle_init ,
|
||||
.Nm callout_init ,
|
||||
.Nm callout_init_mtx ,
|
||||
.Nm callout_stop ,
|
||||
.Nm callout_drain ,
|
||||
.Nm callout_reset ,
|
||||
@ -70,6 +71,8 @@ struct callout_handle handle = CALLOUT_HANDLE_INITIALIZER(&handle)
|
||||
.Fn untimeout "timeout_t *func" "void *arg" "struct callout_handle handle"
|
||||
.Ft void
|
||||
.Fn callout_init "struct callout *c" "int mpsafe"
|
||||
.Ft void
|
||||
.Fn callout_init_mtx "struct callout *c" "struct mtx *mtx" "int flags"
|
||||
.Ft int
|
||||
.Fn callout_stop "struct callout *c"
|
||||
.Ft int
|
||||
@ -178,6 +181,7 @@ Thus they are protected from re-entrancy.
|
||||
.Pp
|
||||
The functions
|
||||
.Fn callout_init ,
|
||||
.Fn callout_init_mtx ,
|
||||
.Fn callout_stop ,
|
||||
.Fn callout_drain
|
||||
and
|
||||
@ -202,6 +206,26 @@ that is,
|
||||
the Giant lock will be acquired before calling the callout function,
|
||||
and released when the callout function returns.
|
||||
.Pp
|
||||
The
|
||||
.Fn callout_init_mtx
|
||||
function may be used as an alternative to
|
||||
.Fn callout_init .
|
||||
The parameter
|
||||
.Fa mtx
|
||||
specifies a mutex that is to be acquired by the callout subsystem
|
||||
before calling the callout function, and released when the callout
|
||||
function returns.
|
||||
The following
|
||||
.Fa flags
|
||||
may be specified:
|
||||
.Bl -tag -width CALLOUT_RETURNUNLOCKED
|
||||
.It Dv CALLOUT_RETURNUNLOCKED
|
||||
The callout function will release
|
||||
.Fa mtx
|
||||
itself, so the callout subsystem should not attempt to unlock it
|
||||
after the callout function returns.
|
||||
.El
|
||||
.Pp
|
||||
The function
|
||||
.Fn callout_stop
|
||||
cancels a callout if it is currently pending.
|
||||
@ -210,6 +234,8 @@ If the callout is pending, then
|
||||
will return a non-zero value.
|
||||
If the callout is not set, has already been serviced or is currently
|
||||
being serviced, then zero will be returned.
|
||||
If the callout has an associated mutex, then that mutex must be
|
||||
held when this function is called.
|
||||
.Pp
|
||||
The function
|
||||
.Fn callout_drain
|
||||
@ -234,6 +260,8 @@ first performs the equivalent of
|
||||
to disestablish the callout, and then establishes a new callout in the
|
||||
same manner as
|
||||
.Fn timeout .
|
||||
If the callout has an associated mutex, then that mutex must be
|
||||
held when this function is called.
|
||||
.Pp
|
||||
The macros
|
||||
.Fn callout_pending ,
|
||||
@ -295,6 +323,27 @@ The callout subsystem provides a number of mechanisms to address these
|
||||
synchronization concerns:
|
||||
.Bl -enum -offset indent
|
||||
.It
|
||||
If the callout has an associated mutex that was specified using the
|
||||
.Fn callout_init_mtx
|
||||
function (or implicitly specified as the
|
||||
.Va Giant
|
||||
mutex using
|
||||
.Fn callout_init
|
||||
with
|
||||
.Fa mpsafe
|
||||
set to
|
||||
.Dv FALSE ) ,
|
||||
then this mutex is used to avoid the race conditions.
|
||||
The associated mutex must be acquired by the caller before calling
|
||||
.Fn callout_stop
|
||||
or
|
||||
.Fn callout_reset
|
||||
and it is guaranteed that the callout will be correctly stopped
|
||||
or reset as expected.
|
||||
Note that it is still necessary to use
|
||||
.Fn callout_drain
|
||||
before destroying the callout or its associated mutex.
|
||||
.It
|
||||
The return value from
|
||||
.Fn callout_stop
|
||||
indicates whether or not the callout was removed.
|
||||
|
||||
@ -53,6 +53,9 @@ SYSCTL_INT(_debug, OID_AUTO, to_avg_depth, CTLFLAG_RD, &avg_depth, 0,
|
||||
static int avg_gcalls;
|
||||
SYSCTL_INT(_debug, OID_AUTO, to_avg_gcalls, CTLFLAG_RD, &avg_gcalls, 0,
|
||||
"Average number of Giant callouts made per softclock call. Units = 1/1000");
|
||||
static int avg_mtxcalls;
|
||||
SYSCTL_INT(_debug, OID_AUTO, to_avg_mtxcalls, CTLFLAG_RD, &avg_mtxcalls, 0,
|
||||
"Average number of mtx callouts made per softclock call. Units = 1/1000");
|
||||
static int avg_mpcalls;
|
||||
SYSCTL_INT(_debug, OID_AUTO, to_avg_mpcalls, CTLFLAG_RD, &avg_mpcalls, 0,
|
||||
"Average number of MP callouts made per softclock call. Units = 1/1000");
|
||||
@ -80,6 +83,12 @@ static struct callout *nextsoftcheck; /* Next callout to be checked. */
|
||||
* If curr_callout is non-NULL, threads waiting on
|
||||
* callout_wait will be woken up as soon as the
|
||||
* relevant callout completes.
|
||||
* curr_cancelled - Changing to 1 with both callout_lock and c_mtx held
|
||||
* guarantees that the current callout will not run.
|
||||
* The softclock() function sets this to 0 before it
|
||||
* drops callout_lock to acquire c_mtx, and it calls
|
||||
* the handler only if curr_cancelled still 0 when
|
||||
* c_mtx is successfully acquired.
|
||||
* wakeup_ctr - Incremented every time a thread wants to wait
|
||||
* for a callout to complete. Modified only when
|
||||
* curr_callout is non-NULL.
|
||||
@ -88,6 +97,7 @@ static struct callout *nextsoftcheck; /* Next callout to be checked. */
|
||||
* cutt_callout is non-NULL.
|
||||
*/
|
||||
static struct callout *curr_callout;
|
||||
static int curr_cancelled;
|
||||
static int wakeup_ctr;
|
||||
static int wakeup_needed;
|
||||
|
||||
@ -181,6 +191,7 @@ softclock(void *dummy)
|
||||
int steps; /* #steps since we last allowed interrupts */
|
||||
int depth;
|
||||
int mpcalls;
|
||||
int mtxcalls;
|
||||
int gcalls;
|
||||
int wakeup_cookie;
|
||||
#ifdef DIAGNOSTIC
|
||||
@ -195,6 +206,7 @@ softclock(void *dummy)
|
||||
#endif /* MAX_SOFTCLOCK_STEPS */
|
||||
|
||||
mpcalls = 0;
|
||||
mtxcalls = 0;
|
||||
gcalls = 0;
|
||||
depth = 0;
|
||||
steps = 0;
|
||||
@ -225,12 +237,14 @@ softclock(void *dummy)
|
||||
} else {
|
||||
void (*c_func)(void *);
|
||||
void *c_arg;
|
||||
struct mtx *c_mtx;
|
||||
int c_flags;
|
||||
|
||||
nextsoftcheck = TAILQ_NEXT(c, c_links.tqe);
|
||||
TAILQ_REMOVE(bucket, c, c_links.tqe);
|
||||
c_func = c->c_func;
|
||||
c_arg = c->c_arg;
|
||||
c_mtx = c->c_mtx;
|
||||
c_flags = c->c_flags;
|
||||
if (c->c_flags & CALLOUT_LOCAL_ALLOC) {
|
||||
c->c_func = NULL;
|
||||
@ -242,11 +256,32 @@ softclock(void *dummy)
|
||||
(c->c_flags & ~CALLOUT_PENDING);
|
||||
}
|
||||
curr_callout = c;
|
||||
curr_cancelled = 0;
|
||||
mtx_unlock_spin(&callout_lock);
|
||||
if (!(c_flags & CALLOUT_MPSAFE)) {
|
||||
mtx_lock(&Giant);
|
||||
gcalls++;
|
||||
CTR1(KTR_CALLOUT, "callout %p", c_func);
|
||||
if (c_mtx != NULL) {
|
||||
mtx_lock(c_mtx);
|
||||
/*
|
||||
* The callout may have been cancelled
|
||||
* while we switched locks.
|
||||
*/
|
||||
if (curr_cancelled) {
|
||||
mtx_unlock(c_mtx);
|
||||
mtx_lock_spin(&callout_lock);
|
||||
goto done_locked;
|
||||
}
|
||||
/* The callout cannot be stopped now. */
|
||||
curr_cancelled = 1;
|
||||
|
||||
if (c_mtx == &Giant) {
|
||||
gcalls++;
|
||||
CTR1(KTR_CALLOUT, "callout %p",
|
||||
c_func);
|
||||
} else {
|
||||
mtxcalls++;
|
||||
CTR1(KTR_CALLOUT,
|
||||
"callout mtx %p",
|
||||
c_func);
|
||||
}
|
||||
} else {
|
||||
mpcalls++;
|
||||
CTR1(KTR_CALLOUT, "callout mpsafe %p",
|
||||
@ -275,9 +310,10 @@ softclock(void *dummy)
|
||||
lastfunc = c_func;
|
||||
}
|
||||
#endif
|
||||
if (!(c_flags & CALLOUT_MPSAFE))
|
||||
mtx_unlock(&Giant);
|
||||
if ((c_flags & CALLOUT_RETURNUNLOCKED) == 0)
|
||||
mtx_unlock(c_mtx);
|
||||
mtx_lock_spin(&callout_lock);
|
||||
done_locked:
|
||||
curr_callout = NULL;
|
||||
if (wakeup_needed) {
|
||||
/*
|
||||
@ -300,6 +336,7 @@ softclock(void *dummy)
|
||||
}
|
||||
avg_depth += (depth * 1000 - avg_depth) >> 8;
|
||||
avg_mpcalls += (mpcalls * 1000 - avg_mpcalls) >> 8;
|
||||
avg_mtxcalls += (mtxcalls * 1000 - avg_mtxcalls) >> 8;
|
||||
avg_gcalls += (gcalls * 1000 - avg_gcalls) >> 8;
|
||||
nextsoftcheck = NULL;
|
||||
mtx_unlock_spin(&callout_lock);
|
||||
@ -397,15 +434,28 @@ callout_reset(c, to_ticks, ftn, arg)
|
||||
void *arg;
|
||||
{
|
||||
|
||||
#ifdef notyet /* Some callers of timeout() do not hold Giant. */
|
||||
if (c->c_mtx != NULL)
|
||||
mtx_assert(c->c_mtx, MA_OWNED);
|
||||
#endif
|
||||
|
||||
mtx_lock_spin(&callout_lock);
|
||||
if (c == curr_callout && wakeup_needed) {
|
||||
if (c == curr_callout) {
|
||||
/*
|
||||
* We're being asked to reschedule a callout which is
|
||||
* currently in progress, and someone has called
|
||||
* callout_drain to kill that callout. Don't reschedule.
|
||||
* currently in progress. If there is a mutex then we
|
||||
* can cancel the callout if it has not really started.
|
||||
*/
|
||||
mtx_unlock_spin(&callout_lock);
|
||||
return;
|
||||
if (c->c_mtx != NULL && !curr_cancelled)
|
||||
curr_cancelled = 1;
|
||||
if (wakeup_needed) {
|
||||
/*
|
||||
* Someone has called callout_drain to kill this
|
||||
* callout. Don't reschedule.
|
||||
*/
|
||||
mtx_unlock_spin(&callout_lock);
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (c->c_flags & CALLOUT_PENDING) {
|
||||
if (nextsoftcheck == c) {
|
||||
@ -446,7 +496,18 @@ _callout_stop_safe(c, safe)
|
||||
struct callout *c;
|
||||
int safe;
|
||||
{
|
||||
int wakeup_cookie;
|
||||
int use_mtx, wakeup_cookie;
|
||||
|
||||
if (!safe && c->c_mtx != NULL) {
|
||||
#ifdef notyet /* Some callers do not hold Giant for Giant-locked callouts. */
|
||||
mtx_assert(c->c_mtx, MA_OWNED);
|
||||
use_mtx = 1;
|
||||
#else
|
||||
use_mtx = mtx_owned(c->c_mtx);
|
||||
#endif
|
||||
} else {
|
||||
use_mtx = 0;
|
||||
}
|
||||
|
||||
mtx_lock_spin(&callout_lock);
|
||||
/*
|
||||
@ -454,7 +515,11 @@ _callout_stop_safe(c, safe)
|
||||
*/
|
||||
if (!(c->c_flags & CALLOUT_PENDING)) {
|
||||
c->c_flags &= ~CALLOUT_ACTIVE;
|
||||
if (c == curr_callout && safe) {
|
||||
if (c != curr_callout) {
|
||||
mtx_unlock_spin(&callout_lock);
|
||||
return (0);
|
||||
}
|
||||
if (safe) {
|
||||
/* We need to wait until the callout is finished. */
|
||||
wakeup_needed = 1;
|
||||
wakeup_cookie = wakeup_ctr++;
|
||||
@ -470,6 +535,11 @@ _callout_stop_safe(c, safe)
|
||||
cv_wait(&callout_wait, &callout_wait_lock);
|
||||
|
||||
mtx_unlock(&callout_wait_lock);
|
||||
} else if (use_mtx && !curr_cancelled) {
|
||||
/* We can stop the callout before it runs. */
|
||||
curr_cancelled = 1;
|
||||
mtx_unlock_spin(&callout_lock);
|
||||
return (1);
|
||||
} else
|
||||
mtx_unlock_spin(&callout_lock);
|
||||
return (0);
|
||||
@ -495,8 +565,29 @@ callout_init(c, mpsafe)
|
||||
int mpsafe;
|
||||
{
|
||||
bzero(c, sizeof *c);
|
||||
if (mpsafe)
|
||||
c->c_flags |= CALLOUT_MPSAFE;
|
||||
if (mpsafe) {
|
||||
c->c_mtx = NULL;
|
||||
c->c_flags = CALLOUT_RETURNUNLOCKED;
|
||||
} else {
|
||||
c->c_mtx = &Giant;
|
||||
c->c_flags = 0;
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
callout_init_mtx(c, mtx, flags)
|
||||
struct callout *c;
|
||||
struct mtx *mtx;
|
||||
int flags;
|
||||
{
|
||||
bzero(c, sizeof *c);
|
||||
c->c_mtx = mtx;
|
||||
KASSERT((flags & ~CALLOUT_RETURNUNLOCKED) == 0,
|
||||
("callout_init_mtx: bad flags %d", flags));
|
||||
/* CALLOUT_RETURNUNLOCKED makes no sense without a mutex. */
|
||||
KASSERT(mtx != NULL || (flags & CALLOUT_RETURNUNLOCKED) == 0,
|
||||
("callout_init_mtx: CALLOUT_RETURNUNLOCKED with no mutex"));
|
||||
c->c_flags = flags & CALLOUT_RETURNUNLOCKED;
|
||||
}
|
||||
|
||||
#ifdef APM_FIXUP_CALLTODO
|
||||
|
||||
@ -40,6 +40,8 @@
|
||||
|
||||
#include <sys/queue.h>
|
||||
|
||||
struct mtx;
|
||||
|
||||
SLIST_HEAD(callout_list, callout);
|
||||
TAILQ_HEAD(callout_tailq, callout);
|
||||
|
||||
@ -51,6 +53,7 @@ struct callout {
|
||||
int c_time; /* ticks to the event */
|
||||
void *c_arg; /* function argument */
|
||||
void (*c_func)(void *); /* function to call */
|
||||
struct mtx *c_mtx; /* mutex to lock */
|
||||
int c_flags; /* state of this entry */
|
||||
};
|
||||
|
||||
@ -58,6 +61,7 @@ struct callout {
|
||||
#define CALLOUT_ACTIVE 0x0002 /* callout is currently active */
|
||||
#define CALLOUT_PENDING 0x0004 /* callout is waiting for timeout */
|
||||
#define CALLOUT_MPSAFE 0x0008 /* callout handler is mp safe */
|
||||
#define CALLOUT_RETURNUNLOCKED 0x0010 /* handler returns with mtx unlocked */
|
||||
|
||||
struct callout_handle {
|
||||
struct callout *callout;
|
||||
@ -75,6 +79,7 @@ extern struct mtx callout_lock;
|
||||
#define callout_deactivate(c) ((c)->c_flags &= ~CALLOUT_ACTIVE)
|
||||
#define callout_drain(c) _callout_stop_safe(c, 1)
|
||||
void callout_init(struct callout *, int);
|
||||
void callout_init_mtx(struct callout *, struct mtx *, int);
|
||||
#define callout_pending(c) ((c)->c_flags & CALLOUT_PENDING)
|
||||
void callout_reset(struct callout *, int, void (*)(void *), void *);
|
||||
#define callout_stop(c) _callout_stop_safe(c, 0)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user