public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6.1] net: stmmac: move the EST lock to struct stmmac_priv
@ 2024-12-10  2:00 bin.lan.cn
  2024-12-10 19:25 ` Sasha Levin
  2024-12-11  8:11 ` Greg KH
  0 siblings, 2 replies; 3+ messages in thread
From: bin.lan.cn @ 2024-12-10  2:00 UTC (permalink / raw)
  To: stable

From: Xiaolei Wang <xiaolei.wang@windriver.com>

[ Upstream commit 36ac9e7f2e5786bd37c5cd91132e1f39c29b8197 ]

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>
Link: https://lore.kernel.org/r/20240513014346.1718740-2-xiaolei.wang@windriver.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
[ Resolve line conflicts ]
Signed-off-by: Bin Lan <bin.lan.cn@windriver.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 46944c02b45e..0a4edc9bd246 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -240,6 +240,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 9c91a3dc8e38..0e5a84694793 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -79,11 +79,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->ioaddr, 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);
@@ -96,7 +96,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, &current_time);
 		current_time_ns = timespec64_to_ktime(current_time);
 		time.tv_nsec = priv->plat->est->btr_reserve[0];
@@ -113,7 +113,7 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
 		priv->plat->est->enable = true;
 		ret = stmmac_est_configure(priv, priv->ioaddr, 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 27187f528620..e4ca722dd2c9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -978,17 +978,19 @@ static int tc_setup_taprio(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->enable;
-	mutex_unlock(&priv->plat->est->lock);
+	mutex_unlock(&priv->est_lock);
 
 	for (i = 0; i < size; i++) {
 		s64 delta_ns = qopt->entries[i].interval;
@@ -1019,7 +1021,7 @@ static int tc_setup_taprio(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, &current_time);
 	current_time_ns = timespec64_to_ktime(current_time);
@@ -1038,7 +1040,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
 	priv->plat->est->ctr[1] = (u32)ctr;
 
 	if (fpe && !priv->dma_cap.fpesel) {
-		mutex_unlock(&priv->plat->est->lock);
+		mutex_unlock(&priv->est_lock);
 		return -EOPNOTSUPP;
 	}
 
@@ -1049,7 +1051,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
 
 	ret = stmmac_est_configure(priv, priv->ioaddr, 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;
@@ -1066,11 +1068,11 @@ static int tc_setup_taprio(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->ioaddr, priv->plat->est,
 				     priv->plat->clk_ptp_rate);
-		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 9f4a4f70270d..216a146c8f22 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -115,7 +115,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.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.1] net: stmmac: move the EST lock to struct stmmac_priv
  2024-12-10  2:00 [PATCH 6.1] net: stmmac: move the EST lock to struct stmmac_priv bin.lan.cn
@ 2024-12-10 19:25 ` Sasha Levin
  2024-12-11  8:11 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2024-12-10 19:25 UTC (permalink / raw)
  To: stable; +Cc: bin.lan.cn, Sasha Levin

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 36ac9e7f2e5786bd37c5cd91132e1f39c29b8197

WARNING: Author mismatch between patch and upstream commit:
Backport author: bin.lan.cn@eng.windriver.com
Commit author: Xiaolei Wang <xiaolei.wang@windriver.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: b538fefeb102)
6.1.y | Not found

Note: The patch differs from the upstream commit:
---
1:  36ac9e7f2e578 ! 1:  5bc5febe7e329 net: stmmac: move the EST lock to struct stmmac_priv
    @@ Metadata
      ## Commit message ##
         net: stmmac: move the EST lock to struct stmmac_priv
     
    +    [ Upstream commit 36ac9e7f2e5786bd37c5cd91132e1f39c29b8197 ]
    +
         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
    @@ Commit message
         Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
         Link: https://lore.kernel.org/r/20240513014346.1718740-2-xiaolei.wang@windriver.com
         Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    +    [ Resolve line conflicts ]
    +    Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
     
      ## drivers/net/ethernet/stmicro/stmmac/stmmac.h ##
     @@ drivers/net/ethernet/stmicro/stmmac/stmmac.h: struct stmmac_priv {
    @@ drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c: static int stmmac_adjust_time(
     -		mutex_lock(&priv->plat->est->lock);
     +		mutex_lock(&priv->est_lock);
      		priv->plat->est->enable = false;
    - 		stmmac_est_configure(priv, priv, priv->plat->est,
    + 		stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
      				     priv->plat->clk_ptp_rate);
     -		mutex_unlock(&priv->plat->est->lock);
     +		mutex_unlock(&priv->est_lock);
    @@ drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c: static int stmmac_adjust_time(
      		time.tv_nsec = priv->plat->est->btr_reserve[0];
     @@ drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c: 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,
    + 		ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
      					   priv->plat->clk_ptp_rate);
     -		mutex_unlock(&priv->plat->est->lock);
     +		mutex_unlock(&priv->est_lock);
    @@ drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c: static int stmmac_adjust_time(
      	}
     
      ## drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c ##
    -@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(struct stmmac_priv *priv,
    +@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_setup_taprio(struct stmmac_priv *priv,
      		if (!plat->est)
      			return -ENOMEM;
      
    @@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(
     -	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;
    + 	priv->plat->est->enable = qopt->enable;
     -	mutex_unlock(&priv->plat->est->lock);
     +	mutex_unlock(&priv->est_lock);
      
      	for (i = 0; i < size; i++) {
      		s64 delta_ns = qopt->entries[i].interval;
    -@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(struct stmmac_priv *priv,
    +@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_setup_taprio(struct stmmac_priv *priv,
      		priv->plat->est->gcl[i] = delta_ns | (gates << wid);
      	}
      
    @@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(
      	/* Adjust for real system time */
      	priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, &current_time);
      	current_time_ns = timespec64_to_ktime(current_time);
    -@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(struct stmmac_priv *priv,
    - 	tc_taprio_map_maxsdu_txq(priv, qopt);
    +@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_setup_taprio(struct stmmac_priv *priv,
    + 	priv->plat->est->ctr[1] = (u32)ctr;
      
      	if (fpe && !priv->dma_cap.fpesel) {
     -		mutex_unlock(&priv->plat->est->lock);
    @@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(
      		return -EOPNOTSUPP;
      	}
      
    -@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(struct stmmac_priv *priv,
    +@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_setup_taprio(struct stmmac_priv *priv,
      
    - 	ret = stmmac_est_configure(priv, priv, priv->plat->est,
    + 	ret = stmmac_est_configure(priv, priv->ioaddr, 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;
    -@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_taprio_configure(struct stmmac_priv *priv,
    +@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: static int tc_setup_taprio(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,
    + 		stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
      				     priv->plat->clk_ptp_rate);
    -@@ drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: 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);
      	}
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.1.y        |  Success    |  Success   |

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.1] net: stmmac: move the EST lock to struct stmmac_priv
  2024-12-10  2:00 [PATCH 6.1] net: stmmac: move the EST lock to struct stmmac_priv bin.lan.cn
  2024-12-10 19:25 ` Sasha Levin
@ 2024-12-11  8:11 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2024-12-11  8:11 UTC (permalink / raw)
  To: bin.lan.cn; +Cc: stable

On Tue, Dec 10, 2024 at 10:00:43AM +0800, bin.lan.cn@eng.windriver.com wrote:
> From: Xiaolei Wang <xiaolei.wang@windriver.com>
> 
> [ Upstream commit 36ac9e7f2e5786bd37c5cd91132e1f39c29b8197 ]

Please cc: all relevant people on backports.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-12-11  8:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-10  2:00 [PATCH 6.1] net: stmmac: move the EST lock to struct stmmac_priv bin.lan.cn
2024-12-10 19:25 ` Sasha Levin
2024-12-11  8:11 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox