git: 5fc3104ae738 - main - callout: retire callout_async_drain()
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 24 Jan 2024 17:34:57 UTC
The branch main has been updated by glebius:
URL: https://cgit.FreeBSD.org/src/commit/?id=5fc3104ae7389a0937f55af5157761c64b8a0668
commit 5fc3104ae7389a0937f55af5157761c64b8a0668
Author: Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-01-24 17:33:27 +0000
Commit: Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-01-24 17:33:27 +0000
callout: retire callout_async_drain()
This function was used only in TCP before 446ccdd08e2a. It was born in
pain in 2016 to plug different complex panics in TCP timers. It wasn't
warmly accepted in phabricator by all of the reviewers and my recollection
of overall agreement was that "if you need this KPI, then you'd better fix
your code to not need it". However, the function served its duty well all
the way to FreeBSD 14. But now that TCP doesn't need it anymore, let's
retire it to reduce complexity of callout code and also to avoid its
further use.
Reviewed by: jhb, markj, kib, rrs
Differential Revision: https://reviews.freebsd.org/D43546
---
ObsoleteFiles.inc | 3 +
share/man/man9/Makefile | 1 -
share/man/man9/callout.9 | 29 +---
sys/kern/kern_timeout.c | 37 +----
sys/modules/tests/Makefile | 1 -
sys/modules/tests/callout_test/Makefile | 14 --
sys/sys/callout.h | 8 +-
sys/tests/callout_test.h | 32 ----
sys/tests/callout_test/callout_test.c | 282 --------------------------------
9 files changed, 12 insertions(+), 395 deletions(-)
diff --git a/ObsoleteFiles.inc b/ObsoleteFiles.inc
index a6c15187e847..2ba4e487ad0b 100644
--- a/ObsoleteFiles.inc
+++ b/ObsoleteFiles.inc
@@ -51,6 +51,9 @@
# xargs -n1 | sort | uniq -d;
# done
+# 20240122: callout_async_drain() removed
+OLD_FILES+=usr/share/man/man9/callout_async_drain.9.gz
+
# 20240114: Removal of geom_map(4)
OLD_FILES+=usr/share/man/man4/geom_map.4
diff --git a/share/man/man9/Makefile b/share/man/man9/Makefile
index 3f9572796fa1..c98849b42a7b 100644
--- a/share/man/man9/Makefile
+++ b/share/man/man9/Makefile
@@ -810,7 +810,6 @@ MLINKS+=byteorder.9 be16dec.9 \
byteorder.9 le64enc.9 \
byteorder.9 le64toh.9
MLINKS+=callout.9 callout_active.9 \
- callout.9 callout_async_drain.9 \
callout.9 callout_deactivate.9 \
callout.9 callout_drain.9 \
callout.9 callout_init.9 \
diff --git a/share/man/man9/callout.9 b/share/man/man9/callout.9
index 96b93283bfc2..0e59ef8ab2b1 100644
--- a/share/man/man9/callout.9
+++ b/share/man/man9/callout.9
@@ -27,13 +27,12 @@
.\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
.\" POSSIBILITY OF SUCH DAMAGE.
.\"
-.Dd September 1, 2021
+.Dd January 22, 2024
.Dt CALLOUT 9
.Os
.Sh NAME
.Nm callout_active ,
.Nm callout_deactivate ,
-.Nm callout_async_drain ,
.Nm callout_drain ,
.Nm callout_init ,
.Nm callout_init_mtx ,
@@ -66,8 +65,6 @@ typedef void callout_func_t (void *);
.Ft void
.Fn callout_deactivate "struct callout *c"
.Ft int
-.Fn callout_async_drain "struct callout *c" "callout_func_t *drain"
-.Ft int
.Fn callout_drain "struct callout *c"
.Ft void
.Fn callout_init "struct callout *c" "int mpsafe"
@@ -293,30 +290,6 @@ If the callout has an associated lock,
then that lock must be held when this function is called.
.Pp
The function
-.Fn callout_async_drain
-is identical to
-.Fn callout_stop
-with one difference.
-When
-.Fn callout_async_drain
-returns zero it will arrange for the function
-.Fa drain
-to be called using the same argument given to the
-.Fn callout_reset
-function.
-.Fn callout_async_drain
-If the callout has an associated lock,
-then that lock must be held when this function is called.
-Note that when stopping multiple callouts that use the same lock it is possible
-to get multiple return's of zero and multiple calls to the
-.Fa drain
-function, depending upon which CPU's the callouts are running.
-The
-.Fa drain
-function itself is called from the context of the completing callout
-i.e. softclock or hardclock, just like a callout itself.
-.Pp
-The function
.Fn callout_drain
is identical to
.Fn callout_stop
diff --git a/sys/kern/kern_timeout.c b/sys/kern/kern_timeout.c
index dbe7ec0a4a76..5f2d78285c66 100644
--- a/sys/kern/kern_timeout.c
+++ b/sys/kern/kern_timeout.c
@@ -142,7 +142,6 @@ static u_int __read_mostly callwheelmask;
*/
struct cc_exec {
struct callout *cc_curr;
- callout_func_t *cc_drain;
void *cc_last_func;
void *cc_last_arg;
#ifdef SMP
@@ -180,7 +179,6 @@ struct callout_cpu {
#define cc_exec_curr(cc, dir) cc->cc_exec_entity[dir].cc_curr
#define cc_exec_last_func(cc, dir) cc->cc_exec_entity[dir].cc_last_func
#define cc_exec_last_arg(cc, dir) cc->cc_exec_entity[dir].cc_last_arg
-#define cc_exec_drain(cc, dir) cc->cc_exec_entity[dir].cc_drain
#define cc_exec_next(cc) cc->cc_next
#define cc_exec_cancel(cc, dir) cc->cc_exec_entity[dir].cc_cancel
#define cc_exec_waiting(cc, dir) cc->cc_exec_entity[dir].cc_waiting
@@ -631,7 +629,7 @@ softclock_call_cc(struct callout *c, struct callout_cpu *cc,
int direct)
{
struct rm_priotracker tracker;
- callout_func_t *c_func, *drain;
+ callout_func_t *c_func;
void *c_arg;
struct lock_class *class;
struct lock_object *c_lock;
@@ -673,7 +671,6 @@ softclock_call_cc(struct callout *c, struct callout_cpu *cc,
cc_exec_last_func(cc, direct) = c_func;
cc_exec_last_arg(cc, direct) = c_arg;
cc_exec_cancel(cc, direct) = false;
- cc_exec_drain(cc, direct) = NULL;
CC_UNLOCK(cc);
if (c_lock != NULL) {
class->lc_lock(c_lock, lock_status);
@@ -739,13 +736,6 @@ skip:
CC_LOCK(cc);
KASSERT(cc_exec_curr(cc, direct) == c, ("mishandled cc_curr"));
cc_exec_curr(cc, direct) = NULL;
- if (cc_exec_drain(cc, direct)) {
- drain = cc_exec_drain(cc, direct);
- cc_exec_drain(cc, direct) = NULL;
- CC_UNLOCK(cc);
- drain(c_arg);
- CC_LOCK(cc);
- }
if (cc_exec_waiting(cc, direct)) {
/*
* There is someone waiting for the
@@ -969,7 +959,7 @@ callout_reset_sbt_on(struct callout *c, sbintime_t sbt, sbintime_t prec,
*/
if (c->c_lock != NULL && !cc_exec_cancel(cc, direct))
cancelled = cc_exec_cancel(cc, direct) = true;
- if (cc_exec_waiting(cc, direct) || cc_exec_drain(cc, direct)) {
+ if (cc_exec_waiting(cc, direct)) {
/*
* Someone has called callout_drain to kill this
* callout. Don't reschedule.
@@ -1080,7 +1070,7 @@ callout_schedule(struct callout *c, int to_ticks)
}
int
-_callout_stop_safe(struct callout *c, int flags, callout_func_t *drain)
+_callout_stop_safe(struct callout *c, int flags)
{
struct callout_cpu *cc, *old_cc;
struct lock_class *class;
@@ -1091,9 +1081,6 @@ _callout_stop_safe(struct callout *c, int flags, callout_func_t *drain)
WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, c->c_lock,
"calling %s", __func__);
- KASSERT((flags & CS_DRAIN) == 0 || drain == NULL,
- ("Cannot set drain callback and CS_DRAIN flag at the same time"));
-
/*
* Some old subsystems don't hold Giant while running a callout_stop(),
* so just discard this check for the moment.
@@ -1228,8 +1215,7 @@ again:
goto again;
}
c->c_flags &= ~CALLOUT_ACTIVE;
- } else if (use_lock &&
- !cc_exec_cancel(cc, direct) && (drain == NULL)) {
+ } else if (use_lock && !cc_exec_cancel(cc, direct)) {
/*
* The current callout is waiting for its
@@ -1237,8 +1223,7 @@ again:
* and return. After our caller drops the
* lock, the callout will be skipped in
* softclock(). This *only* works with a
- * callout_stop() *not* callout_drain() or
- * callout_async_drain().
+ * callout_stop() *not* with callout_drain().
*/
cc_exec_cancel(cc, direct) = true;
CTR3(KTR_CALLOUT, "cancelled %p func %p arg %p",
@@ -1284,23 +1269,11 @@ again:
#endif
CTR3(KTR_CALLOUT, "postponing stop %p func %p arg %p",
c, c->c_func, c->c_arg);
- if (drain) {
- KASSERT(cc_exec_drain(cc, direct) == NULL,
- ("callout drain function already set to %p",
- cc_exec_drain(cc, direct)));
- cc_exec_drain(cc, direct) = drain;
- }
CC_UNLOCK(cc);
return (0);
} else {
CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p",
c, c->c_func, c->c_arg);
- if (drain) {
- KASSERT(cc_exec_drain(cc, direct) == NULL,
- ("callout drain function already set to %p",
- cc_exec_drain(cc, direct)));
- cc_exec_drain(cc, direct) = drain;
- }
}
KASSERT(!sq_locked, ("sleepqueue chain still locked"));
cancelled = 0;
diff --git a/sys/modules/tests/Makefile b/sys/modules/tests/Makefile
index abc077ffa2df..7481a0a3a4b4 100644
--- a/sys/modules/tests/Makefile
+++ b/sys/modules/tests/Makefile
@@ -1,7 +1,6 @@
SUBDIR+= framework
SUBDIR+= .WAIT
-SUBDIR+= callout_test
SUBDIR_PARALLEL=
diff --git a/sys/modules/tests/callout_test/Makefile b/sys/modules/tests/callout_test/Makefile
deleted file mode 100644
index 7cab464b8fc4..000000000000
--- a/sys/modules/tests/callout_test/Makefile
+++ /dev/null
@@ -1,14 +0,0 @@
-#
-#
-
-.PATH: ${SRCTOP}/sys/tests/callout_test
-
-KMOD= callout_test
-SRCS= callout_test.c
-
-#
-# Enable full debugging
-#
-#CFLAGS += -g
-
-.include <bsd.kmod.mk>
diff --git a/sys/sys/callout.h b/sys/sys/callout.h
index 2e2b835fcc43..f2a5bf92ab46 100644
--- a/sys/sys/callout.h
+++ b/sys/sys/callout.h
@@ -81,7 +81,7 @@
*/
#define callout_active(c) ((c)->c_flags & CALLOUT_ACTIVE)
#define callout_deactivate(c) ((c)->c_flags &= ~CALLOUT_ACTIVE)
-#define callout_drain(c) _callout_stop_safe(c, CS_DRAIN, NULL)
+#define callout_drain(c) _callout_stop_safe(c, CS_DRAIN)
void callout_init(struct callout *, int);
void _callout_init_lock(struct callout *, struct lock_object *, int);
#define callout_init_mtx(c, mtx, flags) \
@@ -119,11 +119,9 @@ int callout_schedule(struct callout *, int);
int callout_schedule_on(struct callout *, int, int);
#define callout_schedule_curcpu(c, on_tick) \
callout_schedule_on((c), (on_tick), PCPU_GET(cpuid))
-#define callout_stop(c) _callout_stop_safe(c, 0, NULL)
-int _callout_stop_safe(struct callout *, int, void (*)(void *));
+#define callout_stop(c) _callout_stop_safe(c, 0)
+int _callout_stop_safe(struct callout *, int);
void callout_process(sbintime_t now);
-#define callout_async_drain(c, d) \
- _callout_stop_safe(c, 0, d)
void callout_when(sbintime_t sbt, sbintime_t precision, int flags,
sbintime_t *sbt_res, sbintime_t *prec_res);
#endif
diff --git a/sys/tests/callout_test.h b/sys/tests/callout_test.h
deleted file mode 100644
index 60a02f0c7b56..000000000000
--- a/sys/tests/callout_test.h
+++ /dev/null
@@ -1,32 +0,0 @@
-#ifndef __callout_test_h__
-#define __callout_test_h__
-/*-
- * Copyright (c) 2015 Netflix, Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- */
-struct callout_test {
- int number_of_callouts;
- int test_number;
-};
-#endif
diff --git a/sys/tests/callout_test/callout_test.c b/sys/tests/callout_test/callout_test.c
deleted file mode 100644
index 059d6a728fb6..000000000000
--- a/sys/tests/callout_test/callout_test.c
+++ /dev/null
@@ -1,282 +0,0 @@
-/*-
- * Copyright (c) 2015 Netflix, Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <sys/param.h>
-#include <sys/systm.h>
-#include <sys/conf.h>
-#include <sys/cpuctl.h>
-#include <sys/fcntl.h>
-#include <sys/ioccom.h>
-#include <sys/kernel.h>
-#include <sys/libkern.h>
-#include <sys/malloc.h>
-#include <sys/module.h>
-#include <sys/mutex.h>
-#include <sys/pcpu.h>
-#include <sys/pmckern.h>
-#include <sys/priv.h>
-#include <sys/proc.h>
-#include <sys/queue.h>
-#include <sys/sched.h>
-#include <sys/smp.h>
-#include <sys/sysctl.h>
-#include <sys/uio.h>
-#include <tests/kern_testfrwk.h>
-#include <tests/callout_test.h>
-#include <machine/cpu.h>
-
-MALLOC_DEFINE(M_CALLTMP, "Temp callout Memory", "CalloutTest");
-
-struct callout_run {
- struct mtx lock;
- struct callout *co_array;
- int co_test;
- int co_number_callouts;
- int co_return_npa;
- int co_completed;
- int callout_waiting;
- int drain_calls;
- int cnt_zero;
- int cnt_one;
- int index;
-};
-
-static struct callout_run *comaster[MAXCPU];
-
-uint64_t callout_total = 0;
-
-static void execute_the_co_test(struct callout_run *rn);
-
-static void
-co_saydone(void *arg)
-{
- struct callout_run *rn;
-
- rn = (struct callout_run *)arg;
- printf("The callout test is now complete for thread %d\n",
- rn->index);
- printf("number_callouts:%d\n",
- rn->co_number_callouts);
- printf("Callouts that bailed (Not PENDING or ACTIVE cleared):%d\n",
- rn->co_return_npa);
- printf("Callouts that completed:%d\n", rn->co_completed);
- printf("Drain calls:%d\n", rn->drain_calls);
- printf("Zero returns:%d non-zero:%d\n",
- rn->cnt_zero,
- rn->cnt_one);
-
-}
-
-static void
-drainit(void *arg)
-{
- struct callout_run *rn;
-
- rn = (struct callout_run *)arg;
- mtx_lock(&rn->lock);
- rn->drain_calls++;
- mtx_unlock(&rn->lock);
-}
-
-static void
-test_callout(void *arg)
-{
- struct callout_run *rn;
- int cpu;
-
- critical_enter();
- cpu = curcpu;
- critical_exit();
- rn = (struct callout_run *)arg;
- atomic_add_int(&rn->callout_waiting, 1);
- mtx_lock(&rn->lock);
- if (callout_pending(&rn->co_array[cpu]) ||
- !callout_active(&rn->co_array[cpu])) {
- rn->co_return_npa++;
- atomic_subtract_int(&rn->callout_waiting, 1);
- mtx_unlock(&rn->lock);
- return;
- }
- callout_deactivate(&rn->co_array[cpu]);
- rn->co_completed++;
- mtx_unlock(&rn->lock);
- atomic_subtract_int(&rn->callout_waiting, 1);
-}
-
-void
-execute_the_co_test(struct callout_run *rn)
-{
- int i, ret, cpu;
- uint32_t tk_s, tk_e, tk_d;
-
- mtx_lock(&rn->lock);
- rn->callout_waiting = 0;
- for (i = 0; i < rn->co_number_callouts; i++) {
- if (rn->co_test == 1) {
- /* start all on spread out cpu's */
- cpu = i % mp_ncpus;
- callout_reset_sbt_on(&rn->co_array[i], 3, 0, test_callout, rn,
- cpu, 0);
- } else {
- /* Start all on the same CPU */
- callout_reset_sbt_on(&rn->co_array[i], 3, 0, test_callout, rn,
- rn->index, 0);
- }
- }
- tk_s = ticks;
- while (rn->callout_waiting != rn->co_number_callouts) {
- cpu_spinwait();
- tk_e = ticks;
- tk_d = tk_e - tk_s;
- if (tk_d > 100) {
- break;
- }
- }
- /* OK everyone is waiting and we have the lock */
- for (i = 0; i < rn->co_number_callouts; i++) {
- ret = callout_async_drain(&rn->co_array[i], drainit);
- if (ret) {
- rn->cnt_one++;
- } else {
- rn->cnt_zero++;
- }
- }
- rn->callout_waiting -= rn->cnt_one;
- mtx_unlock(&rn->lock);
- /* Now wait until all are done */
- tk_s = ticks;
- while (rn->callout_waiting > 0) {
- cpu_spinwait();
- tk_e = ticks;
- tk_d = tk_e - tk_s;
- if (tk_d > 100) {
- break;
- }
- }
- co_saydone((void *)rn);
-}
-
-
-static void
-run_callout_test(struct kern_test *test)
-{
- struct callout_test *u;
- size_t sz;
- int i;
- struct callout_run *rn;
- int index = test->tot_threads_running;
-
- u = (struct callout_test *)test->test_options;
- if (comaster[index] == NULL) {
- rn = comaster[index] = malloc(sizeof(struct callout_run), M_CALLTMP, M_WAITOK);
- memset(comaster[index], 0, sizeof(struct callout_run));
- mtx_init(&rn->lock, "callouttest", NULL, MTX_DUPOK);
- rn->index = index;
- } else {
- rn = comaster[index];
- rn->co_number_callouts = rn->co_return_npa = 0;
- rn->co_completed = rn->callout_waiting = 0;
- rn->drain_calls = rn->cnt_zero = rn->cnt_one = 0;
- if (rn->co_array) {
- free(rn->co_array, M_CALLTMP);
- rn->co_array = NULL;
- }
- }
- rn->co_number_callouts = u->number_of_callouts;
- rn->co_test = u->test_number;
- sz = sizeof(struct callout) * rn->co_number_callouts;
- rn->co_array = malloc(sz, M_CALLTMP, M_WAITOK);
- for (i = 0; i < rn->co_number_callouts; i++) {
- callout_init(&rn->co_array[i], CALLOUT_MPSAFE);
- }
- execute_the_co_test(rn);
-}
-
-int callout_test_is_loaded = 0;
-
-static void
-cocleanup(void)
-{
- int i;
-
- for (i = 0; i < MAXCPU; i++) {
- if (comaster[i]) {
- if (comaster[i]->co_array) {
- free(comaster[i]->co_array, M_CALLTMP);
- comaster[i]->co_array = NULL;
- }
- free(comaster[i], M_CALLTMP);
- comaster[i] = NULL;
- }
- }
-}
-
-static int
-callout_test_modevent(module_t mod, int type, void *data)
-{
- int err = 0;
-
- switch (type) {
- case MOD_LOAD:
- err = kern_testframework_register("callout_test",
- run_callout_test);
- if (err) {
- printf("Can't load callout_test err:%d returned\n",
- err);
- } else {
- memset(comaster, 0, sizeof(comaster));
- callout_test_is_loaded = 1;
- }
- break;
- case MOD_QUIESCE:
- err = kern_testframework_deregister("callout_test");
- if (err == 0) {
- callout_test_is_loaded = 0;
- cocleanup();
- }
- break;
- case MOD_UNLOAD:
- if (callout_test_is_loaded) {
- err = kern_testframework_deregister("callout_test");
- if (err == 0) {
- cocleanup();
- callout_test_is_loaded = 0;
- }
- }
- break;
- default:
- return (EOPNOTSUPP);
- }
- return (err);
-}
-
-static moduledata_t callout_test_mod = {
- .name = "callout_test",
- .evhand = callout_test_modevent,
- .priv = 0
-};
-
-MODULE_DEPEND(callout_test, kern_testframework, 1, 1, 1);
-DECLARE_MODULE(callout_test, callout_test_mod, SI_SUB_PSEUDO, SI_ORDER_ANY);