This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
quic: add timer management

This patch introduces 'quic_timer' to unify and manage the five main
timers used in QUIC: loss detection, delayed ACK, path validation,
PMTU probing, and pacing. These timers are critical for driving
retransmissions, connection liveness, and flow control.

[ ... ]

[ ... ]

[ ... ]

[ ... ]

+static void quic_timer_sack_timeout(struct timer_list *t)
+{
+	struct quic_sock *qs = container_of(t, struct quic_sock, timers[QUIC_TIMER_SACK].t);
+	struct sock *sk = &qs->inet.sk;
+
+	bh_lock_sock(sk);
+	if (sock_owned_by_user(sk)) {
+		if (!test_and_set_bit(QUIC_SACK_DEFERRED, &sk->sk_tsq_flags))
+			sock_hold(sk);
+		goto out;
+	}
+
+	quic_timer_sack_handler(sk);
+out:
+	bh_unlock_sock(sk);
+	sock_put(sk);
+}

[ ... ]

+static void quic_timer_loss_timeout(struct timer_list *t)
+{
+	struct quic_sock *qs = container_of(t, struct quic_sock, timers[QUIC_TIMER_LOSS].t);
+	struct sock *sk = &qs->inet.sk;
+
+	bh_lock_sock(sk);
+	if (sock_owned_by_user(sk)) {
+		if (!test_and_set_bit(QUIC_LOSS_DEFERRED, &sk->sk_tsq_flags))
+			sock_hold(sk);
+		goto out;
+	}
+
+	quic_timer_loss_handler(sk);
+out:
+	bh_unlock_sock(sk);
+	sock_put(sk);
+}

[ ... ]

+static void quic_timer_path_timeout(struct timer_list *t)
+{
+	struct quic_sock *qs = container_of(t, struct quic_sock, timers[QUIC_TIMER_PATH].t);
+	struct sock *sk = &qs->inet.sk;
+
+	bh_lock_sock(sk);
+	if (sock_owned_by_user(sk)) {
+		if (!test_and_set_bit(QUIC_PATH_DEFERRED, &sk->sk_tsq_flags))
+			sock_hold(sk);
+		goto out;
+	}
+
+	quic_timer_path_handler(sk);
+out:
+	bh_unlock_sock(sk);
+	sock_put(sk);
+}

[ ... ]

+static void quic_timer_pmtu_timeout(struct timer_list *t)
+{
+	struct quic_sock *qs = container_of(t, struct quic_sock, timers[QUIC_TIMER_PMTU].t);
+	struct sock *sk = &qs->inet.sk;
+
+	bh_lock_sock(sk);
+	if (sock_owned_by_user(sk)) {
+		if (!test_and_set_bit(QUIC_PMTU_DEFERRED, &sk->sk_tsq_flags))
+			sock_hold(sk);
+		goto out;
+	}
+
+	quic_timer_pmtu_handler(sk);
+out:
+	bh_unlock_sock(sk);
+	sock_put(sk);
+}

This isn't a bug, but the four timer_list-based timeout callbacks
(quic_timer_sack_timeout, quic_timer_loss_timeout, quic_timer_path_timeout,
quic_timer_pmtu_timeout) are structurally identical.

Each extracts the sock via container_of, takes bh_lock_sock, checks
sock_owned_by_user to conditionally defer via test_and_set_bit/sock_hold,
otherwise calls the handler directly, then bh_unlock_sock/sock_put. They
differ only in the timer index constant, the deferred bit name, and the
handler function.

This pattern could be consolidated into a single parameterized timeout
callback using a data table that maps timer type to deferred bit and handler
function pointer, reducing around 60 lines of near-duplicate code to
roughly 15 lines. The TCP subsystem uses a similar approach for some of its
timer infrastructure.

That said, this follows the existing per-timer-callback pattern used
elsewhere in the kernel networking stack (for example, tcp_write_timer,
tcp_delack_timer are also separate functions), so this is a minor style
observation rather than a defect.

+void quic_timer_reset(struct sock *sk, u8 type, u64 timeout)
+{
+	struct timer_list *t = quic_timer(sk, type);
+
+	if (timeout && !mod_timer(t, jiffies + usecs_to_jiffies(timeout)))
+		sock_hold(sk);
+}

Can this function handle QUIC_TIMER_PACE safely? It unconditionally casts
the result of quic_timer() to 'struct timer_list *' and calls mod_timer().

Looking at the timer API in this patch:

Both quic_timer_start() and quic_timer_stop() have explicit QUIC_TIMER_PACE
guards that dispatch to hrtimer APIs:

quic_timer_start():
	if (type == QUIC_TIMER_PACE) {
		hr = quic_timer(sk, type);
		if (!hrtimer_is_queued(hr)) {
			hrtimer_start(hr, ns_to_ktime(timeout), ...);
			sock_hold(sk);
		}
		return;
	}

quic_timer_stop():
	if (type == QUIC_TIMER_PACE) {
		if (hrtimer_try_to_cancel(quic_timer(sk, type)) == 1)
			sock_put(sk);
		return;
	}

However, quic_timer_reset() does not have a similar guard. If a future
caller invokes quic_timer_reset(sk, QUIC_TIMER_PACE, timeout), mod_timer()
will operate on memory that is actually a 'struct hrtimer', potentially
causing memory corruption.

Should quic_timer_reset() either add a QUIC_TIMER_PACE guard consistent with
quic_timer_start()/quic_timer_stop(), or document that this function must
only be used with timer_list-based timers?

+void quic_timer_start(struct sock *sk, u8 type, u64 timeout)
+{
+	struct timer_list *t;
+	struct hrtimer *hr;
+
+	if (type == QUIC_TIMER_PACE) {
+		hr = quic_timer(sk, type);
+
+		if (!hrtimer_is_queued(hr)) {
+			hrtimer_start(hr, ns_to_ktime(timeout), HRTIMER_MODE_ABS_PINNED_SOFT);
+			sock_hold(sk);
+		}
+		return;
+	}
+
+	t = quic_timer(sk, type);
+	if (timeout && !timer_pending(t)) {
+		if (!mod_timer(t, jiffies + usecs_to_jiffies(timeout)))
+			sock_hold(sk);
+	}
+}
+
+void quic_timer_stop(struct sock *sk, u8 type)
+{
+	if (type == QUIC_TIMER_PACE) {
+		if (hrtimer_try_to_cancel(quic_timer(sk, type)) == 1)
+			sock_put(sk);
+		return;
+	}
+	if (timer_delete(quic_timer(sk, type)))
+		sock_put(sk);
+}

[ ... ]

[ ... ]