| Message ID | 20240915070334.1267-1-demonihin@gmail.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | e1000e: makes e1000_watchdog_task use queue_delayed_work | expand |
On 9/15/2024 12:03 AM, Dmitrii Ermakov wrote: > > Replaces watchdog timer with delayed_work as advised > in the driver's TODO comment. > > Signed-off-by: Dmitrii Ermakov <demonihin@gmail.com> The Subject should identify which tree to go to, net or net-next. This should be for net-next, e.g. [PATCH net-next]. But be aware that net-next tree is closed during the merge window, so you'll need to wait a week or two before reposting. > --- > drivers/net/ethernet/intel/e1000e/e1000.h | 4 +- > drivers/net/ethernet/intel/e1000e/netdev.c | 43 ++++++++-------------- > 2 files changed, 18 insertions(+), 29 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h > index ba9c19e6994c..5a60372d2158 100644 > --- a/drivers/net/ethernet/intel/e1000e/e1000.h > +++ b/drivers/net/ethernet/intel/e1000e/e1000.h > @@ -189,12 +189,12 @@ struct e1000_phy_regs { > > /* board specific private data structure */ > struct e1000_adapter { > - struct timer_list watchdog_timer; > struct timer_list phy_info_timer; > struct timer_list blink_timer; > > + struct delayed_work watchdog_work; > + > struct work_struct reset_task; > - struct work_struct watchdog_task; > > const struct e1000_info *ei; > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > index 360ee26557f7..5b7a3a1423ed 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -1778,7 +1778,8 @@ static irqreturn_t e1000_intr_msi(int __always_unused irq, void *data) > } > /* guard against interrupt when we're going down */ > if (!test_bit(__E1000_DOWN, &adapter->state)) > - mod_timer(&adapter->watchdog_timer, jiffies + 1); > + queue_delayed_work(system_wq, &adapter->watchdog_work, > + 1); This is not worth the line wrap, keep it one line. > } > > /* Reset on uncorrectable ECC error */ > @@ -1857,7 +1858,8 @@ static irqreturn_t e1000_intr(int __always_unused irq, void *data) > } > /* guard against interrupt when we're going down */ > if (!test_bit(__E1000_DOWN, &adapter->state)) > - mod_timer(&adapter->watchdog_timer, jiffies + 1); > + queue_delayed_work(system_wq, &adapter->watchdog_work, > + 1); ditto > } > > /* Reset on uncorrectable ECC error */ > @@ -1901,7 +1903,8 @@ static irqreturn_t e1000_msix_other(int __always_unused irq, void *data) > hw->mac.get_link_status = true; > /* guard against interrupt when we're going down */ > if (!test_bit(__E1000_DOWN, &adapter->state)) > - mod_timer(&adapter->watchdog_timer, jiffies + 1); > + queue_delayed_work(system_wq, &adapter->watchdog_work, > + 1); ditto > } > > if (!test_bit(__E1000_DOWN, &adapter->state)) > @@ -4293,7 +4296,8 @@ void e1000e_down(struct e1000_adapter *adapter, bool reset) > > napi_synchronize(&adapter->napi); > > - del_timer_sync(&adapter->watchdog_timer); > + cancel_delayed_work_sync(&adapter->watchdog_work); > + > del_timer_sync(&adapter->phy_info_timer); > > spin_lock(&adapter->stats64_lock); > @@ -5164,25 +5168,12 @@ static void e1000e_check_82574_phy_workaround(struct e1000_adapter *adapter) > } > } > > -/** > - * e1000_watchdog - Timer Call-back > - * @t: pointer to timer_list containing private info adapter > - **/ > -static void e1000_watchdog(struct timer_list *t) > -{ > - struct e1000_adapter *adapter = from_timer(adapter, t, watchdog_timer); > - > - /* Do the rest outside of interrupt context */ > - schedule_work(&adapter->watchdog_task); > - > - /* TODO: make this use queue_delayed_work() */ > -} > - > static void e1000_watchdog_task(struct work_struct *work) In keeping with the theme, this could be changed to e1000_watchdog_work() > { > - struct e1000_adapter *adapter = container_of(work, > - struct e1000_adapter, > - watchdog_task); > + struct delayed_work *dwork = > + container_of(work, struct delayed_work, work); > + struct e1000_adapter *adapter = > + container_of(dwork, struct e1000_adapter, watchdog_work); > struct net_device *netdev = adapter->netdev; > struct e1000_mac_info *mac = &adapter->hw.mac; > struct e1000_phy_info *phy = &adapter->hw.phy; > @@ -5411,8 +5402,8 @@ static void e1000_watchdog_task(struct work_struct *work) > > /* Reset the timer */ > if (!test_bit(__E1000_DOWN, &adapter->state)) > - mod_timer(&adapter->watchdog_timer, > - round_jiffies(jiffies + 2 * HZ)); > + queue_delayed_work(system_wq, &adapter->watchdog_work, > + round_jiffies(2 * HZ)); > } > > #define E1000_TX_FLAGS_CSUM 0x00000001 > @@ -7588,11 +7579,10 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > goto err_eeprom; > } > > - timer_setup(&adapter->watchdog_timer, e1000_watchdog, 0); > timer_setup(&adapter->phy_info_timer, e1000_update_phy_info, 0); > + INIT_DELAYED_WORK(&adapter->watchdog_work, e1000_watchdog_task); > > INIT_WORK(&adapter->reset_task, e1000_reset_task); > - INIT_WORK(&adapter->watchdog_task, e1000_watchdog_task); > INIT_WORK(&adapter->downshift_task, e1000e_downshift_workaround); > INIT_WORK(&adapter->update_phy_task, e1000e_update_phy_task); > INIT_WORK(&adapter->print_hang_task, e1000_print_hw_hang); > @@ -7733,11 +7723,10 @@ static void e1000_remove(struct pci_dev *pdev) > * from being rescheduled. > */ > set_bit(__E1000_DOWN, &adapter->state); > - del_timer_sync(&adapter->watchdog_timer); > + cancel_delayed_work_sync(&adapter->watchdog_work); > del_timer_sync(&adapter->phy_info_timer); > > cancel_work_sync(&adapter->reset_task); > - cancel_work_sync(&adapter->watchdog_task); > cancel_work_sync(&adapter->downshift_task); > cancel_work_sync(&adapter->update_phy_task); > cancel_work_sync(&adapter->print_hang_task); > -- > 2.45.2 > >
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h index ba9c19e6994c..5a60372d2158 100644 --- a/drivers/net/ethernet/intel/e1000e/e1000.h +++ b/drivers/net/ethernet/intel/e1000e/e1000.h @@ -189,12 +189,12 @@ struct e1000_phy_regs { /* board specific private data structure */ struct e1000_adapter { - struct timer_list watchdog_timer; struct timer_list phy_info_timer; struct timer_list blink_timer; + struct delayed_work watchdog_work; + struct work_struct reset_task; - struct work_struct watchdog_task; const struct e1000_info *ei; diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 360ee26557f7..5b7a3a1423ed 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -1778,7 +1778,8 @@ static irqreturn_t e1000_intr_msi(int __always_unused irq, void *data) } /* guard against interrupt when we're going down */ if (!test_bit(__E1000_DOWN, &adapter->state)) - mod_timer(&adapter->watchdog_timer, jiffies + 1); + queue_delayed_work(system_wq, &adapter->watchdog_work, + 1); } /* Reset on uncorrectable ECC error */ @@ -1857,7 +1858,8 @@ static irqreturn_t e1000_intr(int __always_unused irq, void *data) } /* guard against interrupt when we're going down */ if (!test_bit(__E1000_DOWN, &adapter->state)) - mod_timer(&adapter->watchdog_timer, jiffies + 1); + queue_delayed_work(system_wq, &adapter->watchdog_work, + 1); } /* Reset on uncorrectable ECC error */ @@ -1901,7 +1903,8 @@ static irqreturn_t e1000_msix_other(int __always_unused irq, void *data) hw->mac.get_link_status = true; /* guard against interrupt when we're going down */ if (!test_bit(__E1000_DOWN, &adapter->state)) - mod_timer(&adapter->watchdog_timer, jiffies + 1); + queue_delayed_work(system_wq, &adapter->watchdog_work, + 1); } if (!test_bit(__E1000_DOWN, &adapter->state)) @@ -4293,7 +4296,8 @@ void e1000e_down(struct e1000_adapter *adapter, bool reset) napi_synchronize(&adapter->napi); - del_timer_sync(&adapter->watchdog_timer); + cancel_delayed_work_sync(&adapter->watchdog_work); + del_timer_sync(&adapter->phy_info_timer); spin_lock(&adapter->stats64_lock); @@ -5164,25 +5168,12 @@ static void e1000e_check_82574_phy_workaround(struct e1000_adapter *adapter) } } -/** - * e1000_watchdog - Timer Call-back - * @t: pointer to timer_list containing private info adapter - **/ -static void e1000_watchdog(struct timer_list *t) -{ - struct e1000_adapter *adapter = from_timer(adapter, t, watchdog_timer); - - /* Do the rest outside of interrupt context */ - schedule_work(&adapter->watchdog_task); - - /* TODO: make this use queue_delayed_work() */ -} - static void e1000_watchdog_task(struct work_struct *work) { - struct e1000_adapter *adapter = container_of(work, - struct e1000_adapter, - watchdog_task); + struct delayed_work *dwork = + container_of(work, struct delayed_work, work); + struct e1000_adapter *adapter = + container_of(dwork, struct e1000_adapter, watchdog_work); struct net_device *netdev = adapter->netdev; struct e1000_mac_info *mac = &adapter->hw.mac; struct e1000_phy_info *phy = &adapter->hw.phy; @@ -5411,8 +5402,8 @@ static void e1000_watchdog_task(struct work_struct *work) /* Reset the timer */ if (!test_bit(__E1000_DOWN, &adapter->state)) - mod_timer(&adapter->watchdog_timer, - round_jiffies(jiffies + 2 * HZ)); + queue_delayed_work(system_wq, &adapter->watchdog_work, + round_jiffies(2 * HZ)); } #define E1000_TX_FLAGS_CSUM 0x00000001 @@ -7588,11 +7579,10 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) goto err_eeprom; } - timer_setup(&adapter->watchdog_timer, e1000_watchdog, 0); timer_setup(&adapter->phy_info_timer, e1000_update_phy_info, 0); + INIT_DELAYED_WORK(&adapter->watchdog_work, e1000_watchdog_task); INIT_WORK(&adapter->reset_task, e1000_reset_task); - INIT_WORK(&adapter->watchdog_task, e1000_watchdog_task); INIT_WORK(&adapter->downshift_task, e1000e_downshift_workaround); INIT_WORK(&adapter->update_phy_task, e1000e_update_phy_task); INIT_WORK(&adapter->print_hang_task, e1000_print_hw_hang); @@ -7733,11 +7723,10 @@ static void e1000_remove(struct pci_dev *pdev) * from being rescheduled. */ set_bit(__E1000_DOWN, &adapter->state); - del_timer_sync(&adapter->watchdog_timer); + cancel_delayed_work_sync(&adapter->watchdog_work); del_timer_sync(&adapter->phy_info_timer); cancel_work_sync(&adapter->reset_task); - cancel_work_sync(&adapter->watchdog_task); cancel_work_sync(&adapter->downshift_task); cancel_work_sync(&adapter->update_phy_task); cancel_work_sync(&adapter->print_hang_task);
Replaces watchdog timer with delayed_work as advised in the driver's TODO comment. Signed-off-by: Dmitrii Ermakov <demonihin@gmail.com> --- drivers/net/ethernet/intel/e1000e/e1000.h | 4 +- drivers/net/ethernet/intel/e1000e/netdev.c | 43 ++++++++-------------- 2 files changed, 18 insertions(+), 29 deletions(-)