[Intel-wired-lan] [PATCH iwl-next 0/5] ice: Improve miscellaneous interrupt code

[Intel-wired-lan] [PATCH iwl-next 0/5] ice: Improve miscellaneous interrupt code

[Jacob Keller]

This series fixes issues that occur due to the interaction with the hard IRQ
ice_misc_intr() function and the soft IRQ ice_misc_intr_thread_fn().

First, Karol fixes an issue with the external timestamp being handled by the
PTP kworker, resulting in a disruption of timestamping an input signal on
the external software defined pins with a frequency of 100Hz.

Second, Karol fixes an issue when running on PREEMPT_RT kernels due to such
kernels not allowing queue_work() inside of the hard IRQ. The scheduling of
the service task is moved into the soft IRQ thread function instead.

Following this, I have three patches intended to fix an issue with the
handling for E822 hardware involving a rather subtle and complicated race
between the hard IRQ function and the soft IRQ thread function. Ultimately
the code is refactored so that we stop polling in the thread function, and
ensure that the OICR (Other interrupt control register) is disabled for the
duration of the thread.

I think that while this does fix a race condition issue, the amount of
changes required to refactor the interrupt function is large enough that it
does not warrant going into net.

The race itself manifests only if multiple Tx timestamps are outstanding at
once. This is atypical behavior for most applications. For example, ptp4l
synchronously waits for timestamps before continuing processing.

Jacob Keller (3):
  ice: introduce ICE_TX_TSTAMP_WORK enumeration
  ice: trigger PFINT_OICR_TSYN_TX interrupt instead of polling
  ice: do not re-enable miscellaneous interrupt until thread_fn
    completes

Karol Kolacinski (2):
  ice: handle extts in the miscellaneous interrupt thread
  ice: schedule service task in IRQ thread_fn

 drivers/net/ethernet/intel/ice/ice.h      |  1 +
 drivers/net/ethernet/intel/ice/ice_main.c | 52 +++++++++++++------
 drivers/net/ethernet/intel/ice/ice_ptp.c  | 61 ++++++++++++-----------
 drivers/net/ethernet/intel/ice/ice_ptp.h  | 16 ++++--
 4 files changed, 83 insertions(+), 47 deletions(-)


base-commit: 1e806efa4f2837a829044df27e1196a4fd520ba3
-- 
2.40.0.471.gbd7f14d9353b

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH iwl-next 1/5] ice: handle extts in the miscellaneous interrupt thread

[Jacob Keller]

From: Karol Kolacinski <karol.kolacinski@intel.com>

The ice_ptp_extts_work() and ice_ptp_periodic_work() functions are both
scheduled on the same kthread worker, pf.ptp.kworker. The
ice_ptp_periodic_work() function sends to the firmware to interact with the
PHY, and must block to wait for responses.

This can cause delay in responding to the PFINT_OICR_TSYN_EVNT interrupt
cause, ultimately resulting in disruption to processing an input signal of
the frequency is high enough. In our testing, even 100 Hz signals get
disrupted.

Fix this by instead processing the signal inside the miscellaneous
interrupt thread prior to handling Tx timestamps.

Note the unconditional usleep_range loop in ice_mics_intr_thread_fn() is
pre-existing. While concerning, it will be removed in the last change in a
following chance that refactors to use a software-triggered interrupt
instead.

Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h      |  1 +
 drivers/net/ethernet/intel/ice/ice_main.c | 33 +++++++++++++++++------
 drivers/net/ethernet/intel/ice/ice_ptp.c  | 12 +++------
 drivers/net/ethernet/intel/ice/ice_ptp.h  |  4 +--
 4 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 176e281dfa24..4256b9f5a25f 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -580,6 +580,7 @@ struct ice_pf {
 	u32 hw_csum_rx_error;
 	u32 oicr_err_reg;
 	struct msi_map oicr_irq;	/* Other interrupt cause MSIX vector */
+	u16 oicr_misc;		/* Misc interrupts to handle in bottom half */
 	u16 max_pf_txqs;	/* Total Tx queues PF wide */
 	u16 max_pf_rxqs;	/* Total Rx queues PF wide */
 	u16 num_lan_msix;	/* Total MSIX vectors for base driver */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index c7d7104dbadc..9e4d7d884115 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3139,20 +3139,28 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 
 	if (oicr & PFINT_OICR_TSYN_TX_M) {
 		ena_mask &= ~PFINT_OICR_TSYN_TX_M;
-		if (!hw->reset_ongoing)
+		if (!hw->reset_ongoing) {
+			pf->oicr_misc |= PFINT_OICR_TSYN_TX_M;
 			ret = IRQ_WAKE_THREAD;
+		}
 	}
 
 	if (oicr & PFINT_OICR_TSYN_EVNT_M) {
 		u8 tmr_idx = hw->func_caps.ts_func_info.tmr_index_owned;
 		u32 gltsyn_stat = rd32(hw, GLTSYN_STAT(tmr_idx));
 
-		/* Save EVENTs from GTSYN register */
-		pf->ptp.ext_ts_irq |= gltsyn_stat & (GLTSYN_STAT_EVENT0_M |
-						     GLTSYN_STAT_EVENT1_M |
-						     GLTSYN_STAT_EVENT2_M);
 		ena_mask &= ~PFINT_OICR_TSYN_EVNT_M;
-		kthread_queue_work(pf->ptp.kworker, &pf->ptp.extts_work);
+
+		if (hw->func_caps.ts_func_info.src_tmr_owned) {
+			/* Save EVENTs from GLTSYN register */
+			pf->ptp.ext_ts_irq |= gltsyn_stat &
+					      (GLTSYN_STAT_EVENT0_M |
+					       GLTSYN_STAT_EVENT1_M |
+					       GLTSYN_STAT_EVENT2_M);
+
+			pf->oicr_misc |= PFINT_OICR_TSYN_EVNT_M;
+			ret = IRQ_WAKE_THREAD;
+		}
 	}
 
 #define ICE_AUX_CRIT_ERR (PFINT_OICR_PE_CRITERR_M | PFINT_OICR_HMC_ERR_M | PFINT_OICR_PE_PUSH_M)
@@ -3196,8 +3204,16 @@ static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
 	if (ice_is_reset_in_progress(pf->state))
 		return IRQ_HANDLED;
 
-	while (!ice_ptp_process_ts(pf))
-		usleep_range(50, 100);
+	if (pf->oicr_misc & PFINT_OICR_TSYN_EVNT_M) {
+		ice_ptp_extts_event(pf);
+		pf->oicr_misc &= ~PFINT_OICR_TSYN_EVNT_M;
+	}
+
+	if (pf->oicr_misc & PFINT_OICR_TSYN_TX_M) {
+		while (!ice_ptp_process_ts(pf))
+			usleep_range(50, 100);
+		pf->oicr_misc &= ~PFINT_OICR_TSYN_TX_M;
+	}
 
 	return IRQ_HANDLED;
 }
@@ -4958,6 +4974,7 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 	pf = ice_allocate_pf(dev);
 	if (!pf)
 		return -ENOMEM;
+	pf->oicr_misc = 0;
 
 	/* initialize Auxiliary index to invalid value */
 	pf->aux_idx = -1;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index d4b6c997141d..6f51ebaf1d70 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1458,15 +1458,11 @@ static int ice_ptp_adjfine(struct ptp_clock_info *info, long scaled_ppm)
 }
 
 /**
- * ice_ptp_extts_work - Workqueue task function
- * @work: external timestamp work structure
- *
- * Service for PTP external clock event
+ * ice_ptp_extts_event - Process PTP external clock event
+ * @pf: Board private structure
  */
-static void ice_ptp_extts_work(struct kthread_work *work)
+void ice_ptp_extts_event(struct ice_pf *pf)
 {
-	struct ice_ptp *ptp = container_of(work, struct ice_ptp, extts_work);
-	struct ice_pf *pf = container_of(ptp, struct ice_pf, ptp);
 	struct ptp_clock_event event;
 	struct ice_hw *hw = &pf->hw;
 	u8 chan, tmr_idx;
@@ -2558,7 +2554,6 @@ void ice_ptp_prepare_for_reset(struct ice_pf *pf)
 	ice_ptp_cfg_timestamp(pf, false);
 
 	kthread_cancel_delayed_work_sync(&ptp->work);
-	kthread_cancel_work_sync(&ptp->extts_work);
 
 	if (test_bit(ICE_PFR_REQ, pf->state))
 		return;
@@ -2656,7 +2651,6 @@ static int ice_ptp_init_work(struct ice_pf *pf, struct ice_ptp *ptp)
 
 	/* Initialize work functions */
 	kthread_init_delayed_work(&ptp->work, ice_ptp_periodic_work);
-	kthread_init_work(&ptp->extts_work, ice_ptp_extts_work);
 
 	/* Allocate a kworker for handling work required for the ports
 	 * connected to the PTP hardware clock.
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 9cda2f43e0e5..9f8902c1e743 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -169,7 +169,6 @@ struct ice_ptp_port {
  * struct ice_ptp - data used for integrating with CONFIG_PTP_1588_CLOCK
  * @port: data for the PHY port initialization procedure
  * @work: delayed work function for periodic tasks
- * @extts_work: work function for handling external Tx timestamps
  * @cached_phc_time: a cached copy of the PHC time for timestamp extension
  * @cached_phc_jiffies: jiffies when cached_phc_time was last updated
  * @ext_ts_chan: the external timestamp channel in use
@@ -190,7 +189,6 @@ struct ice_ptp_port {
 struct ice_ptp {
 	struct ice_ptp_port port;
 	struct kthread_delayed_work work;
-	struct kthread_work extts_work;
 	u64 cached_phc_time;
 	unsigned long cached_phc_jiffies;
 	u8 ext_ts_chan;
@@ -256,6 +254,7 @@ int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr);
 void ice_ptp_cfg_timestamp(struct ice_pf *pf, bool ena);
 int ice_get_ptp_clock_index(struct ice_pf *pf);
 
+void ice_ptp_extts_event(struct ice_pf *pf);
 s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb);
 bool ice_ptp_process_ts(struct ice_pf *pf);
 
@@ -284,6 +283,7 @@ static inline int ice_get_ptp_clock_index(struct ice_pf *pf)
 	return -1;
 }
 
+static inline void ice_ptp_extts_event(struct ice_pf *pf) { }
 static inline s8
 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb)
 {
-- 
2.40.0.471.gbd7f14d9353b

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH iwl-next 2/5] ice: schedule service task in IRQ thread_fn

[Jacob Keller]

From: Karol Kolacinski <karol.kolacinski@intel.com>

If the kernel is configured with CONFIG_PREEMPT_RT, scheduling the service
task in interrupt context can result in a kernel panic. This is a result of
ice_service_task_schedule calling queue_work.

Move the ice_service_task_schedule() call into the miscellaneous IRQ thread
that functions as the interrupt bottom half.

Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 9e4d7d884115..8b59632ec6b1 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3058,7 +3058,6 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 {
 	struct ice_pf *pf = (struct ice_pf *)data;
 	struct ice_hw *hw = &pf->hw;
-	irqreturn_t ret = IRQ_NONE;
 	struct device *dev;
 	u32 oicr, ena_mask;
 
@@ -3139,10 +3138,8 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 
 	if (oicr & PFINT_OICR_TSYN_TX_M) {
 		ena_mask &= ~PFINT_OICR_TSYN_TX_M;
-		if (!hw->reset_ongoing) {
+		if (!hw->reset_ongoing)
 			pf->oicr_misc |= PFINT_OICR_TSYN_TX_M;
-			ret = IRQ_WAKE_THREAD;
-		}
 	}
 
 	if (oicr & PFINT_OICR_TSYN_EVNT_M) {
@@ -3159,7 +3156,6 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 					       GLTSYN_STAT_EVENT2_M);
 
 			pf->oicr_misc |= PFINT_OICR_TSYN_EVNT_M;
-			ret = IRQ_WAKE_THREAD;
 		}
 	}
 
@@ -3180,16 +3176,12 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 		if (oicr & (PFINT_OICR_PCI_EXCEPTION_M |
 			    PFINT_OICR_ECC_ERR_M)) {
 			set_bit(ICE_PFR_REQ, pf->state);
-			ice_service_task_schedule(pf);
 		}
 	}
-	if (!ret)
-		ret = IRQ_HANDLED;
 
-	ice_service_task_schedule(pf);
 	ice_irq_dynamic_ena(hw, NULL, NULL);
 
-	return ret;
+	return IRQ_WAKE_THREAD;
 }
 
 /**
@@ -3204,6 +3196,8 @@ static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
 	if (ice_is_reset_in_progress(pf->state))
 		return IRQ_HANDLED;
 
+	ice_service_task_schedule(pf);
+
 	if (pf->oicr_misc & PFINT_OICR_TSYN_EVNT_M) {
 		ice_ptp_extts_event(pf);
 		pf->oicr_misc &= ~PFINT_OICR_TSYN_EVNT_M;
-- 
2.40.0.471.gbd7f14d9353b

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH iwl-next 2/5] ice: schedule service task in IRQ thread_fn

[Michal Schmidt]

Den 27.05.2023 kl. 00.21 skrev Jacob Keller:
> From: Karol Kolacinski <karol.kolacinski@intel.com>
> 
> If the kernel is configured with CONFIG_PREEMPT_RT, scheduling the service
> task in interrupt context can result in a kernel panic. This is a result of
> ice_service_task_schedule calling queue_work.

Is it really the case on current kernels that one cannot use queue_work 
in that context?
The previous posting of this patch showed a stack trace from a 
3.10-based vendor kernel. Has the crash been seen on anything recent?
I think the workqueue code has been safe to use in atomic context on 
even on PREEMPT_RT since commit fe3bc8a988a4 ("Merge branch 'for-5.8' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq").

That said, the patch looks OK to me. It makes the code cleaner. I object 
only to the description.

Michal

> Move the ice_service_task_schedule() call into the miscellaneous IRQ thread
> that functions as the interrupt bottom half.
> 
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_main.c | 14 ++++----------
>   1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 9e4d7d884115..8b59632ec6b1 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -3058,7 +3058,6 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
>   {
>   	struct ice_pf *pf = (struct ice_pf *)data;
>   	struct ice_hw *hw = &pf->hw;
> -	irqreturn_t ret = IRQ_NONE;
>   	struct device *dev;
>   	u32 oicr, ena_mask;
>   
> @@ -3139,10 +3138,8 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
>   
>   	if (oicr & PFINT_OICR_TSYN_TX_M) {
>   		ena_mask &= ~PFINT_OICR_TSYN_TX_M;
> -		if (!hw->reset_ongoing) {
> +		if (!hw->reset_ongoing)
>   			pf->oicr_misc |= PFINT_OICR_TSYN_TX_M;
> -			ret = IRQ_WAKE_THREAD;
> -		}
>   	}
>   
>   	if (oicr & PFINT_OICR_TSYN_EVNT_M) {
> @@ -3159,7 +3156,6 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
>   					       GLTSYN_STAT_EVENT2_M);
>   
>   			pf->oicr_misc |= PFINT_OICR_TSYN_EVNT_M;
> -			ret = IRQ_WAKE_THREAD;
>   		}
>   	}
>   
> @@ -3180,16 +3176,12 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
>   		if (oicr & (PFINT_OICR_PCI_EXCEPTION_M |
>   			    PFINT_OICR_ECC_ERR_M)) {
>   			set_bit(ICE_PFR_REQ, pf->state);
> -			ice_service_task_schedule(pf);
>   		}
>   	}
> -	if (!ret)
> -		ret = IRQ_HANDLED;
>   
> -	ice_service_task_schedule(pf);
>   	ice_irq_dynamic_ena(hw, NULL, NULL);
>   
> -	return ret;
> +	return IRQ_WAKE_THREAD;
>   }
>   
>   /**
> @@ -3204,6 +3196,8 @@ static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
>   	if (ice_is_reset_in_progress(pf->state))
>   		return IRQ_HANDLED;
>   
> +	ice_service_task_schedule(pf);
> +
>   	if (pf->oicr_misc & PFINT_OICR_TSYN_EVNT_M) {
>   		ice_ptp_extts_event(pf);
>   		pf->oicr_misc &= ~PFINT_OICR_TSYN_EVNT_M;

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH iwl-next 2/5] ice: schedule service task in IRQ thread_fn

[Keller, Jacob E]

> -----Original Message-----
> From: Michal Schmidt <mschmidt@redhat.com>
> Sent: Monday, May 29, 2023 3:43 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>; Intel Wired LAN <intel-wired-
> lan@lists.osuosl.org>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: Kolacinski, Karol <karol.kolacinski@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next 2/5] ice: schedule service task in
> IRQ thread_fn
> 
> Den 27.05.2023 kl. 00.21 skrev Jacob Keller:
> > From: Karol Kolacinski <karol.kolacinski@intel.com>
> >
> > If the kernel is configured with CONFIG_PREEMPT_RT, scheduling the service
> > task in interrupt context can result in a kernel panic. This is a result of
> > ice_service_task_schedule calling queue_work.
> 
> Is it really the case on current kernels that one cannot use queue_work
> in that context?
> The previous posting of this patch showed a stack trace from a
> 3.10-based vendor kernel. Has the crash been seen on anything recent?
> I think the workqueue code has been safe to use in atomic context on
> even on PREEMPT_RT since commit fe3bc8a988a4 ("Merge branch 'for-5.8' of
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq").
> 
> That said, the patch looks OK to me. It makes the code cleaner. I object
> only to the description.
> 
> Michal
> 

Hmm... we had developed this fix some time ago for a customer who was on an older stable kernel that didn't have this fix. I wasn't aware of it, so I assumed it was still a problem when writing this message for upstream. I think I prefer the overall code design here especially since we're moving processing of external timestamps.

I'll re-write the commit message.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration

[Jacob Keller]

The ice_ptp_process_ts() function and its various helper functions return a
boolean value indicating whether any work is remaining. This use of a
boolean has grown confusing as we have multiple helpers that pass status
between each other. Readers must be aware of what "true" and "false" mean,
and it is very easy to get their meaning inverted. The names of the
functions are not standard "yes/no" questions, which is the best practice
for boolean returns.

Replace this use of an enumeration with a custom type, enum
ice_tx_tstamp_work. This enumeration clearly indicates whether all work is
done, or if more work is pending.

To aid in readability, factor the actual list iteration and processing out
into ice_ptp_process_tx_tstamp(), making it void. Then call this in
ice_ptp_tx_tstamp() ensuring that we always check the Tracker list at the
end when determining the appropriate return value.

Now the return value is an explicit name instead of the true or false
value. This is easier to follow and makes reading the resulting callers
much simpler.

In addition, this paves the way for future work to allow E822 hardware to
process timestamps for all functions using a single interrupt on the clock
owning PF.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c |  2 +-
 drivers/net/ethernet/intel/ice/ice_ptp.c  | 49 ++++++++++++++---------
 drivers/net/ethernet/intel/ice/ice_ptp.h  | 12 +++++-
 3 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 8b59632ec6b1..481dccdb95cd 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3204,7 +3204,7 @@ static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
 	}
 
 	if (pf->oicr_misc & PFINT_OICR_TSYN_TX_M) {
-		while (!ice_ptp_process_ts(pf))
+		while (ice_ptp_process_ts(pf) == ICE_TX_TSTAMP_WORK_PENDING)
 			usleep_range(50, 100);
 		pf->oicr_misc &= ~PFINT_OICR_TSYN_TX_M;
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 6f51ebaf1d70..bbb9a44c5616 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -617,7 +617,7 @@ ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
 }
 
 /**
- * ice_ptp_tx_tstamp - Process Tx timestamps for a port
+ * ice_ptp_process_tx_tstamp - Process Tx timestamps for a port
  * @tx: the PTP Tx timestamp tracker
  *
  * Process timestamps captured by the PHY associated with this port. To do
@@ -633,15 +633,6 @@ ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
  * 6) extend the 40 bit timestamp value to get a 64 bit timestamp value
  * 7) send this 64 bit timestamp to the stack
  *
- * Returns true if all timestamps were handled, and false if any slots remain
- * without a timestamp.
- *
- * After looping, if we still have waiting SKBs, return false. This may cause
- * us effectively poll even when not strictly necessary. We do this because
- * it's possible a new timestamp was requested around the same time as the
- * interrupt. In some cases hardware might not interrupt us again when the
- * timestamp is captured.
- *
  * Note that we do not hold the tracking lock while reading the Tx timestamp.
  * This is because reading the timestamp requires taking a mutex that might
  * sleep.
@@ -673,7 +664,7 @@ ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
  * the packet will never be sent by hardware and discard it without reading
  * the timestamp register.
  */
-static bool ice_ptp_tx_tstamp(struct ice_ptp_tx *tx)
+static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
 {
 	struct ice_ptp_port *ptp_port;
 	bool more_timestamps;
@@ -685,7 +676,7 @@ static bool ice_ptp_tx_tstamp(struct ice_ptp_tx *tx)
 	u8 idx;
 
 	if (!tx->init)
-		return true;
+		return;
 
 	ptp_port = container_of(tx, struct ice_ptp_port, tx);
 	pf = ptp_port_to_pf(ptp_port);
@@ -694,7 +685,7 @@ static bool ice_ptp_tx_tstamp(struct ice_ptp_tx *tx)
 	/* Read the Tx ready status first */
 	err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
 	if (err)
-		return false;
+		return;
 
 	/* Drop packets if the link went down */
 	link_up = ptp_port->link_up;
@@ -782,15 +773,34 @@ static bool ice_ptp_tx_tstamp(struct ice_ptp_tx *tx)
 		skb_tstamp_tx(skb, &shhwtstamps);
 		dev_kfree_skb_any(skb);
 	}
+}
 
-	/* Check if we still have work to do. If so, re-queue this task to
-	 * poll for remaining timestamps.
-	 */
+/**
+ * ice_ptp_tx_tstamp - Process Tx timestamps for this function.
+ * @tx: Tx tracking structure to initialize
+ *
+ * Returns: ICE_TX_TSTAMP_WORK_PENDING if there are any outstanding incomplete
+ * Tx timestamps, or ICE_TX_TSTAMP_WORK_DONE otherwise.
+ */
+static enum ice_tx_tstamp_work ice_ptp_tx_tstamp(struct ice_ptp_tx *tx)
+{
+	bool more_timestamps;
+
+	if (!tx->init)
+		return ICE_TX_TSTAMP_WORK_DONE;
+
+	/* Process the Tx timestamp tracker */
+	ice_ptp_process_tx_tstamp(tx);
+
+	/* Check if there are outstanding Tx timestamps */
 	spin_lock(&tx->lock);
 	more_timestamps = tx->init && !bitmap_empty(tx->in_use, tx->len);
 	spin_unlock(&tx->lock);
 
-	return !more_timestamps;
+	if (more_timestamps)
+		return ICE_TX_TSTAMP_WORK_PENDING;
+
+	return ICE_TX_TSTAMP_WORK_DONE;
 }
 
 /**
@@ -2426,9 +2436,10 @@ s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb)
  * ice_ptp_process_ts - Process the PTP Tx timestamps
  * @pf: Board private structure
  *
- * Returns true if timestamps are processed.
+ * Returns: ICE_TX_TSTAMP_WORK_PENDING if there are any outstanding Tx
+ * timestamps that need processing, and ICE_TX_TSTAMP_WORK_DONE otherwise.
  */
-bool ice_ptp_process_ts(struct ice_pf *pf)
+enum ice_tx_tstamp_work ice_ptp_process_ts(struct ice_pf *pf)
 {
 	return ice_ptp_tx_tstamp(&pf->ptp.port.tx);
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 9f8902c1e743..6c90775e1eb0 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -108,6 +108,16 @@ struct ice_tx_tstamp {
 	u64 cached_tstamp;
 };
 
+/**
+ * enum ice_tx_tstamp_work - Status of Tx timestamp work function
+ * @ICE_TX_TSTAMP_WORK_DONE - Tx timestamp processing is complete
+ * @ICE_TX_TSTAMP_WORK_PENDING - More Tx timestamps are pending
+ */
+enum ice_tx_tstamp_work {
+	ICE_TX_TSTAMP_WORK_DONE = 0,
+	ICE_TX_TSTAMP_WORK_PENDING,
+};
+
 /**
  * struct ice_ptp_tx - Tracking structure for all Tx timestamp requests on a port
  * @lock: lock to prevent concurrent access to fields of this struct
@@ -256,7 +266,7 @@ int ice_get_ptp_clock_index(struct ice_pf *pf);
 
 void ice_ptp_extts_event(struct ice_pf *pf);
 s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb);
-bool ice_ptp_process_ts(struct ice_pf *pf);
+enum ice_tx_tstamp_work ice_ptp_process_ts(struct ice_pf *pf);
 
 void
 ice_ptp_rx_hwtstamp(struct ice_rx_ring *rx_ring,
-- 
2.40.0.471.gbd7f14d9353b

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration

[Jacob Keller]

On 5/26/2023 3:21 PM, Jacob Keller wrote:
> - * Returns true if all timestamps were handled, and false if any slots remain
> - * without a timestamp.
> - *
> - * After looping, if we still have waiting SKBs, return false. This may cause
> - * us effectively poll even when not strictly necessary. We do this because
> - * it's possible a new timestamp was requested around the same time as the
> - * interrupt. In some cases hardware might not interrupt us again when the
> - * timestamp is captured.
> - *
>   * Note that we do not hold the tracking lock while reading the Tx timestamp.
>   * This is because reading the timestamp requires taking a mutex that might
>   * sleep.
> @@ -673,7 +664,7 @@ ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
>   * the packet will never be sent by hardware and discard it without reading
>   * the timestamp register.
>   */
> -static bool ice_ptp_tx_tstamp(struct ice_ptp_tx *tx)
> +static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
>  {
>  	struct ice_ptp_port *ptp_port;
>  	bool more_timestamps;

This boolean is no longer used in this function. @Tony, could you amend
this to remove it when applying this? If not, let me know if I should
just send a v2.

Thanks,
Jake
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration

[kernel test robot]

Hi Jacob,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1e806efa4f2837a829044df27e1196a4fd520ba3]

url:    https://github.com/intel-lab-lkp/linux/commits/Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
base:   1e806efa4f2837a829044df27e1196a4fd520ba3
patch link:    https://lore.kernel.org/r/20230526222158.2685796-4-jacob.e.keller%40intel.com
patch subject: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230527/202305271021.Qlv0TxZu-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
        git checkout 23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/ethernet/intel/ice/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305271021.Qlv0TxZu-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/intel/ice/ice_ptp.c: In function 'ice_ptp_process_tx_tstamp':
>> drivers/net/ethernet/intel/ice/ice_ptp.c:670:14: warning: unused variable 'more_timestamps' [-Wunused-variable]
     670 |         bool more_timestamps;
         |              ^~~~~~~~~~~~~~~


vim +/more_timestamps +670 drivers/net/ethernet/intel/ice/ice_ptp.c

3ad5c10bf21d1d Jacob Keller       2022-12-05  618  
06c16d89d2cbe2 Jacob Keller       2021-06-09  619  /**
23cbd0608f6feb Jacob Keller       2023-05-26  620   * ice_ptp_process_tx_tstamp - Process Tx timestamps for a port
1229b33973c7b8 Karol Kolacinski   2022-09-16  621   * @tx: the PTP Tx timestamp tracker
06c16d89d2cbe2 Jacob Keller       2021-06-09  622   *
4b1251bdd18886 Jacob Keller       2022-07-27  623   * Process timestamps captured by the PHY associated with this port. To do
4b1251bdd18886 Jacob Keller       2022-07-27  624   * this, loop over each index with a waiting skb.
4b1251bdd18886 Jacob Keller       2022-07-27  625   *
4b1251bdd18886 Jacob Keller       2022-07-27  626   * If a given index has a valid timestamp, perform the following steps:
4b1251bdd18886 Jacob Keller       2022-07-27  627   *
d40fd60093325c Jacob Keller       2022-12-05  628   * 1) check that the timestamp request is not stale
d40fd60093325c Jacob Keller       2022-12-05  629   * 2) check that a timestamp is ready and available in the PHY memory bank
d40fd60093325c Jacob Keller       2022-12-05  630   * 3) read and copy the timestamp out of the PHY register
d40fd60093325c Jacob Keller       2022-12-05  631   * 4) unlock the index by clearing the associated in_use bit
d40fd60093325c Jacob Keller       2022-12-05  632   * 5) check if the timestamp is stale, and discard if so
d40fd60093325c Jacob Keller       2022-12-05  633   * 6) extend the 40 bit timestamp value to get a 64 bit timestamp value
d40fd60093325c Jacob Keller       2022-12-05  634   * 7) send this 64 bit timestamp to the stack
4b1251bdd18886 Jacob Keller       2022-07-27  635   *
d40fd60093325c Jacob Keller       2022-12-05  636   * Note that we do not hold the tracking lock while reading the Tx timestamp.
d40fd60093325c Jacob Keller       2022-12-05  637   * This is because reading the timestamp requires taking a mutex that might
d40fd60093325c Jacob Keller       2022-12-05  638   * sleep.
0dd92862639238 Jacob Keller       2022-12-05  639   *
d40fd60093325c Jacob Keller       2022-12-05  640   * The only place where we set in_use is when a new timestamp is initiated
d40fd60093325c Jacob Keller       2022-12-05  641   * with a slot index. This is only called in the hard xmit routine where an
d40fd60093325c Jacob Keller       2022-12-05  642   * SKB has a request flag set. The only places where we clear this bit is this
d40fd60093325c Jacob Keller       2022-12-05  643   * function, or during teardown when the Tx timestamp tracker is being
d40fd60093325c Jacob Keller       2022-12-05  644   * removed. A timestamp index will never be re-used until the in_use bit for
d40fd60093325c Jacob Keller       2022-12-05  645   * that index is cleared.
0dd92862639238 Jacob Keller       2022-12-05  646   *
0dd92862639238 Jacob Keller       2022-12-05  647   * If a Tx thread starts a new timestamp, we might not begin processing it
0dd92862639238 Jacob Keller       2022-12-05  648   * right away but we will notice it at the end when we re-queue the task.
0dd92862639238 Jacob Keller       2022-12-05  649   *
0dd92862639238 Jacob Keller       2022-12-05  650   * If a Tx thread starts a new timestamp just after this function exits, the
0dd92862639238 Jacob Keller       2022-12-05  651   * interrupt for that timestamp should re-trigger this function once
0dd92862639238 Jacob Keller       2022-12-05  652   * a timestamp is ready.
0dd92862639238 Jacob Keller       2022-12-05  653   *
d40fd60093325c Jacob Keller       2022-12-05  654   * In cases where the PTP hardware clock was directly adjusted, some
d40fd60093325c Jacob Keller       2022-12-05  655   * timestamps may not be able to safely use the timestamp extension math. In
d40fd60093325c Jacob Keller       2022-12-05  656   * this case, software will set the stale bit for any outstanding Tx
d40fd60093325c Jacob Keller       2022-12-05  657   * timestamps when the clock is adjusted. Then this function will discard
d40fd60093325c Jacob Keller       2022-12-05  658   * those captured timestamps instead of sending them to the stack.
0dd92862639238 Jacob Keller       2022-12-05  659   *
0dd92862639238 Jacob Keller       2022-12-05  660   * If a Tx packet has been waiting for more than 2 seconds, it is not possible
0dd92862639238 Jacob Keller       2022-12-05  661   * to correctly extend the timestamp using the cached PHC time. It is
0dd92862639238 Jacob Keller       2022-12-05  662   * extremely unlikely that a packet will ever take this long to timestamp. If
0dd92862639238 Jacob Keller       2022-12-05  663   * we detect a Tx timestamp request that has waited for this long we assume
0dd92862639238 Jacob Keller       2022-12-05  664   * the packet will never be sent by hardware and discard it without reading
0dd92862639238 Jacob Keller       2022-12-05  665   * the timestamp register.
06c16d89d2cbe2 Jacob Keller       2021-06-09  666   */
23cbd0608f6feb Jacob Keller       2023-05-26  667  static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
06c16d89d2cbe2 Jacob Keller       2021-06-09  668  {
4b1251bdd18886 Jacob Keller       2022-07-27  669  	struct ice_ptp_port *ptp_port;
f0ae124019faaa Jacob Keller       2022-12-05 @670  	bool more_timestamps;
4b1251bdd18886 Jacob Keller       2022-07-27  671  	struct ice_pf *pf;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  672  	struct ice_hw *hw;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  673  	u64 tstamp_ready;
fcc2cef37fed56 Daniel Vacek       2023-01-19  674  	bool link_up;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  675  	int err;
4b1251bdd18886 Jacob Keller       2022-07-27  676  	u8 idx;
06c16d89d2cbe2 Jacob Keller       2021-06-09  677  
4b1251bdd18886 Jacob Keller       2022-07-27  678  	if (!tx->init)
23cbd0608f6feb Jacob Keller       2023-05-26  679  		return;
06c16d89d2cbe2 Jacob Keller       2021-06-09  680  
4b1251bdd18886 Jacob Keller       2022-07-27  681  	ptp_port = container_of(tx, struct ice_ptp_port, tx);
4b1251bdd18886 Jacob Keller       2022-07-27  682  	pf = ptp_port_to_pf(ptp_port);
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  683  	hw = &pf->hw;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  684  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  685  	/* Read the Tx ready status first */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  686  	err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  687  	if (err)
23cbd0608f6feb Jacob Keller       2023-05-26  688  		return;
4b1251bdd18886 Jacob Keller       2022-07-27  689  
fcc2cef37fed56 Daniel Vacek       2023-01-19  690  	/* Drop packets if the link went down */
fcc2cef37fed56 Daniel Vacek       2023-01-19  691  	link_up = ptp_port->link_up;
fcc2cef37fed56 Daniel Vacek       2023-01-19  692  
4b1251bdd18886 Jacob Keller       2022-07-27  693  	for_each_set_bit(idx, tx->in_use, tx->len) {
4b1251bdd18886 Jacob Keller       2022-07-27  694  		struct skb_shared_hwtstamps shhwtstamps = {};
6b5cbc8c4ec71e Sergey Temerkhanov 2022-12-05  695  		u8 phy_idx = idx + tx->offset;
0dd92862639238 Jacob Keller       2022-12-05  696  		u64 raw_tstamp = 0, tstamp;
fcc2cef37fed56 Daniel Vacek       2023-01-19  697  		bool drop_ts = !link_up;
4b1251bdd18886 Jacob Keller       2022-07-27  698  		struct sk_buff *skb;
4b1251bdd18886 Jacob Keller       2022-07-27  699  
0dd92862639238 Jacob Keller       2022-12-05  700  		/* Drop packets which have waited for more than 2 seconds */
0dd92862639238 Jacob Keller       2022-12-05  701  		if (time_is_before_jiffies(tx->tstamps[idx].start + 2 * HZ)) {
0dd92862639238 Jacob Keller       2022-12-05  702  			drop_ts = true;
0dd92862639238 Jacob Keller       2022-12-05  703  
0dd92862639238 Jacob Keller       2022-12-05  704  			/* Count the number of Tx timestamps that timed out */
0dd92862639238 Jacob Keller       2022-12-05  705  			pf->ptp.tx_hwtstamp_timeouts++;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  706  		}
0dd92862639238 Jacob Keller       2022-12-05  707  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  708  		/* Only read a timestamp from the PHY if its marked as ready
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  709  		 * by the tstamp_ready register. This avoids unnecessary
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  710  		 * reading of timestamps which are not yet valid. This is
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  711  		 * important as we must read all timestamps which are valid
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  712  		 * and only timestamps which are valid during each interrupt.
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  713  		 * If we do not, the hardware logic for generating a new
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  714  		 * interrupt can get stuck on some devices.
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  715  		 */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  716  		if (!(tstamp_ready & BIT_ULL(phy_idx))) {
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  717  			if (drop_ts)
0dd92862639238 Jacob Keller       2022-12-05  718  				goto skip_ts_read;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  719  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  720  			continue;
0dd92862639238 Jacob Keller       2022-12-05  721  		}
0dd92862639238 Jacob Keller       2022-12-05  722  
4b1251bdd18886 Jacob Keller       2022-07-27  723  		ice_trace(tx_tstamp_fw_req, tx->tstamps[idx].skb, idx);
4b1251bdd18886 Jacob Keller       2022-07-27  724  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  725  		err = ice_read_phy_tstamp(hw, tx->block, phy_idx, &raw_tstamp);
fcc2cef37fed56 Daniel Vacek       2023-01-19  726  		if (err && !drop_ts)
4b1251bdd18886 Jacob Keller       2022-07-27  727  			continue;
4b1251bdd18886 Jacob Keller       2022-07-27  728  
4b1251bdd18886 Jacob Keller       2022-07-27  729  		ice_trace(tx_tstamp_fw_done, tx->tstamps[idx].skb, idx);
4b1251bdd18886 Jacob Keller       2022-07-27  730  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  731  		/* For PHYs which don't implement a proper timestamp ready
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  732  		 * bitmap, verify that the timestamp value is different
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  733  		 * from the last cached timestamp. If it is not, skip this for
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  734  		 * now assuming it hasn't yet been captured by hardware.
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  735  		 */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  736  		if (!drop_ts && tx->verify_cached &&
4b1251bdd18886 Jacob Keller       2022-07-27  737  		    raw_tstamp == tx->tstamps[idx].cached_tstamp)
4b1251bdd18886 Jacob Keller       2022-07-27  738  			continue;
4b1251bdd18886 Jacob Keller       2022-07-27  739  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  740  		/* Discard any timestamp value without the valid bit set */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  741  		if (!(raw_tstamp & ICE_PTP_TS_VALID))
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  742  			drop_ts = true;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  743  
0dd92862639238 Jacob Keller       2022-12-05  744  skip_ts_read:
4b1251bdd18886 Jacob Keller       2022-07-27  745  		spin_lock(&tx->lock);
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  746  		if (tx->verify_cached && raw_tstamp)
4b1251bdd18886 Jacob Keller       2022-07-27  747  			tx->tstamps[idx].cached_tstamp = raw_tstamp;
4b1251bdd18886 Jacob Keller       2022-07-27  748  		clear_bit(idx, tx->in_use);
4b1251bdd18886 Jacob Keller       2022-07-27  749  		skb = tx->tstamps[idx].skb;
4b1251bdd18886 Jacob Keller       2022-07-27  750  		tx->tstamps[idx].skb = NULL;
d40fd60093325c Jacob Keller       2022-12-05  751  		if (test_and_clear_bit(idx, tx->stale))
d40fd60093325c Jacob Keller       2022-12-05  752  			drop_ts = true;
4b1251bdd18886 Jacob Keller       2022-07-27  753  		spin_unlock(&tx->lock);
06c16d89d2cbe2 Jacob Keller       2021-06-09  754  
0dd92862639238 Jacob Keller       2022-12-05  755  		/* It is unlikely but possible that the SKB will have been
0dd92862639238 Jacob Keller       2022-12-05  756  		 * flushed at this point due to link change or teardown.
4b1251bdd18886 Jacob Keller       2022-07-27  757  		 */
4b1251bdd18886 Jacob Keller       2022-07-27  758  		if (!skb)
4b1251bdd18886 Jacob Keller       2022-07-27  759  			continue;
4b1251bdd18886 Jacob Keller       2022-07-27  760  
0dd92862639238 Jacob Keller       2022-12-05  761  		if (drop_ts) {
0dd92862639238 Jacob Keller       2022-12-05  762  			dev_kfree_skb_any(skb);
0dd92862639238 Jacob Keller       2022-12-05  763  			continue;
0dd92862639238 Jacob Keller       2022-12-05  764  		}
0dd92862639238 Jacob Keller       2022-12-05  765  
4b1251bdd18886 Jacob Keller       2022-07-27  766  		/* Extend the timestamp using cached PHC time */
4b1251bdd18886 Jacob Keller       2022-07-27  767  		tstamp = ice_ptp_extend_40b_ts(pf, raw_tstamp);
4b1251bdd18886 Jacob Keller       2022-07-27  768  		if (tstamp) {
4b1251bdd18886 Jacob Keller       2022-07-27  769  			shhwtstamps.hwtstamp = ns_to_ktime(tstamp);
4b1251bdd18886 Jacob Keller       2022-07-27  770  			ice_trace(tx_tstamp_complete, skb, idx);
06c16d89d2cbe2 Jacob Keller       2021-06-09  771  		}
06c16d89d2cbe2 Jacob Keller       2021-06-09  772  
4b1251bdd18886 Jacob Keller       2022-07-27  773  		skb_tstamp_tx(skb, &shhwtstamps);
4b1251bdd18886 Jacob Keller       2022-07-27  774  		dev_kfree_skb_any(skb);
4b1251bdd18886 Jacob Keller       2022-07-27  775  	}
23cbd0608f6feb Jacob Keller       2023-05-26  776  }
06c16d89d2cbe2 Jacob Keller       2021-06-09  777  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration

[kernel test robot]

Hi Jacob,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1e806efa4f2837a829044df27e1196a4fd520ba3]

url:    https://github.com/intel-lab-lkp/linux/commits/Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
base:   1e806efa4f2837a829044df27e1196a4fd520ba3
patch link:    https://lore.kernel.org/r/20230526222158.2685796-4-jacob.e.keller%40intel.com
patch subject: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230527/202305271021.Qlv0TxZu-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
        git checkout 23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/ethernet/intel/ice/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305271021.Qlv0TxZu-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/intel/ice/ice_ptp.c: In function 'ice_ptp_process_tx_tstamp':
>> drivers/net/ethernet/intel/ice/ice_ptp.c:670:14: warning: unused variable 'more_timestamps' [-Wunused-variable]
     670 |         bool more_timestamps;
         |              ^~~~~~~~~~~~~~~


vim +/more_timestamps +670 drivers/net/ethernet/intel/ice/ice_ptp.c

3ad5c10bf21d1d Jacob Keller       2022-12-05  618  
06c16d89d2cbe2 Jacob Keller       2021-06-09  619  /**
23cbd0608f6feb Jacob Keller       2023-05-26  620   * ice_ptp_process_tx_tstamp - Process Tx timestamps for a port
1229b33973c7b8 Karol Kolacinski   2022-09-16  621   * @tx: the PTP Tx timestamp tracker
06c16d89d2cbe2 Jacob Keller       2021-06-09  622   *
4b1251bdd18886 Jacob Keller       2022-07-27  623   * Process timestamps captured by the PHY associated with this port. To do
4b1251bdd18886 Jacob Keller       2022-07-27  624   * this, loop over each index with a waiting skb.
4b1251bdd18886 Jacob Keller       2022-07-27  625   *
4b1251bdd18886 Jacob Keller       2022-07-27  626   * If a given index has a valid timestamp, perform the following steps:
4b1251bdd18886 Jacob Keller       2022-07-27  627   *
d40fd60093325c Jacob Keller       2022-12-05  628   * 1) check that the timestamp request is not stale
d40fd60093325c Jacob Keller       2022-12-05  629   * 2) check that a timestamp is ready and available in the PHY memory bank
d40fd60093325c Jacob Keller       2022-12-05  630   * 3) read and copy the timestamp out of the PHY register
d40fd60093325c Jacob Keller       2022-12-05  631   * 4) unlock the index by clearing the associated in_use bit
d40fd60093325c Jacob Keller       2022-12-05  632   * 5) check if the timestamp is stale, and discard if so
d40fd60093325c Jacob Keller       2022-12-05  633   * 6) extend the 40 bit timestamp value to get a 64 bit timestamp value
d40fd60093325c Jacob Keller       2022-12-05  634   * 7) send this 64 bit timestamp to the stack
4b1251bdd18886 Jacob Keller       2022-07-27  635   *
d40fd60093325c Jacob Keller       2022-12-05  636   * Note that we do not hold the tracking lock while reading the Tx timestamp.
d40fd60093325c Jacob Keller       2022-12-05  637   * This is because reading the timestamp requires taking a mutex that might
d40fd60093325c Jacob Keller       2022-12-05  638   * sleep.
0dd92862639238 Jacob Keller       2022-12-05  639   *
d40fd60093325c Jacob Keller       2022-12-05  640   * The only place where we set in_use is when a new timestamp is initiated
d40fd60093325c Jacob Keller       2022-12-05  641   * with a slot index. This is only called in the hard xmit routine where an
d40fd60093325c Jacob Keller       2022-12-05  642   * SKB has a request flag set. The only places where we clear this bit is this
d40fd60093325c Jacob Keller       2022-12-05  643   * function, or during teardown when the Tx timestamp tracker is being
d40fd60093325c Jacob Keller       2022-12-05  644   * removed. A timestamp index will never be re-used until the in_use bit for
d40fd60093325c Jacob Keller       2022-12-05  645   * that index is cleared.
0dd92862639238 Jacob Keller       2022-12-05  646   *
0dd92862639238 Jacob Keller       2022-12-05  647   * If a Tx thread starts a new timestamp, we might not begin processing it
0dd92862639238 Jacob Keller       2022-12-05  648   * right away but we will notice it at the end when we re-queue the task.
0dd92862639238 Jacob Keller       2022-12-05  649   *
0dd92862639238 Jacob Keller       2022-12-05  650   * If a Tx thread starts a new timestamp just after this function exits, the
0dd92862639238 Jacob Keller       2022-12-05  651   * interrupt for that timestamp should re-trigger this function once
0dd92862639238 Jacob Keller       2022-12-05  652   * a timestamp is ready.
0dd92862639238 Jacob Keller       2022-12-05  653   *
d40fd60093325c Jacob Keller       2022-12-05  654   * In cases where the PTP hardware clock was directly adjusted, some
d40fd60093325c Jacob Keller       2022-12-05  655   * timestamps may not be able to safely use the timestamp extension math. In
d40fd60093325c Jacob Keller       2022-12-05  656   * this case, software will set the stale bit for any outstanding Tx
d40fd60093325c Jacob Keller       2022-12-05  657   * timestamps when the clock is adjusted. Then this function will discard
d40fd60093325c Jacob Keller       2022-12-05  658   * those captured timestamps instead of sending them to the stack.
0dd92862639238 Jacob Keller       2022-12-05  659   *
0dd92862639238 Jacob Keller       2022-12-05  660   * If a Tx packet has been waiting for more than 2 seconds, it is not possible
0dd92862639238 Jacob Keller       2022-12-05  661   * to correctly extend the timestamp using the cached PHC time. It is
0dd92862639238 Jacob Keller       2022-12-05  662   * extremely unlikely that a packet will ever take this long to timestamp. If
0dd92862639238 Jacob Keller       2022-12-05  663   * we detect a Tx timestamp request that has waited for this long we assume
0dd92862639238 Jacob Keller       2022-12-05  664   * the packet will never be sent by hardware and discard it without reading
0dd92862639238 Jacob Keller       2022-12-05  665   * the timestamp register.
06c16d89d2cbe2 Jacob Keller       2021-06-09  666   */
23cbd0608f6feb Jacob Keller       2023-05-26  667  static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
06c16d89d2cbe2 Jacob Keller       2021-06-09  668  {
4b1251bdd18886 Jacob Keller       2022-07-27  669  	struct ice_ptp_port *ptp_port;
f0ae124019faaa Jacob Keller       2022-12-05 @670  	bool more_timestamps;
4b1251bdd18886 Jacob Keller       2022-07-27  671  	struct ice_pf *pf;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  672  	struct ice_hw *hw;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  673  	u64 tstamp_ready;
fcc2cef37fed56 Daniel Vacek       2023-01-19  674  	bool link_up;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  675  	int err;
4b1251bdd18886 Jacob Keller       2022-07-27  676  	u8 idx;
06c16d89d2cbe2 Jacob Keller       2021-06-09  677  
4b1251bdd18886 Jacob Keller       2022-07-27  678  	if (!tx->init)
23cbd0608f6feb Jacob Keller       2023-05-26  679  		return;
06c16d89d2cbe2 Jacob Keller       2021-06-09  680  
4b1251bdd18886 Jacob Keller       2022-07-27  681  	ptp_port = container_of(tx, struct ice_ptp_port, tx);
4b1251bdd18886 Jacob Keller       2022-07-27  682  	pf = ptp_port_to_pf(ptp_port);
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  683  	hw = &pf->hw;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  684  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  685  	/* Read the Tx ready status first */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  686  	err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  687  	if (err)
23cbd0608f6feb Jacob Keller       2023-05-26  688  		return;
4b1251bdd18886 Jacob Keller       2022-07-27  689  
fcc2cef37fed56 Daniel Vacek       2023-01-19  690  	/* Drop packets if the link went down */
fcc2cef37fed56 Daniel Vacek       2023-01-19  691  	link_up = ptp_port->link_up;
fcc2cef37fed56 Daniel Vacek       2023-01-19  692  
4b1251bdd18886 Jacob Keller       2022-07-27  693  	for_each_set_bit(idx, tx->in_use, tx->len) {
4b1251bdd18886 Jacob Keller       2022-07-27  694  		struct skb_shared_hwtstamps shhwtstamps = {};
6b5cbc8c4ec71e Sergey Temerkhanov 2022-12-05  695  		u8 phy_idx = idx + tx->offset;
0dd92862639238 Jacob Keller       2022-12-05  696  		u64 raw_tstamp = 0, tstamp;
fcc2cef37fed56 Daniel Vacek       2023-01-19  697  		bool drop_ts = !link_up;
4b1251bdd18886 Jacob Keller       2022-07-27  698  		struct sk_buff *skb;
4b1251bdd18886 Jacob Keller       2022-07-27  699  
0dd92862639238 Jacob Keller       2022-12-05  700  		/* Drop packets which have waited for more than 2 seconds */
0dd92862639238 Jacob Keller       2022-12-05  701  		if (time_is_before_jiffies(tx->tstamps[idx].start + 2 * HZ)) {
0dd92862639238 Jacob Keller       2022-12-05  702  			drop_ts = true;
0dd92862639238 Jacob Keller       2022-12-05  703  
0dd92862639238 Jacob Keller       2022-12-05  704  			/* Count the number of Tx timestamps that timed out */
0dd92862639238 Jacob Keller       2022-12-05  705  			pf->ptp.tx_hwtstamp_timeouts++;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  706  		}
0dd92862639238 Jacob Keller       2022-12-05  707  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  708  		/* Only read a timestamp from the PHY if its marked as ready
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  709  		 * by the tstamp_ready register. This avoids unnecessary
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  710  		 * reading of timestamps which are not yet valid. This is
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  711  		 * important as we must read all timestamps which are valid
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  712  		 * and only timestamps which are valid during each interrupt.
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  713  		 * If we do not, the hardware logic for generating a new
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  714  		 * interrupt can get stuck on some devices.
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  715  		 */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  716  		if (!(tstamp_ready & BIT_ULL(phy_idx))) {
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  717  			if (drop_ts)
0dd92862639238 Jacob Keller       2022-12-05  718  				goto skip_ts_read;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  719  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  720  			continue;
0dd92862639238 Jacob Keller       2022-12-05  721  		}
0dd92862639238 Jacob Keller       2022-12-05  722  
4b1251bdd18886 Jacob Keller       2022-07-27  723  		ice_trace(tx_tstamp_fw_req, tx->tstamps[idx].skb, idx);
4b1251bdd18886 Jacob Keller       2022-07-27  724  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  725  		err = ice_read_phy_tstamp(hw, tx->block, phy_idx, &raw_tstamp);
fcc2cef37fed56 Daniel Vacek       2023-01-19  726  		if (err && !drop_ts)
4b1251bdd18886 Jacob Keller       2022-07-27  727  			continue;
4b1251bdd18886 Jacob Keller       2022-07-27  728  
4b1251bdd18886 Jacob Keller       2022-07-27  729  		ice_trace(tx_tstamp_fw_done, tx->tstamps[idx].skb, idx);
4b1251bdd18886 Jacob Keller       2022-07-27  730  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  731  		/* For PHYs which don't implement a proper timestamp ready
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  732  		 * bitmap, verify that the timestamp value is different
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  733  		 * from the last cached timestamp. If it is not, skip this for
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  734  		 * now assuming it hasn't yet been captured by hardware.
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  735  		 */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  736  		if (!drop_ts && tx->verify_cached &&
4b1251bdd18886 Jacob Keller       2022-07-27  737  		    raw_tstamp == tx->tstamps[idx].cached_tstamp)
4b1251bdd18886 Jacob Keller       2022-07-27  738  			continue;
4b1251bdd18886 Jacob Keller       2022-07-27  739  
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  740  		/* Discard any timestamp value without the valid bit set */
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  741  		if (!(raw_tstamp & ICE_PTP_TS_VALID))
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  742  			drop_ts = true;
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  743  
0dd92862639238 Jacob Keller       2022-12-05  744  skip_ts_read:
4b1251bdd18886 Jacob Keller       2022-07-27  745  		spin_lock(&tx->lock);
10e4b4a3a3e1b7 Jacob Keller       2022-12-05  746  		if (tx->verify_cached && raw_tstamp)
4b1251bdd18886 Jacob Keller       2022-07-27  747  			tx->tstamps[idx].cached_tstamp = raw_tstamp;
4b1251bdd18886 Jacob Keller       2022-07-27  748  		clear_bit(idx, tx->in_use);
4b1251bdd18886 Jacob Keller       2022-07-27  749  		skb = tx->tstamps[idx].skb;
4b1251bdd18886 Jacob Keller       2022-07-27  750  		tx->tstamps[idx].skb = NULL;
d40fd60093325c Jacob Keller       2022-12-05  751  		if (test_and_clear_bit(idx, tx->stale))
d40fd60093325c Jacob Keller       2022-12-05  752  			drop_ts = true;
4b1251bdd18886 Jacob Keller       2022-07-27  753  		spin_unlock(&tx->lock);
06c16d89d2cbe2 Jacob Keller       2021-06-09  754  
0dd92862639238 Jacob Keller       2022-12-05  755  		/* It is unlikely but possible that the SKB will have been
0dd92862639238 Jacob Keller       2022-12-05  756  		 * flushed at this point due to link change or teardown.
4b1251bdd18886 Jacob Keller       2022-07-27  757  		 */
4b1251bdd18886 Jacob Keller       2022-07-27  758  		if (!skb)
4b1251bdd18886 Jacob Keller       2022-07-27  759  			continue;
4b1251bdd18886 Jacob Keller       2022-07-27  760  
0dd92862639238 Jacob Keller       2022-12-05  761  		if (drop_ts) {
0dd92862639238 Jacob Keller       2022-12-05  762  			dev_kfree_skb_any(skb);
0dd92862639238 Jacob Keller       2022-12-05  763  			continue;
0dd92862639238 Jacob Keller       2022-12-05  764  		}
0dd92862639238 Jacob Keller       2022-12-05  765  
4b1251bdd18886 Jacob Keller       2022-07-27  766  		/* Extend the timestamp using cached PHC time */
4b1251bdd18886 Jacob Keller       2022-07-27  767  		tstamp = ice_ptp_extend_40b_ts(pf, raw_tstamp);
4b1251bdd18886 Jacob Keller       2022-07-27  768  		if (tstamp) {
4b1251bdd18886 Jacob Keller       2022-07-27  769  			shhwtstamps.hwtstamp = ns_to_ktime(tstamp);
4b1251bdd18886 Jacob Keller       2022-07-27  770  			ice_trace(tx_tstamp_complete, skb, idx);
06c16d89d2cbe2 Jacob Keller       2021-06-09  771  		}
06c16d89d2cbe2 Jacob Keller       2021-06-09  772  
4b1251bdd18886 Jacob Keller       2022-07-27  773  		skb_tstamp_tx(skb, &shhwtstamps);
4b1251bdd18886 Jacob Keller       2022-07-27  774  		dev_kfree_skb_any(skb);
4b1251bdd18886 Jacob Keller       2022-07-27  775  	}
23cbd0608f6feb Jacob Keller       2023-05-26  776  }
06c16d89d2cbe2 Jacob Keller       2021-06-09  777  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration

[kernel test robot]

Hi Jacob,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1e806efa4f2837a829044df27e1196a4fd520ba3]

url:    https://github.com/intel-lab-lkp/linux/commits/Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
base:   1e806efa4f2837a829044df27e1196a4fd520ba3
patch link:    https://lore.kernel.org/r/20230526222158.2685796-4-jacob.e.keller%40intel.com
patch subject: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration
config: x86_64-randconfig-x096-20230526 (https://download.01.org/0day-ci/archive/20230527/202305271105.qPWszTyo-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
        git checkout 23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/ethernet/intel/ice/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305271105.qPWszTyo-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/intel/ice/ice_ptp.c:670:7: warning: unused variable 'more_timestamps' [-Wunused-variable]
           bool more_timestamps;
                ^
   1 warning generated.


vim +/more_timestamps +670 drivers/net/ethernet/intel/ice/ice_ptp.c

3ad5c10bf21d1d6 Jacob Keller       2022-12-05  618  
06c16d89d2cbe28 Jacob Keller       2021-06-09  619  /**
23cbd0608f6febe Jacob Keller       2023-05-26  620   * ice_ptp_process_tx_tstamp - Process Tx timestamps for a port
1229b33973c7b89 Karol Kolacinski   2022-09-16  621   * @tx: the PTP Tx timestamp tracker
06c16d89d2cbe28 Jacob Keller       2021-06-09  622   *
4b1251bdd18886c Jacob Keller       2022-07-27  623   * Process timestamps captured by the PHY associated with this port. To do
4b1251bdd18886c Jacob Keller       2022-07-27  624   * this, loop over each index with a waiting skb.
4b1251bdd18886c Jacob Keller       2022-07-27  625   *
4b1251bdd18886c Jacob Keller       2022-07-27  626   * If a given index has a valid timestamp, perform the following steps:
4b1251bdd18886c Jacob Keller       2022-07-27  627   *
d40fd60093325cd Jacob Keller       2022-12-05  628   * 1) check that the timestamp request is not stale
d40fd60093325cd Jacob Keller       2022-12-05  629   * 2) check that a timestamp is ready and available in the PHY memory bank
d40fd60093325cd Jacob Keller       2022-12-05  630   * 3) read and copy the timestamp out of the PHY register
d40fd60093325cd Jacob Keller       2022-12-05  631   * 4) unlock the index by clearing the associated in_use bit
d40fd60093325cd Jacob Keller       2022-12-05  632   * 5) check if the timestamp is stale, and discard if so
d40fd60093325cd Jacob Keller       2022-12-05  633   * 6) extend the 40 bit timestamp value to get a 64 bit timestamp value
d40fd60093325cd Jacob Keller       2022-12-05  634   * 7) send this 64 bit timestamp to the stack
4b1251bdd18886c Jacob Keller       2022-07-27  635   *
d40fd60093325cd Jacob Keller       2022-12-05  636   * Note that we do not hold the tracking lock while reading the Tx timestamp.
d40fd60093325cd Jacob Keller       2022-12-05  637   * This is because reading the timestamp requires taking a mutex that might
d40fd60093325cd Jacob Keller       2022-12-05  638   * sleep.
0dd928626392386 Jacob Keller       2022-12-05  639   *
d40fd60093325cd Jacob Keller       2022-12-05  640   * The only place where we set in_use is when a new timestamp is initiated
d40fd60093325cd Jacob Keller       2022-12-05  641   * with a slot index. This is only called in the hard xmit routine where an
d40fd60093325cd Jacob Keller       2022-12-05  642   * SKB has a request flag set. The only places where we clear this bit is this
d40fd60093325cd Jacob Keller       2022-12-05  643   * function, or during teardown when the Tx timestamp tracker is being
d40fd60093325cd Jacob Keller       2022-12-05  644   * removed. A timestamp index will never be re-used until the in_use bit for
d40fd60093325cd Jacob Keller       2022-12-05  645   * that index is cleared.
0dd928626392386 Jacob Keller       2022-12-05  646   *
0dd928626392386 Jacob Keller       2022-12-05  647   * If a Tx thread starts a new timestamp, we might not begin processing it
0dd928626392386 Jacob Keller       2022-12-05  648   * right away but we will notice it at the end when we re-queue the task.
0dd928626392386 Jacob Keller       2022-12-05  649   *
0dd928626392386 Jacob Keller       2022-12-05  650   * If a Tx thread starts a new timestamp just after this function exits, the
0dd928626392386 Jacob Keller       2022-12-05  651   * interrupt for that timestamp should re-trigger this function once
0dd928626392386 Jacob Keller       2022-12-05  652   * a timestamp is ready.
0dd928626392386 Jacob Keller       2022-12-05  653   *
d40fd60093325cd Jacob Keller       2022-12-05  654   * In cases where the PTP hardware clock was directly adjusted, some
d40fd60093325cd Jacob Keller       2022-12-05  655   * timestamps may not be able to safely use the timestamp extension math. In
d40fd60093325cd Jacob Keller       2022-12-05  656   * this case, software will set the stale bit for any outstanding Tx
d40fd60093325cd Jacob Keller       2022-12-05  657   * timestamps when the clock is adjusted. Then this function will discard
d40fd60093325cd Jacob Keller       2022-12-05  658   * those captured timestamps instead of sending them to the stack.
0dd928626392386 Jacob Keller       2022-12-05  659   *
0dd928626392386 Jacob Keller       2022-12-05  660   * If a Tx packet has been waiting for more than 2 seconds, it is not possible
0dd928626392386 Jacob Keller       2022-12-05  661   * to correctly extend the timestamp using the cached PHC time. It is
0dd928626392386 Jacob Keller       2022-12-05  662   * extremely unlikely that a packet will ever take this long to timestamp. If
0dd928626392386 Jacob Keller       2022-12-05  663   * we detect a Tx timestamp request that has waited for this long we assume
0dd928626392386 Jacob Keller       2022-12-05  664   * the packet will never be sent by hardware and discard it without reading
0dd928626392386 Jacob Keller       2022-12-05  665   * the timestamp register.
06c16d89d2cbe28 Jacob Keller       2021-06-09  666   */
23cbd0608f6febe Jacob Keller       2023-05-26  667  static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
06c16d89d2cbe28 Jacob Keller       2021-06-09  668  {
4b1251bdd18886c Jacob Keller       2022-07-27  669  	struct ice_ptp_port *ptp_port;
f0ae124019faaa0 Jacob Keller       2022-12-05 @670  	bool more_timestamps;
4b1251bdd18886c Jacob Keller       2022-07-27  671  	struct ice_pf *pf;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  672  	struct ice_hw *hw;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  673  	u64 tstamp_ready;
fcc2cef37fed567 Daniel Vacek       2023-01-19  674  	bool link_up;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  675  	int err;
4b1251bdd18886c Jacob Keller       2022-07-27  676  	u8 idx;
06c16d89d2cbe28 Jacob Keller       2021-06-09  677  
4b1251bdd18886c Jacob Keller       2022-07-27  678  	if (!tx->init)
23cbd0608f6febe Jacob Keller       2023-05-26  679  		return;
06c16d89d2cbe28 Jacob Keller       2021-06-09  680  
4b1251bdd18886c Jacob Keller       2022-07-27  681  	ptp_port = container_of(tx, struct ice_ptp_port, tx);
4b1251bdd18886c Jacob Keller       2022-07-27  682  	pf = ptp_port_to_pf(ptp_port);
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  683  	hw = &pf->hw;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  684  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  685  	/* Read the Tx ready status first */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  686  	err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  687  	if (err)
23cbd0608f6febe Jacob Keller       2023-05-26  688  		return;
4b1251bdd18886c Jacob Keller       2022-07-27  689  
fcc2cef37fed567 Daniel Vacek       2023-01-19  690  	/* Drop packets if the link went down */
fcc2cef37fed567 Daniel Vacek       2023-01-19  691  	link_up = ptp_port->link_up;
fcc2cef37fed567 Daniel Vacek       2023-01-19  692  
4b1251bdd18886c Jacob Keller       2022-07-27  693  	for_each_set_bit(idx, tx->in_use, tx->len) {
4b1251bdd18886c Jacob Keller       2022-07-27  694  		struct skb_shared_hwtstamps shhwtstamps = {};
6b5cbc8c4ec71e4 Sergey Temerkhanov 2022-12-05  695  		u8 phy_idx = idx + tx->offset;
0dd928626392386 Jacob Keller       2022-12-05  696  		u64 raw_tstamp = 0, tstamp;
fcc2cef37fed567 Daniel Vacek       2023-01-19  697  		bool drop_ts = !link_up;
4b1251bdd18886c Jacob Keller       2022-07-27  698  		struct sk_buff *skb;
4b1251bdd18886c Jacob Keller       2022-07-27  699  
0dd928626392386 Jacob Keller       2022-12-05  700  		/* Drop packets which have waited for more than 2 seconds */
0dd928626392386 Jacob Keller       2022-12-05  701  		if (time_is_before_jiffies(tx->tstamps[idx].start + 2 * HZ)) {
0dd928626392386 Jacob Keller       2022-12-05  702  			drop_ts = true;
0dd928626392386 Jacob Keller       2022-12-05  703  
0dd928626392386 Jacob Keller       2022-12-05  704  			/* Count the number of Tx timestamps that timed out */
0dd928626392386 Jacob Keller       2022-12-05  705  			pf->ptp.tx_hwtstamp_timeouts++;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  706  		}
0dd928626392386 Jacob Keller       2022-12-05  707  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  708  		/* Only read a timestamp from the PHY if its marked as ready
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  709  		 * by the tstamp_ready register. This avoids unnecessary
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  710  		 * reading of timestamps which are not yet valid. This is
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  711  		 * important as we must read all timestamps which are valid
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  712  		 * and only timestamps which are valid during each interrupt.
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  713  		 * If we do not, the hardware logic for generating a new
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  714  		 * interrupt can get stuck on some devices.
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  715  		 */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  716  		if (!(tstamp_ready & BIT_ULL(phy_idx))) {
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  717  			if (drop_ts)
0dd928626392386 Jacob Keller       2022-12-05  718  				goto skip_ts_read;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  719  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  720  			continue;
0dd928626392386 Jacob Keller       2022-12-05  721  		}
0dd928626392386 Jacob Keller       2022-12-05  722  
4b1251bdd18886c Jacob Keller       2022-07-27  723  		ice_trace(tx_tstamp_fw_req, tx->tstamps[idx].skb, idx);
4b1251bdd18886c Jacob Keller       2022-07-27  724  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  725  		err = ice_read_phy_tstamp(hw, tx->block, phy_idx, &raw_tstamp);
fcc2cef37fed567 Daniel Vacek       2023-01-19  726  		if (err && !drop_ts)
4b1251bdd18886c Jacob Keller       2022-07-27  727  			continue;
4b1251bdd18886c Jacob Keller       2022-07-27  728  
4b1251bdd18886c Jacob Keller       2022-07-27  729  		ice_trace(tx_tstamp_fw_done, tx->tstamps[idx].skb, idx);
4b1251bdd18886c Jacob Keller       2022-07-27  730  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  731  		/* For PHYs which don't implement a proper timestamp ready
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  732  		 * bitmap, verify that the timestamp value is different
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  733  		 * from the last cached timestamp. If it is not, skip this for
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  734  		 * now assuming it hasn't yet been captured by hardware.
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  735  		 */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  736  		if (!drop_ts && tx->verify_cached &&
4b1251bdd18886c Jacob Keller       2022-07-27  737  		    raw_tstamp == tx->tstamps[idx].cached_tstamp)
4b1251bdd18886c Jacob Keller       2022-07-27  738  			continue;
4b1251bdd18886c Jacob Keller       2022-07-27  739  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  740  		/* Discard any timestamp value without the valid bit set */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  741  		if (!(raw_tstamp & ICE_PTP_TS_VALID))
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  742  			drop_ts = true;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  743  
0dd928626392386 Jacob Keller       2022-12-05  744  skip_ts_read:
4b1251bdd18886c Jacob Keller       2022-07-27  745  		spin_lock(&tx->lock);
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  746  		if (tx->verify_cached && raw_tstamp)
4b1251bdd18886c Jacob Keller       2022-07-27  747  			tx->tstamps[idx].cached_tstamp = raw_tstamp;
4b1251bdd18886c Jacob Keller       2022-07-27  748  		clear_bit(idx, tx->in_use);
4b1251bdd18886c Jacob Keller       2022-07-27  749  		skb = tx->tstamps[idx].skb;
4b1251bdd18886c Jacob Keller       2022-07-27  750  		tx->tstamps[idx].skb = NULL;
d40fd60093325cd Jacob Keller       2022-12-05  751  		if (test_and_clear_bit(idx, tx->stale))
d40fd60093325cd Jacob Keller       2022-12-05  752  			drop_ts = true;
4b1251bdd18886c Jacob Keller       2022-07-27  753  		spin_unlock(&tx->lock);
06c16d89d2cbe28 Jacob Keller       2021-06-09  754  
0dd928626392386 Jacob Keller       2022-12-05  755  		/* It is unlikely but possible that the SKB will have been
0dd928626392386 Jacob Keller       2022-12-05  756  		 * flushed at this point due to link change or teardown.
4b1251bdd18886c Jacob Keller       2022-07-27  757  		 */
4b1251bdd18886c Jacob Keller       2022-07-27  758  		if (!skb)
4b1251bdd18886c Jacob Keller       2022-07-27  759  			continue;
4b1251bdd18886c Jacob Keller       2022-07-27  760  
0dd928626392386 Jacob Keller       2022-12-05  761  		if (drop_ts) {
0dd928626392386 Jacob Keller       2022-12-05  762  			dev_kfree_skb_any(skb);
0dd928626392386 Jacob Keller       2022-12-05  763  			continue;
0dd928626392386 Jacob Keller       2022-12-05  764  		}
0dd928626392386 Jacob Keller       2022-12-05  765  
4b1251bdd18886c Jacob Keller       2022-07-27  766  		/* Extend the timestamp using cached PHC time */
4b1251bdd18886c Jacob Keller       2022-07-27  767  		tstamp = ice_ptp_extend_40b_ts(pf, raw_tstamp);
4b1251bdd18886c Jacob Keller       2022-07-27  768  		if (tstamp) {
4b1251bdd18886c Jacob Keller       2022-07-27  769  			shhwtstamps.hwtstamp = ns_to_ktime(tstamp);
4b1251bdd18886c Jacob Keller       2022-07-27  770  			ice_trace(tx_tstamp_complete, skb, idx);
06c16d89d2cbe28 Jacob Keller       2021-06-09  771  		}
06c16d89d2cbe28 Jacob Keller       2021-06-09  772  
4b1251bdd18886c Jacob Keller       2022-07-27  773  		skb_tstamp_tx(skb, &shhwtstamps);
4b1251bdd18886c Jacob Keller       2022-07-27  774  		dev_kfree_skb_any(skb);
4b1251bdd18886c Jacob Keller       2022-07-27  775  	}
23cbd0608f6febe Jacob Keller       2023-05-26  776  }
06c16d89d2cbe28 Jacob Keller       2021-06-09  777  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration

[kernel test robot]

Hi Jacob,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1e806efa4f2837a829044df27e1196a4fd520ba3]

url:    https://github.com/intel-lab-lkp/linux/commits/Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
base:   1e806efa4f2837a829044df27e1196a4fd520ba3
patch link:    https://lore.kernel.org/r/20230526222158.2685796-4-jacob.e.keller%40intel.com
patch subject: [Intel-wired-lan] [PATCH iwl-next 3/5] ice: introduce ICE_TX_TSTAMP_WORK enumeration
config: x86_64-randconfig-x096-20230526 (https://download.01.org/0day-ci/archive/20230527/202305271105.qPWszTyo-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jacob-Keller/ice-handle-extts-in-the-miscellaneous-interrupt-thread/20230527-062501
        git checkout 23cbd0608f6febe437dc272b1d38fe6fb96e7b7a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/ethernet/intel/ice/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305271105.qPWszTyo-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/intel/ice/ice_ptp.c:670:7: warning: unused variable 'more_timestamps' [-Wunused-variable]
           bool more_timestamps;
                ^
   1 warning generated.


vim +/more_timestamps +670 drivers/net/ethernet/intel/ice/ice_ptp.c

3ad5c10bf21d1d6 Jacob Keller       2022-12-05  618  
06c16d89d2cbe28 Jacob Keller       2021-06-09  619  /**
23cbd0608f6febe Jacob Keller       2023-05-26  620   * ice_ptp_process_tx_tstamp - Process Tx timestamps for a port
1229b33973c7b89 Karol Kolacinski   2022-09-16  621   * @tx: the PTP Tx timestamp tracker
06c16d89d2cbe28 Jacob Keller       2021-06-09  622   *
4b1251bdd18886c Jacob Keller       2022-07-27  623   * Process timestamps captured by the PHY associated with this port. To do
4b1251bdd18886c Jacob Keller       2022-07-27  624   * this, loop over each index with a waiting skb.
4b1251bdd18886c Jacob Keller       2022-07-27  625   *
4b1251bdd18886c Jacob Keller       2022-07-27  626   * If a given index has a valid timestamp, perform the following steps:
4b1251bdd18886c Jacob Keller       2022-07-27  627   *
d40fd60093325cd Jacob Keller       2022-12-05  628   * 1) check that the timestamp request is not stale
d40fd60093325cd Jacob Keller       2022-12-05  629   * 2) check that a timestamp is ready and available in the PHY memory bank
d40fd60093325cd Jacob Keller       2022-12-05  630   * 3) read and copy the timestamp out of the PHY register
d40fd60093325cd Jacob Keller       2022-12-05  631   * 4) unlock the index by clearing the associated in_use bit
d40fd60093325cd Jacob Keller       2022-12-05  632   * 5) check if the timestamp is stale, and discard if so
d40fd60093325cd Jacob Keller       2022-12-05  633   * 6) extend the 40 bit timestamp value to get a 64 bit timestamp value
d40fd60093325cd Jacob Keller       2022-12-05  634   * 7) send this 64 bit timestamp to the stack
4b1251bdd18886c Jacob Keller       2022-07-27  635   *
d40fd60093325cd Jacob Keller       2022-12-05  636   * Note that we do not hold the tracking lock while reading the Tx timestamp.
d40fd60093325cd Jacob Keller       2022-12-05  637   * This is because reading the timestamp requires taking a mutex that might
d40fd60093325cd Jacob Keller       2022-12-05  638   * sleep.
0dd928626392386 Jacob Keller       2022-12-05  639   *
d40fd60093325cd Jacob Keller       2022-12-05  640   * The only place where we set in_use is when a new timestamp is initiated
d40fd60093325cd Jacob Keller       2022-12-05  641   * with a slot index. This is only called in the hard xmit routine where an
d40fd60093325cd Jacob Keller       2022-12-05  642   * SKB has a request flag set. The only places where we clear this bit is this
d40fd60093325cd Jacob Keller       2022-12-05  643   * function, or during teardown when the Tx timestamp tracker is being
d40fd60093325cd Jacob Keller       2022-12-05  644   * removed. A timestamp index will never be re-used until the in_use bit for
d40fd60093325cd Jacob Keller       2022-12-05  645   * that index is cleared.
0dd928626392386 Jacob Keller       2022-12-05  646   *
0dd928626392386 Jacob Keller       2022-12-05  647   * If a Tx thread starts a new timestamp, we might not begin processing it
0dd928626392386 Jacob Keller       2022-12-05  648   * right away but we will notice it at the end when we re-queue the task.
0dd928626392386 Jacob Keller       2022-12-05  649   *
0dd928626392386 Jacob Keller       2022-12-05  650   * If a Tx thread starts a new timestamp just after this function exits, the
0dd928626392386 Jacob Keller       2022-12-05  651   * interrupt for that timestamp should re-trigger this function once
0dd928626392386 Jacob Keller       2022-12-05  652   * a timestamp is ready.
0dd928626392386 Jacob Keller       2022-12-05  653   *
d40fd60093325cd Jacob Keller       2022-12-05  654   * In cases where the PTP hardware clock was directly adjusted, some
d40fd60093325cd Jacob Keller       2022-12-05  655   * timestamps may not be able to safely use the timestamp extension math. In
d40fd60093325cd Jacob Keller       2022-12-05  656   * this case, software will set the stale bit for any outstanding Tx
d40fd60093325cd Jacob Keller       2022-12-05  657   * timestamps when the clock is adjusted. Then this function will discard
d40fd60093325cd Jacob Keller       2022-12-05  658   * those captured timestamps instead of sending them to the stack.
0dd928626392386 Jacob Keller       2022-12-05  659   *
0dd928626392386 Jacob Keller       2022-12-05  660   * If a Tx packet has been waiting for more than 2 seconds, it is not possible
0dd928626392386 Jacob Keller       2022-12-05  661   * to correctly extend the timestamp using the cached PHC time. It is
0dd928626392386 Jacob Keller       2022-12-05  662   * extremely unlikely that a packet will ever take this long to timestamp. If
0dd928626392386 Jacob Keller       2022-12-05  663   * we detect a Tx timestamp request that has waited for this long we assume
0dd928626392386 Jacob Keller       2022-12-05  664   * the packet will never be sent by hardware and discard it without reading
0dd928626392386 Jacob Keller       2022-12-05  665   * the timestamp register.
06c16d89d2cbe28 Jacob Keller       2021-06-09  666   */
23cbd0608f6febe Jacob Keller       2023-05-26  667  static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
06c16d89d2cbe28 Jacob Keller       2021-06-09  668  {
4b1251bdd18886c Jacob Keller       2022-07-27  669  	struct ice_ptp_port *ptp_port;
f0ae124019faaa0 Jacob Keller       2022-12-05 @670  	bool more_timestamps;
4b1251bdd18886c Jacob Keller       2022-07-27  671  	struct ice_pf *pf;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  672  	struct ice_hw *hw;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  673  	u64 tstamp_ready;
fcc2cef37fed567 Daniel Vacek       2023-01-19  674  	bool link_up;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  675  	int err;
4b1251bdd18886c Jacob Keller       2022-07-27  676  	u8 idx;
06c16d89d2cbe28 Jacob Keller       2021-06-09  677  
4b1251bdd18886c Jacob Keller       2022-07-27  678  	if (!tx->init)
23cbd0608f6febe Jacob Keller       2023-05-26  679  		return;
06c16d89d2cbe28 Jacob Keller       2021-06-09  680  
4b1251bdd18886c Jacob Keller       2022-07-27  681  	ptp_port = container_of(tx, struct ice_ptp_port, tx);
4b1251bdd18886c Jacob Keller       2022-07-27  682  	pf = ptp_port_to_pf(ptp_port);
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  683  	hw = &pf->hw;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  684  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  685  	/* Read the Tx ready status first */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  686  	err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  687  	if (err)
23cbd0608f6febe Jacob Keller       2023-05-26  688  		return;
4b1251bdd18886c Jacob Keller       2022-07-27  689  
fcc2cef37fed567 Daniel Vacek       2023-01-19  690  	/* Drop packets if the link went down */
fcc2cef37fed567 Daniel Vacek       2023-01-19  691  	link_up = ptp_port->link_up;
fcc2cef37fed567 Daniel Vacek       2023-01-19  692  
4b1251bdd18886c Jacob Keller       2022-07-27  693  	for_each_set_bit(idx, tx->in_use, tx->len) {
4b1251bdd18886c Jacob Keller       2022-07-27  694  		struct skb_shared_hwtstamps shhwtstamps = {};
6b5cbc8c4ec71e4 Sergey Temerkhanov 2022-12-05  695  		u8 phy_idx = idx + tx->offset;
0dd928626392386 Jacob Keller       2022-12-05  696  		u64 raw_tstamp = 0, tstamp;
fcc2cef37fed567 Daniel Vacek       2023-01-19  697  		bool drop_ts = !link_up;
4b1251bdd18886c Jacob Keller       2022-07-27  698  		struct sk_buff *skb;
4b1251bdd18886c Jacob Keller       2022-07-27  699  
0dd928626392386 Jacob Keller       2022-12-05  700  		/* Drop packets which have waited for more than 2 seconds */
0dd928626392386 Jacob Keller       2022-12-05  701  		if (time_is_before_jiffies(tx->tstamps[idx].start + 2 * HZ)) {
0dd928626392386 Jacob Keller       2022-12-05  702  			drop_ts = true;
0dd928626392386 Jacob Keller       2022-12-05  703  
0dd928626392386 Jacob Keller       2022-12-05  704  			/* Count the number of Tx timestamps that timed out */
0dd928626392386 Jacob Keller       2022-12-05  705  			pf->ptp.tx_hwtstamp_timeouts++;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  706  		}
0dd928626392386 Jacob Keller       2022-12-05  707  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  708  		/* Only read a timestamp from the PHY if its marked as ready
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  709  		 * by the tstamp_ready register. This avoids unnecessary
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  710  		 * reading of timestamps which are not yet valid. This is
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  711  		 * important as we must read all timestamps which are valid
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  712  		 * and only timestamps which are valid during each interrupt.
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  713  		 * If we do not, the hardware logic for generating a new
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  714  		 * interrupt can get stuck on some devices.
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  715  		 */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  716  		if (!(tstamp_ready & BIT_ULL(phy_idx))) {
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  717  			if (drop_ts)
0dd928626392386 Jacob Keller       2022-12-05  718  				goto skip_ts_read;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  719  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  720  			continue;
0dd928626392386 Jacob Keller       2022-12-05  721  		}
0dd928626392386 Jacob Keller       2022-12-05  722  
4b1251bdd18886c Jacob Keller       2022-07-27  723  		ice_trace(tx_tstamp_fw_req, tx->tstamps[idx].skb, idx);
4b1251bdd18886c Jacob Keller       2022-07-27  724  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  725  		err = ice_read_phy_tstamp(hw, tx->block, phy_idx, &raw_tstamp);
fcc2cef37fed567 Daniel Vacek       2023-01-19  726  		if (err && !drop_ts)
4b1251bdd18886c Jacob Keller       2022-07-27  727  			continue;
4b1251bdd18886c Jacob Keller       2022-07-27  728  
4b1251bdd18886c Jacob Keller       2022-07-27  729  		ice_trace(tx_tstamp_fw_done, tx->tstamps[idx].skb, idx);
4b1251bdd18886c Jacob Keller       2022-07-27  730  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  731  		/* For PHYs which don't implement a proper timestamp ready
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  732  		 * bitmap, verify that the timestamp value is different
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  733  		 * from the last cached timestamp. If it is not, skip this for
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  734  		 * now assuming it hasn't yet been captured by hardware.
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  735  		 */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  736  		if (!drop_ts && tx->verify_cached &&
4b1251bdd18886c Jacob Keller       2022-07-27  737  		    raw_tstamp == tx->tstamps[idx].cached_tstamp)
4b1251bdd18886c Jacob Keller       2022-07-27  738  			continue;
4b1251bdd18886c Jacob Keller       2022-07-27  739  
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  740  		/* Discard any timestamp value without the valid bit set */
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  741  		if (!(raw_tstamp & ICE_PTP_TS_VALID))
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  742  			drop_ts = true;
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  743  
0dd928626392386 Jacob Keller       2022-12-05  744  skip_ts_read:
4b1251bdd18886c Jacob Keller       2022-07-27  745  		spin_lock(&tx->lock);
10e4b4a3a3e1b70 Jacob Keller       2022-12-05  746  		if (tx->verify_cached && raw_tstamp)
4b1251bdd18886c Jacob Keller       2022-07-27  747  			tx->tstamps[idx].cached_tstamp = raw_tstamp;
4b1251bdd18886c Jacob Keller       2022-07-27  748  		clear_bit(idx, tx->in_use);
4b1251bdd18886c Jacob Keller       2022-07-27  749  		skb = tx->tstamps[idx].skb;
4b1251bdd18886c Jacob Keller       2022-07-27  750  		tx->tstamps[idx].skb = NULL;
d40fd60093325cd Jacob Keller       2022-12-05  751  		if (test_and_clear_bit(idx, tx->stale))
d40fd60093325cd Jacob Keller       2022-12-05  752  			drop_ts = true;
4b1251bdd18886c Jacob Keller       2022-07-27  753  		spin_unlock(&tx->lock);
06c16d89d2cbe28 Jacob Keller       2021-06-09  754  
0dd928626392386 Jacob Keller       2022-12-05  755  		/* It is unlikely but possible that the SKB will have been
0dd928626392386 Jacob Keller       2022-12-05  756  		 * flushed at this point due to link change or teardown.
4b1251bdd18886c Jacob Keller       2022-07-27  757  		 */
4b1251bdd18886c Jacob Keller       2022-07-27  758  		if (!skb)
4b1251bdd18886c Jacob Keller       2022-07-27  759  			continue;
4b1251bdd18886c Jacob Keller       2022-07-27  760  
0dd928626392386 Jacob Keller       2022-12-05  761  		if (drop_ts) {
0dd928626392386 Jacob Keller       2022-12-05  762  			dev_kfree_skb_any(skb);
0dd928626392386 Jacob Keller       2022-12-05  763  			continue;
0dd928626392386 Jacob Keller       2022-12-05  764  		}
0dd928626392386 Jacob Keller       2022-12-05  765  
4b1251bdd18886c Jacob Keller       2022-07-27  766  		/* Extend the timestamp using cached PHC time */
4b1251bdd18886c Jacob Keller       2022-07-27  767  		tstamp = ice_ptp_extend_40b_ts(pf, raw_tstamp);
4b1251bdd18886c Jacob Keller       2022-07-27  768  		if (tstamp) {
4b1251bdd18886c Jacob Keller       2022-07-27  769  			shhwtstamps.hwtstamp = ns_to_ktime(tstamp);
4b1251bdd18886c Jacob Keller       2022-07-27  770  			ice_trace(tx_tstamp_complete, skb, idx);
06c16d89d2cbe28 Jacob Keller       2021-06-09  771  		}
06c16d89d2cbe28 Jacob Keller       2021-06-09  772  
4b1251bdd18886c Jacob Keller       2022-07-27  773  		skb_tstamp_tx(skb, &shhwtstamps);
4b1251bdd18886c Jacob Keller       2022-07-27  774  		dev_kfree_skb_any(skb);
4b1251bdd18886c Jacob Keller       2022-07-27  775  	}
23cbd0608f6febe Jacob Keller       2023-05-26  776  }
06c16d89d2cbe28 Jacob Keller       2021-06-09  777  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[Intel-wired-lan] [PATCH iwl-next 4/5] ice: trigger PFINT_OICR_TSYN_TX interrupt instead of polling

[Jacob Keller]

In ice_misc_intr_thread_fn(), if we do not complete all Tx timestamp work,
the thread function will poll continuously forever.

For E822 hardware, this wastes time as the return value from
ice_ptp_process_ts() is accurate and always reports correctly that the PHY
actually has new timestamp data.

In addition, if we receive enough timestamps at the right pacing, we might
never exit this polling and thus prevent other interrupt tasks from being
processed.

Fix this by instead writing to PFINT_OICR, causing an emulated interrupt to
be triggered immediately. This does take slightly more processing than just
re-checking the timestamps. However, it allows all of the other interrupt
causes a chance to be processed first in the hard IRQ function.

Note that the OICR interrupt is throttled to 8K per second, so the hardware
will not let the interrupt trigger more often than once every 124
microseconds, so this should not cause a significant increase in CPU usage
vs the sleeping method.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 481dccdb95cd..72e1b919b2d3 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3204,8 +3204,16 @@ static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
 	}
 
 	if (pf->oicr_misc & PFINT_OICR_TSYN_TX_M) {
-		while (ice_ptp_process_ts(pf) == ICE_TX_TSTAMP_WORK_PENDING)
-			usleep_range(50, 100);
+		struct ice_hw *hw = &pf->hw;
+
+		/* Process outstanding Tx timestamps. If there is more work,
+		 * re-arm the interrupt to trigger again.
+		 */
+		if (ice_ptp_process_ts(pf) == ICE_TX_TSTAMP_WORK_PENDING) {
+			wr32(hw, PFINT_OICR, PFINT_OICR_TSYN_TX_M);
+			ice_flush(hw);
+		}
+
 		pf->oicr_misc &= ~PFINT_OICR_TSYN_TX_M;
 	}
 
-- 
2.40.0.471.gbd7f14d9353b

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH iwl-next 5/5] ice: do not re-enable miscellaneous interrupt until thread_fn completes

[Jacob Keller]

At the end of ice_misc_intr() the driver calls ice_irq_dynamic_ena() to
re-enable the miscellaneous interrupt. This is done before the
ice_misc_intr_thread_fn can run and complete.

According to the kernel function comment documentation for
request_threaded_irq(), the interrupt should remain disabled until the
thread function completes its task.

By re-enabling the interrupt at the end of the hard IRQ, it is possible for
a new interrupt to trigger while the thread function is processing. This is
problematic for PTP Tx timestamps.

For E822 devices, the hardware in the PHY keeps track of how many
outstanding timestamps are generated and how many timestamps are read from
the PHY.

This counter is incremented once for each timestamp that is captured by
hardware, and decremented once each time a timestamp is read from the PHY.
The PHY will not generate a new interrupt unless this internal counter is
zero before the most recently captured timestamp.

Because of this counter behavior, a race with the hard IRQ and threaded IRQ
function can result in the potential for the counter to get stuck such that
no new interrupts will be triggered until the device is reset.

Consider the following flow:

 1 -> Tx timestamp completes in hardware
 2 -> timestamp interrupt occurs
 3 -> ice_misc_intr() re-enables timestamp interrupt, and wakes the
      thread_fn
 4 -> thread_fn is running and processing Tx timestamp
 5 -> the Tx timestamp is read from PHY, clearing the counter
 6 -> a new Tx timestamp completes in hardware, triggering interrupt
 7 -> the thread_fn hasn't exited and reported IRQ handled
 8 -> ice_misc_intr() triggers and sees PTP interrupt, so tries to wake thread
 9 -> thread_fn is already running (IRQTF_RUNTHREAD is set still!) so we
      skip running the thread...
10 -> an outstanding timestamp is remaining but we never read it
11 -> interrupt never triggers again

The fix for this complicated race condition is simple: do not re-enable the
miscellaneous interrupt until *after* the thread function completes. If a
new timestamp event triggers while the interrupt is disabled, it will be
remembered and should cause the interrupt to trigger again immediately
after we re-enable the interrupt.

Fixes: 1229b33973c7 ("ice: Add low latency Tx timestamp read")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 72e1b919b2d3..51fe3da0d54f 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3179,8 +3179,6 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 		}
 	}
 
-	ice_irq_dynamic_ena(hw, NULL, NULL);
-
 	return IRQ_WAKE_THREAD;
 }
 
@@ -3192,6 +3190,9 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
 {
 	struct ice_pf *pf = data;
+	struct ice_hw *hw;
+
+	hw = &pf->hw;
 
 	if (ice_is_reset_in_progress(pf->state))
 		return IRQ_HANDLED;
@@ -3204,8 +3205,6 @@ static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
 	}
 
 	if (pf->oicr_misc & PFINT_OICR_TSYN_TX_M) {
-		struct ice_hw *hw = &pf->hw;
-
 		/* Process outstanding Tx timestamps. If there is more work,
 		 * re-arm the interrupt to trigger again.
 		 */
@@ -3217,6 +3216,8 @@ static irqreturn_t ice_misc_intr_thread_fn(int __always_unused irq, void *data)
 		pf->oicr_misc &= ~PFINT_OICR_TSYN_TX_M;
 	}
 
+	ice_irq_dynamic_ena(hw, NULL, NULL);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.40.0.471.gbd7f14d9353b

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan