Hi Martin, On Wed, Jun 24, 2020 at 4:02 AM Martin Pieuchot <m...@openbsd.org> wrote: > Yes, that might be a better way. If I understood your original mail the > issue is related to ifunit(), right? ifunit() is not used in packet- > processing contexts, that's why we did not protect it by the NET_LOCK(). > > I'm not sure if all the ifunit() usages are necessary, many of them are > certainly exposing races with attach/destroy.
No, not _just_ ifunit, but ifunit is one of the places where this is hit. But just one. > Taking a rwlock for all ioctl(2) has a big impact. The 'default' case > of the switch in ifioctl() goes into other subsystems and per-ifp ioctl > handler. > > Many drivers sleep in their ioctl(2) handler, most USB and wireless one > to name a few. Past experience showed that holding a rwlock around all > the handlers, led to (temporarily) "deadlock". This is why vio(4) mailbox > sleep has been turned into a rwsleep releasing the `netlock'. > > One can argue that such deadlocks happen because the scope of the lock is > huge. This is easy to understand with the `netlock' and questionable, > at the time of the diff, with the proposed `if_list_lock'. But saying > so would miss the point: throwing a lock around a huge part of code to > make symptoms disappear is a big hammer. Oh, that's your concern. I understand what you're concerned with now. However, I think that in light of that, you've misunderstood the original patch, and I'm now more convinced that the original patch is the correct route. The original patchset: a. Uses an exclusive lock for clone/destroy. b. Uses a shared lock for all other ioctls. This means that all ioctls except clone/destroy can run without blocking each other. So, there's no deadlock issues, and no speed issues, and no lack of coarseness of locking. What this patch set does add is: 1. Other ioctls are not permitted to run at the same time as clone/destroy. 2. Clone and destroy are not allowed to run at the same time as each other. However: 3. Other ioctls ARE allowed to run at the same time as other ioctls, so no blocking or deadlock issues. Given the object lifetime and lookup structure design, these seem to be the optimal set of circumstances. Jason