| Message ID | 20200205214422.3657-4-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: qla2xxx: fixes for driver unloading | expand |
On Wed, Feb 05, 2020 at 10:44:22PM +0100, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > The purpose of the UNLOADING flag is to avoid port login procedures > to continue when a controller is in the process of shutting down. > It makes sense to set this flag before starting session teardown. > The only operations that must be able to continue are LOGO, PRLO, > and the like. > > Furthermore, use atomic test_and_set_bit() to avoid the shutdown > being run multiple times in parallel. In qla2x00_disable_board_on_pci_error(), > the test for UNLOADING is postponed until after the check for an already > disabled PCI board. > > Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Daniel Wagner <dwagner@suse.de>
On Wed, Feb 05, 2020 at 10:44:22PM +0100, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > The purpose of the UNLOADING flag is to avoid port login procedures > to continue when a controller is in the process of shutting down. > It makes sense to set this flag before starting session teardown. > The only operations that must be able to continue are LOGO, PRLO, > and the like. > > Furthermore, use atomic test_and_set_bit() to avoid the shutdown > being run multiple times in parallel. In qla2x00_disable_board_on_pci_error(), > the test for UNLOADING is postponed until after the check for an already > disabled PCI board. > > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > drivers/scsi/qla2xxx/qla_os.c | 25 +++++++++++-------------- > 1 file changed, 11 insertions(+), 14 deletions(-) > Hi Martin, Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com> Thanks, Roman
On Wed, 5 Feb 2020, 1:44pm, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > The purpose of the UNLOADING flag is to avoid port login procedures > to continue when a controller is in the process of shutting down. > It makes sense to set this flag before starting session teardown. > The only operations that must be able to continue are LOGO, PRLO, > and the like. > > Furthermore, use atomic test_and_set_bit() to avoid the shutdown > being run multiple times in parallel. In qla2x00_disable_board_on_pci_error(), > the test for UNLOADING is postponed until after the check for an already > disabled PCI board. > > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > drivers/scsi/qla2xxx/qla_os.c | 25 +++++++++++-------------- > 1 file changed, 11 insertions(+), 14 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c > index e81080d..8329f95 100644 > --- a/drivers/scsi/qla2xxx/qla_os.c > +++ b/drivers/scsi/qla2xxx/qla_os.c > @@ -3720,16 +3720,15 @@ qla2x00_remove_one(struct pci_dev *pdev) > } > qla2x00_wait_for_hba_ready(base_vha); > > - qla2x00_wait_for_sess_deletion(base_vha); > - > /* > - * if UNLOAD flag is already set, then continue unload, > + * if UNLOADING flag is already set, then continue unload, > * where it was set first. > */ > - if (test_bit(UNLOADING, &base_vha->dpc_flags)) > + if (test_and_set_bit(UNLOADING, &base_vha->dpc_flags)) > return; > > - set_bit(UNLOADING, &base_vha->dpc_flags); > + qla2x00_wait_for_sess_deletion(base_vha); > + > if (IS_QLA25XX(ha) || IS_QLA2031(ha) || IS_QLA27XX(ha) || > IS_QLA28XX(ha)) { > if (ha->flags.fw_started) > @@ -6046,13 +6045,6 @@ qla2x00_disable_board_on_pci_error(struct work_struct *work) > struct pci_dev *pdev = ha->pdev; > scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev); > > - /* > - * if UNLOAD flag is already set, then continue unload, > - * where it was set first. > - */ > - if (test_bit(UNLOADING, &base_vha->dpc_flags)) > - return; > - > ql_log(ql_log_warn, base_vha, 0x015b, > "Disabling adapter.\n"); > > @@ -6063,9 +6055,14 @@ qla2x00_disable_board_on_pci_error(struct work_struct *work) > return; > } > > - qla2x00_wait_for_sess_deletion(base_vha); > + /* > + * if UNLOADING flag is already set, then continue unload, > + * where it was set first. > + */ > + if (test_and_set_bit(UNLOADING, &base_vha->dpc_flags)) > + return; > > - set_bit(UNLOADING, &base_vha->dpc_flags); > + qla2x00_wait_for_sess_deletion(base_vha); > > qla2x00_delete_all_vps(ha, base_vha); > > This was done on top of PATCH 2/3, please resend dropping 2. The test_and_set_bit() part looks ok. Regards, -Arun
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index e81080d..8329f95 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -3720,16 +3720,15 @@ qla2x00_remove_one(struct pci_dev *pdev) } qla2x00_wait_for_hba_ready(base_vha); - qla2x00_wait_for_sess_deletion(base_vha); - /* - * if UNLOAD flag is already set, then continue unload, + * if UNLOADING flag is already set, then continue unload, * where it was set first. */ - if (test_bit(UNLOADING, &base_vha->dpc_flags)) + if (test_and_set_bit(UNLOADING, &base_vha->dpc_flags)) return; - set_bit(UNLOADING, &base_vha->dpc_flags); + qla2x00_wait_for_sess_deletion(base_vha); + if (IS_QLA25XX(ha) || IS_QLA2031(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha)) { if (ha->flags.fw_started) @@ -6046,13 +6045,6 @@ qla2x00_disable_board_on_pci_error(struct work_struct *work) struct pci_dev *pdev = ha->pdev; scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev); - /* - * if UNLOAD flag is already set, then continue unload, - * where it was set first. - */ - if (test_bit(UNLOADING, &base_vha->dpc_flags)) - return; - ql_log(ql_log_warn, base_vha, 0x015b, "Disabling adapter.\n"); @@ -6063,9 +6055,14 @@ qla2x00_disable_board_on_pci_error(struct work_struct *work) return; } - qla2x00_wait_for_sess_deletion(base_vha); + /* + * if UNLOADING flag is already set, then continue unload, + * where it was set first. + */ + if (test_and_set_bit(UNLOADING, &base_vha->dpc_flags)) + return; - set_bit(UNLOADING, &base_vha->dpc_flags); + qla2x00_wait_for_sess_deletion(base_vha); qla2x00_delete_all_vps(ha, base_vha);