From: Uma Krishnan <hidden> Date: 2016-09-02 20:36:52
This patch set contains miscellaneous fixes in the device cleanup and
EEH recovery paths. One of the patches simplifies the WWPN assignment
routine.
There are a couple of fixes that resolve regressions introduced by
Commit 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards") and Commit 888baf069f49 ("scsi: cxlflash: Add kref to context")
This series is based upon mkp's 4.9/scsi-queue and is bisectable.
Matthew R. Ochs (4):
cxlflash: Fix to avoid EEH and host reset collisions
cxlflash: Improve EEH recovery time
cxlflash: Refactor WWPN setup
cxlflash: Fix context reference tracking on detach
Uma Krishnan (2):
cxlflash: Scan host only after the port is ready for I/O
cxlflash: Remove the device cleanly in the system shutdown path
drivers/scsi/cxlflash/main.c | 81 +++++++++++++++++----------------------
drivers/scsi/cxlflash/superpipe.c | 3 +-
2 files changed, 37 insertions(+), 47 deletions(-)
--
2.1.0
From: Uma Krishnan <hidden> Date: 2016-09-02 20:39:24
When a port link is established, the AFU sends a 'link up' interrupt.
After the link is up, corresponding initialization steps are performed
on the card. Following that, when the card is ready for I/O, the AFU
sends 'login succeeded' interrupt. Today, cxlflash invokes
scsi_scan_host() upon receipt of both interrupts.
SCSI commands sent to the port prior to the 'login succeeded' interrupt
will fail with 'port not available' error. This is not desirable.
Moreover, when async_scan is active for the host, subsequent scan calls
are terminated with error. Due to this, the scsi_scan_host() call
performed after 'login succeeded' interrupt could portentially return
error and the devices may not be scanned properly.
To avoid this problem, scsi_scan_host() should be called only after the
'login succeeded' interrupt.
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Uma Krishnan <hidden> Date: 2016-09-02 20:39:32
Commit 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards") was recently introduced to notify the AFU when a system is going
down. Due to the position of the cxlflash driver in the device stack,
cxlflash devices are _always_ removed during a reboot/shutdown. This can
lead to a crash if the cxlflash shutdown hook is invoked _after_ the
shutdown hook for the owning virtual PHB. Furthermore, the current
implementation of shutdown/remove hooks for cxlflash are not tolerant to
being invoked when the device is not enabled. This can also lead to a
crash in situations where the remove hook is invoked after the device has
been removed via the vPHBs shutdown hook. An example of this scenario
would be an EEH reset failure while a reboot/shutdown is in progress.
To solve both problems, the shutdown hook for cxlflash is updated to
simply remove the device. This path already includes the AFU notification
and thus this solution will continue to perform the original intent. At
the same time, the remove hook is updated to protect against being
called when the device is not enabled.
Fixes: 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards")
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/main.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
@@ -844,6 +833,11 @@ static void cxlflash_remove(struct pci_dev *pdev)structcxlflash_cfg*cfg=pci_get_drvdata(pdev);ulonglock_flags;+if(!pci_is_enabled(pdev)){+pr_debug("%s: Device is disabled\n",__func__);+return;+}+/* If a Task Management Function is active, wait for it to complete*beforecontinuingwithremove.*/
From: Uma Krishnan <hidden> Date: 2016-09-02 20:39:44
From: "Matthew R. Ochs" <redacted>
The EEH reset handler is ignorant to the current state of the
driver when processing a frozen event and initiating a device
reset. This can be an issue if an EEH event occurs while a user
or stack initiated reset is executing. More specifically, if an
EEH occurs while the SCSI host reset handler is active, the reset
initiated by the EEH thread will likely collide with the host reset
thread. This can leave the device in an inconsistent state, or worse,
cause a system crash.
As a remedy, the EEH handler is updated to evaluate the device state
and take appropriate action (proceed, wait, or disconnect host). The
host reset handler is also updated to handle situations where an EEH
occurred during a host reset. In such situations, the host reset handler
will delay reporting back a success to give the EEH reset an opportunity
to complete.
Signed-off-by: Matthew R. Ochs <redacted>
---
drivers/scsi/cxlflash/main.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
@@ -2074,7 +2079,8 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)}elsecfg->state=STATE_NORMAL;wake_up_all(&cfg->reset_waitq);-break;+ssleep(1);+/* fall through */caseSTATE_RESET:wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);if(cfg->state==STATE_NORMAL)
From: Uma Krishnan <hidden> Date: 2016-09-02 20:40:19
From: "Matthew R. Ochs" <redacted>
When an EEH occurs during device initialization, the port timeout
logic can cause excessive delays as MMIO reads will fail. Depending
on where they are experienced, these delays can lead to a prolonged
reset, causing an unnecessary triggering of other timeout logic in
the SCSI stack or user applications.
To expedite recovery, the port timeout logic is updated to decay the
timeout at a much faster rate when in the presence of a likely EEH
frozen event.
Signed-off-by: Matthew R. Ochs <redacted>
---
drivers/scsi/cxlflash/main.c | 4 ++++
1 file changed, 4 insertions(+)
From: Uma Krishnan <hidden> Date: 2016-09-02 20:40:37
From: "Matthew R. Ochs" <redacted>
Commit 964497b3bf3f ("cxlflash: Remove dual port online dependency")
logically removed the ability for the WWPN setup routine afu_set_wwpn()
to return a non-success value. This routine can safely be made a void
to simplify the code as there is no longer a need to report a failure.
Signed-off-by: Matthew R. Ochs <redacted>
---
drivers/scsi/cxlflash/main.c | 40 +++++++++-------------------------------
1 file changed, 9 insertions(+), 31 deletions(-)
@@ -1093,42 +1093,25 @@ static int wait_port_offline(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)*online.Thistogglingactioncancausethisroutinetodelayuptoafew*seconds.WhenconfiguredtousetheinternalLUNfeatureoftheAFU,a*failuretocomeonlineisoverridden.-*-*Return:-*0whentheWWPNissuccessfullywrittenandtheportcomesbackonline-*-1whentheportfailstogoofflineorcomebackuponline*/-staticintafu_set_wwpn(structafu*afu,intport,__be64__iomem*fc_regs,-u64wwpn)+staticvoidafu_set_wwpn(structafu*afu,intport,__be64__iomem*fc_regs,+u64wwpn){-intrc=0;-set_port_offline(fc_regs);-if(!wait_port_offline(fc_regs,FC_PORT_STATUS_RETRY_INTERVAL_US,FC_PORT_STATUS_RETRY_CNT)){pr_debug("%s: wait on port %d to go offline timed out\n",__func__,port);-rc=-1;/* but continue on to leave the port back online */}-if(rc==0)-writeq_be(wwpn,&fc_regs[FC_PNAME/8]);--/* Always return success after programming WWPN */-rc=0;+writeq_be(wwpn,&fc_regs[FC_PNAME/8]);set_port_online(fc_regs);-if(!wait_port_online(fc_regs,FC_PORT_STATUS_RETRY_INTERVAL_US,FC_PORT_STATUS_RETRY_CNT)){-pr_err("%s: wait on port %d to go online timed out\n",-__func__,port);+pr_debug("%s: wait on port %d to go online timed out\n",+__func__,port);}--pr_debug("%s: returning rc=%d\n",__func__,rc);--returnrc;}/**
@@ -1629,15 +1612,10 @@ static int init_global(struct cxlflash_cfg *cfg)[FC_CRC_THRESH/8]);/* Set WWPNs. If already programmed, wwpn[i] is 0 */-if(wwpn[i]!=0&&-afu_set_wwpn(afu,i,-&afu->afu_map->global.fc_regs[i][0],-wwpn[i])){-dev_err(dev,"%s: failed to set WWPN on port %d\n",-__func__,i);-rc=-EIO;-gotoout;-}+if(wwpn[i]!=0)+afu_set_wwpn(afu,i,+&afu->afu_map->global.fc_regs[i][0],+wwpn[i]);/* Programming WWPN back to back causes additional*offline/onlinetransitionsandaPLOGI*/
From: Uma Krishnan <hidden> Date: 2016-09-02 20:40:50
From: "Matthew R. Ochs" <redacted>
Commit 888baf069f49 ("scsi: cxlflash: Add kref to context") introduced a
kref to the context. In particular, the detach routine was updated to use
the kref services for managing the removal and destruction of a context.
As part of this change, the tracking mechanism internal to the detach
handler was refactored. This introduced a bug that can cause the tracking
state to be lost. This can lead to a situation where exclusive access to a
context is prematurely [and unknowingly] relinquished for the executing
thread.
To remedy, only update the tracking state when the kref operation indicates
the context was removed.
Fixes: 888baf069f49 ("scsi: cxlflash: Add kref to context")
Signed-off-by: Matthew R. Ochs <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Andrew Donnellan <hidden> Date: 2016-09-05 07:13:04
On 03/09/16 06:39, Uma Krishnan wrote:
quoted hunk
Commit 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards") was recently introduced to notify the AFU when a system is going
down. Due to the position of the cxlflash driver in the device stack,
cxlflash devices are _always_ removed during a reboot/shutdown. This can
lead to a crash if the cxlflash shutdown hook is invoked _after_ the
shutdown hook for the owning virtual PHB. Furthermore, the current
implementation of shutdown/remove hooks for cxlflash are not tolerant to
being invoked when the device is not enabled. This can also lead to a
crash in situations where the remove hook is invoked after the device has
been removed via the vPHBs shutdown hook. An example of this scenario
would be an EEH reset failure while a reboot/shutdown is in progress.
To solve both problems, the shutdown hook for cxlflash is updated to
simply remove the device. This path already includes the AFU notification
and thus this solution will continue to perform the original intent. At
the same time, the remove hook is updated to protect against being
called when the device is not enabled.
Fixes: 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards")
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/main.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
You can get rid of the second parameter to notify_shutdown() now.
quoted hunk
-}
-
-/**
* cxlflash_remove() - PCI entry point to tear down host
* @pdev: PCI device associated with the host.
*
@@ -844,6 +833,11 @@ static void cxlflash_remove(struct pci_dev *pdev) struct cxlflash_cfg *cfg = pci_get_drvdata(pdev); ulong lock_flags;+ if (!pci_is_enabled(pdev)) {+ pr_debug("%s: Device is disabled\n", __func__);+ return;+ }+ /* If a Task Management Function is active, wait for it to complete * before continuing with remove. */
What's the justification for using cxlflash_remove() as the shutdown
hook, rather than just not having a shutdown hook at all?
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Uma Krishnan <hidden> Date: 2016-09-06 20:06:17
On 9/5/2016 2:12 AM, Andrew Donnellan wrote:
On 03/09/16 06:39, Uma Krishnan wrote:
quoted
Commit 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards") was recently introduced to notify the AFU when a system is going
down. Due to the position of the cxlflash driver in the device stack,
cxlflash devices are _always_ removed during a reboot/shutdown. This can
lead to a crash if the cxlflash shutdown hook is invoked _after_ the
shutdown hook for the owning virtual PHB. Furthermore, the current
implementation of shutdown/remove hooks for cxlflash are not tolerant to
being invoked when the device is not enabled. This can also lead to a
crash in situations where the remove hook is invoked after the device has
been removed via the vPHBs shutdown hook. An example of this scenario
would be an EEH reset failure while a reboot/shutdown is in progress.
To solve both problems, the shutdown hook for cxlflash is updated to
simply remove the device. This path already includes the AFU notification
and thus this solution will continue to perform the original intent. At
the same time, the remove hook is updated to protect against being
called when the device is not enabled.
Fixes: 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards")
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/main.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
You can get rid of the second parameter to notify_shutdown() now.
Valid comment. This was considered while working the patch. I left it
for now, in case we need this flag in future. If we end up not having
to use it, I will clean it up in a future patch.
quoted
-}
-
-/**
* cxlflash_remove() - PCI entry point to tear down host
* @pdev: PCI device associated with the host.
*
@@ -844,6 +833,11 @@ static void cxlflash_remove(struct pci_dev *pdev) struct cxlflash_cfg *cfg = pci_get_drvdata(pdev); ulong lock_flags;+ if (!pci_is_enabled(pdev)) {+ pr_debug("%s: Device is disabled\n", __func__);+ return;+ }+ /* If a Task Management Function is active, wait for it to complete * before continuing with remove. */
What's the justification for using cxlflash_remove() as the shutdown
hook, rather than just not having a shutdown hook at all?
Even though cxlflash gets cleaned up via cxl_remove() shutdown
hook, today it is a dependency on the code external to our driver.
To protect us from future API changes that could possibly impact
cxlflash, we want to maintain a clean and reliable way to get
called in the shutdown path.
From: Matthew R. Ochs <hidden> Date: 2016-09-07 23:46:21
On Sep 2, 2016, at 3:38 PM, Uma Krishnan [off-list ref] =
wrote:
=20
When a port link is established, the AFU sends a 'link up' interrupt.
After the link is up, corresponding initialization steps are performed
on the card. Following that, when the card is ready for I/O, the AFU
sends 'login succeeded' interrupt. Today, cxlflash invokes
scsi_scan_host() upon receipt of both interrupts.
=20
SCSI commands sent to the port prior to the 'login succeeded' =
interrupt
will fail with 'port not available' error. This is not desirable.
Moreover, when async_scan is active for the host, subsequent scan =
calls
are terminated with error. Due to this, the scsi_scan_host() call
performed after 'login succeeded' interrupt could portentially return
error and the devices may not be scanned properly.
=20
To avoid this problem, scsi_scan_host() should be called only after =
the
'login succeeded' interrupt.
=20
Signed-off-by: Uma Krishna <redacted>
From: Matthew R. Ochs <hidden> Date: 2016-09-07 23:46:45
On Sep 2, 2016, at 3:39 PM, Uma Krishnan [off-list ref] =
wrote:
=20
Commit 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards") was recently introduced to notify the AFU when a system is =
going
down. Due to the position of the cxlflash driver in the device stack,
cxlflash devices are _always_ removed during a reboot/shutdown. This =
can
lead to a crash if the cxlflash shutdown hook is invoked _after_ the
shutdown hook for the owning virtual PHB. Furthermore, the current
implementation of shutdown/remove hooks for cxlflash are not tolerant =
to
being invoked when the device is not enabled. This can also lead to a
crash in situations where the remove hook is invoked after the device =
has
been removed via the vPHBs shutdown hook. An example of this scenario
would be an EEH reset failure while a reboot/shutdown is in progress.
=20
To solve both problems, the shutdown hook for cxlflash is updated to
simply remove the device. This path already includes the AFU =
notification
and thus this solution will continue to perform the original intent. =
At
the same time, the remove hook is updated to protect against being
called when the device is not enabled.
=20
Fixes: 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
cards")
Signed-off-by: Uma Krishna <redacted>
From: Uma Krishnan <hidden> Date: 2016-09-09 22:13:32
On 9/2/2016 3:39 PM, Uma Krishnan wrote:
From: "Matthew R. Ochs" <redacted>
The EEH reset handler is ignorant to the current state of the
driver when processing a frozen event and initiating a device
reset. This can be an issue if an EEH event occurs while a user
or stack initiated reset is executing. More specifically, if an
EEH occurs while the SCSI host reset handler is active, the reset
initiated by the EEH thread will likely collide with the host reset
thread. This can leave the device in an inconsistent state, or worse,
cause a system crash.
As a remedy, the EEH handler is updated to evaluate the device state
and take appropriate action (proceed, wait, or disconnect host). The
host reset handler is also updated to handle situations where an EEH
occurred during a host reset. In such situations, the host reset handler
will delay reporting back a success to give the EEH reset an opportunity
to complete.
Signed-off-by: Matthew R. Ochs <redacted>
From: Uma Krishnan <hidden> Date: 2016-09-09 22:14:09
On 9/2/2016 3:40 PM, Uma Krishnan wrote:
From: "Matthew R. Ochs" <redacted>
When an EEH occurs during device initialization, the port timeout
logic can cause excessive delays as MMIO reads will fail. Depending
on where they are experienced, these delays can lead to a prolonged
reset, causing an unnecessary triggering of other timeout logic in
the SCSI stack or user applications.
To expedite recovery, the port timeout logic is updated to decay the
timeout at a much faster rate when in the presence of a likely EEH
frozen event.
Signed-off-by: Matthew R. Ochs <redacted>
From: Uma Krishnan <hidden> Date: 2016-09-09 22:14:36
On 9/2/2016 3:40 PM, Uma Krishnan wrote:
From: "Matthew R. Ochs" <redacted>
Commit 964497b3bf3f ("cxlflash: Remove dual port online dependency")
logically removed the ability for the WWPN setup routine afu_set_wwpn()
to return a non-success value. This routine can safely be made a void
to simplify the code as there is no longer a need to report a failure.
Signed-off-by: Matthew R. Ochs <redacted>
From: Uma Krishnan <hidden> Date: 2016-09-09 22:14:57
On 9/2/2016 3:40 PM, Uma Krishnan wrote:
From: "Matthew R. Ochs" <redacted>
Commit 888baf069f49 ("scsi: cxlflash: Add kref to context") introduced a
kref to the context. In particular, the detach routine was updated to use
the kref services for managing the removal and destruction of a context.
As part of this change, the tracking mechanism internal to the detach
handler was refactored. This introduced a bug that can cause the tracking
state to be lost. This can lead to a situation where exclusive access to a
context is prematurely [and unknowingly] relinquished for the executing
thread.
To remedy, only update the tracking state when the kref operation indicates
the context was removed.
Fixes: 888baf069f49 ("scsi: cxlflash: Add kref to context")
Signed-off-by: Matthew R. Ochs <redacted>