Re: [PATCH iwl-net v1] i40e: fix races in PTP external timestamp work handling

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 05/08/2026 08:13, luoxuanqiang wrote:
> 
> 在 2026/8/5 05:33, Vadim Fedorenko 写道:
>> On 24/07/2026 10:34, xuanqiang.luo@linux.dev wrote:
>>>
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/ 
>>> net/ethernet/intel/i40e/i40e_ptp.c
>>> index ff62b5f2c8150..fd51ab8c10c20 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>> @@ -169,6 +169,17 @@ static void i40e_ptp_extts0_work(struct 
>>> work_struct *work)
>>>       ptp_clock_event(pf->ptp_clock, &event);
>>>   }
>>>   +/**
>>> + * i40e_ptp_init_work - Initialize PTP work for a PF
>>> + * @pf: Board private structure
>>> + *
>>> + * Initialize work which must remain valid for the lifetime of the PF.
>>> + */
>>> +void i40e_ptp_init_work(struct i40e_pf *pf)
>>> +{
>>> +    INIT_WORK(&pf->ptp_extts0_work, i40e_ptp_extts0_work);
>>> +}
>>> +
>>
>> why do you need extra function for a single line of code? why cannot 
>> you put it into i40e_ptp_init?
>>
>>
> Thanks for pointing this out.
> 
> i40e_ptp_init() is also called from the reset/rebuild path:
> 
>    i40e_service_task()
>      -> i40e_reset_subtask()
>        -> i40e_reset_and_rebuild()
>          -> i40e_rebuild()
>            -> i40e_setup_pf_switch()
>              -> i40e_ptp_init()
> 
> Putting INIT_WORK() there would reinitialize the work item during reset or
> recovery. If the work is pending or running, this races with the workqueue,
> resets its bookkeeping, and breaks the workqueue's non-reentrance
> guarantee.

ok, but then this driver has bigger problem. the same i40e_ptp_init()
re-inits tmreg_lock mutex and ptp_rx_lock spinlock. While latter might
be ok during reset (hopefully no traffic processed while the nic is in
reset), tmreg_lock mutex can be used for adjfine/adjtime/gettimex64/
settime64 callbacks as ptp device is still visible to user space.

I believe ptp part of the driver has to be redesigned properly.

> 
> The work item is therefore initialized once per PF from i40e_sw_init().
> The helper also keeps the work callback private to i40e_ptp.c.