diff mbox series

[iwl-net,v1,5/5] igc: Add lock preventing multiple simultaneous PTM transactions

Message ID 20240807003032.10300-6-christopher.s.hall@intel.com
State Changes Requested
Headers show
Series igc: PTM timeout fix | expand

Commit Message

Hall, Christopher S Aug. 7, 2024, 12:30 a.m. UTC 
From: Christopher S M Hall <christopher.s.hall@intel.com>

Add a mutex around the PTM transaction to prevent multiple transactors

Multiple processes try to initiate a PTM transaction, one or all may
fail. This can be reproduced by running two instances of the
following:

$ sudo phc2sys -O 0 -i tsn0 -m

PHC2SYS exits with:

"ioctl PTP_OFFSET_PRECISE: Connection timed out" when the PTM transaction
 fails

Note: Normally two instance of PHC2SYS will not run, but one process
 should not break another.

Fixes: a90ec8483732 ("igc: Add support for PTP getcrosststamp()")
Signed-off-by: Christopher S M Hall <christopher.s.hall@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h     |  1 +
 drivers/net/ethernet/intel/igc/igc_ptp.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

Comments

Corinna Vinschen Aug. 8, 2024, 3:17 p.m. UTC | #1 
Hi Christopher,

On Aug  6 17:30, christopher.s.hall@intel.com wrote:
> From: Christopher S M Hall <christopher.s.hall@intel.com>
> 
> Add a mutex around the PTM transaction to prevent multiple transactors
> 
> Multiple processes try to initiate a PTM transaction, one or all may
> fail. This can be reproduced by running two instances of the
> following:

I saw a former version of the patch which additionally added a mutex
lock/unlock in igc_ptp_reset() just before calling igc_ptm_trigger().
Is it safe to skip that?  igc_ptp_reset() is called from igc_reset()
which in turn is called from quite a few places.


Thanks,
Corinna
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index c38b4d0f00ce..fbac02c79178 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -315,6 +315,7 @@  struct igc_adapter {
 	struct timespec64 prev_ptp_time; /* Pre-reset PTP clock */
 	ktime_t ptp_reset_start; /* Reset time in clock mono */
 	struct system_time_snapshot snapshot;
+	struct mutex ptm_lock; /* Only allow one PTM transaction at a time */
 
 	char fw_version[32];
 
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index f770e39650ef..c70a6393c210 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -1068,9 +1068,16 @@  static int igc_ptp_getcrosststamp(struct ptp_clock_info *ptp,
 {
 	struct igc_adapter *adapter = container_of(ptp, struct igc_adapter,
 						   ptp_caps);
+	int ret;
 
-	return get_device_system_crosststamp(igc_phc_get_syncdevicetime,
-					     adapter, &adapter->snapshot, cts);
+	/* This blocks until any in progress PTM transactions complete */
+	mutex_lock(&adapter->ptm_lock);
+
+	ret = get_device_system_crosststamp(igc_phc_get_syncdevicetime,
+					    adapter, &adapter->snapshot, cts);
+	mutex_unlock(&adapter->ptm_lock);
+
+	return ret;
 }
 
 static int igc_ptp_getcyclesx64(struct ptp_clock_info *ptp,
@@ -1302,6 +1309,7 @@  void igc_ptp_reset(struct igc_adapter *adapter)
 		wr32(IGC_PTM_CTRL, ctrl);
 
 		/* Force the first cycle to run. */
+		mutex_init(&adapter->ptm_lock);
 		igc_ptm_trigger(hw);
 
 		if (readx_poll_timeout_atomic(rd32, IGC_PTM_STAT, stat,