Date: Wed, 21 Sep 2005 15:45:37 -0400 From: John Baldwin <jhb@FreeBSD.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: Robert Watson <rwatson@FreeBSD.org>, Ruslan Ermilov <ru@FreeBSD.org>, "M. Warner Losh" <imp@bsdimp.com>, cvs-src@FreeBSD.org Subject: Re: cleanup of interface shutdown/detach Was: cvs commit: src/sys/dev/an Message-ID: <200509211545.39246.jhb@FreeBSD.org> In-Reply-To: <20050921190250.GX36166@cell.sick.ru> References: <200509190310.j8J3ALgt095979@repoman.freebsd.org> <200509211455.59154.jhb@FreeBSD.org> <20050921190250.GX36166@cell.sick.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509211545.39246.jhb>