On 1/29/26 22:48, Paul Menzel wrote:
Dear Petr,
Thank you for the patch.
Am 29.01.26 um 10:57 schrieb Petr Oros:
Commit 7c01dbfc8a1c5f ("iavf: periodically cache PHC time") introduced a
worker to cache PHC time, but failed to stop it during reset or disable.
This creates a race condition where `iavf_reset_task()` or
`iavf_disable_vf()` free adapter resources (AQ) while the worker is
still
running. If the worker triggers `iavf_queue_ptp_cmd()` during
teardown, it
accesses freed memory/locks, leading to a crash.
Do you have a stacktrace, and could you add an excerpt, so people
hitting this, can more easily find the commit?
I have some stack traces. The problem is that the race window is so wide
that it sometimes crashes in the PTP subsystem, looking like:
[ 5611.939379] Call Trace:
[ 5611.941831] <TASK>
[ 5611.943937] ? show_trace_log_lvl+0x1b0/0x2f0
[ 5611.948295] ? show_trace_log_lvl+0x1b0/0x2f0
[ 5611.952656] ? ptp_aux_kworker+0x1d/0x40
[ 5611.956584] ? __die_body.cold+0x8/0x12
[ 5611.960422] ? page_fault_oops+0x148/0x160
[ 5611.964520] ? exc_page_fault+0x73/0x160
[ 5611.968445] ? asm_exc_page_fault+0x26/0x30
[ 5611.972634] ? __pfx_ptp_aux_kworker+0x10/0x10
[ 5611.977082] ? __pfx_ptp_aux_kworker+0x10/0x10
[ 5611.981525] ptp_aux_kworker+0x1d/0x40
[ 5611.985278] kthread_worker_fn+0xa0/0x260
[ 5611.989291] ? __pfx_kthread_worker_fn+0x10/0x10
[ 5611.993911] kthread+0xfd/0x240
[ 5611.997056] ? __pfx_kthread+0x10/0x10
[ 5612.000809] ret_from_fork+0x34/0x50
[ 5612.004386] ? __pfx_kthread+0x10/0x10
[ 5612.008140] ret_from_fork_asm+0x1a/0x30
[ 5612.012069] </TASK>
and other times in iavf, looking like:
3476.640150] Call Trace:
[ 3476.642597] <TASK>
[ 3476.644702] ? show_trace_log_lvl+0x1b0/0x2f0
[ 3476.649062] ? show_trace_log_lvl+0x1b0/0x2f0
[ 3476.653420] ? mod_delayed_work_on+0x9f/0xb0
[ 3476.657691] ? __die_body.cold+0x8/0x12
[ 3476.661530] ? page_fault_oops+0x148/0x160
[ 3476.665629] ? exc_page_fault+0x7f/0x150
[ 3476.669556] ? asm_exc_page_fault+0x26/0x30
[ 3476.673743] ? __queue_work.part.0+0x44/0x320
[ 3476.678100] ? __pfx_ptp_aux_kworker+0x10/0x10
[ 3476.682547] mod_delayed_work_on+0x9f/0xb0
[ 3476.686647] iavf_send_phc_read+0xb0/0xd0 [iavf]
[ 3476.691283] iavf_ptp_do_aux_work+0x39/0x50 [iavf]
[ 3476.696083] ptp_aux_kworker+0x1d/0x40
[ 3476.699835] kthread_worker_fn+0xa3/0x260
[ 3476.703848] ? __pfx_kthread_worker_fn+0x10/0x10
[ 3476.708465] kthread+0xfa/0x240
[ 3476.711613] ? __pfx_kthread+0x10/0x10
[ 3476.715365] ret_from_fork+0x34/0x50
[ 3476.718943] ? __pfx_kthread+0x10/0x10
[ 3476.722695] ret_from_fork_asm+0x1a/0x30
[ 3476.726622] </TASK>
, etc.
Fix this by calling `iavf_ptp_release()` before tearing down the
adapter.
This ensures `ptp_clock_unregister()` synchronously cancels the
worker and
cleans up the chardev before the backing resources are destroyed.
Fixes: 7c01dbfc8a1c5f ("iavf: periodically cache PHC time")
Signed-off-by: Petr Oros <[email protected]>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 4b0fc8f354bc90..0dd58ce5a53ab1 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -3025,6 +3025,8 @@ static void iavf_disable_vf(struct iavf_adapter
*adapter)
adapter->flags |= IAVF_FLAG_PF_COMMS_FAILED;
+ iavf_ptp_release(adapter);
+
/* We don't use netif_running() because it may be true prior to
* ndo_open() returning, so we can't assume it means all our open
* tasks have finished, since we're not holding the rtnl_lock
here.
@@ -3200,6 +3202,8 @@ static void iavf_reset_task(struct work_struct
*work)
iavf_change_state(adapter, __IAVF_RESETTING);
adapter->flags &= ~IAVF_FLAG_RESET_PENDING;
+ iavf_ptp_release(adapter);
+
/* free the Tx/Rx rings and descriptors, might be better to just
* re-use them sometime in the future
*/
Reviewed-by: Paul Menzel <[email protected]>
Kind regards,
Paul
