* [PATCH net-next 0/2] ionic watchdog training
@ 2020-09-29 22:19 Shannon Nelson
2020-09-29 22:19 ` [PATCH net-next 1/2] ionic: stop watchdog timer earlier on remove Shannon Nelson
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Shannon Nelson @ 2020-09-29 22:19 UTC (permalink / raw)
To: netdev, davem; +Cc: Shannon Nelson
Our link watchdog displayed a couple of unfriendly behaviors in some recent
stress testing. These patches change the startup and stop timing in order
to be sure that expected structures are ready to be used by the watchdog.
Shannon Nelson (2):
ionic: stop watchdog timer earlier on remove
ionic: prevent early watchdog check
drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 5 +++--
drivers/net/ethernet/pensando/ionic/ionic_dev.c | 10 ++++------
drivers/net/ethernet/pensando/ionic/ionic_dev.h | 1 -
3 files changed, 7 insertions(+), 9 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next 1/2] ionic: stop watchdog timer earlier on remove 2020-09-29 22:19 [PATCH net-next 0/2] ionic watchdog training Shannon Nelson @ 2020-09-29 22:19 ` Shannon Nelson 2020-09-30 0:15 ` Jakub Kicinski 2020-09-29 22:19 ` [PATCH net-next 2/2] ionic: prevent early watchdog check Shannon Nelson 2020-09-30 1:20 ` [PATCH net-next 0/2] ionic watchdog training David Miller 2 siblings, 1 reply; 8+ messages in thread From: Shannon Nelson @ 2020-09-29 22:19 UTC (permalink / raw) To: netdev, davem; +Cc: Shannon Nelson We need to be better at making sure we don't have a link check watchdog go off while we're shutting things down, so let's stop the timer as soon as we start the remove. Meanwhile, since that was the only thing in ionic_dev_teardown(), simplify and remove that function. Signed-off-by: Shannon Nelson <snelson@pensando.io> --- drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 5 +++-- drivers/net/ethernet/pensando/ionic/ionic_dev.c | 5 ----- drivers/net/ethernet/pensando/ionic/ionic_dev.h | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c index d1d6fb6669e5..2749ce009ebc 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c @@ -350,7 +350,7 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) err_out_reset: ionic_reset(ionic); err_out_teardown: - ionic_dev_teardown(ionic); + del_timer_sync(&ionic->watchdog_timer); pci_clear_master(pdev); /* Don't fail the probe for these errors, keep * the hw interface around for inspection @@ -378,6 +378,8 @@ static void ionic_remove(struct pci_dev *pdev) if (!ionic) return; + del_timer_sync(&ionic->watchdog_timer); + if (ionic->lif) { ionic_devlink_unregister(ionic); ionic_lif_unregister(ionic->lif); @@ -389,7 +391,6 @@ static void ionic_remove(struct pci_dev *pdev) ionic_port_reset(ionic); ionic_reset(ionic); - ionic_dev_teardown(ionic); pci_clear_master(pdev); ionic_unmap_bars(ionic); pci_release_regions(pdev); diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c index 6068f51a11d9..306e9401b09b 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c @@ -98,11 +98,6 @@ int ionic_dev_setup(struct ionic *ionic) return 0; } -void ionic_dev_teardown(struct ionic *ionic) -{ - del_timer_sync(&ionic->watchdog_timer); -} - /* Devcmd Interface */ int ionic_heartbeat_check(struct ionic *ionic) { diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h index 8842dc4a716f..c109cd5a0471 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h @@ -283,7 +283,6 @@ static inline bool ionic_q_has_space(struct ionic_queue *q, unsigned int want) void ionic_init_devinfo(struct ionic *ionic); int ionic_dev_setup(struct ionic *ionic); -void ionic_dev_teardown(struct ionic *ionic); void ionic_dev_cmd_go(struct ionic_dev *idev, union ionic_dev_cmd *cmd); u8 ionic_dev_cmd_status(struct ionic_dev *idev); -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] ionic: stop watchdog timer earlier on remove 2020-09-29 22:19 ` [PATCH net-next 1/2] ionic: stop watchdog timer earlier on remove Shannon Nelson @ 2020-09-30 0:15 ` Jakub Kicinski 0 siblings, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2020-09-30 0:15 UTC (permalink / raw) To: Shannon Nelson; +Cc: netdev, davem On Tue, 29 Sep 2020 15:19:55 -0700 Shannon Nelson wrote: > We need to be better at making sure we don't have a link check > watchdog go off while we're shutting things down, so let's stop > the timer as soon as we start the remove. > > Meanwhile, since that was the only thing in > ionic_dev_teardown(), simplify and remove that function. > > Signed-off-by: Shannon Nelson <snelson@pensando.io> The asymmetry of when the watchdog is started and stopped is a little strange. Won't there be a similar problem now with the watchdog starting too early? ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] ionic: prevent early watchdog check 2020-09-29 22:19 [PATCH net-next 0/2] ionic watchdog training Shannon Nelson 2020-09-29 22:19 ` [PATCH net-next 1/2] ionic: stop watchdog timer earlier on remove Shannon Nelson @ 2020-09-29 22:19 ` Shannon Nelson 2020-09-30 0:15 ` Jakub Kicinski 2020-09-30 1:20 ` [PATCH net-next 0/2] ionic watchdog training David Miller 2 siblings, 1 reply; 8+ messages in thread From: Shannon Nelson @ 2020-09-29 22:19 UTC (permalink / raw) To: netdev, davem; +Cc: Shannon Nelson In one corner case scenario, the driver device lif setup can get delayed such that the ionic_watchdog_cb() timer goes off before the ionic->lif is set, thus causing a NULL pointer panic. We catch the problem by checking for a NULL lif just a little earlier in the callback. Signed-off-by: Shannon Nelson <snelson@pensando.io> --- drivers/net/ethernet/pensando/ionic/ionic_dev.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c index 306e9401b09b..f90322ec3e18 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c @@ -19,9 +19,12 @@ static void ionic_watchdog_cb(struct timer_list *t) mod_timer(&ionic->watchdog_timer, round_jiffies(jiffies + ionic->watchdog_period)); + if (!ionic->lif) + return; + hb = ionic_heartbeat_check(ionic); - if (hb >= 0 && ionic->lif) + if (hb >= 0) ionic_link_status_check_request(ionic->lif); } -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] ionic: prevent early watchdog check 2020-09-29 22:19 ` [PATCH net-next 2/2] ionic: prevent early watchdog check Shannon Nelson @ 2020-09-30 0:15 ` Jakub Kicinski 2020-09-30 0:17 ` Shannon Nelson 0 siblings, 1 reply; 8+ messages in thread From: Jakub Kicinski @ 2020-09-30 0:15 UTC (permalink / raw) To: Shannon Nelson; +Cc: netdev, davem On Tue, 29 Sep 2020 15:19:56 -0700 Shannon Nelson wrote: > In one corner case scenario, the driver device lif setup can > get delayed such that the ionic_watchdog_cb() timer goes off > before the ionic->lif is set, thus causing a NULL pointer panic. > We catch the problem by checking for a NULL lif just a little > earlier in the callback. > > Signed-off-by: Shannon Nelson <snelson@pensando.io> Hah, I should have looked at the second patch :D ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] ionic: prevent early watchdog check 2020-09-30 0:15 ` Jakub Kicinski @ 2020-09-30 0:17 ` Shannon Nelson 2020-09-30 0:26 ` Jakub Kicinski 0 siblings, 1 reply; 8+ messages in thread From: Shannon Nelson @ 2020-09-30 0:17 UTC (permalink / raw) To: Jakub Kicinski; +Cc: netdev, davem On 9/29/20 5:15 PM, Jakub Kicinski wrote: > On Tue, 29 Sep 2020 15:19:56 -0700 Shannon Nelson wrote: >> In one corner case scenario, the driver device lif setup can >> get delayed such that the ionic_watchdog_cb() timer goes off >> before the ionic->lif is set, thus causing a NULL pointer panic. >> We catch the problem by checking for a NULL lif just a little >> earlier in the callback. >> >> Signed-off-by: Shannon Nelson <snelson@pensando.io> > Hah, I should have looked at the second patch :D Am I making my patches too small now? :-) sln ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] ionic: prevent early watchdog check 2020-09-30 0:17 ` Shannon Nelson @ 2020-09-30 0:26 ` Jakub Kicinski 0 siblings, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2020-09-30 0:26 UTC (permalink / raw) To: Shannon Nelson; +Cc: netdev, davem On Tue, 29 Sep 2020 17:17:45 -0700 Shannon Nelson wrote: > On 9/29/20 5:15 PM, Jakub Kicinski wrote: > > On Tue, 29 Sep 2020 15:19:56 -0700 Shannon Nelson wrote: > >> In one corner case scenario, the driver device lif setup can > >> get delayed such that the ionic_watchdog_cb() timer goes off > >> before the ionic->lif is set, thus causing a NULL pointer panic. > >> We catch the problem by checking for a NULL lif just a little > >> earlier in the callback. > >> > >> Signed-off-by: Shannon Nelson <snelson@pensando.io> > > Hah, I should have looked at the second patch :D > > Am I making my patches too small now? :-) Just right :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] ionic watchdog training 2020-09-29 22:19 [PATCH net-next 0/2] ionic watchdog training Shannon Nelson 2020-09-29 22:19 ` [PATCH net-next 1/2] ionic: stop watchdog timer earlier on remove Shannon Nelson 2020-09-29 22:19 ` [PATCH net-next 2/2] ionic: prevent early watchdog check Shannon Nelson @ 2020-09-30 1:20 ` David Miller 2 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2020-09-30 1:20 UTC (permalink / raw) To: snelson; +Cc: netdev From: Shannon Nelson <snelson@pensando.io> Date: Tue, 29 Sep 2020 15:19:54 -0700 > Our link watchdog displayed a couple of unfriendly behaviors in some recent > stress testing. These patches change the startup and stop timing in order > to be sure that expected structures are ready to be used by the watchdog. This doesn't apply cleanly, almost certainly because of conflicts with Thomas Gleixner's patch series. Always do a quick update of your net-next tree before submitting patches to avoid this problem in the future. Thank you. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-30 1:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-29 22:19 [PATCH net-next 0/2] ionic watchdog training Shannon Nelson 2020-09-29 22:19 ` [PATCH net-next 1/2] ionic: stop watchdog timer earlier on remove Shannon Nelson 2020-09-30 0:15 ` Jakub Kicinski 2020-09-29 22:19 ` [PATCH net-next 2/2] ionic: prevent early watchdog check Shannon Nelson 2020-09-30 0:15 ` Jakub Kicinski 2020-09-30 0:17 ` Shannon Nelson 2020-09-30 0:26 ` Jakub Kicinski 2020-09-30 1:20 ` [PATCH net-next 0/2] ionic watchdog training David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.