svn commit: r332432 - head/sys/net

Sean Bruno sbruno at FreeBSD.org
Thu Apr 12 14:35:38 UTC 2018


Author: sbruno
Date: Thu Apr 12 14:35:37 2018
New Revision: 332432
URL: https://svnweb.freebsd.org/changeset/base/332432

Log:
  Restore r332389 after resolution of locking fixes.
  
  Add one extra lock initialization to iflib_register() that was missed
  in the git<->phab conversion.
  
  Split out flag manipulation from general context manipulation in iflib
  
  To avoid blocking on the context lock in the swi thread and risk potential
  deadlocks, this change protects lighter weight updates that only need to
  be consistent with each other with their own lock.
  
  Submitted by:   Matthew Macy <mmacy at mattmacy.io>
  Reviewed by:    shurd
  Sponsored by:   Limelight Networks
  Differential Revision:  https://reviews.freebsd.org/D14967

Modified:
  head/sys/net/iflib.c

Modified: head/sys/net/iflib.c
==============================================================================
--- head/sys/net/iflib.c	Thu Apr 12 14:32:26 2018	(r332431)
+++ head/sys/net/iflib.c	Thu Apr 12 14:35:37 2018	(r332432)
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2014-2017, Matthew Macy <mmacy at mattmacy.io>
+ * Copyright (c) 2014-2018, Matthew Macy <mmacy at mattmacy.io>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -163,7 +163,8 @@ struct iflib_ctx {
 	if_shared_ctx_t ifc_sctx;
 	struct if_softc_ctx ifc_softc_ctx;
 
-	struct mtx ifc_mtx;
+	struct mtx ifc_ctx_mtx;
+	struct mtx ifc_state_mtx;
 
 	uint16_t ifc_nhwtxqs;
 
@@ -317,8 +318,10 @@ typedef struct iflib_sw_tx_desc_array {
 #define	IFC_INIT_DONE		0x020
 #define	IFC_PREFETCH		0x040
 #define	IFC_DO_RESET		0x080
-#define	IFC_CHECK_HUNG		0x100
+#define	IFC_DO_WATCHDOG		0x100
+#define	IFC_CHECK_HUNG		0x200
 
+
 #define CSUM_OFFLOAD		(CSUM_IP_TSO|CSUM_IP6_TSO|CSUM_IP| \
 				 CSUM_IP_UDP|CSUM_IP_TCP|CSUM_IP_SCTP| \
 				 CSUM_IP6_UDP|CSUM_IP6_TCP|CSUM_IP6_SCTP)
@@ -534,13 +537,19 @@ rxd_info_zero(if_rxd_info_t ri)
 
 #define CTX_ACTIVE(ctx) ((if_getdrvflags((ctx)->ifc_ifp) & IFF_DRV_RUNNING))
 
-#define CTX_LOCK_INIT(_sc, _name)  mtx_init(&(_sc)->ifc_mtx, _name, "iflib ctx lock", MTX_DEF)
+#define CTX_LOCK_INIT(_sc, _name)  mtx_init(&(_sc)->ifc_ctx_mtx, _name, "iflib ctx lock", MTX_DEF)
+#define CTX_LOCK(ctx) mtx_lock(&(ctx)->ifc_ctx_mtx)
+#define CTX_UNLOCK(ctx) mtx_unlock(&(ctx)->ifc_ctx_mtx)
+#define CTX_LOCK_DESTROY(ctx) mtx_destroy(&(ctx)->ifc_ctx_mtx)
 
-#define CTX_LOCK(ctx) mtx_lock(&(ctx)->ifc_mtx)
-#define CTX_UNLOCK(ctx) mtx_unlock(&(ctx)->ifc_mtx)
-#define CTX_LOCK_DESTROY(ctx) mtx_destroy(&(ctx)->ifc_mtx)
 
+#define STATE_LOCK_INIT(_sc, _name)  mtx_init(&(_sc)->ifc_state_mtx, _name, "iflib state lock", MTX_DEF)
+#define STATE_LOCK(ctx) mtx_lock(&(ctx)->ifc_state_mtx)
+#define STATE_UNLOCK(ctx) mtx_unlock(&(ctx)->ifc_state_mtx)
+#define STATE_LOCK_DESTROY(ctx) mtx_destroy(&(ctx)->ifc_state_mtx)
 
+
+
 #define CALLOUT_LOCK(txq)	mtx_lock(&txq->ift_mtx)
 #define CALLOUT_UNLOCK(txq) 	mtx_unlock(&txq->ift_mtx)
 
@@ -2143,18 +2152,14 @@ iflib_timer(void *arg)
 	if (if_getdrvflags(ctx->ifc_ifp) & IFF_DRV_RUNNING) 
 		callout_reset_on(&txq->ift_timer, hz/2, iflib_timer, txq, txq->ift_timer.c_cpu);
 	return;
-hung:
-	CTX_LOCK(ctx);
-	if_setdrvflagbits(ctx->ifc_ifp, IFF_DRV_OACTIVE, IFF_DRV_RUNNING);
+ hung:
 	device_printf(ctx->ifc_dev,  "TX(%d) desc avail = %d, pidx = %d\n",
 				  txq->ift_id, TXQ_AVAIL(txq), txq->ift_pidx);
-
-	IFDI_WATCHDOG_RESET(ctx);
-	ctx->ifc_watchdog_events++;
-
-	ctx->ifc_flags |= IFC_DO_RESET;
+	STATE_LOCK(ctx);
+	if_setdrvflagbits(ctx->ifc_ifp, IFF_DRV_OACTIVE, IFF_DRV_RUNNING);
+	ctx->ifc_flags |= (IFC_DO_WATCHDOG|IFC_DO_RESET);
 	iflib_admin_intr_deferred(ctx);
-	CTX_UNLOCK(ctx);
+	STATE_UNLOCK(ctx);
 }
 
 static void
@@ -2672,10 +2677,10 @@ iflib_rxeof(iflib_rxq_t rxq, qidx_t budget)
 		return true;
 	return (iflib_rxd_avail(ctx, rxq, *cidxp, 1));
 err:
-	CTX_LOCK(ctx);
+	STATE_LOCK(ctx);
 	ctx->ifc_flags |= IFC_DO_RESET;
 	iflib_admin_intr_deferred(ctx);
-	CTX_UNLOCK(ctx);
+	STATE_UNLOCK(ctx);
 	return (false);
 }
 
@@ -3705,27 +3710,35 @@ _task_fn_admin(void *context)
 	if_softc_ctx_t sctx = &ctx->ifc_softc_ctx;
 	iflib_txq_t txq;
 	int i;
+	bool oactive, running, do_reset, do_watchdog;
 
-	if (!(if_getdrvflags(ctx->ifc_ifp) & IFF_DRV_RUNNING)) {
-		if (!(if_getdrvflags(ctx->ifc_ifp) & IFF_DRV_OACTIVE)) {
-			return;
-		}
-	}
+	STATE_LOCK(ctx);
+	running = (if_getdrvflags(ctx->ifc_ifp) & IFF_DRV_RUNNING);
+	oactive = (if_getdrvflags(ctx->ifc_ifp) & IFF_DRV_OACTIVE);
+	do_reset = (ctx->ifc_flags & IFC_DO_RESET);
+	do_watchdog = (ctx->ifc_flags & IFC_DO_WATCHDOG);
+	ctx->ifc_flags &= ~(IFC_DO_RESET|IFC_DO_WATCHDOG);
+	STATE_UNLOCK(ctx);
 
+	if (!running & !oactive)
+		return;
+
 	CTX_LOCK(ctx);
 	for (txq = ctx->ifc_txqs, i = 0; i < sctx->isc_ntxqsets; i++, txq++) {
 		CALLOUT_LOCK(txq);
 		callout_stop(&txq->ift_timer);
 		CALLOUT_UNLOCK(txq);
 	}
+	if (do_watchdog) {
+		ctx->ifc_watchdog_events++;
+		IFDI_WATCHDOG_RESET(ctx);
+	}
 	IFDI_UPDATE_ADMIN_STATUS(ctx);
 	for (txq = ctx->ifc_txqs, i = 0; i < sctx->isc_ntxqsets; i++, txq++)
 		callout_reset_on(&txq->ift_timer, hz/2, iflib_timer, txq, txq->ift_timer.c_cpu);
 	IFDI_LINK_INTR_ENABLE(ctx);
-	if (ctx->ifc_flags & IFC_DO_RESET) {
-		ctx->ifc_flags &= ~IFC_DO_RESET;
+	if (do_reset)
 		iflib_if_init_locked(ctx);
-	}
 	CTX_UNLOCK(ctx);
 
 	if (LINK_ACTIVE(ctx) == 0)
@@ -3869,15 +3882,15 @@ iflib_if_qflush(if_t ifp)
 	iflib_txq_t txq = ctx->ifc_txqs;
 	int i;
 
-	CTX_LOCK(ctx);
+	STATE_LOCK(ctx);
 	ctx->ifc_flags |= IFC_QFLUSH;
-	CTX_UNLOCK(ctx);
+	STATE_UNLOCK(ctx);
 	for (i = 0; i < NTXQSETS(ctx); i++, txq++)
 		while (!(ifmp_ring_is_idle(txq->ift_br) || ifmp_ring_is_stalled(txq->ift_br)))
 			iflib_txq_check_drain(txq, 0);
-	CTX_LOCK(ctx);
+	STATE_LOCK(ctx);
 	ctx->ifc_flags &= ~IFC_QFLUSH;
-	CTX_UNLOCK(ctx);
+	STATE_UNLOCK(ctx);
 
 	if_qflush(ifp);
 }
@@ -3934,14 +3947,18 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data)
 		iflib_stop(ctx);
 
 		if ((err = IFDI_MTU_SET(ctx, ifr->ifr_mtu)) == 0) {
+			STATE_LOCK(ctx);
 			if (ifr->ifr_mtu > ctx->ifc_max_fl_buf_size)
 				ctx->ifc_flags |= IFC_MULTISEG;
 			else
 				ctx->ifc_flags &= ~IFC_MULTISEG;
+			STATE_UNLOCK(ctx);
 			err = if_setmtu(ifp, ifr->ifr_mtu);
 		}
 		iflib_init_locked(ctx);
+		STATE_LOCK(ctx);
 		if_setdrvflags(ifp, bits);
+		STATE_UNLOCK(ctx);
 		CTX_UNLOCK(ctx);
 		break;
 	case SIOCSIFFLAGS:
@@ -4025,10 +4042,14 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data)
 			bits = if_getdrvflags(ifp);
 			if (bits & IFF_DRV_RUNNING)
 				iflib_stop(ctx);
+			STATE_LOCK(ctx);
 			if_togglecapenable(ifp, setmask);
+			STATE_UNLOCK(ctx);
 			if (bits & IFF_DRV_RUNNING)
 				iflib_init_locked(ctx);
+			STATE_LOCK(ctx);
 			if_setdrvflags(ifp, bits);
+			STATE_UNLOCK(ctx);
 			CTX_UNLOCK(ctx);
 		}
 		break;
@@ -4683,6 +4704,7 @@ iflib_register(if_ctx_t ctx)
 
 	CTX_LOCK_INIT(ctx, device_get_nameunit(ctx->ifc_dev));
 
+	STATE_LOCK_INIT(ctx, device_get_nameunit(ctx->ifc_dev));
 	ifp = ctx->ifc_ifp = if_gethandle(IFT_ETHER);
 	if (ifp == NULL) {
 		device_printf(dev, "can not allocate ifnet structure\n");
@@ -5431,9 +5453,11 @@ iflib_link_state_change(if_ctx_t ctx, int link_state, 
 	iflib_txq_t txq = ctx->ifc_txqs;
 
 	if_setbaudrate(ifp, baudrate);
-	if (baudrate >= IF_Gbps(10))
+	if (baudrate >= IF_Gbps(10)) {
+		STATE_LOCK(ctx);
 		ctx->ifc_flags |= IFC_PREFETCH;
-
+		STATE_UNLOCK(ctx);
+	}
 	/* If link down, disable watchdog */
 	if ((ctx->ifc_link_state == LINK_STATE_UP) && (link_state == LINK_STATE_DOWN)) {
 		for (int i = 0; i < ctx->ifc_softc_ctx.isc_ntxqsets; i++, txq++)
@@ -5492,7 +5516,7 @@ struct mtx *
 iflib_ctx_lock_get(if_ctx_t ctx)
 {
 
-	return (&ctx->ifc_mtx);
+	return (&ctx->ifc_ctx_mtx);
 }
 
 static int


More information about the svn-src-all mailing list