RE: [PATCH iwl-net 01/12] ice: use reference counting and RCU for PTP port access

> -----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]>