From: Mateusz Polchlopek <mateusz.polchlo...@intel.com> Date: Tue, 1 Oct 2024 09:20:14 +0200
> > > On 8/21/2024 4:31 PM, Alexander Lobakin wrote: >> From: Wojciech Drewek <wojciech.dre...@intel.com> >> Date: Wed, 21 Aug 2024 14:15:32 +0200 >> >>> From: Jacob Keller <jacob.e.kel...@intel.com> >>> >>> Implement support for reading the PHC time indirectly via the >>> VIRTCHNL_OP_1588_PTP_GET_TIME operation. >> >> [...] >> >>> +/** >>> + * iavf_queue_ptp_cmd - Queue PTP command for sending over virtchnl >>> + * @adapter: private adapter structure >>> + * @cmd: the command structure to send >>> + * >>> + * Queue the given command structure into the PTP virtchnl command >>> queue tos >>> + * end to the PF. >>> + */ >>> +static void iavf_queue_ptp_cmd(struct iavf_adapter *adapter, >>> + struct iavf_ptp_aq_cmd *cmd) >>> +{ >>> + mutex_lock(&adapter->ptp.aq_cmd_lock); >>> + list_add_tail(&cmd->list, &adapter->ptp.aq_cmds); >>> + mutex_unlock(&adapter->ptp.aq_cmd_lock); >>> + >>> + adapter->aq_required |= IAVF_FLAG_AQ_SEND_PTP_CMD; >>> + mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0); >> >> Are you sure you need delayed_work here? delayed_work is used only when >> you need to run it after a delay. If the delay is always 0, then you >> only need work_struct and queue_work(). >> > > Sorry for late response but I was on quite long vacation. > > Regarding your question - I think it is needed to have > mod_delayed_work() here, at least as for now. I agree with your > suggestion to use queue_work() but this function takes as parameter > work_struct and in our case the adapter->watchdog_task field is of > struct delayed_work type. It uses the function iavf_watchdog_task() > which does plenty of things (including sending ptp cmd). Changing > adapter->watchdog_task to be just struct work_struct is not applicable > here as in iavf_main.c file in few places we add it to queue with > different time. Aaaah I'm sorry I didn't notice that watchdog_task is used in other placed, not only here. Ack, leave it as it is. > > Make long story short - I agree with your argument but as far as this > 0 delay is not causing performance regression then I will stick with > this solution implemented by Jake. Thanks, Olek