Date: Sat, 3 Dec 2016 00:20:17 -0500 From: Ryan Stone <rysto32@gmail.com> To: Chris Torek <torek@torek.net> Cc: freebsd-net <freebsd-net@freebsd.org> Subject: Re: mutex usage in if_bridge vs other drivers Message-ID: <CAFMmRNxf8FoDSOEDO6AA1An6nNK6i0X44-bMUH9v_kvSM06J%2BA@mail.gmail.com> In-Reply-To: <201612022042.uB2KgOkO055419@elf.torek.net> References: <201612022042.uB2KgOkO055419@elf.torek.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Dec 2, 2016 at 3:42 PM, Chris Torek <torek@torek.net> wrote: > THE QUESTION: > > - Who is wrong, the bxe driver or the bridge code? I.e., > does the bridge driver need to release its lock here, > and if so, is that actually safe to do? (We might need > to restart the loop over all the members if we drop the > lock.) > > - Or if the bridge driver should retain its lock, can it > use an sx lock here, to permit members to also use sx > locks? > bxe is not the only driver that sleeps in its config path, so I would say that if_bridge is de facto incorrect. Dropping the lock is entirely the wrong thing to do -- as you note, if we do, then the bridge members can change out from under us. The only path forward is to use an sx lock, but unfortunately it's not quite as simple as just converting the lock to an sx lock. The problem is that BRIDGE_LOCK() is also called in the transmit and receive paths, and those paths absolutely may not sleep for any reason. I believe that the correct fix would be to introduce an sx lock to if_bridge in additional to the current lock. In code paths that call ioctls() on bridge members, replace the use of BRIDGE_LOCK with the new sx lock. In code paths that modify the list of bridge members, hold both the BRIDGE_LOCK and the new sx lock. In the transmit and receive paths, nothing should change.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFMmRNxf8FoDSOEDO6AA1An6nNK6i0X44-bMUH9v_kvSM06J%2BA>