Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 13 Oct 2025 20:26:02 GMT
From:      Michael Tuexen <tuexen@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: e79941bcb9d8 - main - tcp: fix KASSERT in HPTS
Message-ID:  <202510132026.59DKQ2Iu014236@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by tuexen:

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

commit e79941bcb9d8758070a97d780582e097bb82396e
Author:     Nick Banks <nickbanks@netflix.com>
AuthorDate: 2025-10-13 20:24:52 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2025-10-13 20:24:52 +0000

    tcp: fix KASSERT in HPTS
    
    Reviewed by:    tuexen
    Sponsored by:   Netflix, Inc.
---
 sys/netinet/tcp_hpts.c | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/sys/netinet/tcp_hpts.c b/sys/netinet/tcp_hpts.c
index b022cfba451d..fbab912b9a1f 100644
--- a/sys/netinet/tcp_hpts.c
+++ b/sys/netinet/tcp_hpts.c
@@ -1486,24 +1486,32 @@ no_run:
 	 */
 	hpts->p_wheel_complete = 1;
 	/*
-	 * Now did we spend too long running input and need to run more?
-	 * Note that if wrap_loop_cnt < 2 then we should have the conditions
-	 * in the KASSERT's true. But if the wheel is behind i.e. wrap_loop_cnt
-	 * is greater than 2, then the condtion most likely are *not* true.
-	 * Also if we are called not from the callout, we don't run the wheel
-	 * multiple times so the slots may not align either.
-	 */
-	KASSERT(((hpts->p_prev_slot == hpts->p_cur_slot) ||
-		 (hpts->p_on_queue_cnt == 0) ||
-		 (wrap_loop_cnt >= 2) || !from_callout),
-		("H:%p p_prev_slot:%u not equal to p_cur_slot:%u", hpts,
-		 hpts->p_prev_slot, hpts->p_cur_slot));
-	KASSERT((!tcp_hpts_different_slots(cts, cts_last_run) ||
-		 (hpts->p_on_queue_cnt == 0) ||
-		 (wrap_loop_cnt >= 2) || !from_callout),
-		("H:%p cts_last_run:%u not close enough to cts:%u", hpts,
-		 cts_last_run, cts));
-	if (from_callout && ((cts - cts_last_run) >= HPTS_USECS_PER_SLOT)) {
+	* If enough time has elapsed that we should be processing the next
+	* slot(s), then we should have kept running and not marked the wheel as
+	* complete.
+	*
+	* But there are several other conditions where we would have stopped
+	* processing, so the prev/cur slots and cts variables won't match.
+	* These conditions are:
+	*
+	* - Calls not from callouts don't run multiple times
+	* - The wheel is empty
+	* - We've processed more than max_pacer_loops times
+	* - We've wrapped more than 2 times
+	*
+	* This assert catches when the logic above has violated this design.
+	*
+	*/
+	KASSERT((!from_callout || (hpts->p_on_queue_cnt == 0) ||
+		 (loop_cnt > max_pacer_loops) || (wrap_loop_cnt >= 2) ||
+		 ((hpts->p_prev_slot == hpts->p_cur_slot) &&
+		  !tcp_hpts_different_slots(cts, cts_last_run))),
+		("H:%p Shouldn't be done! prev_slot:%u, cur_slot:%u, "
+		 "cts_last_run:%u, cts:%u, loop_cnt:%d, wrap_loop_cnt:%d",
+		 hpts, hpts->p_prev_slot, hpts->p_cur_slot,
+		 cts_last_run, cts, loop_cnt, wrap_loop_cnt));
+
+	if (from_callout && tcp_hpts_different_slots(cts, cts_last_run)){
 		cts_last_run = tcp_pace.cts_last_ran[hpts->p_num];
 		tcp_pace.cts_last_ran[hpts->p_num] = cts = tcp_get_usecs(&tv);
 		hpts->p_cur_slot = cts_to_wheel(cts);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202510132026.59DKQ2Iu014236>