On Tue, Dec 26, 2017 at 4:30 PM, Paul Goyette <p...@whooppee.com> wrote: > 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.
The right thing here is to kill softnet_lock. IMHO, we shouldn't straightforwardly cope with softnet_lock and change the code a lot due to softnet_lock. Backing to callout_stop is a good compromise, I think. ozaki-r