svn commit: r332389 - head/sys/net
K. Macy
kmacy at freebsd.org
Wed Apr 11 17:44:55 UTC 2018
Yup. git<->phab patch update fail. Maybe some day we can move to git.
-M
On Wed, Apr 11, 2018 at 10:12 AM, Jonathan T. Looney <jtl at freebsd.org> wrote:
> It appears this is causing panics (see the emails on freebsd-current@).
>
> From a brief glance, it appears that the new STATE_LOCK macros is used (and
> causes panics), but the STATE_LOCK_INIT macro is not used. Is it possible
> the code is missing an initialization call for the new mutex?
>
> Jonathan
>
> On Tue, Apr 10, 2018 at 3:48 PM, Stephen Hurd <shurd at freebsd.org> wrote:
>
>> Author: shurd
>> Date: Tue Apr 10 19:48:24 2018
>> New Revision: 332389
>> URL: https://svnweb.freebsd.org/changeset/base/332389
>>
>> Log:
>> 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 Tue Apr 10 19:42:50 2018 (r332388)
>> +++ head/sys/net/iflib.c Tue Apr 10 19:48:24 2018 (r332389)
>> @@ -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;
>> uint16_t ifc_nhwrxqs;
>> @@ -318,8 +319,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)
>> @@ -535,13 +538,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)
>>
>> @@ -2144,18 +2153,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
>> @@ -2673,10 +2678,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);
>> }
>>
>> @@ -3706,27 +3711,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)
>> @@ -3870,15 +3883,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);
>> }
>> @@ -3935,14 +3948,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:
>> @@ -4026,10 +4043,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;
>> @@ -5431,9 +5452,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 +5515,7 @@ struct mtx *
>> iflib_ctx_lock_get(if_ctx_t ctx)
>> {
>>
>> - return (&ctx->ifc_mtx);
>> + return (&ctx->ifc_ctx_mtx);
>> }
>>
>> static int
>>
>>
> _______________________________________________
> svn-src-head at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> To unsubscribe, send any mail to "svn-src-head-unsubscribe at freebsd.org"
More information about the svn-src-head
mailing list