d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add a mechanism for associating a mutex with a callout when the

Browse Source 
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:
iedowse 2005-02-07 02:47:33 +00:00
parent b3dbb534b1
commit 8cf324095d
4 changed files with 167 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
share/man/man9/Makefile
View File

@ -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

51
share/man/man9/timeout.9
View File

@ -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.

121
sys/kern/kern_timeout.c
View File

@ -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

5
sys/sys/callout.h
View File

@ -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)