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
_______________________________________________
dev-commits-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all
To unsubscribe, send any mail to "dev-commits-src-all-unsubscr...@freebsd.org"
