Date: Thu, 22 Sep 2005 11:18:48 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: John Baldwin <jhb@FreeBSD.org> Cc: cvs-src@FreeBSD.org, Gleb Smirnoff <glebius@FreeBSD.org>, Ruslan Ermilov <ru@FreeBSD.org>, "M. Warner Losh" <imp@bsdimp.com> Subject: Re: cleanup of interface shutdown/detach Was: cvs commit: src/sys/dev/an Message-ID: <20050922111533.G34322@fledge.watson.org> In-Reply-To: <200509211545.39246.jhb@FreeBSD.org> References: <200509190310.j8J3ALgt095979@repoman.freebsd.org> <200509211455.59154.jhb@FreeBSD.org> <20050921190250.GX36166@cell.sick.ru> <200509211545.39246.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050922111533.G34322>