> -----Original Message----- > From: Keller, Jacob E <[email protected]> > Sent: Friday, August 21, 2026 5:13 PM > To: Intel Wired LAN <[email protected]> > Cc: [email protected]; Machnikowski, Maciej > <[email protected]>; Nguyen, Anthony L > <[email protected]>; Korba, Przemyslaw <[email protected]>; > Nitka, Grzegorz <[email protected]>; Oros, Petr <[email protected]>; > Nowlin, Alexander <[email protected]>; Bross, Kevin > <[email protected]>; Cavatur, Ranjit <[email protected]>; Keller, > Jacob E <[email protected]>; Machnikowski, Maciej > <[email protected]> > Subject: [PATCH iwl-net 01/12] ice: use reference counting and RCU for PTP > port access > > The ice adapter structure maintains a list of ports associated with the > adapter. This is used for supporting PTP, where the clock owner must handle > many operations that require access to the PTP port structures > of the > associated PFs. > > This is implemented using a linked list and a mutex. This sort of works, but > a few places within the code do not acquire the mutex when iterating the > list. This includes ice_ptp_flush_all_tx_tracker(), > ice_ptp_restart_all_phy(), and ice_ptp_prepare_rebuild_sec(). > > Fixing this is tricky, especially since it is not clear if we can simply > acquire the lock around the complete iterations. > > The pattern of use for the port list is read-mostly with modifications only > happening during PF initialization when elements are inserted. This typically > only happens during early boot, though a PF could in principle be removed or > loaded at arbitrary times via bind and unbind operations. > > The use of a mutex does mean the driver can sleep while holding it, but it > still creates complicates with lock ordering and prevents iterating the list > in any code path that *can't* sleep. > > Instead, use the RCU primitives for the port linked list, along with a > reference count on the port. The kref reference counter ensures that we can > safely acquire pointers with a guarantee of their lifetime, ensuring > the > associated PF will not be removed until the reference is released. > > For port iterations which are short and definitely can't sleep, wrap the > entire loop with rcu_read_lock() and rcu_read_unlock(). > > For longer operations, or those which might sleep, we need to close the > critical section between each loop iteration. To make this safe, start the > loop iteration with rcu_read_lock(), then acquire a reference for the port > with kref_get_unless_zero. If this returns 0, the port is already in the > process of being removed, so that port should be skipped when iterating. Once > a reference to the port is acquired, exit the RCU critical > section. Then, > perform the desired operations on the port, followed by releasing the > reference with kref_put and then re-entering a critical section at the end of > the loop body. Note that kref_put() is done outside the RCU critical section. > This is safe, as the port will not be freed until all references are dropped. > > The ice_ptp_release_port_rcu() function is used as the release function for > the kref_put() call. To avoid a potential infinite loop of new references, > the release function simply uses a wake_up_var() call to wake the closing > thread. The ice_ptp_cleanup_pf() function will remove the port from the > linked list using list_del_rcu, then release its primary reference, then wait > for all references to drop via wait queue. Finally synchronize_rcu() is > called to guarantee the port remains valid for at least one RCU grace period. > Then PF removal will continue. > > This flow ensures that all accesses to ports via the port list will remain > valid until either the RCU critical sections end, or the references have been > dropped. > > One major complication of this reference count is that ice_ptp_port is > embedded inside of other structures and not merely allocated. As a result, we > can't use the standard pattern of kfree_rcu() to just delay freeing until > references are dropped, and instead are delaying PF port teardown. If any > code path leaks the reference, the driver will be unable to teardown. > Instead, a 15 second timeout with a WARN() is used when waiting to finally > allow PF teardown to continue. This has the risk of potentially allowing > use-after-free, assuming some path really is stuck for 15 seconds. However, > this both less likely and a less bad outcome compared to blocking > indefinitely on a reference leak. > > Fixes: e800654e85b5 ("ice: Use ice_adapter for PTP shared data instead of > auxdev") > Signed-off-by: Jacob Keller <[email protected]> > Reviewed-by: Maciek Machnikowski <[email protected]> > --- > drivers/net/ethernet/intel/ice/ice_adapter.h | 6 +- > drivers/net/ethernet/intel/ice/ice_ptp.h | 4 + > drivers/net/ethernet/intel/ice/ice_adapter.c | 7 +- > drivers/net/ethernet/intel/ice/ice_ptp.c | 118 > +++++++++++++++++++-------- > 4 files changed, 96 insertions(+), 39 deletions(-)
Tested-by: Alexander Nowlin <[email protected]>
