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

Reply via email to