Date: Fri, 08 May 2009 14:50:12 -0400 From: John Baldwin <jhb@FreeBSD.org> To: Andrew Thompson <thompsa@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r191912 - in head/sys/dev: ipw iwi Message-ID: <4A047EE4.4080007@FreeBSD.org> In-Reply-To: <20090508174914.GC93351@citylink.fud.org.nz> References: <200905081344.n48DiYJI092605@svn.freebsd.org> <4A046EB0.9060009@FreeBSD.org> <20090508174914.GC93351@citylink.fud.org.nz>
next in thread | previous in thread | raw e-mail | index | archive | help
Andrew Thompson wrote: > On Fri, May 08, 2009 at 01:41:04PM -0400, John Baldwin wrote: >> Andrew Thompson wrote: >>> Author: thompsa >>> Date: Fri May 8 13:44:33 2009 >>> New Revision: 191912 >>> URL: http://svn.freebsd.org/changeset/base/191912 >>> >>> Log: >>> Drain the tasks before the interface stop call in case a restart was queued. >> Actually, you have to drain it after if_detach() so you can safely destroy >> the lock. The proper order should be something like this: >> >> bpfdetach(ifp); >> ieee80211_ifdetach(ic); >> >> ipw_stop(sc); >> >> callout_drain(); >> ieee80211_draintask(); >> >> ipw_release(sc); >> >> This is the order other NIC drivers use where they do something like: >> >> ether_ifdetach(ifp); >> FOO_LOCK(sc); >> foo_stop(sc); // calls callout_stop() >> FOO_UNLOCK(sc); >> >> callout_drain(); >> taskqueue_drain(); // only if it uses tasks >> >> if_free(); >> mtx_destroy(); > > ieee80211_ifdetach() will actually free the taskqueue, it doesnt use one > of the persistent system ones. It wasnt incorrect before as the > interface would still be brought down before net80211 detached, it was > just to reduce the flip flopping. With that noted does it still need > reordering? Assuming that free'ing a taskqueue drains any tasks, then you don't actually need to drain the task queue in that case. However, you should still move the foo_stop() after if_detach() as userland ioctl requests are still able to call your driver's ioctl() routine until if_detach() returns. So perhaps something like this: bpfdetach() ieee80211_ifdetach() ipw_stop(); callout_drain(); ipw_release(); In general, it isn't really safe to shutdown the hardware (foo_stop()) until you have detached the driver from external code paths (ifnet, bpf, etc.). -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A047EE4.4080007>