Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 7 Dec 2016 17:41:51 +0800
From:      Sepherosa Ziehau <sepherosa@gmail.com>
To:        Chris Torek <torek@elf.torek.net>
Cc:        rysto32@gmail.com, "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>
Subject:   Re: mutex usage in if_bridge vs other drivers
Message-ID:  <CAMOc5cxknfT6N224nSi4eyhh6s2E9LyA3_S95ym7fEk3TyU5qQ@mail.gmail.com>
In-Reply-To: <201612030736.uB37aArG057471@elf.torek.net>
References:  <CAFMmRNxf8FoDSOEDO6AA1An6nNK6i0X44-bMUH9v_kvSM06J%2BA@mail.gmail.com> <201612030736.uB37aArG057471@elf.torek.net>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

Thanks,
sephe

-- 
Tomorrow Will Never Die



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAMOc5cxknfT6N224nSi4eyhh6s2E9LyA3_S95ym7fEk3TyU5qQ>