Hi guys, On 19/09/15 10:55, Hans Petter Selasky wrote: > On 09/18/15 22:33, Davide Italiano wrote: >> On Thu, Sep 17, 2015 at 12:20 AM, Hans Petter Selasky >> <h...@selasky.org> wrote: >>> On 09/17/15 00:05, Gleb Smirnoff wrote: >> [...] 2) Your commit message didn't explain what (if any) is the >> use case > for this. > > It currently has one critical client, and that is destruction of > TCP connections. > > The last 6 months there has been terribly much discussion, bugs > and panics in the callout area, and there seems no end with recent > panics posted to -current. Even the updates which rss & more did > slipped in new bugs. > > I'm going to let Julien finish his work first. If he doesn't need > the KPI it will be removed. Else I want that it stays in. [...]
It is too much pressure on my shoulders, I am a callout rookie after all. :) Let me continue this discussion at a technical level: o Short answers: - Is callout_drain_async() currently required for TCP timers callout: No - Are we using the same logic than callout_drain_async() in current TCP timers code: Yes - Would callout_drain_async() make TCP timer code cleaner: Yes - Is callout(9) with an associated mutex hard to use: No - Is callout(9) without an associated mutex (mpsafe callout) hard to use: Yes o My callout (rookie) solutions: - Short-term: At least, improve man callout(9) page for mpsafe callout usage. I am working on it. - Mid-term: Improve current mpsafe callout KPI. hps is working on it. - Long-term I: Deprecate mpsafe callout usage. On TCP stack side it would require to remove INP_INFO lock, this is achievable as soon as we have RCU-ish locks in kernel. gnn is working on getting RCU-ish lock. (@gnn you might feel a gentle backpressure here :) - Long-term II: Come with a new callout-ish KPI only for that brings more guarantee in mpsafe callout cases and use it (Lot of work!). o Long answers: - Is callout_drain_async() KPI required for TCP timers: No - Are we using the same logic than callout_drain_async() in current TCP timers code: Yes All TCP timers race conditions (so far) have been fixed in two commits: https://svnweb.freebsd.org/base?view=revision&revision=281599 https://svnweb.freebsd.org/base?view=revision&revision=284245 This logic is used currently in release/10.2.0, stable/10 and HEAD with no issues (so far). And it is the same logic than callout_drain_async(). - Would it make the TCP time code cleaner: Yes Below code with mpsafe callout is wrong: if (callout_stop(c)) { /* Free resources used in callout callback */ free_res(); <- Kernel panic here } else { /* Defer free resources used in callout callback */ defer_free_res(); } Below code with mpsafe callout is right, but tricky as in general callout_reset() and callout_stop() calls are far away (current TCP timers code login): r = callout_reset(c); ... s = callout_stop(c); if (r && s) { /* Free resources used in callout callback */ free_res(); } else { /* Defer free resources used in callout callback */ defer_free_res(); } Below code with mpsafe callout is right, and less tricky: if (callout_stop_async(c, defer_free_res)) { /* Free resources used in callout callback */ free_res(); } Sorry for this too long answer. -- Julien
signature.asc
Description: OpenPGP digital signature