From owner-freebsd-net@freebsd.org Wed Dec 7 09:41:52 2016 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E15D3C6987A for ; Wed, 7 Dec 2016 09:41:52 +0000 (UTC) (envelope-from sepherosa@gmail.com) Received: from mail-ua0-x236.google.com (mail-ua0-x236.google.com [IPv6:2607:f8b0:400c:c08::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9BA17A54 for ; Wed, 7 Dec 2016 09:41:52 +0000 (UTC) (envelope-from sepherosa@gmail.com) Received: by mail-ua0-x236.google.com with SMTP id 51so409491302uai.1 for ; Wed, 07 Dec 2016 01:41:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=VWrB+AwhVNJMzsvD9iIBGGcGIeYqhS9sPW2sIu6Ojfc=; b=ZNyMVH8Qs57pyN7gygmC6OtK9nI8rPvh+z+QY8JADpnxN2BhEe6frzsUtXGy/DNkPd t60CS51JQ/GNmxoTl4CNptnOJLAGOsk20wJdfPXPkgXDnLhDYo20aONEbIYoi/bFsp9Z UOCvCbxQ5ADQDAaDkNqKtr5r5cLqBr0tAe/YJ1OM4ZW9zeh4n0CXg33Ds8X0fZTLN72z 4zCduTxmdRBzTtc3Fa6Zpwxo7vbKr/S27SLqGlzWfZAg+gIgdUgTkfhYLomroAfZ94N7 0hMnuTPcLXYC9gVsE0ixZv4Zt7RIaKPtmqLwtipzTiKdSYIJ7JyB8ODawHpx9ptn9APK WUoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=VWrB+AwhVNJMzsvD9iIBGGcGIeYqhS9sPW2sIu6Ojfc=; b=Ba0hN4QWDU28LmImDQ8MHIjOleft7XPQsPBa/2QFkq9btLrvKpJ7n1NjDgKin0Kf9K jx3OK3o93lcntxLguQ5EeoREsKkIwlrgICD3zIQnuWkSS01joRoIRERHsg7RIONBmg+x EmO8rpCRW1QZgBd5xxLRGZwNrV0h/eFe3GPBC7NnaAF6gozrni6n/ef7Z7BxqdZKvYBs inLAEujJiO0rDk1Qrv3UuaTK9atW6QiJRc/0D4HZAjal6Dp3IvP/jqch+ayk0UgQv8vN sfLqI5OSsR/eS0vlmFsImnt8PBWk/XcJDACIRzL+wi1NpvsCPIcM8YB2H58C91AKwDHc xr8w== X-Gm-Message-State: AKaTC00V17HSEOis9eqy4eOsKzBmWimVX0onAj10w26S1iRViJJdBWFU0cWvRpKVVSxVRXnsWPXI/3XA74L9Cw== X-Received: by 10.176.83.151 with SMTP id k23mr52949534uaa.90.1481103711818; Wed, 07 Dec 2016 01:41:51 -0800 (PST) MIME-Version: 1.0 Received: by 10.176.5.198 with HTTP; Wed, 7 Dec 2016 01:41:51 -0800 (PST) In-Reply-To: <201612030736.uB37aArG057471@elf.torek.net> References: <201612030736.uB37aArG057471@elf.torek.net> From: Sepherosa Ziehau Date: Wed, 7 Dec 2016 17:41:51 +0800 Message-ID: Subject: Re: mutex usage in if_bridge vs other drivers To: Chris Torek Cc: rysto32@gmail.com, "freebsd-net@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Dec 2016 09:41:53 -0000 On Sat, Dec 3, 2016 at 3:36 PM, Chris Torek 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