Received-SPF: pass (google.com: domain of linux-kernel+bounces-172684-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Xiaolei Wang <xiaolei.wang@windriver.com>
To: alexandre.torgue@foss.st.com, joabreu@synopsys.com, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        mcoquelin.stm32@gmail.com, richardcochran@gmail.com,
        bartosz.golaszewski@linaro.org, horms@kernel.org, ahalaney@redhat.com,
        rohan.g.thomas@intel.com, j.zink@pengutronix.de,
        rmk+kernel@armlinux.org.uk, leong.ching.swee@intel.com
Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3] net: stmmac: move the lock to struct plat_stmmacenet_data
Date: Wed,  8 May 2024 12:52:57 +0800
Message-Id: <20240508045257.2470698-1-xiaolei.wang@windriver.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?/6R2a/2jEeWtIN8RpN9x8OVfc5O7BFNBv7px1LXzbXUsgAfhiwyFGtrDVQjI?=
 =?us-ascii?Q?TWx4xUb/gBPXJhZE6u7Npnkd/oqQ/RjzDhwGgEbRc/VkX3d+9A1opNZKFsv9?=
 =?us-ascii?Q?90ypNDlvrOROTb1WuR9u2PW6oMmrOoCGLd/tcowBKyt4ldyxJgyqp2hnw075?=
 =?us-ascii?Q?EwfL8/3JhEPXeTaICY/mpgSxV80ulIM44iqOuwcT62HEsa8wMCzE9guqay1H?=
 =?us-ascii?Q?nzThvtEHw8eXNnjXYdFg9iGG9Qa5FbG+0Qjfu7TeDf/B6oPmT2bkSHwqjOsi?=
 =?us-ascii?Q?OwGfePLjpV9vxvI5mMXqqBa4jawZDCs94G7JuzSJHv1Tjl7bMbSE/tspjPPC?=
 =?us-ascii?Q?mSQ4xxhsPTsuQoyVj26Dlsf2JezFacTh4w2K9PTru0MVhrvG6L6auha79haY?=
 =?us-ascii?Q?0ffa/bxYY08TF2k9aUxGIjOOfhRDHbovAeUPzkulgiGVLe+sKHQq0tKpupUV?=
 =?us-ascii?Q?pxXMlCJmeQMpCiP8qrvYeWhR1HCEe3FjNqHYM4TuCB02AgFG6YfP4j40CXEi?=
 =?us-ascii?Q?Z+I4u7AJTKxtSxGofxAfJKpS2eg9BTLkYpFgvBcWdLIiJEX4QYhR+WJKAOo6?=
 =?us-ascii?Q?AdarfWuhf9KTxQeBga9EdCFk838J/h9WDqxKKelsZfPaMLVVq9JKuAyZgl7P?=
 =?us-ascii?Q?RvHAhEk1lTJFddz79tMolKaw6iJfUwJvloCYRRUKbf3Dml+StJE2huLtmN6x?=
 =?us-ascii?Q?YXsSTwwACZdlSDulIXiNlh9Q1XoGAZySmSIDIj6cV8URc+0TmlIwpfUf6kZ+?=
 =?us-ascii?Q?lC43zddSXedq7ALPxEDaiqzBKEjJgO+ryjzrJBzzsiOf5++evwh+7ZccphL+?=
 =?us-ascii?Q?AUDtq2OGchqMlgOKPb2f8iXxbXBMardFhiZv7BS6sKaAZN3x0nXKkW5EvKUG?=
 =?us-ascii?Q?qqOqwzm2INqSD6FaGGKAcGN6d3noxOh+xwFZDvEFD24RLuSTV/KpUmhmNzbq?=
 =?us-ascii?Q?Yz2QiPHMo1YJCrXxl2Jq/N42Ssj9xKnoPToT2pEpfKot/RTBD8vuJjscUQJE?=
 =?us-ascii?Q?ZC6f8EyxdCvW3UR0IIb2vsnh9bhP7axzwOGlxc143kNgdN0ro+8Hs49W8bTF?=
 =?us-ascii?Q?zyhRsCISUC16pftN8hWXYVC+D79pQG65rAAtRLpl8ySh4EuBA8zM6kDrcSY0?=
 =?us-ascii?Q?2h+WuSOK4fq2KF5PRLJfzsWmRsPGGjcqkh/pkphEEC3E2GV53Oj3K4OLeB4j?=
 =?us-ascii?Q?TTyUncu21ZCXmN4HE9yxxY2BlWrMkzUyKrr6hGq9zLf2X/ZbnSmLzUyjgSeq?=
 =?us-ascii?Q?PvLS7tjylwottzG31NaTmWb8rWuC3AmZwyX+rE+fP/4STsPuv2fJePkzwtlt?=
 =?us-ascii?Q?CjctT1n5Tae4oyN2gQcHX7uTOC18OcQEuEjdIZcAnN/vKeM9Ng8sgBkfJkhc?=
 =?us-ascii?Q?7kE09E4rJltv7AjKdYPCQ33s+PCfccbu+RRYP5nvELB0n8MgCoQbS1Q9ri0v?=
 =?us-ascii?Q?Sdd6j914wbAy116KuSkqR6l3vJMK4gdhW+OTJn2ilxLxJ09A4gO8EruGuQIv?=
 =?us-ascii?Q?3IG40Tp8pwPoNo26IHpPvY3lgwwacce7Fgvw5lcVwyD3PqBF5MmZnFw5kLZe?=
 =?us-ascii?Q?0+umXOK3qK6I7x1hHX9Q0fRMCrVQHiVk1BaFNl2qVgCPTmS6kxwZZSwCDn9W?=
 =?us-ascii?Q?tw=3D=3D?=
X-OriginatorOrg: windriver.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0b51b0f9-ed31-4bad-8684-08dc6f1ac639
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5764.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 May 2024 04:53:16.6974
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ddb2873-a1ad-4a18-ae4e-4644631433be
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9P/+xJPgYfb7t5fBKnMusQcoAtaxsAVqLmv20IFTFFlAdKlGgKHtr2Mbs5V8orIxsb8bdR2BklMaqEhXJyvS28+rPHzEWKzI4Y2o41e8Myw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7829
X-Proofpoint-ORIG-GUID: 8emv-XHsOq-YecytePY84ygNUAIOc_pR
X-Proofpoint-GUID: 8emv-XHsOq-YecytePY84ygNUAIOc_pR
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-08_01,2024-05-06_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 impostorscore=0
 malwarescore=0 adultscore=0 spamscore=0 priorityscore=1501 mlxlogscore=999
 phishscore=0 suspectscore=0 mlxscore=0 lowpriorityscore=0 clxscore=1015
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2405010000
 definitions=main-2405080034

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 plat_stmmacenet_data.
We also need to require 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

Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
v1 -> v2:
 - move the lock to struct plat_stmmacenet_data
v2 -> v3:
 - Add require the mutex lock for reinitialization

 .../net/ethernet/stmicro/stmmac/stmmac_ptp.c   |  8 ++++----
 .../net/ethernet/stmicro/stmmac/stmmac_tc.c    | 18 ++++++++++--------
 include/linux/stmmac.h                         |  2 +-
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index e04830a3a1fb..82b7577fea9e 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->plat->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->plat->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->plat->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];
@@ -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->plat->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..c0b720f08d77 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->plat->lock);
 	} else {
+		mutex_lock(&priv->plat->lock);
 		memset(plat->est, 0, sizeof(*plat->est));
+		mutex_unlock(&priv->plat->lock);
 	}
 
 	size = qopt->num_entries;
 
-	mutex_lock(&priv->plat->est->lock);
+	mutex_lock(&priv->plat->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->plat->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->plat->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);
@@ -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->plat->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->plat->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->plat->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->plat->lock);
 	}
 
 	priv->plat->fpe_cfg->enable = false;
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dfa1828cd756..316ff7eb8b33 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];
@@ -246,6 +245,7 @@ struct plat_stmmacenet_data {
 	struct fwnode_handle *port_node;
 	struct device_node *mdio_node;
 	struct stmmac_dma_cfg *dma_cfg;
+	struct mutex lock;
 	struct stmmac_est *est;
 	struct stmmac_fpe_cfg *fpe_cfg;
 	struct stmmac_safety_feature_cfg *safety_feat_cfg;
-- 
2.25.1