Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Jul 2019 19:54:38 +0000 (UTC)
From:      Vincenzo Maffione <vmaffione@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r349692 - stable/12/usr.sbin/bhyve
Message-ID:  <201907031954.x63Jsc7F080046@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: vmaffione
Date: Wed Jul  3 19:54:37 2019
New Revision: 349692
URL: https://svnweb.freebsd.org/changeset/base/349692

Log:
  MFC r348834
  
  bhyve: vtnet: simplify thread synchronization
  
  On vtnet device reset it is necessary to wait for threads to stop TX and
  RX processing. However, the rx_in_progress variable (used for to wait for
  RX processing to stop) is actually useless, and can be removed. Acquiring
  and releasing the RX lock is enough to synchronize correctly. Moreover,
  it is possible to reset the device while holding both TX and RX locks, so
  that the "resetting" variable becomes unnecessary for the RX thread, and
  can be protected by the TX lock (instead of being volatile).
  
  Reviewed by:    jhb, markj
  Differential Revision:  https://reviews.freebsd.org/D20543

Modified:
  stable/12/usr.sbin/bhyve/pci_virtio_net.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/usr.sbin/bhyve/pci_virtio_net.c
==============================================================================
--- stable/12/usr.sbin/bhyve/pci_virtio_net.c	Wed Jul  3 19:52:24 2019	(r349691)
+++ stable/12/usr.sbin/bhyve/pci_virtio_net.c	Wed Jul  3 19:54:37 2019	(r349692)
@@ -147,14 +147,13 @@ struct pci_vtnet_softc {
 	struct nm_desc	*vsc_nmd;
 
 	int		vsc_rx_ready;
-	volatile int	resetting;	/* set and checked outside lock */
+	int		resetting;	/* protected by tx_mtx */
 
 	uint64_t	vsc_features;	/* negotiated features */
 	
 	struct virtio_net_config vsc_config;
 
 	pthread_mutex_t	rx_mtx;
-	int		rx_in_progress;
 	int		rx_vhdrlen;
 	int		rx_merge;	/* merged rx bufs in use */
 
@@ -186,62 +185,39 @@ static struct virtio_consts vtnet_vi_consts = {
 	VTNET_S_HOSTCAPS,	/* our capabilities */
 };
 
-/*
- * If the transmit thread is active then stall until it is done.
- */
 static void
-pci_vtnet_txwait(struct pci_vtnet_softc *sc)
+pci_vtnet_reset(void *vsc)
 {
+	struct pci_vtnet_softc *sc = vsc;
 
+	DPRINTF(("vtnet: device reset requested !\n"));
+
+	/* Acquire the RX lock to block RX processing. */
+	pthread_mutex_lock(&sc->rx_mtx);
+
+	/* Set sc->resetting and give a chance to the TX thread to stop. */
 	pthread_mutex_lock(&sc->tx_mtx);
+	sc->resetting = 1;
 	while (sc->tx_in_progress) {
 		pthread_mutex_unlock(&sc->tx_mtx);
 		usleep(10000);
 		pthread_mutex_lock(&sc->tx_mtx);
 	}
-	pthread_mutex_unlock(&sc->tx_mtx);
-}
 
-/*
- * If the receive thread is active then stall until it is done.
- */
-static void
-pci_vtnet_rxwait(struct pci_vtnet_softc *sc)
-{
-
-	pthread_mutex_lock(&sc->rx_mtx);
-	while (sc->rx_in_progress) {
-		pthread_mutex_unlock(&sc->rx_mtx);
-		usleep(10000);
-		pthread_mutex_lock(&sc->rx_mtx);
-	}
-	pthread_mutex_unlock(&sc->rx_mtx);
-}
-
-static void
-pci_vtnet_reset(void *vsc)
-{
-	struct pci_vtnet_softc *sc = vsc;
-
-	DPRINTF(("vtnet: device reset requested !\n"));
-
-	sc->resetting = 1;
-
-	/*
-	 * Wait for the transmit and receive threads to finish their
-	 * processing.
-	 */
-	pci_vtnet_txwait(sc);
-	pci_vtnet_rxwait(sc);
-
 	sc->vsc_rx_ready = 0;
 	sc->rx_merge = 1;
 	sc->rx_vhdrlen = sizeof(struct virtio_net_rxhdr);
 
-	/* now reset rings, MSI-X vectors, and negotiated capabilities */
+	/*
+	 * Now reset rings, MSI-X vectors, and negotiated capabilities.
+	 * Do that with the TX lock held, since we need to reset
+	 * sc->resetting.
+	 */
 	vi_reset_dev(&sc->vsc_vs);
 
 	sc->resetting = 0;
+	pthread_mutex_unlock(&sc->tx_mtx);
+	pthread_mutex_unlock(&sc->rx_mtx);
 }
 
 /*
@@ -315,9 +291,9 @@ pci_vtnet_tap_rx(struct pci_vtnet_softc *sc)
 
 	/*
 	 * But, will be called when the rx ring hasn't yet
-	 * been set up or the guest is resetting the device.
+	 * been set up.
 	 */
-	if (!sc->vsc_rx_ready || sc->resetting) {
+	if (!sc->vsc_rx_ready) {
 		/*
 		 * Drop the packet and try later.
 		 */
@@ -512,9 +488,9 @@ pci_vtnet_netmap_rx(struct pci_vtnet_softc *sc)
 
 	/*
 	 * But, will be called when the rx ring hasn't yet
-	 * been set up or the guest is resetting the device.
+	 * been set up.
 	 */
-	if (!sc->vsc_rx_ready || sc->resetting) {
+	if (!sc->vsc_rx_ready) {
 		/*
 		 * Drop the packet and try later.
 		 */
@@ -591,9 +567,7 @@ pci_vtnet_rx_callback(int fd, enum ev_type type, void 
 	struct pci_vtnet_softc *sc = param;
 
 	pthread_mutex_lock(&sc->rx_mtx);
-	sc->rx_in_progress = 1;
 	sc->pci_vtnet_rx(sc);
-	sc->rx_in_progress = 0;
 	pthread_mutex_unlock(&sc->rx_mtx);
 
 }
@@ -918,7 +892,6 @@ pci_vtnet_init(struct vmctx *ctx, struct pci_devinst *
 
 	sc->rx_merge = 1;
 	sc->rx_vhdrlen = sizeof(struct virtio_net_rxhdr);
-	sc->rx_in_progress = 0;
 	pthread_mutex_init(&sc->rx_mtx, NULL); 
 
 	/* 



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