linux-kernel - [PATCH net-next v2] ptp: remove unnecessary mutex lock in ptp_clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250703055340.55158-1-aha310510@gmail.com>
Date: Thu,  3 Jul 2025 14:53:40 +0900
From: Jeongjun Park <aha310510@...il.com>
To: richardcochran@...il.com
Cc: andrew+netdev@...n.ch,
	davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	vladimir.oltean@....com,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Jeongjun Park <aha310510@...il.com>
Subject: [PATCH net-next v2] ptp: remove unnecessary mutex lock in ptp_clock_unregister()

ptp_clock_unregister() is called by ptp core and several drivers that
require ptp clock feature. And in this function, ptp_vclock_in_use()
is called to check if ptp virtual clock is in use, and
ptp->is_virtual_clock, ptp->n_vclocks are checked.

It is true that you should always check ptp->is_virtual_clock to see if
you are using ptp virtual clock, but you do not necessarily need to
check ptp->n_vclocks.

ptp->n_vclocks is a feature need by ptp sysfs or some ptp cores, so in
most cases, except for these callers, it is not necessary to check.

The problem is that ptp_clock_unregister() checks ptp->n_vclocks even
when called by a driver other than the ptp core, and acquires
ptp->n_vclocks_mux to avoid concurrency issues when checking.

I think this logic is inefficient, so I think it would be appropriate to
modify the caller function that must check ptp->n_vclocks to check
ptp->n_vclocks in advance before calling ptp_clock_unregister().

Cc: Vladimir Oltean <vladimir.oltean@....com>
Signed-off-by: Jeongjun Park <aha310510@...il.com>
---
v2: Add CC Vladimir
- Link to v1: https://lore.kernel.org/all/20250701170353.7255-1-aha310510@gmail.com/
---
 drivers/ptp/ptp_clock.c   |  2 +-
 drivers/ptp/ptp_private.h | 34 +++++++++-------------------------
 2 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 36f57d7b4a66..db6e03072fba 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -413,7 +413,7 @@ static int unregister_vclock(struct device *dev, void *data)
 
 int ptp_clock_unregister(struct ptp_clock *ptp)
 {
-	if (ptp_vclock_in_use(ptp)) {
+	if (!ptp->is_virtual_clock) {
 		device_for_each_child(&ptp->dev, NULL, unregister_vclock);
 	}
 
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index a6aad743c282..9b308461fcc8 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -95,39 +95,23 @@ static inline int queue_cnt(const struct timestamp_event_queue *q)
 	return cnt < 0 ? PTP_MAX_TIMESTAMPS + cnt : cnt;
 }
 
-/* Check if ptp virtual clock is in use */
-static inline bool ptp_vclock_in_use(struct ptp_clock *ptp)
+/* Check if ptp clock shall be free running */
+static inline bool ptp_clock_freerun(struct ptp_clock *ptp)
 {
-	bool in_use = false;
-
-	/* Virtual clocks can't be stacked on top of virtual clocks.
-	 * Avoid acquiring the n_vclocks_mux on virtual clocks, to allow this
-	 * function to be called from code paths where the n_vclocks_mux of the
-	 * parent physical clock is already held. Functionally that's not an
-	 * issue, but lockdep would complain, because they have the same lock
-	 * class.
-	 */
-	if (ptp->is_virtual_clock)
-		return false;
+	bool ret = false;
+
+	if (ptp->has_cycles)
+		return ret;
 
 	if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
 		return true;
 
-	if (ptp->n_vclocks)
-		in_use = true;
+	if (!ptp->is_virtual_clock && ptp->n_vclocks)
+		ret = true;
 
 	mutex_unlock(&ptp->n_vclocks_mux);
 
-	return in_use;
-}
-
-/* Check if ptp clock shall be free running */
-static inline bool ptp_clock_freerun(struct ptp_clock *ptp)
-{
-	if (ptp->has_cycles)
-		return false;
-
-	return ptp_vclock_in_use(ptp);
+	return ret;
 }
 
 extern const struct class ptp_class;
--