diff mbox series

[v3,1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available

Message ID 1537770916-16337-2-git-send-email-suganath-prabu.subramani@broadcom.com (mailing list archive)
State Changes Requested
Headers show
Series mpt3sas: Hot-Plug Surprise removal support on IOC. | expand

Commit Message

Suganath Prabu S Sept. 24, 2018, 6:35 a.m. UTC
* Driver uses "pci_device_is_present" to check whether
If Hot unplugged:
the outstanding IOs with 'DID_NO_CONNECT' before removing the drives
attached to the HBA.
"DID_NO_CONNECT" status and free the smid, if driver detects that
HBA is hot unplugged.

* In the hard reset flush out all the outstanding IOs even if diag reset
fails and also if driver detects that HBA is hot unplugged.

v1 change set:

Comments

Andy Shevchenko Sept. 25, 2018, 8:40 a.m. UTC | #1
On Mon, Sep 24, 2018 at 9:36 AM Suganath Prabu S
<suganath-prabu.subramani@broadcom.com> wrote:
>
> * Driver uses "pci_device_is_present" to check whether
> If Hot unplugged:
> the outstanding IOs with 'DID_NO_CONNECT' before removing the drives
> attached to the HBA.
> "DID_NO_CONNECT" status and free the smid, if driver detects that
> HBA is hot unplugged.
>
> * In the hard reset flush out all the outstanding IOs even if diag reset
> fails and also if driver detects that HBA is hot unplugged.

> +       if (!mpt3sas_base_pci_device_is_available(ioc)) {

> +               pr_err(MPT3SAS_FMT
> +                   "%s: pci error recovery reset or"
> +                   " pci device unplug occurred\n",

This should be just one line.

> +                   ioc->name, __func__);
> +               return;
> +       }
Suganath Prabu S Sept. 25, 2018, 9:46 a.m. UTC | #2
Hi Andy,

I forgot to add Lukas in last patch CC list, But i have sent a note
regarding the updated patch to him.
 Also added linux-pci

On Tue, Sep 25, 2018 at 2:10 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Sep 24, 2018 at 9:36 AM Suganath Prabu S
> <suganath-prabu.subramani@broadcom.com> wrote:
> >
> > * Driver uses "pci_device_is_present" to check whether
> > If Hot unplugged:
> > the outstanding IOs with 'DID_NO_CONNECT' before removing the drives
> > attached to the HBA.
> > "DID_NO_CONNECT" status and free the smid, if driver detects that
> > HBA is hot unplugged.
> >
> > * In the hard reset flush out all the outstanding IOs even if diag reset
> > fails and also if driver detects that HBA is hot unplugged.
>
> > +       if (!mpt3sas_base_pci_device_is_available(ioc)) {
>
> > +               pr_err(MPT3SAS_FMT
> > +                   "%s: pci error recovery reset or"
> > +                   " pci device unplug occurred\n",
>
> This should be just one line.
Above line crosses over 80 characters, so it is in two lines.
>
> > +                   ioc->name, __func__);
> > +               return;
> > +       }
>
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko Sept. 25, 2018, 9:51 a.m. UTC | #3
On Tue, Sep 25, 2018 at 12:46 PM Suganath Prabu Subramani
<suganath-prabu.subramani@broadcom.com> wrote:

> > > +               pr_err(MPT3SAS_FMT
> > > +                   "%s: pci error recovery reset or"
> > > +                   " pci device unplug occurred\n",
> >
> > This should be just one line.
> Above line crosses over 80 characters, so it is in two lines.

For years there is no such requirement for string literals.
Please, don't split string literals in ugly way like this.
Suganath Prabu S Sept. 25, 2018, 10:29 a.m. UTC | #4
On Tue, Sep 25, 2018 at 3:22 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Sep 25, 2018 at 12:46 PM Suganath Prabu Subramani
> <suganath-prabu.subramani@broadcom.com> wrote:
>
> > > > +               pr_err(MPT3SAS_FMT
> > > > +                   "%s: pci error recovery reset or"
> > > > +                   " pci device unplug occurred\n",
> > >
> > > This should be just one line.
> > Above line crosses over 80 characters, so it is in two lines.
>
> For years there is no such requirement for string literals.
Below is the one i was mentioning,
WARNING: line over 80 characters
#30: FILE: drivers/scsi/mpt3sas/mpt3sas_base.c:6861:
+ pr_err(MPT3SAS_FMT "%s: pci error recovery reset or pci device
unplug occurred\n", ioc->name, __func__);

Warning comes for both characters over 80 chars and string split.
Do you want us to ignore warning and repost like above in single line ?
> Please, don't split string literals in ugly way like this.
>
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko Sept. 25, 2018, 1:34 p.m. UTC | #5
On Tue, Sep 25, 2018 at 1:29 PM Suganath Prabu Subramani
<suganath-prabu.subramani@broadcom.com> wrote:
>
> On Tue, Sep 25, 2018 at 3:22 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Tue, Sep 25, 2018 at 12:46 PM Suganath Prabu Subramani
> > <suganath-prabu.subramani@broadcom.com> wrote:
> >
> > > > > +               pr_err(MPT3SAS_FMT
> > > > > +                   "%s: pci error recovery reset or"
> > > > > +                   " pci device unplug occurred\n",
> > > >
> > > > This should be just one line.
> > > Above line crosses over 80 characters, so it is in two lines.
> >
> > For years there is no such requirement for string literals.
> Below is the one i was mentioning,
> WARNING: line over 80 characters
> #30: FILE: drivers/scsi/mpt3sas/mpt3sas_base.c:6861:
> + pr_err(MPT3SAS_FMT "%s: pci error recovery reset or pci device
> unplug occurred\n", ioc->name, __func__);
>
> Warning comes for both characters over 80 chars and string split.
> Do you want us to ignore warning and repost like above in single line ?
> > Please, don't split string literals in ugly way like this.

I didn't tell anything about other parameters to be on the same line.
So, please, try again.
diff mbox series

Patch

==============
unlock mutex before goto "out_unlocked",
if active reset is in progress.

v2 change set:
==============
1) Use pci_device_is_present instead of
mpt3sas_base_pci_device_is_unplugged.
2) As suggested by Lukas, removed using
watchdog thread for checking hba hot unplug(Patch 02 of V1).
Added Hot unplug checks in scan finish and reset paths.

v3 Change Set:
=============
Simplified function "mpt3sas_base_pci_device_is_available" and
made inline.

Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c  | 41 +++++++++++++++++++++++++++++
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  3 ++-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 50 ++++++++++++++++++++++++++++++++----
 3 files changed, 88 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 59d7844..c98d8e2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -543,6 +543,20 @@  static int mpt3sas_remove_dead_ioc_func(void *arg)
 }
 
 /**
+ * mpt3sas_base_pci_device_is_available - check whether pci device is
+ *			available for any transactions with FW
+ *
+ * @ioc: per adapter object
+ *
+ * Return 1 if pci device state is up and running else return 0.
+ */
+inline bool
+mpt3sas_base_pci_device_is_available(struct MPT3SAS_ADAPTER *ioc)
+{
+	return !ioc->pci_error_recovery && pci_device_is_present(ioc->pdev);
+}
+
+/**
  * _base_fault_reset_work - workq handling ioc fault conditions
  * @work: input argument, used to derive ioc
  *
@@ -6122,6 +6136,11 @@  _base_diag_reset(struct MPT3SAS_ADAPTER *ioc)
 
 	count = 0;
 	do {
+		if (!pci_device_is_present(ioc->pdev)) {
+			ioc->remove_host = 1;
+			pr_err(MPT3SAS_FMT "Hba Hot unplugged\n", ioc->name);
+			goto out;
+		}
 		/* Write magic sequence to WriteSequence register
 		 * Loop until in diagnostic mode
 		 */
@@ -6853,6 +6872,14 @@  mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
 
 	ioc->pending_io_count = 0;
 
+	if (!mpt3sas_base_pci_device_is_available(ioc)) {
+		pr_err(MPT3SAS_FMT
+		    "%s: pci error recovery reset or"
+		    " pci device unplug occurred\n",
+		    ioc->name, __func__);
+		return;
+	}
+
 	ioc_state = mpt3sas_base_get_iocstate(ioc, 0);
 	if ((ioc_state & MPI2_IOC_STATE_MASK) != MPI2_IOC_STATE_OPERATIONAL)
 		return;
@@ -6899,6 +6926,20 @@  mpt3sas_base_hard_reset_handler(struct MPT3SAS_ADAPTER *ioc,
 	/* wait for an active reset in progress to complete */
 	mutex_lock(&ioc->reset_in_progress_mutex);
 
+	if (!mpt3sas_base_pci_device_is_available(ioc)) {
+		pr_err(MPT3SAS_FMT
+		    "%s: pci error recovery reset or"
+		    " pci device unplug occurred\n",
+		    ioc->name, __func__);
+		if (!pci_device_is_present(ioc->pdev))
+			ioc->schedule_dead_ioc_flush_running_cmds(ioc);
+		r = 0;
+		mutex_unlock(&ioc->reset_in_progress_mutex);
+		goto out_unlocked;
+	}
+
+	mpt3sas_halt_firmware(ioc);
+
 	spin_lock_irqsave(&ioc->ioc_reset_in_progress_lock, flags);
 	ioc->shost_recovery = 1;
 	spin_unlock_irqrestore(&ioc->ioc_reset_in_progress_lock, flags);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 96dc15e..a802ad4 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1474,7 +1474,8 @@  void mpt3sas_base_update_missing_delay(struct MPT3SAS_ADAPTER *ioc,
 	u16 device_missing_delay, u8 io_missing_delay);
 
 int mpt3sas_port_enable(struct MPT3SAS_ADAPTER *ioc);
-
+inline bool  mpt3sas_base_pci_device_is_available(
+	struct MPT3SAS_ADAPTER *ioc);
 void
 mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc);
 
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 53133cf..566a550 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2846,9 +2846,19 @@  scsih_abort(struct scsi_cmnd *scmd)
 		"attempting task abort! scmd(%p)\n", scmd);
 	_scsih_tm_display_info(ioc, scmd);
 
+	if (!pci_device_is_present(ioc->pdev) || ioc->remove_host) {
+		sdev_printk(KERN_INFO, scmd->device, "%s scmd(%p)\n",
+		    ((ioc->remove_host) ? ("shost is getting removed!") :
+		    ("pci device been removed!")), scmd);
+		if (st && st->smid)
+			mpt3sas_base_free_smid(ioc, st->smid);
+		scmd->result = DID_NO_CONNECT << 16;
+		r = FAST_IO_FAIL;
+		goto out;
+	}
+
 	sas_device_priv_data = scmd->device->hostdata;
-	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
-	    ioc->remove_host) {
+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
 		sdev_printk(KERN_INFO, scmd->device,
 			"device been deleted! scmd(%p)\n", scmd);
 		scmd->result = DID_NO_CONNECT << 16;
@@ -2918,6 +2928,15 @@  scsih_dev_reset(struct scsi_cmnd *scmd)
 		"attempting device reset! scmd(%p)\n", scmd);
 	_scsih_tm_display_info(ioc, scmd);
 
+	if (!pci_device_is_present(ioc->pdev) || ioc->remove_host) {
+		sdev_printk(KERN_INFO, scmd->device, "%s scmd(%p)\n",
+		    ((ioc->remove_host) ? ("shost is getting removed!") :
+		    ("pci device been removed!")), scmd);
+		scmd->result = DID_NO_CONNECT << 16;
+		r = FAST_IO_FAIL;
+		goto out;
+	}
+
 	sas_device_priv_data = scmd->device->hostdata;
 	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
 	    ioc->remove_host) {
@@ -2995,9 +3014,17 @@  scsih_target_reset(struct scsi_cmnd *scmd)
 		scmd);
 	_scsih_tm_display_info(ioc, scmd);
 
+	if ((!pci_device_is_present(ioc->pdev)) || ioc->remove_host) {
+		sdev_printk(KERN_INFO, scmd->device, "%s scmd(%p)\n",
+		    ((ioc->remove_host) ? ("shost is getting removed!") :
+		    ("pci device been removed!")), scmd);
+		scmd->result = DID_NO_CONNECT << 16;
+		r = FAST_IO_FAIL;
+		goto out;
+	}
+
 	sas_device_priv_data = scmd->device->hostdata;
-	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
-	    ioc->remove_host) {
+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
 		starget_printk(KERN_INFO, starget, "target been deleted! scmd(%p)\n",
 			scmd);
 		scmd->result = DID_NO_CONNECT << 16;
@@ -4474,7 +4501,9 @@  _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc)
 		st = scsi_cmd_priv(scmd);
 		mpt3sas_base_clear_st(ioc, st);
 		scsi_dma_unmap(scmd);
-		if (ioc->pci_error_recovery || ioc->remove_host)
+
+		if ((!mpt3sas_base_pci_device_is_available(ioc))
+				|| ioc->remove_host)
 			scmd->result = DID_NO_CONNECT << 16;
 		else
 			scmd->result = DID_RESET << 16;
@@ -9726,6 +9755,9 @@  _scsih_ir_shutdown(struct MPT3SAS_ADAPTER *ioc)
 	if (list_empty(&ioc->raid_device_list))
 		return;
 
+	if (!pci_device_is_present(ioc->pdev))
+		return;
+
 	mutex_lock(&ioc->scsih_cmds.mutex);
 
 	if (ioc->scsih_cmds.status != MPT3_CMD_NOT_USED) {
@@ -10247,6 +10279,14 @@  scsih_scan_finished(struct Scsi_Host *shost, unsigned long time)
 {
 	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
 
+	if (!pci_device_is_present(ioc->pdev)) {
+		complete(&ioc->port_enable_cmds.done);
+		ioc->port_enable_cmds.status = MPT3_CMD_NOT_USED;
+		ioc->is_driver_loading = 0;
+		ioc->remove_host = 1;
+		return 1;
+	}
+
 	if (disable_discovery > 0) {
 		ioc->is_driver_loading = 0;
 		ioc->wait_for_discovery_to_complete = 0;