mbox series

[v4,0/2] scsi: qla2xxx: fixes for driver unloading

Message ID 20200421204621.19228-1-mwilck@suse.com (mailing list archive)
Headers show
Series scsi: qla2xxx: fixes for driver unloading | expand

Message

Martin Wilck April 21, 2020, 8:46 p.m. UTC
From: Martin Wilck <mwilck@suse.com>

Hello Martin, Arun, Himanshu, all,

here is v4 of the little series I first submitted on Nov 29, 2019.
I dropped the more controversial part, hoping to get the actual fix
for the issue merged.

Reviews welcome.
Martin

Changes since v3:
 - In patch 2, moved check from qla2x00_post_async_XYZ_work() to
   qla2x00_alloc_work() as suggested by Arun
 - Dropped patch 3-5. With 1 and 2 applied, I haven't been able to reproduce
   shutdown hangs any more.

Changes since v2:
 - Removed "scsi: qla2xxx: avoid sending mailbox commands if firmware is
   stopped", because the first hunk is obsoleted by the (new) 1/3, and Arun
   suggested to use a different approach (which is now in 4/3) for the second
   hunk.
 - Removed "scsi: qla2xxx: don't shut down firmware before closing sessions"
   (nak'd by Arun).
 - Former 3/3 is now 1/5
 - Added "scsi: qla2xxx: check UNLOADING before posting async work". This one
   is key for avoiding lags when qla2xxx is unloaded.
 - Added revert of "scsi: qla2xxx: Fix unbound sleep in fcport delete path.",
   as I believe it's now obsolete.
   If we ever encounter unbound sleep there again, we should rather figure
   out the reason than simply abort waiting.
 - Added patch 4 and 5, a new attempt at avoiding mailbox and HW request queue
   access at low level. 4/5 was motivated by Arun's comments on my v2 series.
   5/5 is obviously similar in spirit to 77ddb94a4853 ("scsi: qla2xxx: Only
   allow operational MBX to proceed during RESET."), but I found that the
   rom_cmds list contains commands that would hang when the FW is stopped,
   so I created a new list. Perhaps some day the two can be consolidated.

Changes since v1:
 - Added patch 3 to set the UNLOADING flag before waiting for sessions
   to end (Roman)
 - Use test_and_set_bit() for UNLOADING (Hannes)

Martin Wilck (2):
  scsi: qla2xxx: set UNLOADING before waiting for session deletion
  scsi: qla2xxx: check UNLOADING before posting async work

 drivers/scsi/qla2xxx/qla_os.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Comments

Arun Easi April 21, 2020, 9:25 p.m. UTC | #1
Series look good.

Thank you for the patches and testing, Martin.

Regards,
-Arun

On Tue, 21 Apr 2020, 1:46pm, mwilck@suse.com wrote:

> External Email
> 
> ----------------------------------------------------------------------
> From: Martin Wilck <mwilck@suse.com>
> 
> Hello Martin, Arun, Himanshu, all,
> 
> here is v4 of the little series I first submitted on Nov 29, 2019.
> I dropped the more controversial part, hoping to get the actual fix
> for the issue merged.
> 
> Reviews welcome.
> Martin
> 
> Changes since v3:
>  - In patch 2, moved check from qla2x00_post_async_XYZ_work() to
>    qla2x00_alloc_work() as suggested by Arun
>  - Dropped patch 3-5. With 1 and 2 applied, I haven't been able to reproduce
>    shutdown hangs any more.
> 
> Changes since v2:
>  - Removed "scsi: qla2xxx: avoid sending mailbox commands if firmware is
>    stopped", because the first hunk is obsoleted by the (new) 1/3, and Arun
>    suggested to use a different approach (which is now in 4/3) for the second
>    hunk.
>  - Removed "scsi: qla2xxx: don't shut down firmware before closing sessions"
>    (nak'd by Arun).
>  - Former 3/3 is now 1/5
>  - Added "scsi: qla2xxx: check UNLOADING before posting async work". This one
>    is key for avoiding lags when qla2xxx is unloaded.
>  - Added revert of "scsi: qla2xxx: Fix unbound sleep in fcport delete path.",
>    as I believe it's now obsolete.
>    If we ever encounter unbound sleep there again, we should rather figure
>    out the reason than simply abort waiting.
>  - Added patch 4 and 5, a new attempt at avoiding mailbox and HW request queue
>    access at low level. 4/5 was motivated by Arun's comments on my v2 series.
>    5/5 is obviously similar in spirit to 77ddb94a4853 ("scsi: qla2xxx: Only
>    allow operational MBX to proceed during RESET."), but I found that the
>    rom_cmds list contains commands that would hang when the FW is stopped,
>    so I created a new list. Perhaps some day the two can be consolidated.
> 
> Changes since v1:
>  - Added patch 3 to set the UNLOADING flag before waiting for sessions
>    to end (Roman)
>  - Use test_and_set_bit() for UNLOADING (Hannes)
> 
> Martin Wilck (2):
>   scsi: qla2xxx: set UNLOADING before waiting for session deletion
>   scsi: qla2xxx: check UNLOADING before posting async work
> 
>  drivers/scsi/qla2xxx/qla_os.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
>
Martin K. Petersen April 22, 2020, 3:54 a.m. UTC | #2
Martin,

> here is v4 of the little series I first submitted on Nov 29, 2019.  I
> dropped the more controversial part, hoping to get the actual fix for
> the issue merged.

Applied to 5.7/scsi-fixes, thanks!