Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 28 Jul 2012 17:41:50 -0400
From:      Arnaud Lacombe <lacombar@gmail.com>
To:        Adrian Chadd <adrian@freebsd.org>
Cc:        Dimitry Andric <dim@freebsd.org>, current@freebsd.org
Subject:   Re: panic: _mtx_lock_sleep: recursed on non-recursive mutex em0 @ /usr/src/sys/dev/e1000/if_lem.c:881
Message-ID:  <CACqU3MWjePb5mswxh6=gMnHG9VF8ezjgtZwX6Ehk8hVo6Y8bog@mail.gmail.com>
In-Reply-To: <CAJ-VmomrNdDJKTp8DW5AyffkGEhWvCdu23Q17%2Bm-LnEL-Ujq1g@mail.gmail.com>
References:  <20120726154610.GC1587@albert.catwhisker.org> <5012E233.3050007@FreeBSD.org> <CAJ-Vmo=Tgg-sYVB5b1RhP0adCYJLrp%2B4W4nvpN6M6giTnBjw7w@mail.gmail.com> <CACqU3MVLuf%2BH-ujVhTqod10uWTioeC0eLgM_fgVHUeH22o55Sg@mail.gmail.com> <CAJ-VmomrNdDJKTp8DW5AyffkGEhWvCdu23Q17%2Bm-LnEL-Ujq1g@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail 

[-- Attachment #1 --]
Hi,

On Sat, Jul 28, 2012 at 4:04 PM, Adrian Chadd <adrian@freebsd.org> wrote:
> On 28 July 2012 12:09, Arnaud Lacombe <lacombar@gmail.com> wrote:
>
>> How would a single ATH_LOCK() helps here ? AFAICS, the panic seem to
>> be a classical fallout from direct dispatch where you can re-enter the
>> driver from the driver itself through the network stack.
>
> Take a look at iwn. It has a single lock - IWN_LOCK() - which it
> releases before punting the frame up the stack.
>
oh, I see. So, what happens in lem(4) is that we enter the stack with
RX unlocked, but TX and CORE are still locked. The whole locking of
lem(4) seems rather inconsistent. Through DEVICE_POLLING, lem_rxeof()
is called with TX and CORE unlocked. Through EN_LEGACY_IRQ it is
called with TX and CORE locked, and from MSI interrupt handler, is is
caller with TX unlocked (CORE assumed to be unlock). I'd assume that
lem(4) is just poorly maintained[0]... I would make a huge shot in the
dark by proposing the completely untested attached patch :/

 - Arnaud

[0]: like code claiming support for Intel 82574 when this chipset
*cannot* be used by lem(4), as there is no E1000_DEV_ID_82574* entries
in `lem_vendor_info_array'.

[-- Attachment #2 --]
diff --git a/sys/dev/e1000/if_lem.c b/sys/dev/e1000/if_lem.c
index 4ade180..1596d66 100644
--- a/sys/dev/e1000/if_lem.c
+++ b/sys/dev/e1000/if_lem.c
@@ -1304,11 +1304,15 @@ lem_intr(void *arg)
 	if (reg_icr & E1000_ICR_RXO)
 		adapter->rx_overruns++;
 
-	if ((reg_icr == 0xffffffff) || (reg_icr == 0))
-			goto out;
+	if ((reg_icr == 0xffffffff) || (reg_icr == 0)) {
+		EM_CORE_UNLOCK(adapter);
+		goto out;
+	}
 
-	if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0)
-			goto out;
+	if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
+		EM_CORE_UNLOCK(adapter);
+		goto out;
+	}
 
 	if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC)) {
 		callout_stop(&adapter->timer);
@@ -1320,17 +1324,17 @@ lem_intr(void *arg)
 		    lem_local_timer, adapter);
 		goto out;
 	}
+	EM_CORE_UNLOCK(adapter);
 
-	EM_TX_LOCK(adapter);
 	lem_rxeof(adapter, -1, NULL);
+
+	EM_TX_LOCK(adapter);
 	lem_txeof(adapter);
-	if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
-	    !IFQ_DRV_IS_EMPTY(&ifp->if_snd))
+	if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
 		lem_start_locked(ifp);
 	EM_TX_UNLOCK(adapter);
 
 out:
-	EM_CORE_UNLOCK(adapter);
 	return;
 }
help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACqU3MWjePb5mswxh6=gMnHG9VF8ezjgtZwX6Ehk8hVo6Y8bog>