RE: [Intel-wired-lan] [PATCH iwl-next v3] igb: Retrieve Tx timestamp directly from interrupt for i210

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf
> Of Kurt Kanzenbach
> Sent: Thursday, February 5, 2026 12:58 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@xxxxxxxxx>; Nguyen,
> Anthony L <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>
> Cc: Paul Menzel <pmenzel@xxxxxxxxxxxxx>; Vadim Fedorenko
> <vadim.fedorenko@xxxxxxxxx>; Gomes, Vinicius
> <vinicius.gomes@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; Richard Cochran
> <richardcochran@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Andrew Lunn
> <andrew+netdev@xxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; intel-
> wired-lan@xxxxxxxxxxxxxxxx; Keller, Jacob E
> <jacob.e.keller@xxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo
> Abeni <pabeni@xxxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>;
> Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v3] igb: Retrieve Tx
> timestamp directly from interrupt for i210
> 
> On Thu Feb 05 2026, Loktionov, Aleksandr wrote:
> >> +/**
> >> + * igb_ptp_tx_tstamp_event
> >> + * @adapter: pointer to igb adapter
> >> + *
> >> + * This function checks the TSYNCTXCTL valid bit and stores the Tx
> >> +hardware
> >> + * timestamp at the current skb.
> >> + **/
> >> +void igb_ptp_tx_tstamp_event(struct igb_adapter *adapter) {
> >> +	struct e1000_hw *hw = &adapter->hw;
> >> +	u32 tsynctxctl;
> >> +
> >> +	if (!adapter->ptp_tx_skb)
> >> +		return;
> >> +
> >> +	tsynctxctl = rd32(E1000_TSYNCTXCTL);
> >> +	if (WARN_ON_ONCE(!(tsynctxctl & E1000_TSYNCTXCTL_VALID)))
> >> +		return;
> >> +
> >> +	igb_ptp_tx_hwtstamp(adapter); <-Calls existing function
> designed for work queue!
> >
> > skb_tstamp_tx() can sleep
> > Smells like sleep-in-atomic isn't it?
> 
> AFAICS skb_tstamp_tx() is safe to call here.
> 
> > spin_lock_irqsave(&wq_head->lock, flags);  <- RT mutex can sleep
> 
> In case you're worried about PREEMPT_RT: On -RT the IRQ runs a
> dedicated thread. BTW I've tested this with and without -RT and with
> CONFIG_DEBUG_ATOMIC_SLEEP.
> 
> Thanks,
> Kurt

Thank you, Kurt for sharing your experience. I don't have so many experience with RT Linux.
For me calling a function, not designed to be called from IRQ context is a SUS.
So, I rose the question about sleeping.

But there is also a question about non-RT Kernels with Shared IRQ Vectors...
I.e. regular packet processing (NAPI poll in softirq) and PTP timestamp events (in hardirq).

I suspect we have potentially broken drivers with shared vectors (MSI-X should be safe I hope).

I'd like the comment to be added:
/* Safe to call from IRQ: dedicated misc vector + RT-safe on PREEMPT_RT */
igb_ptp_tx_hwtstamp(adapter);

But in long term perspective prefer to refactor moving to NAPI is the safer, more maintainable pattern.

What do you think?
Alex