Re: MP-safe DAD timer destruction with callout_stop

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Fri, Jan 12, 2018 at 1:33 PM, Christos Zoulas <christos%astron.com@localhost> wrote:
> In article <CAKrYomiYk93wa12jrbr6Yw5BkJ60fcU1BVJj=_ommC=Gb5w-eA%mail.gmail.com@localhost>,
> Ryota Ozaki  <ozaki-r%netbsd.org@localhost> wrote:
>>Hi,
>>
>>For a certain reason(*), DAD timers are hard to use
>>callout_halt to destroy DAD timer objects. So I was going
>>to fall back to callout_stop (as of NetBSD 7) that is
>>almost safe but still has a possibility of going wrong(**).
>>
>>(*) See the thread starting at
>>http://mail-index.netbsd.org/source-changes-d/2017/12/26/msg009744.html
>>(**) http://mail-index.netbsd.org/source-changes-d/2018/01/11/msg009802.html
>>
>>Discussing with @riastradh, we found a possible way to
>>make DAD timer destructions MP-safe with callout_stop.
>>This is a PoC patch:
>>  http://www.netbsd.org/~ozaki-r/dad-callout_stop.v2.diff
>>
>>The issue of using callout_stop instead of callout_halt is
>>that callout_stop doesn't actually stop the running callout.
>>So we cannot safety free a related data after calling
>>callout_stop.
>>
>>The new approach copes with the issue by delegating the
>>destruction of a callout to callout itself, which allows us
>>to not wait the callout to finish. This can be done thanks
>>to that DAD objects are separated from other data such as
>>ifa.
>
> Even then (with callout_halt) nothing prevents something from
> calling callout_schedule() or callout_reset() again between
> that time and callout_destroy()... I have code that adds another
> flag that stops further callouts from being reschuled. Does this
> help you?

Not quite in this case. The cause of a race condition in this case
is an in-flight callout handler and the new flag probably doesn't
help for the case. And also DADs can prevent extra callout_schedule
and callout_reset by theirselves (by checking the presence of a
target DAD object).

Nonetheless the feature may be useful in general, although I don't
have use cases now.

  ozaki-r