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 Signed-off-by: Jeongjun Park --- 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; --