Date: Mon, 7 Oct 2019 10:09:27 +0000 (UTC) From: Hans Petter Selasky <hselasky@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: r353251 - stable/12/sys/dev/mlx5/mlx5_en Message-ID: <201910071009.x97A9RhY012079@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: hselasky Date: Mon Oct 7 10:09:26 2019 New Revision: 353251 URL: https://svnweb.freebsd.org/changeset/base/353251 Log: MFC r352989: Seal transmit path with regards to using destroyed mutex in mlx5en(4). It may happen during link down that the running state may be observed non-zero in the transmit routine, right before the running state is cleared. This may end up using a destroyed mutex. Make all channel mutexes and callouts persistant. Preserve receive and send queue statistics during link toggle. Sponsored by: Mellanox Technologies Modified: stable/12/sys/dev/mlx5/mlx5_en/en.h stable/12/sys/dev/mlx5/mlx5_en/mlx5_en_main.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/dev/mlx5/mlx5_en/en.h ============================================================================== --- stable/12/sys/dev/mlx5/mlx5_en/en.h Mon Oct 7 09:58:36 2019 (r353250) +++ stable/12/sys/dev/mlx5/mlx5_en/en.h Mon Oct 7 10:09:26 2019 (r353251) @@ -138,6 +138,10 @@ #define MLX5E_100MB (100000) #define MLX5E_1GB (1000000) +#define MLX5E_ZERO(ptr, field) \ + memset(&(ptr)->field, 0, \ + sizeof(*(ptr)) - __offsetof(__typeof(*(ptr)), field)) + MALLOC_DECLARE(M_MLX5EN); struct mlx5_core_dev; @@ -740,15 +744,18 @@ struct mlx5e_rq_mbuf { }; struct mlx5e_rq { + /* persistant fields */ + struct mtx mtx; + struct mlx5e_rq_stats stats; + /* data path */ +#define mlx5e_rq_zero_start wq struct mlx5_wq_ll wq; - struct mtx mtx; bus_dma_tag_t dma_tag; u32 wqe_sz; u32 nsegs; struct mlx5e_rq_mbuf *mbuf; struct ifnet *ifp; - struct mlx5e_rq_stats stats; struct mlx5e_cq cq; struct lro_ctrl lro; volatile int enabled; @@ -782,11 +789,15 @@ struct mlx5e_snd_tag { }; struct mlx5e_sq { - /* data path */ + /* persistant fields */ struct mtx lock; - bus_dma_tag_t dma_tag; struct mtx comp_lock; + struct mlx5e_sq_stats stats; + /* data path */ +#define mlx5e_sq_zero_start dma_tag + bus_dma_tag_t dma_tag; + /* dirtied @completion */ u16 cc; @@ -805,7 +816,6 @@ struct mlx5e_sq { u32 d32[2]; u64 d64; } doorbell; - struct mlx5e_sq_stats stats; struct mlx5e_cq cq; @@ -858,13 +868,9 @@ mlx5e_sq_queue_level(struct mlx5e_sq *sq) } struct mlx5e_channel { - /* data path */ struct mlx5e_rq rq; struct mlx5e_snd_tag tag; struct mlx5e_sq sq[MLX5E_MAX_TX_NUM_TC]; - u8 num_tc; - - /* control */ struct mlx5e_priv *priv; int ix; } __aligned(MLX5E_CACHELINE_SIZE); Modified: stable/12/sys/dev/mlx5/mlx5_en/mlx5_en_main.c ============================================================================== --- stable/12/sys/dev/mlx5/mlx5_en/mlx5_en_main.c Mon Oct 7 09:58:36 2019 (r353250) +++ stable/12/sys/dev/mlx5/mlx5_en/mlx5_en_main.c Mon Oct 7 10:09:26 2019 (r353251) @@ -1480,8 +1480,6 @@ mlx5e_close_rq(struct mlx5e_rq *rq) callout_stop(&rq->watchdog); mtx_unlock(&rq->mtx); - callout_drain(&rq->watchdog); - mlx5e_modify_rq(rq, MLX5_RQC_STATE_RDY, MLX5_RQC_STATE_ERR); } @@ -1563,7 +1561,7 @@ mlx5e_refresh_sq_inline_sub(struct mlx5e_priv *priv, s { int i; - for (i = 0; i != c->num_tc; i++) { + for (i = 0; i != priv->num_tc; i++) { mtx_lock(&c->sq[i].lock); mlx5e_update_sq_inline(&c->sq[i]); mtx_unlock(&c->sq[i].lock); @@ -1750,6 +1748,12 @@ mlx5e_open_sq(struct mlx5e_channel *c, { int err; + sq->cev_factor = c->priv->params_ethtool.tx_completion_fact; + + /* ensure the TX completion event factor is not zero */ + if (sq->cev_factor == 0) + sq->cev_factor = 1; + err = mlx5e_create_sq(c, tc, param, sq); if (err) return (err); @@ -1861,9 +1865,6 @@ mlx5e_drain_sq(struct mlx5e_sq *sq) mlx5e_sq_send_nops_locked(sq, 1); mtx_unlock(&sq->lock); - /* make sure it is safe to free the callout */ - callout_drain(&sq->cev_callout); - /* wait till SQ is empty or link is down */ mtx_lock(&sq->lock); while (sq->cc != sq->pc && @@ -2048,7 +2049,7 @@ mlx5e_open_tx_cqs(struct mlx5e_channel *c, int err; int tc; - for (tc = 0; tc < c->num_tc; tc++) { + for (tc = 0; tc < c->priv->num_tc; tc++) { /* open completion queue */ err = mlx5e_open_cq(c->priv, &cparam->tx_cq, &c->sq[tc].cq, &mlx5e_tx_cq_comp, c->ix); @@ -2069,7 +2070,7 @@ mlx5e_close_tx_cqs(struct mlx5e_channel *c) { int tc; - for (tc = 0; tc < c->num_tc; tc++) + for (tc = 0; tc < c->priv->num_tc; tc++) mlx5e_close_cq(&c->sq[tc].cq); } @@ -2080,7 +2081,7 @@ mlx5e_open_sqs(struct mlx5e_channel *c, int err; int tc; - for (tc = 0; tc < c->num_tc; tc++) { + for (tc = 0; tc < c->priv->num_tc; tc++) { err = mlx5e_open_sq(c, tc, &cparam->sq, &c->sq[tc]); if (err) goto err_close_sqs; @@ -2100,20 +2101,28 @@ mlx5e_close_sqs_wait(struct mlx5e_channel *c) { int tc; - for (tc = 0; tc < c->num_tc; tc++) + for (tc = 0; tc < c->priv->num_tc; tc++) mlx5e_close_sq_wait(&c->sq[tc]); } static void -mlx5e_chan_mtx_init(struct mlx5e_channel *c) +mlx5e_chan_static_init(struct mlx5e_priv *priv, struct mlx5e_channel *c, int ix) { int tc; + /* setup priv and channel number */ + c->priv = priv; + c->ix = ix; + + /* setup send tag */ + c->tag.m_snd_tag.ifp = priv->ifp; + c->tag.type = IF_SND_TAG_TYPE_UNLIMITED; + mtx_init(&c->rq.mtx, "mlx5rx", MTX_NETWORK_LOCK, MTX_DEF); callout_init_mtx(&c->rq.watchdog, &c->rq.mtx, 0); - for (tc = 0; tc < c->num_tc; tc++) { + for (tc = 0; tc != MLX5E_MAX_TX_NUM_TC; tc++) { struct mlx5e_sq *sq = c->sq + tc; mtx_init(&sq->lock, "mlx5tx", @@ -2122,47 +2131,37 @@ mlx5e_chan_mtx_init(struct mlx5e_channel *c) MTX_NETWORK_LOCK " TX", MTX_DEF); callout_init_mtx(&sq->cev_callout, &sq->lock, 0); - - sq->cev_factor = c->priv->params_ethtool.tx_completion_fact; - - /* ensure the TX completion event factor is not zero */ - if (sq->cev_factor == 0) - sq->cev_factor = 1; } } static void -mlx5e_chan_mtx_destroy(struct mlx5e_channel *c) +mlx5e_chan_static_destroy(struct mlx5e_channel *c) { int tc; + callout_drain(&c->rq.watchdog); + mtx_destroy(&c->rq.mtx); - for (tc = 0; tc < c->num_tc; tc++) { + for (tc = 0; tc != MLX5E_MAX_TX_NUM_TC; tc++) { + callout_drain(&c->sq[tc].cev_callout); mtx_destroy(&c->sq[tc].lock); mtx_destroy(&c->sq[tc].comp_lock); } } static int -mlx5e_open_channel(struct mlx5e_priv *priv, int ix, +mlx5e_open_channel(struct mlx5e_priv *priv, struct mlx5e_channel_param *cparam, struct mlx5e_channel *c) { - int err; + int i, err; - memset(c, 0, sizeof(*c)); + /* zero non-persistant data */ + MLX5E_ZERO(&c->rq, mlx5e_rq_zero_start); + for (i = 0; i != priv->num_tc; i++) + MLX5E_ZERO(&c->sq[i], mlx5e_sq_zero_start); - c->priv = priv; - c->ix = ix; - /* setup send tag */ - c->tag.m_snd_tag.ifp = priv->ifp; - c->tag.type = IF_SND_TAG_TYPE_UNLIMITED; - c->num_tc = priv->num_tc; - - /* init mutexes */ - mlx5e_chan_mtx_init(c); - /* open transmit completion queue */ err = mlx5e_open_tx_cqs(c, cparam); if (err) @@ -2197,8 +2196,6 @@ err_close_tx_cqs: mlx5e_close_tx_cqs(c); err_free: - /* destroy mutexes */ - mlx5e_chan_mtx_destroy(c); return (err); } @@ -2214,8 +2211,6 @@ mlx5e_close_channel_wait(struct mlx5e_channel *c) mlx5e_close_rq_wait(&c->rq); mlx5e_close_sqs_wait(c); mlx5e_close_tx_cqs(c); - /* destroy mutexes */ - mlx5e_chan_mtx_destroy(c); } static int @@ -2411,14 +2406,16 @@ mlx5e_build_channel_param(struct mlx5e_priv *priv, static int mlx5e_open_channels(struct mlx5e_priv *priv) { - struct mlx5e_channel_param cparam; + struct mlx5e_channel_param *cparam; int err; int i; int j; - mlx5e_build_channel_param(priv, &cparam); + cparam = malloc(sizeof(*cparam), M_MLX5EN, M_WAITOK); + + mlx5e_build_channel_param(priv, cparam); for (i = 0; i < priv->params.num_channels; i++) { - err = mlx5e_open_channel(priv, i, &cparam, &priv->channel[i]); + err = mlx5e_open_channel(priv, cparam, &priv->channel[i]); if (err) goto err_close_channels; } @@ -2428,6 +2425,7 @@ mlx5e_open_channels(struct mlx5e_priv *priv) if (err) goto err_close_channels; } + free(cparam, M_MLX5EN); return (0); err_close_channels: @@ -2435,6 +2433,7 @@ err_close_channels: mlx5e_close_channel(&priv->channel[i]); mlx5e_close_channel_wait(&priv->channel[i]); } + free(cparam, M_MLX5EN); return (err); } @@ -2544,7 +2543,7 @@ mlx5e_refresh_channel_params_sub(struct mlx5e_priv *pr if (err) goto done; - for (i = 0; i != c->num_tc; i++) { + for (i = 0; i != priv->num_tc; i++) { err = mlx5e_refresh_sq_params(priv, &c->sq[i]); if (err) goto done; @@ -3599,17 +3598,26 @@ static const char *mlx5e_pport_stats_desc[] = { }; static void -mlx5e_priv_mtx_init(struct mlx5e_priv *priv) +mlx5e_priv_static_init(struct mlx5e_priv *priv, const uint32_t channels) { + uint32_t x; + mtx_init(&priv->async_events_mtx, "mlx5async", MTX_NETWORK_LOCK, MTX_DEF); sx_init(&priv->state_lock, "mlx5state"); callout_init_mtx(&priv->watchdog, &priv->async_events_mtx, 0); MLX5_INIT_DOORBELL_LOCK(&priv->doorbell_lock); + for (x = 0; x != channels; x++) + mlx5e_chan_static_init(priv, &priv->channel[x], x); } static void -mlx5e_priv_mtx_destroy(struct mlx5e_priv *priv) +mlx5e_priv_static_destroy(struct mlx5e_priv *priv, const uint32_t channels) { + uint32_t x; + + for (x = 0; x != channels; x++) + mlx5e_chan_static_destroy(&priv->channel[x]); + callout_drain(&priv->watchdog); mtx_destroy(&priv->async_events_mtx); sx_destroy(&priv->state_lock); } @@ -3639,7 +3647,7 @@ mlx5e_disable_tx_dma(struct mlx5e_channel *ch) { int i; - for (i = 0; i < ch->num_tc; i++) + for (i = 0; i < ch->priv->num_tc; i++) mlx5e_drain_sq(&ch->sq[i]); } @@ -3691,7 +3699,7 @@ mlx5e_enable_tx_dma(struct mlx5e_channel *ch) { int i; - for (i = 0; i < ch->num_tc; i++) + for (i = 0; i < ch->priv->num_tc; i++) mlx5e_resume_sq(&ch->sq[i]); } @@ -3706,8 +3714,6 @@ mlx5e_disable_rx_dma(struct mlx5e_channel *ch) callout_stop(&rq->watchdog); mtx_unlock(&rq->mtx); - callout_drain(&rq->watchdog); - err = mlx5e_modify_rq(rq, MLX5_RQC_STATE_RDY, MLX5_RQC_STATE_ERR); if (err != 0) { mlx5_en_err(rq->ifp, @@ -4126,13 +4132,15 @@ mlx5e_create_ifp(struct mlx5_core_dev *mdev) priv = malloc(sizeof(*priv) + (sizeof(priv->channel[0]) * mdev->priv.eq_table.num_comp_vectors), M_MLX5EN, M_WAITOK | M_ZERO); - mlx5e_priv_mtx_init(priv); ifp = priv->ifp = if_alloc(IFT_ETHER); if (ifp == NULL) { mlx5_core_err(mdev, "if_alloc() failed\n"); goto err_free_priv; } + /* setup all static fields */ + mlx5e_priv_static_init(priv, mdev->priv.eq_table.num_comp_vectors); + ifp->if_softc = priv; if_initname(ifp, "mce", device_get_unit(mdev->pdev->dev.bsddev)); ifp->if_mtu = ETHERMTU; @@ -4359,10 +4367,10 @@ err_free_sysctl: sysctl_ctx_free(&priv->sysctl_ctx); if (priv->sysctl_debug) sysctl_ctx_free(&priv->stats.port_stats_debug.ctx); + mlx5e_priv_static_destroy(priv, mdev->priv.eq_table.num_comp_vectors); if_free(ifp); err_free_priv: - mlx5e_priv_mtx_destroy(priv); free(priv, M_MLX5EN); return (NULL); } @@ -4416,7 +4424,6 @@ mlx5e_destroy_ifp(struct mlx5_core_dev *mdev, void *vp /* unregister device */ ifmedia_removeall(&priv->media); ether_ifdetach(ifp); - if_free(ifp); #ifdef RATELIMIT mlx5e_rl_cleanup(priv); @@ -4434,7 +4441,8 @@ mlx5e_destroy_ifp(struct mlx5_core_dev *mdev, void *vp mlx5_unmap_free_uar(priv->mdev, &priv->cq_uar); mlx5e_disable_async_events(priv); flush_workqueue(priv->wq); - mlx5e_priv_mtx_destroy(priv); + mlx5e_priv_static_destroy(priv, mdev->priv.eq_table.num_comp_vectors); + if_free(ifp); free(priv, M_MLX5EN); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201910071009.x97A9RhY012079>