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.
+------------------+--------------------------+----------------------------+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org | +------------------+--------------------------+----------------------------+