Date: Wed, 7 Dec 2016 12:09:42 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Sepherosa Ziehau <sepherosa@gmail.com> Cc: Chris Torek <torek@mail.torek.net>, "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, rysto32@gmail.com Subject: Re: mutex usage in if_bridge vs other drivers Message-ID: <20161207100942.GU54029@kib.kiev.ua> In-Reply-To: <CAMOc5cxknfT6N224nSi4eyhh6s2E9LyA3_S95ym7fEk3TyU5qQ@mail.gmail.com> References: <CAFMmRNxf8FoDSOEDO6AA1An6nNK6i0X44-bMUH9v_kvSM06J%2BA@mail.gmail.com> <201612030736.uB37aArG057471@elf.torek.net> <CAMOc5cxknfT6N224nSi4eyhh6s2E9LyA3_S95ym7fEk3TyU5qQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Dec 07, 2016 at 05:41:51PM +0800, Sepherosa Ziehau wrote: > On Sat, Dec 3, 2016 at 3:36 PM, Chris Torek <torek@elf.torek.net> wrote: > >>... 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 ... > > [snip] > >>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. > > > > That should work, but I hate the cost of obtaining two locks -- > > not so much the lock acquire/release cost, but the "mental cost" > > of keeping the lock ordering straight. > > > > The other option I thought of was gathering the list of members > > that need an ioctl done on them in one pass, then doing all the > > ioctls after dropping the lock. But that runs the risk of doing > > ioctls on interfaces that are no longer members of the bridge. > > > > I guess the "mental cost" is not that high, since the lock order > > requirement is guaranteed to be "sx lock first, then mtx lock" > > (because the other way around causes sleeping while holding a > > regular mutex!). > > This is what did for Hyper-V network drivers: > > while (sx_try_xlock(drv_conf_sx) == 0) > DELAY(1000); > > since that mainlock, i.e. configuration lock, is not on hot path for drivers. > > In this way, driver can sleep freely on IF_UP/DOWN/detach path, which > is convenient, though other paths e.g. promisc, add/delmulti still > requires busy-wait instead of sleeping. Consider what happens if some other thread already owns the conf sx lock and tries to acquire the mutex. If the thread blocks execution by whatever mean, be it lock primitive putting the thread off cpu, or thread executing spin loop (as in your case), reversing the order still causes the deadlock.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161207100942.GU54029>