From owner-svn-src-head@FreeBSD.ORG Tue Jul 31 17:59:43 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8AA2E106566B; Tue, 31 Jul 2012 17:59:43 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 597F28FC0C; Tue, 31 Jul 2012 17:59:43 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id AF8E8B94A; Tue, 31 Jul 2012 13:59:42 -0400 (EDT) From: John Baldwin To: Garrett Cooper Date: Tue, 31 Jul 2012 13:43:31 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p17; KDE/4.5.5; amd64; ; ) References: <201207251128.q6PBSFlt052575@svn.freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201207311343.31777.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 31 Jul 2012 13:59:42 -0400 (EDT) Cc: Adrian Chadd , src-committers@freebsd.org, svn-src-all@freebsd.org, Andrey Zonov , Luigi Rizzo , Jack Vogel , svn-src-head@freebsd.org Subject: Re: svn commit: r238765 - head/sys/dev/e1000 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jul 2012 17:59:43 -0000 On Tuesday, July 31, 2012 12:29:32 pm Garrett Cooper wrote: > On Tue, Jul 31, 2012 at 9:20 AM, Jack Vogel wrote: > > Yes, I agree John, that was ugly, I'm already taking care of it with my > > changes, > > I'll send you a copy to check out. > > Like so: > > --- //depot/user/gcooper/atf-head/src/sys/dev/e1000/if_lem.c > 2012-07-25 17:11:00.000000000 0000 > +++ /scratch/p4/user/gcooper/atf-head/src/sys/dev/e1000/if_lem.c > 2012-07-25 17:11:00.000000000 0000 > @@ -1320,9 +1320,10 @@ > lem_local_timer, adapter); > goto out; > } > + EM_CORE_UNLOCK(adapter); > > + lem_rxeof(adapter, -1, NULL); > EM_TX_LOCK(adapter); > - lem_rxeof(adapter, -1, NULL); > lem_txeof(adapter); > if (ifp->if_drv_flags & IFF_DRV_RUNNING && > !IFQ_DRV_IS_EMPTY(&ifp->if_snd)) > @@ -1330,8 +1331,8 @@ > EM_TX_UNLOCK(adapter); > > out: > - EM_CORE_UNLOCK(adapter); > - return; > + if (mtx_owned(&adapter->core_mtx)) > + EM_CORE_UNLOCK(adapter); > } No, mtx_owned() is equally bad (I'd rather people be intentional about what locks they hold when, mtx_owned() allows people to be lazy IMO). Just do this: EM_TX_UNLOCK(adapter); return; out: EM_CORE_UNLOCK(adapter); } -- John Baldwin