Merge branch 'ice-e82x-reduced-timestamp-processing-logic-fixes'

Jacob Keller says:

====================
ice: E82x: reduced timestamp processing logic fixes [part]

This is a reduced set patches originally posted as part of [1] which
previously passes Sashiko review without comments. It does not completely
resolve the outstanding issues that we had, but can hopefully unblock some
progress.

First, the locking around the PTP ports list in the adapter structure is
converted to use RCU primitives and a spinlock, resolving a couple of
reports from Petr about places where the original list was accessed without
lock protection. Note that an older version of this fix used an xarray
instead of the list. The xarray has more overhead and results in an
increase of ~25 microseconds to the average latency for processing Tx
timestamps. The list is simpler and avoids this overhead.

Next, the ice_ptp_request_ts() function is updated to sequence the marking
of the in_use bitmap in order to work properly with the lockless reader in
the IRQ thread. This issue was reported by Sashiko during development.

Next, Przemyslaw Korba fixes the E822 timestamping logic to keep timestamp
requests disabled until Vernier calibration completes.
====================

Link: https://patch.msgid.link/20261027-reduction-v1-0-81237c67a361@intel.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2026-10-28 18:09:12 -07:00
commit 280352de4f
4 changed files with 160 additions and 58 deletions

View File

@ -6,6 +6,7 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/srcu.h>
#include <linux/xarray.h>
#include "ice_adapter.h"
#include "ice.h"
@ -54,11 +55,18 @@ static unsigned long ice_adapter_xa_index(struct pci_dev *pdev)
static struct ice_adapter *ice_adapter_new(struct pci_dev *pdev)
{
struct ice_adapter *adapter;
int err;
adapter = kzalloc_obj(*adapter);
if (!adapter)
return NULL;
err = init_srcu_struct(&adapter->ports.srcu);
if (err) {
kfree(adapter);
return NULL;
}
adapter->index = ice_adapter_index(pdev);
spin_lock_init(&adapter->ptp_gltsyn_time_lock);
spin_lock_init(&adapter->txq_ctx_lock);
@ -66,18 +74,19 @@ static struct ice_adapter *ice_adapter_new(struct pci_dev *pdev)
mutex_init(&adapter->cpi_phy_lock[i]);
refcount_set(&adapter->refcount, 1);
mutex_init(&adapter->ports.lock);
INIT_LIST_HEAD(&adapter->ports.ports);
spin_lock_init(&adapter->ports.lock);
INIT_LIST_HEAD(&adapter->ports.list);
return adapter;
}
static void ice_adapter_free(struct ice_adapter *adapter)
{
WARN_ON(!list_empty(&adapter->ports.ports));
WARN_ON(!list_empty(&adapter->ports.list));
for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
mutex_destroy(&adapter->cpi_phy_lock[i]);
mutex_destroy(&adapter->ports.lock);
cleanup_srcu_struct(&adapter->ports.srcu);
kfree(adapter);
}

View File

@ -6,6 +6,7 @@
#include <linux/types.h>
#include <linux/mutex.h>
#include <linux/srcu.h>
#include <linux/spinlock_types.h>
#include <linux/refcount_types.h>
@ -17,15 +18,19 @@ struct ice_pf;
/**
* struct ice_port_list - data used to store the list of adapter ports
*
* This structure contains data used to maintain a list of adapter ports
* This structure contains data used to maintain a list of adapter ports.
* Writers modifying the list *must* acquire the lock, and use SRCU safe list
* operations. Readers should use srcu_read_lock() on the provided domain.
*
* @ports: list of ports
* @lock: protect access to the ports list
* @list: list of ports
* @lock: protect write access to the list
* @srcu: Sleepable RCU domain for this adapter
*/
struct ice_port_list {
struct list_head ports;
/* To synchronize the ports list operations */
struct mutex lock;
struct list_head list;
/* To synchronize write operations on the port list */
spinlock_t lock;
struct srcu_struct srcu;
};
/**

View File

@ -1,6 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (C) 2021, Intel Corporation. */
#include <linux/rculist.h>
#include <linux/srcu.h>
#include <linux/wait_bit.h>
#include "ice.h"
#include "ice_lib.h"
#include "ice_trace.h"
@ -367,7 +370,7 @@ ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
{
lockdep_assert_held(&tx->lock);
return tx->init && !tx->calibrating;
return tx->init && !tx->disabled;
}
/**
@ -592,6 +595,12 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
bool drop_ts = !link_up;
struct sk_buff *skb;
/* Paired with smp_mb__before_atomic() in ice_ptp_request_ts()
* to ensure ordering the in_use bitmap and the indexed fields
* in the tstamps array.
*/
smp_rmb();
/* Drop packets which have waited for more than 2 seconds */
if (time_is_before_jiffies(tx->tstamps[idx].start + 2 * HZ)) {
drop_ts = true;
@ -676,20 +685,37 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
pf->ptp.tx_hwtstamp_good += tstamp_good;
}
static void ice_ptp_release_port_srcu(struct kref *ref)
{
/* wake_up_var() requires full ordering but kref_put only provides
* release ordering.
*/
smp_mb__after_atomic();
wake_up_var(ref);
}
static void ice_ptp_tx_tstamp_owner(struct ice_pf *pf)
{
struct ice_port_list *ports = &pf->adapter->ports;
struct ice_ptp_port *port;
int srcu_idx;
mutex_lock(&pf->adapter->ports.lock);
list_for_each_entry(port, &pf->adapter->ports.ports, list_node) {
srcu_idx = srcu_read_lock(&ports->srcu);
list_for_each_entry_srcu(port, &ports->list, list_node,
srcu_read_lock_held(&ports->srcu)) {
struct ice_ptp_tx *tx = &port->tx;
if (!tx || !tx->init)
if (!tx->init)
continue;
if (!kref_get_unless_zero(&port->ref))
continue;
ice_ptp_process_tx_tstamp(tx);
kref_put(&port->ref, ice_ptp_release_port_srcu);
}
mutex_unlock(&pf->adapter->ports.lock);
srcu_read_unlock(&ports->srcu, srcu_idx);
}
/**
@ -809,10 +835,19 @@ ice_ptp_mark_tx_tracker_stale(struct ice_ptp_tx *tx)
static void
ice_ptp_flush_all_tx_tracker(struct ice_pf *pf)
{
struct ice_port_list *ports = &pf->adapter->ports;
struct ice_ptp_port *port;
int srcu_idx;
list_for_each_entry(port, &pf->adapter->ports.ports, list_node)
srcu_idx = srcu_read_lock(&ports->srcu);
list_for_each_entry_srcu(port, &ports->list, list_node,
srcu_read_lock_held(&ports->srcu)) {
if (!kref_get_unless_zero(&port->ref))
continue;
ice_ptp_flush_tx_tracker(ptp_port_to_pf(port), &port->tx);
kref_put(&port->ref, ice_ptp_release_port_srcu);
}
srcu_read_unlock(&ports->srcu, srcu_idx);
}
/**
@ -1136,6 +1171,7 @@ static int ice_ptp_check_tx_fifo(struct ice_ptp_port *port)
static void ice_ptp_wait_for_offsets(struct kthread_work *work)
{
struct ice_ptp_port *port;
unsigned long flags;
struct ice_pf *pf;
struct ice_hw *hw;
int tx_err;
@ -1156,9 +1192,26 @@ static void ice_ptp_wait_for_offsets(struct kthread_work *work)
tx_err = ice_ptp_check_tx_fifo(port);
if (!tx_err)
tx_err = ice_phy_cfg_tx_offset_e82x(hw, port->port_num);
if (!tx_err) {
/* Tx offset has been configured, re-enable Tx timestamps */
spin_lock_irqsave(&port->tx.lock, flags);
if (port->tx.disabled) {
port->tx.disabled = false;
dev_dbg(ice_pf_to_dev(pf), "PTP Tx offset valid for port %u, Tx timestamps enabled\n",
port->port_num);
}
spin_unlock_irqrestore(&port->tx.lock, flags);
}
rx_err = ice_phy_cfg_rx_offset_e82x(hw, port->port_num);
if (tx_err || rx_err) {
/* Tx and/or Rx offset not yet configured, try again later */
/* Tx and/or Rx offset not yet configured, try again later.
* This is expected during normal link-up: the vernier offset
* calibration cannot complete until at least one packet has
* been transmitted, so the first retries routinely land here.
*/
dev_dbg(ice_pf_to_dev(pf), "PTP offset not yet valid for port %u (tx_err=%d rx_err=%d)\n",
port->port_num, tx_err, rx_err);
kthread_queue_delayed_work(pf->ptp.kworker,
&port->ov_work,
msecs_to_jiffies(100));
@ -1236,11 +1289,9 @@ ice_ptp_port_phy_restart(struct ice_ptp_port *ptp_port)
/* Start the PHY timer in Vernier mode */
kthread_cancel_delayed_work_sync(&ptp_port->ov_work);
/* temporarily disable Tx timestamps while calibrating
* PHY offset
*/
/* Disable Tx timestamps until PHY calibration completes */
spin_lock_irqsave(&ptp_port->tx.lock, flags);
ptp_port->tx.calibrating = true;
ptp_port->tx.disabled = true;
spin_unlock_irqrestore(&ptp_port->tx.lock, flags);
ptp_port->tx_fifo_busy_cnt = 0;
@ -1249,11 +1300,10 @@ ice_ptp_port_phy_restart(struct ice_ptp_port *ptp_port)
if (err)
break;
/* Enable Tx timestamps right away */
spin_lock_irqsave(&ptp_port->tx.lock, flags);
ptp_port->tx.calibrating = false;
spin_unlock_irqrestore(&ptp_port->tx.lock, flags);
/* Do not clear disabled flag here. Tx timestamp requests
* remain disabled until ice_ptp_wait_for_offsets() has
* verified that the Tx offset calibration has completed.
*/
kthread_queue_delayed_work(pf->ptp.kworker, &ptp_port->ov_work,
0);
break;
@ -1427,16 +1477,22 @@ static void ice_ptp_reset_phy_timestamping(struct ice_pf *pf)
*/
static void ice_ptp_restart_all_phy(struct ice_pf *pf)
{
struct list_head *entry;
struct ice_port_list *ports = &pf->adapter->ports;
struct ice_ptp_port *port;
int srcu_idx;
list_for_each(entry, &pf->adapter->ports.ports) {
struct ice_ptp_port *port = list_entry(entry,
struct ice_ptp_port,
list_node);
srcu_idx = srcu_read_lock(&ports->srcu);
list_for_each_entry_srcu(port, &ports->list, list_node,
srcu_read_lock_held(&ports->srcu)) {
if (!kref_get_unless_zero(&port->ref))
continue;
if (port->link_up)
ice_ptp_port_phy_restart(port);
kref_put(&port->ref, ice_ptp_release_port_srcu);
}
srcu_read_unlock(&ports->srcu, srcu_idx);
}
/**
@ -2643,11 +2699,13 @@ s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb)
* a reference to the skb and the start time to allow discarding old
* requests.
*/
set_bit(idx, tx->in_use);
clear_bit(idx, tx->stale);
tx->tstamps[idx].start = jiffies;
tx->tstamps[idx].skb = skb_get(skb);
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
clear_bit(idx, tx->stale);
/* Ensure index is setup before marking it as used */
smp_mb__before_atomic();
set_bit(idx, tx->in_use);
ice_trace(tx_tstamp_request, skb, idx);
}
@ -2699,19 +2757,28 @@ static bool ice_port_has_timestamps(struct ice_ptp_tx *tx)
static bool ice_any_port_has_timestamps(struct ice_pf *pf)
{
struct ice_port_list *ports = &pf->adapter->ports;
bool have_tstamps = false;
struct ice_ptp_port *port;
int srcu_idx;
scoped_guard(mutex, &pf->adapter->ports.lock) {
list_for_each_entry(port, &pf->adapter->ports.ports,
list_node) {
struct ice_ptp_tx *tx = &port->tx;
srcu_idx = srcu_read_lock(&ports->srcu);
list_for_each_entry_srcu(port, &ports->list, list_node,
srcu_read_lock_held(&ports->srcu)) {
if (!kref_get_unless_zero(&port->ref))
continue;
if (ice_port_has_timestamps(tx))
return true;
}
if (ice_port_has_timestamps(&port->tx))
have_tstamps = true;
kref_put(&port->ref, ice_ptp_release_port_srcu);
if (have_tstamps)
break;
}
srcu_read_unlock(&ports->srcu, srcu_idx);
return false;
return have_tstamps;
}
bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf)
@ -2970,14 +3037,18 @@ void ice_ptp_queue_work(struct ice_pf *pf)
static void ice_ptp_prepare_rebuild_sec(struct ice_pf *pf, bool rebuild,
enum ice_reset_req reset_type)
{
struct list_head *entry;
struct ice_port_list *ports = &pf->adapter->ports;
struct ice_ptp_port *port;
int srcu_idx;
list_for_each(entry, &pf->adapter->ports.ports) {
struct ice_ptp_port *port = list_entry(entry,
struct ice_ptp_port,
list_node);
srcu_idx = srcu_read_lock(&ports->srcu);
list_for_each_entry_srcu(port, &ports->list, list_node,
srcu_read_lock_held(&ports->srcu)) {
struct ice_pf *peer_pf = ptp_port_to_pf(port);
if (!kref_get_unless_zero(&port->ref))
continue;
if (!ice_is_primary(&peer_pf->hw)) {
if (rebuild) {
/* TODO: When implementing rebuild=true:
@ -2989,7 +3060,10 @@ static void ice_ptp_prepare_rebuild_sec(struct ice_pf *pf, bool rebuild,
ice_ptp_prepare_for_reset(peer_pf, reset_type);
}
}
kref_put(&port->ref, ice_ptp_release_port_srcu);
}
srcu_read_unlock(&ports->srcu, srcu_idx);
}
/**
@ -3169,11 +3243,11 @@ static int ice_ptp_setup_pf(struct ice_pf *pf)
return -ENODEV;
INIT_LIST_HEAD(&ptp->port.list_node);
mutex_lock(&pf->adapter->ports.lock);
kref_init(&ptp->port.ref);
list_add(&ptp->port.list_node,
&pf->adapter->ports.ports);
mutex_unlock(&pf->adapter->ports.lock);
spin_lock(&pf->adapter->ports.lock);
list_add_rcu(&ptp->port.list_node, &pf->adapter->ports.list);
spin_unlock(&pf->adapter->ports.lock);
/* Seed the per-PHY Tx reference clock usage map for this port.
* Only meaningful on E825 (other MAC types don't expose tx-clk
@ -3195,13 +3269,23 @@ static int ice_ptp_setup_pf(struct ice_pf *pf)
static void ice_ptp_cleanup_pf(struct ice_pf *pf)
{
struct ice_port_list *ports = &pf->adapter->ports;
struct ice_ptp *ptp = &pf->ptp;
struct kref *ref;
if (pf->hw.mac_type != ICE_MAC_UNKNOWN) {
mutex_lock(&pf->adapter->ports.lock);
list_del(&ptp->port.list_node);
mutex_unlock(&pf->adapter->ports.lock);
}
if (pf->hw.mac_type == ICE_MAC_UNKNOWN)
return;
spin_lock(&ports->lock);
list_del_rcu(&ptp->port.list_node);
spin_unlock(&ports->lock);
ref = &ptp->port.ref;
kref_put(ref, ice_ptp_release_port_srcu);
wait_var_event(ref, !kref_read(ref));
synchronize_srcu(&ports->srcu);
}
/**

View File

@ -5,6 +5,8 @@
#define _ICE_PTP_H_
#include <linux/ptp_clock_kernel.h>
#include <linux/rculist.h>
#include <linux/kref.h>
#include <linux/kthread.h>
#include "ice_ptp_hw.h"
@ -104,8 +106,8 @@ enum ice_tx_tstamp_work {
* @offset: offset into timestamp block to get the real index
* @len: length of the tstamps and in_use fields.
* @init: if true, the tracker is initialized;
* @calibrating: if true, the PHY is calibrating the Tx offset. During this
* window, timestamps are temporarily disabled.
* @disabled: set to true when new Tx timestamp requests are disabled, such as
* while the PHY Tx offset configuration has not yet completed.
* @has_ready_bitmap: if true, the hardware has a valid Tx timestamp ready
* bitmap register. If false, fall back to verifying new
* timestamp values against previously cached copy.
@ -120,7 +122,7 @@ struct ice_ptp_tx {
u8 offset;
u8 len;
u8 init : 1;
u8 calibrating : 1;
u8 disabled : 1;
u8 has_ready_bitmap : 1;
s8 last_ll_ts_idx_read;
};
@ -138,6 +140,7 @@ struct ice_ptp_tx {
* and determine when the port's PHY offset is valid.
*
* @list_node: list member structure
* @ref: reference counter for use with adapter ports list
* @tx: Tx timestamp tracking for this port
* @ov_work: delayed work task for tracking when PHY offset is valid
* @ps_lock: mutex used to protect the overall PTP PHY start procedure
@ -149,6 +152,7 @@ struct ice_ptp_tx {
*/
struct ice_ptp_port {
struct list_head list_node;
struct kref ref;
struct ice_ptp_tx tx;
struct kthread_delayed_work ov_work;
struct mutex ps_lock; /* protects overall PTP PHY start procedure */