On Tue, 26 Dec 2017, Ryota Ozaki wrote:
Well, since the lock _might_ be released (and subsequently reacquired)
by callout_halt(), it might be easiest to modify all the callers to
just unlock it before calling nd6_dad_stoptimer(), and reacquire the
mutex after it returns (as well as re-establishing any values that
might have changed while the mutex was released).
The callers needs to be prepared for the release/reacquire situation
anyway, so the change should not be significant. As noted in the
callout_halt(9) man page
...to avoid race conditions the caller should always assume that
[the] interlock has been released and reacquired, and act
accordingly.
Alternatively, you could modify all the callers to always acquire the
mutex before calling nd6_dad_stoptimer().
I mean such changes are tough and mess up many codes which we don't hope.
Yes, the changes are not trivial. But the first option above should already
have been done when you changed from callout_stop() to _halt().
But making a run-time decision with mutex_owned() is not a good idea.
If it must be respected we can back to callout_stop because it's unsafe
but NetBSD 7 uses it without any issues; using callout_stop in
nd6_dad_stoptimer might be safe somehow (if not NET_MPSAFE).
IMHO, we definitely do not want to use mutex_owned() in this way.
I do not believe that going backwards to callout_stop() is the right
approach.
Again IMHO, the _right_ thing to do is to continue using callout_halt() and
modify the callers. Yes, it might be a lot of work, and it might initially
introduce new errors, but in the long run it is the correct approach. IMHO.