Re: CVS commit: src/sys/dev/pci

[Nick Hudson <nick.hudson%gmx.co.uk@localhost>]

Hi,


On 21/09/2022 08:56, matthew green wrote:
> > > this asserts for me.  perhaps ryo@ didn't have LOCKDEBUG?
> >
> > I had LOCKDEBUG on, but not DEBUG. https://nxr.netbsd.org/xref/src/sys/sys/systm.h#760
> > I see that adding options DEBUG does indeed cause a panic with ASSERT_SLEEPABLE()...
>
> ah!  that would explain it.  nick asked me to test switching
> sc_mutex to IPL_SOFTCLOCK mutex, and this works, though it
> exposed another bug in reboot that i also needed a fix for
> (ifnet locked), see patch below with both fixes.
>
> there's another with the rev 1.32 and rev 1.33+patch driver
> when doing "ifconfig aq0 down up", i get this shortly after
> and packets no longer flow:
>
> aq0: watchdog timeout -- resetting
> aq0: aq_handle_reset_work: INTR_MASK/STATUS = 0001ffff/00000000
> aq0: aq_handle_reset_work: TXring[0] HEAD/TAIL=26/51
> aq0: aq_handle_reset_work: TXring[1] HEAD/TAIL=153/170
> aq0: aq_handle_reset_work: TXring[2] HEAD/TAIL=0/0
> aq0: aq_handle_reset_work: TXring[3] HEAD/TAIL=0/0
> aq0: aq_handle_reset_work: TXring[4] HEAD/TAIL=0/0
> aq0: aq_handle_reset_work: TXring[5] HEAD/TAIL=0/0
> aq0: aq_handle_reset_work: TXring[6] HEAD/TAIL=0/0
> aq0: aq_handle_reset_work: TXring[7] HEAD/TAIL=0/0


I think this diff is slightly better and might even fix the problem
you're seeing with "ifconfig aq0 down up"

Nick

Index: sys/dev/pci/if_aq.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_aq.c,v
retrieving revision 1.33
diff -u -p -r1.33 if_aq.c
--- sys/dev/pci/if_aq.c	16 Sep 2022 03:55:53 -0000	1.33
+++ sys/dev/pci/if_aq.c	21 Sep 2022 08:15:26 -0000
@@ -1278,7 +1278,7 @@ aq_attach(device_t parent, device_t self
 	int error;
 
 	sc->sc_dev = self;
-	mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_NET);
+	mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_SOFTCLOCK);
 	mutex_init(&sc->sc_mpi_mutex, MUTEX_DEFAULT, IPL_NET);
 
 	sc->sc_pc = pc = pa->pa_pc;
@@ -1588,7 +1588,9 @@ aq_detach(device_t self, int flags __unu
 
 	if (sc->sc_iosize != 0) {
 		if (ifp->if_softc != NULL) {
-			aq_stop(ifp, 0);
+			IFNET_LOCK(ifp);
+			aq_stop(ifp, 1);
+			IFNET_UNLOCK(ifp);
 		}
 
 		for (i = 0; i < AQ_NINTR_MAX; i++) {
@@ -1616,8 +1618,6 @@ aq_detach(device_t self, int flags __unu
 		sc->sc_iosize = 0;
 	}
 
-	callout_stop(&sc->sc_tick_ch);
-
 #if NSYSMON_ENVSYS > 0
 	if (sc->sc_sme != NULL) {
 		/* all sensors associated with this will also be detached */