cleanup of interface shutdown/detach Was: cvs commit: src/sys/dev/an

[John Baldwin]

On Wednesday 21 September 2005 03:02 pm, Gleb Smirnoff wrote:
> On Wed, Sep 21, 2005 at 02:55:57PM -0400, John Baldwin wrote:
> J> > 4.  Remove IFF_DRV_RUNNING check from ether_output().
>
> <skip>
>
> J> I think you can leave #4 in if the race doesn't hurt anything.
>
> This check confuses people, is incorrect and useless. It confuses
> people, because they think that the !IFF_DRV_RUNNING condition is
> checked.  It is incorrect because upper layer must not touch/look
> at if_drv_flags. It is useless because the flag is checked without
> driver mutex being acquired, and thus does not protect from anything.
>
> Yesterday I have fixed panic in em(4) that was "protected" by this
> check. The correct way is to check the flag in interface start
> method, with driver mutex held.

It can sometimes be ok to check a flag twice to optimize the common case:

	if (!(foo & IF_FOO))
		return;
	FOO_LOCK(foo);
	if (!(foo & IF_FOO)) {
		FOO_UNLOCK(foo);
		return;
	}
	...
	FOO_UNLOCK(foo);

This can be useful if IF_FOO is often false and if you don't lose anything by 
reading a stale value for the check (for example, if you poll it every so 
often then if you lose the race you just lose it until the next poll).

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org