On Thu, 2020-02-06 at 13:33 +0100, Daniel Wagner wrote:
> Hi Martin,
>
> On Wed, Feb 05, 2020 at 10:44:20PM +0100, mwilck@xxxxxxxx wrote:
> > From: Martin Wilck <mwilck@xxxxxxxx>
> >
> > Since commit 45235022da99 ("scsi: qla2xxx: Fix driver unload by
> > shutting down chip"),
> > it is possible that FC commands are scheduled after the adapter
> > firmware
> > has been shut down. IO sent to the firmware in this situation hangs
> > indefinitely. Avoid this for the LOGO code path that is typically
> > taken
> > when adapters are shut down.
> >
> > Fixes: 45235022da99 ("scsi: qla2xxx: Fix driver unload by shutting
> > down chip")
> > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> > Reviewed-by: Roman Bolshakov <r.bolshakov@xxxxxxxxx>
>
> Just to understand it correctly: 45235022da99 ("scsi: qla2xxx: Fix
> driver unload by shutting down chip") is saying FW is not able to
> shutdown propably and therefore we kill it first and still try to do
> some cleanup.
Yes, that's what I observed. Mailbox access hangs in this case, as the
mailbox simply won't execute the command and the expected status change
will not happen.
> Are you sure you got all the necessary places fixed up?
>
> I tend to say we should add
>
> if (!ha->flags.fw_started)
> return QLA_FUNCTION_FAILED;
>
> do qla2x00_mailbox_command() and handle the errors one by one. Just
> an
> idea.
I had that idea too. I even tried it out. IIRC it's not that easy. Some
commands need to be sent to the mailbox even in shut-down state
(MBC_EXECUTE_FIRMWARE, for example?). I admit I didn't dare going down
the path you're suggesting, I thought it had quite a potential to cause
regressions.
Did you mean this as a negative review, or rather an additional
suggestion?
Thanks
Martin
