On Sat, Jan 20, 2018 at 15:17 +0100, Alexander Bluhm wrote:
> Hi,
>
Hi,
While I'm not against making all TCP timeouts look similar, I'd like
to understand if there's any other reason to do it other than
"consistency".
> The tcp reaper timeout is still imlemented as soft timeout. So it
> can run while net lock is held by others and it is not synchronized
> with the other tcp timeouts.
Am I right to say that this is not an issue? Neither pool_put nor
tcpstat_inc requre a NET_LOCK, correct?
> Convert it to an ordinary tcp timeout
> so it is scheduled on the same timeout thread. It grabs the net
> lock to make sure that softnet has finished its work.
>
This just makes other threads who want to grab NET_LOCK wait for
a pool_put to finish, but where's the benefit?
> ok?
>
> bluhm
>
What's up with the diagnostics? Do you suspect a race of some sorts?
> @@ -462,5 +464,26 @@ tcp_timer_2msl(void *arg)
> tp = tcp_close(tp);
>
> out:
> + NET_UNLOCK();
> +}
> +
> +void
> +tcp_timer_reaper(void *arg)
> +{
> + struct tcpcb *tp = arg;
> + int i;
> +
> + NET_LOCK();
> +#ifdef DIAGNOSTIC
> + if ((tp->t_flags & TF_DEAD) == 0)
> + panic("%s: tcpcb %p is not dead", __func__, tp);
> + for (i = 0; i < TCPT_NTIMERS; i++) {
> + if (TCP_TIMER_ISARMED(tp, i))
> + panic("%s: tcpcb %p timer %d is armed",
> + __func__, tp, i);
> + }
> +#endif
> + pool_put(&tcpcb_pool, tp);
> + tcpstat_inc(tcps_closed);
> NET_UNLOCK();
> }
