On Sun, 6 Sep 2020 20:48:04 -0700 Michael Chan wrote: > On Sun, Sep 6, 2020 at 8:13 PM Jakub Kicinski <k...@kernel.org> wrote: > > > > On Sun, 6 Sep 2020 15:07:02 -0700 Michael Chan wrote: > > > On Sun, Sep 6, 2020 at 12:25 PM Jakub Kicinski <k...@kernel.org> wrote: > > > > > > > > devlink can itself scheduler a recovery via: > > > > > > > > bnxt_fw_fatal_recover() -> bnxt_fw_reset() > > > > > > > > > > Yes, this is how it is initiated when we call devlink_health_report() > > > to report the error condition. From bnxt_fw_reset(), we use a > > > workqueue because we have to go through many states, requiring > > > sleeping/polling to transition through the states. > > > > > > > no? Maybe don't make the devlink recovery path need to go via a > > > > workqueue? > > > > > > Current implementation is going through a work queue. > > > > What I'm saying is the code looks like this after this patch: > > > > + clear_bit(BNXT_STATE_IN_FW_RESET, &bp->state); > > + bnxt_cancel_sp_work(bp); > > + bp->sp_event = 0; > > + > > bnxt_dl_fw_reporters_destroy(bp, true); > > > > It cancels the work, _then_ destroys the reporter. But I think the > > reported can be used to schedule a recovery from command line. So the > > work may get re-scheduled after it has been canceled. > > bnxt_en does not support recovery from the command line. We return > -EOPNOTSUPP when it comes from the command line. > > Recovery has to be triggered from a firmware reported error or a > driver detected error.
I see it now, thanks.
