Message ID | 20240513014346.1718740-2-xiaolei.wang@windriver.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 36ac9e7f2e5786bd37c5cd91132e1f39c29b8197 |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Move EST lock and EST structure to struct stmmac_priv | expand |
On Mon, May 13, 2024 at 09:43:45AM GMT, Xiaolei Wang wrote: > Reinitialize the whole EST structure would also reset the mutex > lock which is embedded in the EST structure, and then trigger > the following warning. To address this, move the lock to struct > stmmac_priv. We also need to reacquire the mutex lock when doing > this initialization. > > DEBUG_LOCKS_WARN_ON(lock->magic != lock) > WARNING: CPU: 3 PID: 505 at kernel/locking/mutex.c:587 __mutex_lock+0xd84/0x1068 > Modules linked in: > CPU: 3 PID: 505 Comm: tc Not tainted 6.9.0-rc6-00053-g0106679839f7-dirty #29 > Hardware name: NXP i.MX8MPlus EVK board (DT) > pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : __mutex_lock+0xd84/0x1068 > lr : __mutex_lock+0xd84/0x1068 > sp : ffffffc0864e3570 > x29: ffffffc0864e3570 x28: ffffffc0817bdc78 x27: 0000000000000003 > x26: ffffff80c54f1808 x25: ffffff80c9164080 x24: ffffffc080d723ac > x23: 0000000000000000 x22: 0000000000000002 x21: 0000000000000000 > x20: 0000000000000000 x19: ffffffc083bc3000 x18: ffffffffffffffff > x17: ffffffc08117b080 x16: 0000000000000002 x15: ffffff80d2d40000 > x14: 00000000000002da x13: ffffff80d2d404b8 x12: ffffffc082b5a5c8 > x11: ffffffc082bca680 x10: ffffffc082bb2640 x9 : ffffffc082bb2698 > x8 : 0000000000017fe8 x7 : c0000000ffffefff x6 : 0000000000000001 > x5 : ffffff8178fe0d48 x4 : 0000000000000000 x3 : 0000000000000027 > x2 : ffffff8178fe0d50 x1 : 0000000000000000 x0 : 0000000000000000 > Call trace: > __mutex_lock+0xd84/0x1068 > mutex_lock_nested+0x28/0x34 > tc_setup_taprio+0x118/0x68c > stmmac_setup_tc+0x50/0xf0 > taprio_change+0x868/0xc9c > > Fixes: b2aae654a479 ("net: stmmac: add mutex lock to protect est parameters") > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> > Reviewed-by: Simon Horman <horms@kernel.org> > Reviewed-by: Serge Semin <fancer.lancer@gmail.com> Reviewed-by: Andrew Halaney <ahalaney@redhat.com> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 ++ > .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 8 ++++---- > .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 18 ++++++++++-------- > include/linux/stmmac.h | 1 - > 4 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h > index dddcaa9220cc..64b21c83e2b8 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h > @@ -261,6 +261,8 @@ struct stmmac_priv { > struct stmmac_extra_stats xstats ____cacheline_aligned_in_smp; > struct stmmac_safety_stats sstats; > struct plat_stmmacenet_data *plat; > + /* Protect est parameters */ > + struct mutex est_lock; > struct dma_features dma_cap; > struct stmmac_counters mmc; > int hw_cap_support; > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c > index e04830a3a1fb..0c5aab6dd7a7 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c > @@ -70,11 +70,11 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta) > /* If EST is enabled, disabled it before adjust ptp time. */ > if (priv->plat->est && priv->plat->est->enable) { > est_rst = true; > - mutex_lock(&priv->plat->est->lock); > + mutex_lock(&priv->est_lock); > priv->plat->est->enable = false; > stmmac_est_configure(priv, priv, priv->plat->est, > priv->plat->clk_ptp_rate); > - mutex_unlock(&priv->plat->est->lock); > + mutex_unlock(&priv->est_lock); > } > > write_lock_irqsave(&priv->ptp_lock, flags); > @@ -87,7 +87,7 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta) > ktime_t current_time_ns, basetime; > u64 cycle_time; > > - mutex_lock(&priv->plat->est->lock); > + mutex_lock(&priv->est_lock); > priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time); > current_time_ns = timespec64_to_ktime(current_time); > time.tv_nsec = priv->plat->est->btr_reserve[0]; > @@ -104,7 +104,7 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta) > priv->plat->est->enable = true; > ret = stmmac_est_configure(priv, priv, priv->plat->est, > priv->plat->clk_ptp_rate); > - mutex_unlock(&priv->plat->est->lock); > + mutex_unlock(&priv->est_lock); > if (ret) > netdev_err(priv->dev, "failed to configure EST\n"); > } > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > index cce00719937d..620c16e9be3a 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > @@ -1004,17 +1004,19 @@ static int tc_taprio_configure(struct stmmac_priv *priv, > if (!plat->est) > return -ENOMEM; > > - mutex_init(&priv->plat->est->lock); > + mutex_init(&priv->est_lock); > } else { > + mutex_lock(&priv->est_lock); > memset(plat->est, 0, sizeof(*plat->est)); > + mutex_unlock(&priv->est_lock); > } > > size = qopt->num_entries; > > - mutex_lock(&priv->plat->est->lock); > + mutex_lock(&priv->est_lock); > priv->plat->est->gcl_size = size; > priv->plat->est->enable = qopt->cmd == TAPRIO_CMD_REPLACE; > - mutex_unlock(&priv->plat->est->lock); > + mutex_unlock(&priv->est_lock); > > for (i = 0; i < size; i++) { > s64 delta_ns = qopt->entries[i].interval; > @@ -1045,7 +1047,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, > priv->plat->est->gcl[i] = delta_ns | (gates << wid); > } > > - mutex_lock(&priv->plat->est->lock); > + mutex_lock(&priv->est_lock); > /* Adjust for real system time */ > priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time); > current_time_ns = timespec64_to_ktime(current_time); > @@ -1068,7 +1070,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, > tc_taprio_map_maxsdu_txq(priv, qopt); > > if (fpe && !priv->dma_cap.fpesel) { > - mutex_unlock(&priv->plat->est->lock); > + mutex_unlock(&priv->est_lock); > return -EOPNOTSUPP; > } > > @@ -1079,7 +1081,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, > > ret = stmmac_est_configure(priv, priv, priv->plat->est, > priv->plat->clk_ptp_rate); > - mutex_unlock(&priv->plat->est->lock); > + mutex_unlock(&priv->est_lock); > if (ret) { > netdev_err(priv->dev, "failed to configure EST\n"); > goto disable; > @@ -1096,7 +1098,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, > > disable: > if (priv->plat->est) { > - mutex_lock(&priv->plat->est->lock); > + mutex_lock(&priv->est_lock); > priv->plat->est->enable = false; > stmmac_est_configure(priv, priv, priv->plat->est, > priv->plat->clk_ptp_rate); > @@ -1105,7 +1107,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, > priv->xstats.max_sdu_txq_drop[i] = 0; > priv->xstats.mtl_est_txq_hlbf[i] = 0; > } > - mutex_unlock(&priv->plat->est->lock); > + mutex_unlock(&priv->est_lock); > } > > priv->plat->fpe_cfg->enable = false; > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > index dfa1828cd756..c0d74f97fd18 100644 > --- a/include/linux/stmmac.h > +++ b/include/linux/stmmac.h > @@ -117,7 +117,6 @@ struct stmmac_axi { > > #define EST_GCL 1024 > struct stmmac_est { > - struct mutex lock; > int enable; > u32 btr_reserve[2]; > u32 btr_offset[2]; > -- > 2.25.1 >
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index dddcaa9220cc..64b21c83e2b8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -261,6 +261,8 @@ struct stmmac_priv { struct stmmac_extra_stats xstats ____cacheline_aligned_in_smp; struct stmmac_safety_stats sstats; struct plat_stmmacenet_data *plat; + /* Protect est parameters */ + struct mutex est_lock; struct dma_features dma_cap; struct stmmac_counters mmc; int hw_cap_support; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c index e04830a3a1fb..0c5aab6dd7a7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c @@ -70,11 +70,11 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta) /* If EST is enabled, disabled it before adjust ptp time. */ if (priv->plat->est && priv->plat->est->enable) { est_rst = true; - mutex_lock(&priv->plat->est->lock); + mutex_lock(&priv->est_lock); priv->plat->est->enable = false; stmmac_est_configure(priv, priv, priv->plat->est, priv->plat->clk_ptp_rate); - mutex_unlock(&priv->plat->est->lock); + mutex_unlock(&priv->est_lock); } write_lock_irqsave(&priv->ptp_lock, flags); @@ -87,7 +87,7 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta) ktime_t current_time_ns, basetime; u64 cycle_time; - mutex_lock(&priv->plat->est->lock); + mutex_lock(&priv->est_lock); priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time); current_time_ns = timespec64_to_ktime(current_time); time.tv_nsec = priv->plat->est->btr_reserve[0]; @@ -104,7 +104,7 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta) priv->plat->est->enable = true; ret = stmmac_est_configure(priv, priv, priv->plat->est, priv->plat->clk_ptp_rate); - mutex_unlock(&priv->plat->est->lock); + mutex_unlock(&priv->est_lock); if (ret) netdev_err(priv->dev, "failed to configure EST\n"); } diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c index cce00719937d..620c16e9be3a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c @@ -1004,17 +1004,19 @@ static int tc_taprio_configure(struct stmmac_priv *priv, if (!plat->est) return -ENOMEM; - mutex_init(&priv->plat->est->lock); + mutex_init(&priv->est_lock); } else { + mutex_lock(&priv->est_lock); memset(plat->est, 0, sizeof(*plat->est)); + mutex_unlock(&priv->est_lock); } size = qopt->num_entries; - mutex_lock(&priv->plat->est->lock); + mutex_lock(&priv->est_lock); priv->plat->est->gcl_size = size; priv->plat->est->enable = qopt->cmd == TAPRIO_CMD_REPLACE; - mutex_unlock(&priv->plat->est->lock); + mutex_unlock(&priv->est_lock); for (i = 0; i < size; i++) { s64 delta_ns = qopt->entries[i].interval; @@ -1045,7 +1047,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, priv->plat->est->gcl[i] = delta_ns | (gates << wid); } - mutex_lock(&priv->plat->est->lock); + mutex_lock(&priv->est_lock); /* Adjust for real system time */ priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time); current_time_ns = timespec64_to_ktime(current_time); @@ -1068,7 +1070,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, tc_taprio_map_maxsdu_txq(priv, qopt); if (fpe && !priv->dma_cap.fpesel) { - mutex_unlock(&priv->plat->est->lock); + mutex_unlock(&priv->est_lock); return -EOPNOTSUPP; } @@ -1079,7 +1081,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, ret = stmmac_est_configure(priv, priv, priv->plat->est, priv->plat->clk_ptp_rate); - mutex_unlock(&priv->plat->est->lock); + mutex_unlock(&priv->est_lock); if (ret) { netdev_err(priv->dev, "failed to configure EST\n"); goto disable; @@ -1096,7 +1098,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, disable: if (priv->plat->est) { - mutex_lock(&priv->plat->est->lock); + mutex_lock(&priv->est_lock); priv->plat->est->enable = false; stmmac_est_configure(priv, priv, priv->plat->est, priv->plat->clk_ptp_rate); @@ -1105,7 +1107,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, priv->xstats.max_sdu_txq_drop[i] = 0; priv->xstats.mtl_est_txq_hlbf[i] = 0; } - mutex_unlock(&priv->plat->est->lock); + mutex_unlock(&priv->est_lock); } priv->plat->fpe_cfg->enable = false; diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index dfa1828cd756..c0d74f97fd18 100644 --- a/include/linux/stmmac.h +++ b/include/linux/stmmac.h @@ -117,7 +117,6 @@ struct stmmac_axi { #define EST_GCL 1024 struct stmmac_est { - struct mutex lock; int enable; u32 btr_reserve[2]; u32 btr_offset[2];