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

[Robert Watson]

On Wed, 21 Sep 2005, John Baldwin wrote:

>> 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).

In the case of IFF_DRV_RUNNING it's probably not a big deal, because we 
want to optimize for the driver running case rather than the driver not 
running case.  However, for IFF_DRV_OACTIVE, we actually want to optimize 
for the case where the output routine is already active and we're rapidly 
enqueueing packets for the driver to send on its next interrupt, I think. 
So adding additional lock acquires in that case may be a problem.

Something we discussed at BSDCan was eliminating the ifq lock by having 
the driver softc (or softc send) lock protect the queue.  This potentially 
increases contention on enqueue because driver locks are held a fair 
amount on send.  However, it reduces the total locking operations by 4 for 
a simple packet send.  We'd need to carefully work out the contention 
costs though.  If we do successfully start breaking out network device 
drivers into two locks: send and receive paths, then the content is likely 
to be much less of a problem because the send enqueue would no longer 
contend with the receive processing.

Robert N M Watson