git: 2694c869ff9f - main - ktls: fix a panic with INVARIANTS

[Andrew Gallatin]

On 8/5/21 1:41 PM, Ian Lepore wrote:

       if (nbufs != ktls_max_alloc) {
> 
> Finding a different way to spell "forever" is not a valid way to fix a
> problem where you're being warned that it is not safe to sleep forever.
> 
> The assert was warning you that the code was vulnerable to hanging
> forever due to a missed wakeup.  The code is still vulnerable to that,
> but now the problem is hidden and will be very difficult to find (more
> so because the wait message also violates the convention of using a
> short name that can be displayed by tools such as ps(1) and SIGINFO,
> where the wait-channel display is currently likely to show as
> "waitin").
> 
> I haven't looked at the code outside of the few lines shown in the
> commit diff, but based on the names involved, I suspect the right fix
> is to protect sc->running with a mutex and use msleep() instead of
> trying to roll-your-own with atomics.  That would certainly be true if
> the wakeup code is some form of "if (!sc->running) wakeup(sc);"
> 
> -- Ian
> 

The code is a case where a missing wakeup does not matter.

The thread is woken up by an allocation failure, which
are themselves rate-limited to one attempt per second
(since failures are expensive, and there is a less expensive
fallback).  So the worst thing that can happen is that we wait
at most an extra second.

Adding a mutex adds nothing except unneeded complexity.

Drew