From owner-svn-src-head@freebsd.org Wed Apr 11 17:44:55 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5B539F82FF9; Wed, 11 Apr 2018 17:44:55 +0000 (UTC) (envelope-from kmacybsd@gmail.com) Received: from mail-ot0-x22e.google.com (mail-ot0-x22e.google.com [IPv6:2607:f8b0:4003:c0f::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E20616FBBD; Wed, 11 Apr 2018 17:44:54 +0000 (UTC) (envelope-from kmacybsd@gmail.com) Received: by mail-ot0-x22e.google.com with SMTP id v64-v6so2930662otb.13; Wed, 11 Apr 2018 10:44:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=qevC9bhbDpWH0NvX8MZ/TXaBINoFBiE4uEKN0V6mDVE=; b=WME9EQL7572OO0rJvQT795WFlggbOuoqcXNnFLFQ3tRnFbYRoUEzuGOr1jPPF2PAyy a9XTi4GthOOw1UJ9Ykh3wP6bkJc3gIiwqs6F7VyXJFOIPqQDMBtHJ8PTAzk7mpzBmR2R 5+jZkpYzjxrqZ2kn/7NXD3qursAxCjutkMarEOKnn6UB75Rpxw1SnzteRjJGdB4AWv8S kZGIVGfbv1BhLwN7lBcJ8ZDZl2jBhRgfU4xfJIhA790ms95zQYK+s233BFrnLssC1yfh 5E20RuqztxaoXU0h+OwfDtDxSNxZOppKRKDTj1L5lSDqS54phn57VbuUv/6u0OC0KbkG IHUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=qevC9bhbDpWH0NvX8MZ/TXaBINoFBiE4uEKN0V6mDVE=; b=ZUkiZ0YAuXHNPnNHpzWksmGosJVjwD9xWi85jFFAeilI9wanK2tJ6YYSW+WfsGv05a M6nevYFFwqTRjW7u0QlnMeNms24SmZK1pDeKOAg6H8o6HnULv4+52CCTCNgEh5jVd7uP d8namyw254tLRPcQ9n8ERVyXZklrPVy2L6eDzg/a+OlMQSYceSE7IkaPPV85fXHAkox0 fwtJoeopcxh2jRl0IjUDjsKYzZaFyRbXVpOjGR4+fFTo+Hkkb8wn1o7kx/6f7bnkOmnM Y1uijyqVb01VWf2AkXF22px3Mlst0sngQJ9FYwGMh368gBt67VALYNVj6ycSP+YB1Xse 40+g== X-Gm-Message-State: ALQs6tDtZvvzQTskUAOwflvitIjGgQUU9RgvUK972vlzcx+bnwGxSWyn aA6ZWD8YpbX8ZqxIXZib6BLV/OriTN4gqYvrMBRByQ== X-Google-Smtp-Source: AIpwx49yHPCrYvqsHlBQbZtFBMoTbi/4XtczxorW4yXdaI4AeEQgYv5Co5SSD5aCJEPapFOMqwFv7pBSXfqZvaPVnrE= X-Received: by 2002:a9d:7388:: with SMTP id j8-v6mr3629887otk.29.1523468693811; Wed, 11 Apr 2018 10:44:53 -0700 (PDT) MIME-Version: 1.0 Sender: kmacybsd@gmail.com Received: by 2002:a9d:4782:0:0:0:0:0 with HTTP; Wed, 11 Apr 2018 10:44:53 -0700 (PDT) In-Reply-To: References: <201804101948.w3AJmOt2066564@repo.freebsd.org> From: "K. Macy" Date: Wed, 11 Apr 2018 10:44:53 -0700 X-Google-Sender-Auth: lC7z7--nIyxsWSiHqKjFdHu9qRg Message-ID: Subject: Re: svn commit: r332389 - head/sys/net To: "Jonathan T. Looney" Cc: Stephen Hurd , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Apr 2018 17:44:55 -0000 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 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 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 >> 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 >> + * Copyright (c) 2014-2018, Matthew Macy >> * 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@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/svn-src-head > To unsubscribe, send any mail to "svn-src-head-unsubscribe@freebsd.org"