Re: CVS commit: src/sys [freeze on boot]

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

Hi.

On 2020/01/21 2:06, Patrick Welche wrote:
> On Mon, Jan 20, 2020 at 04:12:45PM +0000, Patrick Welche wrote:
>> On Mon, Jan 20, 2020 at 12:51:00PM +0000, Andrew Doran wrote:
>>> This also happened the last time I touched rw_downgrade(), and I backed out
>>> the change then, but both times I don't see the bug.  I have some questions:
>>>
>>> - Are you running DIAGNOSTIC and/or LOCKDEBUG?  I would be very interested
>>>   to see what happens with a LOCKDEBUG kernel here.
>>
>> One worked with the addition of LOCKDEBUG. The other didn't, but it seems
>> to be unrelated:
>>
>> db{0}> show panic
>> Panic string: mutex_vector_enter,510: uninitialized lock (lock=0xffffbd012366609
>> 0, from=ffffffff8033dc9d)
>> bt
>> breakpoint() at netbsd:breakpoint+0x5
>> vpanic() at netbsd:vpanic+0x178
>> snprintf() at netbsd:snprintf
>> lockdebug_wantlock() at netbsd:lockdebug_wantlock+0x166
>> mutex_enter() at netbsd:mutex_enter+0x37c
>> ixgbe_getext() at netbsd:ixgbe_getext+0x1d
> 
> ixgbe_getext does mutex_enter(&eh->eh_mtx) but...


I suspect the location of your panic is after the following message
(because of ixgbe_allocate_msix()'s failure):

>         aprint_normal(" ETrackID %08x\n", ((uint32_t)high << 16) | low);

If so, could you try the following diff?

------------------
 Fix the freeing code for some error paths. Found by Patrick Welche.

Index: ix_txrx.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/ixgbe/ix_txrx.c,v
retrieving revision 1.59
diff -u -p -r1.59 ix_txrx.c
--- ix_txrx.c	20 Jan 2020 07:19:04 -0000	1.59
+++ ix_txrx.c	21 Jan 2020 06:24:16 -0000
@@ -2353,3 +2353,24 @@ err_tx_desc:
 	free(adapter->queues, M_DEVBUF);
 	return (error);
 } /* ixgbe_allocate_queues */
+
+/************************************************************************
+ * ixgbe_free_queues
+ *
+ *   Free descriptors for the transmit and receive rings, and then
+ *   the memory associated with each.
+ ************************************************************************/
+void
+ixgbe_free_queues(struct adapter *adapter)
+{
+	struct ix_queue *que;
+	int i;
+
+	ixgbe_free_transmit_structures(adapter);
+	ixgbe_free_receive_structures(adapter);
+	for (i = 0; i < adapter->num_queues; i++) {
+		que = &adapter->queues[i];
+		mutex_destroy(&que->dc_mtx);
+	}
+	free(adapter->queues, M_DEVBUF);
+} /* ixgbe_free_queues */
Index: ixgbe.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/ixgbe/ixgbe.c,v
retrieving revision 1.220
diff -u -p -r1.220 ixgbe.c
--- ixgbe.c	3 Jan 2020 12:59:46 -0000	1.220
+++ ixgbe.c	21 Jan 2020 06:24:16 -0000
@@ -1059,9 +1059,7 @@ ixgbe_attach(device_t parent, device_t d
 		error = ixgbe_allocate_msix(adapter, pa);
 		if (error) {
 			/* Free allocated queue structures first */
-			ixgbe_free_transmit_structures(adapter);
-			ixgbe_free_receive_structures(adapter);
-			free(adapter->queues, M_DEVBUF);
+			ixgbe_free_queues(adapter);
 
 			/* Fallback to legacy interrupt */
 			adapter->feat_en &= ~IXGBE_FEATURE_MSIX;
@@ -1236,9 +1234,7 @@ ixgbe_attach(device_t parent, device_t d
 	return;
 
 err_late:
-	ixgbe_free_transmit_structures(adapter);
-	ixgbe_free_receive_structures(adapter);
-	free(adapter->queues, M_DEVBUF);
+	ixgbe_free_queues(adapter);
 err_out:
 	ctrl_ext = IXGBE_READ_REG(&adapter->hw, IXGBE_CTRL_EXT);
 	ctrl_ext &= ~IXGBE_CTRL_EXT_DRV_LOAD;
@@ -3712,13 +3708,7 @@ ixgbe_detach(device_t dev, int flags)
 	evcnt_detach(&stats->ptc1023);
 	evcnt_detach(&stats->ptc1522);
 
-	ixgbe_free_transmit_structures(adapter);
-	ixgbe_free_receive_structures(adapter);
-	for (i = 0; i < adapter->num_queues; i++) {
-		struct ix_queue * que = &adapter->queues[i];
-		mutex_destroy(&que->dc_mtx);
-	}
-	free(adapter->queues, M_DEVBUF);
+	ixgbe_free_queues(adapter);
 	free(adapter->mta, M_DEVBUF);
 
 	IXGBE_CORE_LOCK_DESTROY(adapter);
Index: ixgbe.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/ixgbe/ixgbe.h,v
retrieving revision 1.61
diff -u -p -r1.61 ixgbe.h
--- ixgbe.h	20 Jan 2020 07:19:04 -0000	1.61
+++ ixgbe.h	21 Jan 2020 06:24:16 -0000
@@ -768,6 +768,7 @@ void ixgbe_deferred_mq_start_work(struct
 void ixgbe_drain_all(struct adapter *);
 
 int  ixgbe_allocate_queues(struct adapter *);
+void ixgbe_free_queues(struct adapter *);
 int  ixgbe_setup_transmit_structures(struct adapter *);
 void ixgbe_free_transmit_structures(struct adapter *);
 int  ixgbe_setup_receive_structures(struct adapter *);
Index: ixgbe_netbsd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/ixgbe/ixgbe_netbsd.c,v
retrieving revision 1.11
diff -u -p -r1.11 ixgbe_netbsd.c
--- ixgbe_netbsd.c	20 Jan 2020 07:19:04 -0000	1.11
+++ ixgbe_netbsd.c	21 Jan 2020 06:24:16 -0000
@@ -225,10 +225,10 @@ ixgbe_jcl_destroy(struct adapter *adapte
 {
 	ixgbe_extmem_head_t *eh = &rxr->jcl_head;
 
-	/* Free all dmamem */
-	ixgbe_jcl_freeall(adapter, rxr);
-
 	if (eh->eh_initialized) {
+		/* Free all dmamem */
+		ixgbe_jcl_freeall(adapter, rxr);
+
 		mutex_destroy(&eh->eh_mtx);
 		eh->eh_initialized = false;
 	}
Index: ixv.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/ixgbe/ixv.c,v
retrieving revision 1.143
diff -u -p -r1.143 ixv.c
--- ixv.c	17 Dec 2019 05:49:01 -0000	1.143
+++ ixv.c	21 Jan 2020 06:24:17 -0000
@@ -547,9 +547,7 @@ ixv_attach(device_t parent, device_t dev
 	return;
 
 err_late:
-	ixgbe_free_transmit_structures(adapter);
-	ixgbe_free_receive_structures(adapter);
-	free(adapter->queues, M_DEVBUF);
+	ixgbe_free_queues(adapter);
 err_out:
 	ixv_free_pci_resources(adapter);
 	IXGBE_CORE_LOCK_DESTROY(adapter);
@@ -674,13 +672,7 @@ ixv_detach(device_t dev, int flags)
 	evcnt_detach(&hw->mbx.stats.reqs);
 	evcnt_detach(&hw->mbx.stats.rsts);
 
-	ixgbe_free_transmit_structures(adapter);
-	ixgbe_free_receive_structures(adapter);
-	for (int i = 0; i < adapter->num_queues; i++) {
-		struct ix_queue *lque = &adapter->queues[i];
-		mutex_destroy(&lque->dc_mtx);
-	}
-	free(adapter->queues, M_DEVBUF);
+	ixgbe_free_queues(adapter);
 
 	IXGBE_CORE_LOCK_DESTROY(adapter);
 
------------------

The same diff is at:
	 http://www.netbsd.org/~msaitoh/ixgbe-20200121-0.dif

-- 
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)