git: 621fd9dcb2d8 - main - timecounter: Lock the timecounter list

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Mon, 18 Oct 2021 14:08:16 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=621fd9dcb2d83daab477c130bc99b905f6fc27dc

commit 621fd9dcb2d83daab477c130bc99b905f6fc27dc
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-10-16 13:46:55 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-10-18 13:56:59 +0000

    timecounter: Lock the timecounter list
    
    Timecounter registration is dynamic, i.e., there is no requirement that
    timecounters must be registered during single-threaded boot.  Loadable
    drivers may in principle register timecounters (which can be switched to
    automatically).  Timecounters cannot be unregistered, though this could
    be implemented.
    
    Registered timecounters belong to a global linked list.  Add a mutex to
    synchronize insertions and the traversals done by (mpsafe) sysctl
    handlers.  No functional change intended.
    
    Reviewed by:    imp, kib
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D32511
---
 sys/kern/kern_tc.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c
index be4142e19d77..9a928ca37aff 100644
--- a/sys/kern/kern_tc.c
+++ b/sys/kern/kern_tc.c
@@ -97,6 +97,10 @@ static struct timehands *volatile timehands = &ths[0];
 struct timecounter *timecounter = &dummy_timecounter;
 static struct timecounter *timecounters = &dummy_timecounter;
 
+/* Mutex to protect the timecounter list. */
+static struct mtx tc_lock;
+MTX_SYSINIT(tc_lock, &tc_lock, "tc", MTX_DEF);
+
 int tc_min_ticktock_freq = 1;
 
 volatile time_t time_second = 1;
@@ -1188,8 +1192,6 @@ tc_init(struct timecounter *tc)
 		    tc->tc_quality);
 	}
 
-	tc->tc_next = timecounters;
-	timecounters = tc;
 	/*
 	 * Set up sysctl tree for this counter.
 	 */
@@ -1211,6 +1213,11 @@ tc_init(struct timecounter *tc)
 	SYSCTL_ADD_INT(NULL, SYSCTL_CHILDREN(tc_root), OID_AUTO,
 	    "quality", CTLFLAG_RD, &(tc->tc_quality), 0,
 	    "goodness of time counter");
+
+	mtx_lock(&tc_lock);
+	tc->tc_next = timecounters;
+	timecounters = tc;
+
 	/*
 	 * Do not automatically switch if the current tc was specifically
 	 * chosen.  Never automatically use a timecounter with negative quality.
@@ -1218,22 +1225,24 @@ tc_init(struct timecounter *tc)
 	 * worse since this timecounter may not be monotonic.
 	 */
 	if (tc_chosen)
-		return;
+		goto unlock;
 	if (tc->tc_quality < 0)
-		return;
+		goto unlock;
 	if (tc_from_tunable[0] != '\0' &&
 	    strcmp(tc->tc_name, tc_from_tunable) == 0) {
 		tc_chosen = 1;
 		tc_from_tunable[0] = '\0';
 	} else {
 		if (tc->tc_quality < timecounter->tc_quality)
-			return;
+			goto unlock;
 		if (tc->tc_quality == timecounter->tc_quality &&
 		    tc->tc_frequency < timecounter->tc_frequency)
-			return;
+			goto unlock;
 	}
 	(void)tc->tc_get_timecount(tc);
 	timecounter = tc;
+unlock:
+	mtx_unlock(&tc_lock);
 }
 
 /* Report the frequency of the current timecounter. */
@@ -1479,16 +1488,22 @@ sysctl_kern_timecounter_hardware(SYSCTL_HANDLER_ARGS)
 	struct timecounter *newtc, *tc;
 	int error;
 
+	mtx_lock(&tc_lock);
 	tc = timecounter;
 	strlcpy(newname, tc->tc_name, sizeof(newname));
+	mtx_unlock(&tc_lock);
 
 	error = sysctl_handle_string(oidp, &newname[0], sizeof(newname), req);
 	if (error != 0 || req->newptr == NULL)
 		return (error);
+
+	mtx_lock(&tc_lock);
 	/* Record that the tc in use now was specifically chosen. */
 	tc_chosen = 1;
-	if (strcmp(newname, tc->tc_name) == 0)
+	if (strcmp(newname, tc->tc_name) == 0) {
+		mtx_unlock(&tc_lock);
 		return (0);
+	}
 	for (newtc = timecounters; newtc != NULL; newtc = newtc->tc_next) {
 		if (strcmp(newname, newtc->tc_name) != 0)
 			continue;
@@ -1506,11 +1521,11 @@ sysctl_kern_timecounter_hardware(SYSCTL_HANDLER_ARGS)
 		 * use any locking and that it can be called in hard interrupt
 		 * context via 'tc_windup()'.
 		 */
-		return (0);
+		break;
 	}
-	return (EINVAL);
+	mtx_unlock(&tc_lock);
+	return (newtc != NULL ? 0 : EINVAL);
 }
-
 SYSCTL_PROC(_kern_timecounter, OID_AUTO, hardware,
     CTLTYPE_STRING | CTLFLAG_RWTUN | CTLFLAG_NOFETCH | CTLFLAG_MPSAFE, 0, 0,
     sysctl_kern_timecounter_hardware, "A",
@@ -1524,12 +1539,17 @@ sysctl_kern_timecounter_choice(SYSCTL_HANDLER_ARGS)
 	struct timecounter *tc;
 	int error;
 
+	error = sysctl_wire_old_buffer(req, 0);
+	if (error != 0)
+		return (error);
 	sbuf_new_for_sysctl(&sb, NULL, 0, req);
+	mtx_lock(&tc_lock);
 	for (tc = timecounters; tc != NULL; tc = tc->tc_next) {
 		if (tc != timecounters)
 			sbuf_putc(&sb, ' ');
 		sbuf_printf(&sb, "%s(%d)", tc->tc_name, tc->tc_quality);
 	}
+	mtx_unlock(&tc_lock);
 	error = sbuf_finish(&sb);
 	sbuf_delete(&sb);
 	return (error);