Commit 642436a1 authored by Yannick Vignon's avatar Yannick Vignon Committed by Jakub Kicinski
Browse files

net: stmmac: optimize locking around PTP clock reads 



Reading the PTP clock is a simple operation requiring only 3 register
reads. Under a PREEMPT_RT kernel, protecting those reads by a spin_lock is
counter-productive: if the 2nd task preempting the 1st has a higher prio
but needs to read time as well, it will require 2 context switches, which
will pretty much always be more costly than just disabling preemption for
the duration of the reads. Moreover, with the code logic recently added
to get_systime(), disabling preemption is not even required anymore:
reads and writes just need to be protected from each other, to prevent a
clock read while the clock is being updated.

Improve the above situation by replacing the PTP spinlock by a rwlock, and
using read_lock for PTP clock reads so simultaneous reads do not block
each other.

Signed-off-by: default avatarYannick Vignon <yannick.vignon@nxp.com>
Link: https://lore.kernel.org/r/20220204135545.2770625-1-yannick.vignon@oss.nxp.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent d1d5bd64

drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c

+2 −2
Original line number Diff line number Diff line
@@ -383,10 +383,10 @@ static int intel_crosststamp(ktime_t *device, 


	/* Repeat until the timestamps are from the FIFO last segment */

	for (i = 0; i < num_snapshot; i++) {

		spin_lock_irqsave(&priv->ptp_lock, flags);

		read_lock_irqsave(&priv->ptp_lock, flags);

		stmmac_get_ptptime(priv, ptpaddr, &ptp_time);

		*device = ns_to_ktime(ptp_time);

		spin_unlock_irqrestore(&priv->ptp_lock, flags);

		read_unlock_irqrestore(&priv->ptp_lock, flags);

		get_arttime(priv->mii, intel_priv->mdio_adhoc_addr, &art_time);

		*system = convert_art_to_tsc(art_time);

	}

drivers/net/ethernet/stmicro/stmmac/stmmac.h

+1 −1
Original line number Diff line number Diff line
@@ -263,7 +263,7 @@ struct stmmac_priv { 
	u32 adv_ts;

	int use_riwt;

	int irq_wake;

	spinlock_t ptp_lock;

	rwlock_t ptp_lock;

	/* Protects auxiliary snapshot registers from concurrent access. */

	struct mutex aux_ts_lock;

drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c

+2 −2
Original line number Diff line number Diff line
@@ -196,9 +196,9 @@ static void timestamp_interrupt(struct stmmac_priv *priv) 
		       GMAC_TIMESTAMP_ATSNS_SHIFT;



	for (i = 0; i < num_snapshot; i++) {

		spin_lock_irqsave(&priv->ptp_lock, flags);

		read_lock_irqsave(&priv->ptp_lock, flags);

		get_ptptime(priv->ptpaddr, &ptp_time);

		spin_unlock_irqrestore(&priv->ptp_lock, flags);

		read_unlock_irqrestore(&priv->ptp_lock, flags);

		event.type = PTP_CLOCK_EXTTS;

		event.index = 0;

		event.timestamp = ptp_time;

drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c

+11 −11
Original line number Diff line number Diff line
@@ -39,9 +39,9 @@ static int stmmac_adjust_freq(struct ptp_clock_info *ptp, s32 ppb) 
	diff = div_u64(adj, 1000000000ULL);

	addend = neg_adj ? (addend - diff) : (addend + diff);



	spin_lock_irqsave(&priv->ptp_lock, flags);

	write_lock_irqsave(&priv->ptp_lock, flags);

	stmmac_config_addend(priv, priv->ptpaddr, addend);

	spin_unlock_irqrestore(&priv->ptp_lock, flags);

	write_unlock_irqrestore(&priv->ptp_lock, flags);



	return 0;

}
@@ -86,9 +86,9 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta) 
		mutex_unlock(&priv->plat->est->lock);

	}



	spin_lock_irqsave(&priv->ptp_lock, flags);

	write_lock_irqsave(&priv->ptp_lock, flags);

	stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj, xmac);

	spin_unlock_irqrestore(&priv->ptp_lock, flags);

	write_unlock_irqrestore(&priv->ptp_lock, flags);



	/* Caculate new basetime and re-configured EST after PTP time adjust. */

	if (est_rst) {
@@ -137,9 +137,9 @@ static int stmmac_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts) 
	unsigned long flags;

	u64 ns = 0;



	spin_lock_irqsave(&priv->ptp_lock, flags);

	read_lock_irqsave(&priv->ptp_lock, flags);

	stmmac_get_systime(priv, priv->ptpaddr, &ns);

	spin_unlock_irqrestore(&priv->ptp_lock, flags);

	read_unlock_irqrestore(&priv->ptp_lock, flags);



	*ts = ns_to_timespec64(ns);
@@ -162,9 +162,9 @@ static int stmmac_set_time(struct ptp_clock_info *ptp, 
	    container_of(ptp, struct stmmac_priv, ptp_clock_ops);

	unsigned long flags;



	spin_lock_irqsave(&priv->ptp_lock, flags);

	write_lock_irqsave(&priv->ptp_lock, flags);

	stmmac_init_systime(priv, priv->ptpaddr, ts->tv_sec, ts->tv_nsec);

	spin_unlock_irqrestore(&priv->ptp_lock, flags);

	write_unlock_irqrestore(&priv->ptp_lock, flags);



	return 0;

}
@@ -194,12 +194,12 @@ static int stmmac_enable(struct ptp_clock_info *ptp, 
		cfg->period.tv_sec = rq->perout.period.sec;

		cfg->period.tv_nsec = rq->perout.period.nsec;



		spin_lock_irqsave(&priv->ptp_lock, flags);

		write_lock_irqsave(&priv->ptp_lock, flags);

		ret = stmmac_flex_pps_config(priv, priv->ioaddr,

					     rq->perout.index, cfg, on,

					     priv->sub_second_inc,

					     priv->systime_flags);

		spin_unlock_irqrestore(&priv->ptp_lock, flags);

		write_unlock_irqrestore(&priv->ptp_lock, flags);

		break;

	case PTP_CLK_REQ_EXTTS:

		priv->plat->ext_snapshot_en = on;
@@ -314,7 +314,7 @@ void stmmac_ptp_register(struct stmmac_priv *priv) 
	stmmac_ptp_clock_ops.n_per_out = priv->dma_cap.pps_out_num;

	stmmac_ptp_clock_ops.n_ext_ts = priv->dma_cap.aux_snapshot_n;



	spin_lock_init(&priv->ptp_lock);

	rwlock_init(&priv->ptp_lock);

	mutex_init(&priv->aux_ts_lock);

	priv->ptp_clock_ops = stmmac_ptp_clock_ops;

drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c

+4 −4
Original line number Diff line number Diff line
@@ -1777,9 +1777,9 @@ static int stmmac_test_tbs(struct stmmac_priv *priv) 
	if (ret)

		return ret;



	spin_lock_irqsave(&priv->ptp_lock, flags);

	read_lock_irqsave(&priv->ptp_lock, flags);

	stmmac_get_systime(priv, priv->ptpaddr, &curr_time);

	spin_unlock_irqrestore(&priv->ptp_lock, flags);

	read_unlock_irqrestore(&priv->ptp_lock, flags);



	if (!curr_time) {

		ret = -EOPNOTSUPP;
@@ -1799,9 +1799,9 @@ static int stmmac_test_tbs(struct stmmac_priv *priv) 
		goto fail_disable;



	/* Check if expected time has elapsed */

	spin_lock_irqsave(&priv->ptp_lock, flags);

	read_lock_irqsave(&priv->ptp_lock, flags);

	stmmac_get_systime(priv, priv->ptpaddr, &curr_time);

	spin_unlock_irqrestore(&priv->ptp_lock, flags);

	read_unlock_irqrestore(&priv->ptp_lock, flags);



	if ((curr_time - start_time) < STMMAC_TBS_LT_OFFSET)

		ret = -EINVAL;