Skip site navigation (1)Skip section navigation (2)
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>