From owner-svn-src-all@freebsd.org Fri Mar 18 18:05:14 2016 Return-Path: Delivered-To: svn-src-all@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 697AAAD4D4E; Fri, 18 Mar 2016 18:05:14 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 481C5104F; Fri, 18 Mar 2016 18:05:14 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 59E1DB977; Fri, 18 Mar 2016 14:05:13 -0400 (EDT) From: John Baldwin To: Adrian Chadd Cc: Gleb Smirnoff , Hans Petter Selasky , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Subject: Re: svn commit: r296909 - head/sys/ofed/drivers/infiniband/ulp/ipoib Date: Fri, 18 Mar 2016 10:43:32 -0700 Message-ID: <208354486.x7l0Vtjzkk@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.2-STABLE; KDE/4.14.3; amd64; ; ) In-Reply-To: References: <201603151547.u2FFlQKN078643@repo.freebsd.org> <5463538.GTDDlnsCR6@ralph.baldwin.cx> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 18 Mar 2016 14:05:13 -0400 (EDT) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Mar 2016 18:05:14 -0000 On Wednesday, March 16, 2016 03:37:09 PM Adrian Chadd wrote: > I've fought this problem in USB drivers. Ideally you'd also have the > detach path /also/ take said lock and drain anyone currently doing > anything active (tx, rx, ioctls, net80211 methods, etc) to completion > before continuing. > > A lot of drivers don't do this very well, or at all. There isn't a lock to take in detach to hold over drains. Instead, the various detach APIs (like if_detach) need to drain internally. Only the ifnet layer can keep track of if an ifnet is currently "busy" (any methods currently executing for example). You can't do that in the driver. This is why bus_teardown_intr() does a drain for example (as well as callout_drain()). destroy_dev() also handles this more correctly as it does a drain (and drivers can even provide a d_purge callback to wakeup any waiters inside of cdevsw methods during the drain). if_detach() doesn't do any drain at all and is broken. It has to be fixed in the ifnet layer. Changes in drivers are just band-aids (the current band-aids in this commit and in other NIC drivers previously only hide witness warnings, they do nothing to solve the problem of some thread that is already inside of an if_foo method still running when detach is called, particularly if an if_foo method can sleep). -- John Baldwin