Skip to content

Commit c7d6cb9

Browse files
jacob-kelleranguy11
authored andcommitted
igc: fix lock order in igc_ptp_reset
Commit 1a931c4 ("igc: add lock preventing multiple simultaneous PTM transactions") added a new mutex to protect concurrent PTM transactions. This lock is acquired in igc_ptp_reset() in order to ensure the PTM registers are properly disabled after a device reset. The flow where the lock is acquired already holds a spinlock, so acquiring a mutex leads to a sleep-while-locking bug, reported both by smatch, and the kernel test robot. The critical section in igc_ptp_reset() does correctly use the readx_poll_timeout_atomic variants, but the standard PTM flow uses regular sleeping variants. This makes converting the mutex to a spinlock a bit tricky. Instead, re-order the locking in igc_ptp_reset. Acquire the mutex first, and then the tmreg_lock spinlock. This is safe because there is no other ordering dependency on these locks, as this is the only place where both locks were acquired simultaneously. Indeed, any other flow acquiring locks in that order would be wrong regardless. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Fixes: 1a931c4 ("igc: add lock preventing multiple simultaneous PTM transactions") Link: https://lore.kernel.org/intel-wired-lan/Z_-P-Hc1yxcw0lTB@stanley.mountain/ Link: https://lore.kernel.org/intel-wired-lan/202504211511.f7738f5d-lkp@intel.com/T/#u Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Reviewed-by: Vitaly Lifshits <vitaly.lifshits@intel.com> Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
1 parent ed375b1 commit c7d6cb9

File tree

1 file changed

+4
-2
lines changed

1 file changed

+4
-2
lines changed

drivers/net/ethernet/intel/igc/igc_ptp.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,6 +1290,8 @@ void igc_ptp_reset(struct igc_adapter *adapter)
12901290
/* reset the tstamp_config */
12911291
igc_ptp_set_timestamp_mode(adapter, &adapter->tstamp_config);
12921292

1293+
mutex_lock(&adapter->ptm_lock);
1294+
12931295
spin_lock_irqsave(&adapter->tmreg_lock, flags);
12941296

12951297
switch (adapter->hw.mac.type) {
@@ -1308,7 +1310,6 @@ void igc_ptp_reset(struct igc_adapter *adapter)
13081310
if (!igc_is_crosststamp_supported(adapter))
13091311
break;
13101312

1311-
mutex_lock(&adapter->ptm_lock);
13121313
wr32(IGC_PCIE_DIG_DELAY, IGC_PCIE_DIG_DELAY_DEFAULT);
13131314
wr32(IGC_PCIE_PHY_DELAY, IGC_PCIE_PHY_DELAY_DEFAULT);
13141315

@@ -1332,7 +1333,6 @@ void igc_ptp_reset(struct igc_adapter *adapter)
13321333
netdev_err(adapter->netdev, "Timeout reading IGC_PTM_STAT register\n");
13331334

13341335
igc_ptm_reset(hw);
1335-
mutex_unlock(&adapter->ptm_lock);
13361336
break;
13371337
default:
13381338
/* No work to do. */
@@ -1349,5 +1349,7 @@ void igc_ptp_reset(struct igc_adapter *adapter)
13491349
out:
13501350
spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
13511351

1352+
mutex_unlock(&adapter->ptm_lock);
1353+
13521354
wrfl();
13531355
}

0 commit comments

Comments
 (0)