small set of qwx bug fixes

Stefan Sperling <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
I found 3 distinct bugs in qwx while looking for causes of qwx instability
across suspend/resume cycles.
Not sure yet if these fixes really make suspend/resume more stable. Across
about a dozen syspend/resume cycles I couldn't trigger problem anymore
with these changes applied. But it might just be down to luck.

1. Ensure that rx_ring->bufs_max and freemap are always initialized correctly.
   If bus_dmamap_create() fails part-way during the loop then we leave
   bufs_max set to a zero value, and the cleanup path won't free any maps.

2. The init task should not wait for the ioctl lock to become available.
   The task could trigger a pointless down/up cycle if the rwlock is
   already contended by ifconfig from userland or by the ACPI thread.

3. Do not ignore errors which occur in qwx_qmi_fw_init_done().
   Any errors were effectively ignored, including DMA allocation failures
   in callees of qwx_qmi_fw_init_done().

ok?

M  sys/dev/ic/qwx.c  |  17+  11-

1 file changed, 17 insertions(+), 11 deletions(-)

commit - 510a3c8701b503ed3d6bbf6e2c2c1d66315cb14c
commit + 8c088bd41d300a603d5464ec5cf71694983b8b78
blob - 4c744b205a09dec658faed98adb3e4e7a7f557be
blob + c2526028507bb5d92bd5d4f6924ac90d01bd51dd
--- sys/dev/ic/qwx.c
+++ sys/dev/ic/qwx.c
@@ -15352,19 +15352,19 @@ qwx_dp_rxdma_ring_buf_setup(struct qwx_softc *sc,
 	    M_DEVBUF, M_NOWAIT | M_ZERO);
 	if (rx_ring->rx_data == NULL)
 		return ENOMEM;
 
+	rx_ring->bufs_max = num_entries;
+	memset(rx_ring->freemap, 0xff, sizeof(rx_ring->freemap));
+
 	for (i = 0; i < num_entries; i++) {
 		struct qwx_rx_data *rx_data = &rx_ring->rx_data[i];
 
 		if (bus_dmamap_create(sc->sc_dmat, DP_RX_BUFFER_SIZE, 1,
 		    DP_RX_BUFFER_SIZE, 0, BUS_DMA_NOWAIT, &rx_data->map))
 			return ENOMEM;
 	}
 
-	rx_ring->bufs_max = num_entries;
-	memset(rx_ring->freemap, 0xff, sizeof(rx_ring->freemap));
-
 	return qwx_dp_rxbufs_replenish(sc, dp->mac_id, rx_ring, num_entries,
 	    sc->hw_params.hal_params->rx_buf_rbm);
 }
 
@@ -20969,9 +20969,9 @@ err_firmware_stop:
 
 	return ret;
 }
 
-void
+int
 qwx_qmi_fw_init_done(struct qwx_softc *sc)
 {
 	int ret = 0;
 
@@ -20982,13 +20982,11 @@ qwx_qmi_fw_init_done(struct qwx_softc *sc)
 	} else {
 		clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
 		clear_bit(ATH11K_FLAG_RECOVERY, sc->sc_flags);
 		ret = qwx_core_qmi_firmware_ready(sc);
-		if (ret) {
-			set_bit(ATH11K_FLAG_QMI_FAIL, sc->sc_flags);
-			return;
-		}
 	}
+
+	return ret;
 }
 
 int
 qwx_qmi_event_server_arrive(struct qwx_softc *sc)
@@ -21046,10 +21044,9 @@ qwx_qmi_event_server_arrive(struct qwx_softc *sc)
 			return -1;
 		}
 	}
 
-	qwx_qmi_fw_init_done(sc);
-	return 0;
+	return qwx_qmi_fw_init_done(sc);
 }
 
 int
 qwx_core_init(struct qwx_softc *sc)
@@ -24322,10 +24319,19 @@ qwx_init_task(void *arg)
 {
 	struct qwx_softc *sc = arg;
 	struct ifnet *ifp = &sc->sc_ic.ic_if;
 	int s = splnet();
-	rw_enter_write(&sc->ioctl_rwl);
 
+	/*
+	 * Do not sleep for this lock. The init task is a one-shot
+	 * recovery mechanism. If the ioctl handler is busy then
+	 * we are being reconfigured or reset already.
+	 */
+	if (rw_enter(&sc->ioctl_rwl, RW_WRITE | RW_NOSLEEP) != 0) {
+		splx(s);
+		return;
+	}
+
 	if (ifp->if_flags & IFF_RUNNING)
 		qwx_stop(ifp);
 
 	if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) == IFF_UP)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.