[PATCH 0/6] cxlflash: Miscellaneous fixes

[PATCH 0/6] cxlflash: Miscellaneous fixes

[Uma Krishnan]

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

[PATCH 1/6] cxlflash: Scan host only after the port is ready for I/O

[Uma Krishnan]

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 <ukrishn@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 661bb94..b063c41 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1187,7 +1187,7 @@ static const struct asyc_intr_info ainfo[] = {
 	{SISL_ASTATUS_FC0_LOGI_F, "login failed", 0, CLR_FC_ERROR},
 	{SISL_ASTATUS_FC0_LOGI_S, "login succeeded", 0, SCAN_HOST},
 	{SISL_ASTATUS_FC0_LINK_DN, "link down", 0, 0},
-	{SISL_ASTATUS_FC0_LINK_UP, "link up", 0, SCAN_HOST},
+	{SISL_ASTATUS_FC0_LINK_UP, "link up", 0, 0},
 	{SISL_ASTATUS_FC1_OTHER, "other error", 1, CLR_FC_ERROR | LINK_RESET},
 	{SISL_ASTATUS_FC1_LOGO, "target initiated LOGO", 1, 0},
 	{SISL_ASTATUS_FC1_CRC_T, "CRC threshold exceeded", 1, LINK_RESET},
@@ -1195,7 +1195,7 @@ static const struct asyc_intr_info ainfo[] = {
 	{SISL_ASTATUS_FC1_LOGI_F, "login failed", 1, CLR_FC_ERROR},
 	{SISL_ASTATUS_FC1_LOGI_S, "login succeeded", 1, SCAN_HOST},
 	{SISL_ASTATUS_FC1_LINK_DN, "link down", 1, 0},
-	{SISL_ASTATUS_FC1_LINK_UP, "link up", 1, SCAN_HOST},
+	{SISL_ASTATUS_FC1_LINK_UP, "link up", 1, 0},
 	{0x0, "", 0, 0}		/* terminator */
 };
 
-- 
2.1.0

Re: [PATCH 1/6] cxlflash: Scan host only after the port is ready for I/O

[Matthew R. Ochs]

> On Sep 2, 2016, at 3:38 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> wrote:
> 
> 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 Krishna <ukrishn@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

[PATCH 2/6] cxlflash: Remove the device cleanly in the system shutdown path

[Uma Krishnan]

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 <ukrishn@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index b063c41..4c2559a 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -823,17 +823,6 @@ static void notify_shutdown(struct cxlflash_cfg *cfg, bool wait)
 }
 
 /**
- * cxlflash_shutdown() - shutdown handler
- * @pdev:	PCI device associated with the host.
- */
-static void cxlflash_shutdown(struct pci_dev *pdev)
-{
-	struct cxlflash_cfg *cfg = pci_get_drvdata(pdev);
-
-	notify_shutdown(cfg, false);
-}
-
-/**
  * 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.
 	 */
@@ -2685,7 +2679,7 @@ static struct pci_driver cxlflash_driver = {
 	.id_table = cxlflash_pci_table,
 	.probe = cxlflash_probe,
 	.remove = cxlflash_remove,
-	.shutdown = cxlflash_shutdown,
+	.shutdown = cxlflash_remove,
 	.err_handler = &cxlflash_err_handler,
 };
 
-- 
2.1.0

Re: [PATCH 2/6] cxlflash: Remove the device cleanly in the system shutdown path

[Andrew Donnellan]

On 03/09/16 06:39, Uma Krishnan wrote:
> 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 <ukrishn@linux.vnet.ibm.com>
> ---
>  drivers/scsi/cxlflash/main.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index b063c41..4c2559a 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -823,17 +823,6 @@ static void notify_shutdown(struct cxlflash_cfg *cfg, bool wait)
>  }
>
>  /**
> - * cxlflash_shutdown() - shutdown handler
> - * @pdev:	PCI device associated with the host.
> - */
> -static void cxlflash_shutdown(struct pci_dev *pdev)
> -{
> -	struct cxlflash_cfg *cfg = pci_get_drvdata(pdev);
> -
> -	notify_shutdown(cfg, false);

You can get rid of the second parameter to notify_shutdown() now.

> -}
> -
> -/**
>   * 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.
>  	 */
> @@ -2685,7 +2679,7 @@ static struct pci_driver cxlflash_driver = {
>  	.id_table = cxlflash_pci_table,
>  	.probe = cxlflash_probe,
>  	.remove = cxlflash_remove,
> -	.shutdown = cxlflash_shutdown,
> +	.shutdown = cxlflash_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

Re: [PATCH 2/6] cxlflash: Remove the device cleanly in the system shutdown path

[Uma Krishnan]

On 9/5/2016 2:12 AM, Andrew Donnellan wrote:
> On 03/09/16 06:39, Uma Krishnan wrote:
>> 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 <ukrishn@linux.vnet.ibm.com>
>> ---
>>  drivers/scsi/cxlflash/main.c | 18 ++++++------------
>>  1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
>> index b063c41..4c2559a 100644
>> --- a/drivers/scsi/cxlflash/main.c
>> +++ b/drivers/scsi/cxlflash/main.c
>> @@ -823,17 +823,6 @@ static void notify_shutdown(struct cxlflash_cfg
>> *cfg, bool wait)
>>  }
>>
>>  /**
>> - * cxlflash_shutdown() - shutdown handler
>> - * @pdev:    PCI device associated with the host.
>> - */
>> -static void cxlflash_shutdown(struct pci_dev *pdev)
>> -{
>> -    struct cxlflash_cfg *cfg = pci_get_drvdata(pdev);
>> -
>> -    notify_shutdown(cfg, false);
>
> 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.

>> -}
>> -
>> -/**
>>   * 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.
>>       */
>> @@ -2685,7 +2679,7 @@ static struct pci_driver cxlflash_driver = {
>>      .id_table = cxlflash_pci_table,
>>      .probe = cxlflash_probe,
>>      .remove = cxlflash_remove,
>> -    .shutdown = cxlflash_shutdown,
>> +    .shutdown = cxlflash_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.

Re: [PATCH 2/6] cxlflash: Remove the device cleanly in the system shutdown path

[Matthew R. Ochs]

> On Sep 2, 2016, at 3:39 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> wrote:
> 
> 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 Krishna <ukrishn@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

[PATCH 3/6] cxlflash: Fix to avoid EEH and host reset collisions

[Uma Krishnan]

From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>

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 <mrochs@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 4c2559a..4ef5235 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2042,6 +2042,11 @@ retry:
  * cxlflash_eh_host_reset_handler() - reset the host adapter
  * @scp:	SCSI command from stack identifying host.
  *
+ * Following a reset, the state is evaluated again in case an EEH occurred
+ * during the reset. In such a scenario, the host reset will either yield
+ * until the EEH recovery is complete or return success or failure based
+ * upon the current device state.
+ *
  * Return:
  *	SUCCESS as defined in scsi/scsi.h
  *	FAILED as defined in scsi/scsi.h
@@ -2074,7 +2079,8 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)
 		} else
 			cfg->state = STATE_NORMAL;
 		wake_up_all(&cfg->reset_waitq);
-		break;
+		ssleep(1);
+		/* fall through */
 	case STATE_RESET:
 		wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
 		if (cfg->state == STATE_NORMAL)
@@ -2590,6 +2596,9 @@ out_remove:
  * @pdev:	PCI device struct.
  * @state:	PCI channel state.
  *
+ * When an EEH occurs during an active reset, wait until the reset is
+ * complete and then take action based upon the device state.
+ *
  * Return: PCI_ERS_RESULT_NEED_RESET or PCI_ERS_RESULT_DISCONNECT
  */
 static pci_ers_result_t cxlflash_pci_error_detected(struct pci_dev *pdev,
@@ -2603,6 +2612,10 @@ static pci_ers_result_t cxlflash_pci_error_detected(struct pci_dev *pdev,
 
 	switch (state) {
 	case pci_channel_io_frozen:
+		wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
+		if (cfg->state == STATE_FAILTERM)
+			return PCI_ERS_RESULT_DISCONNECT;
+
 		cfg->state = STATE_RESET;
 		scsi_block_requests(cfg->host);
 		drain_ioctls(cfg);
-- 
2.1.0

Re: [PATCH 3/6] cxlflash: Fix to avoid EEH and host reset collisions

[Uma Krishnan]

On 9/2/2016 3:39 PM, Uma Krishnan wrote:
> From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>
>
> 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 <mrochs@linux.vnet.ibm.com>

Acked-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

Re: [PATCH 3/6] cxlflash: Fix to avoid EEH and host reset collisions

[Martin K. Petersen]

>>>>> "Uma" == Uma Krishnan <ukrishn@linux.vnet.ibm.com> writes:

Applied patches 3 through 6 to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 4/6] cxlflash: Improve EEH recovery time

[Uma Krishnan]

From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>

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 <mrochs@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 4ef5235..42970a4 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1040,6 +1040,8 @@ static int wait_port_online(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)
 	do {
 		msleep(delay_us / 1000);
 		status = readq_be(&fc_regs[FC_MTIP_STATUS / 8]);
+		if (status == U64_MAX)
+			nretry /= 2;
 	} while ((status & FC_MTIP_STATUS_MASK) != FC_MTIP_STATUS_ONLINE &&
 		 nretry--);
 
@@ -1071,6 +1073,8 @@ static int wait_port_offline(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)
 	do {
 		msleep(delay_us / 1000);
 		status = readq_be(&fc_regs[FC_MTIP_STATUS / 8]);
+		if (status == U64_MAX)
+			nretry /= 2;
 	} while ((status & FC_MTIP_STATUS_MASK) != FC_MTIP_STATUS_OFFLINE &&
 		 nretry--);
 
-- 
2.1.0

Re: [PATCH 4/6] cxlflash: Improve EEH recovery time

[Uma Krishnan]

On 9/2/2016 3:40 PM, Uma Krishnan wrote:
> From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>
>
> 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 <mrochs@linux.vnet.ibm.com>

Acked-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

[PATCH 5/6] cxlflash: Refactor WWPN setup

[Uma Krishnan]

From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>

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 <mrochs@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 40 +++++++++-------------------------------
 1 file changed, 9 insertions(+), 31 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 42970a4..b301655 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1093,42 +1093,25 @@ static int wait_port_offline(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)
  * online. This toggling action can cause this routine to delay up to a few
  * seconds. When configured to use the internal LUN feature of the AFU, a
  * failure to come online is overridden.
- *
- * Return:
- *	0 when the WWPN is successfully written and the port comes back online
- *	-1 when the port fails to go offline or come back up online
  */
-static int afu_set_wwpn(struct afu *afu, int port, __be64 __iomem *fc_regs,
-			u64 wwpn)
+static void afu_set_wwpn(struct afu *afu, int port, __be64 __iomem *fc_regs,
+			 u64 wwpn)
 {
-	int rc = 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);
-
-	return rc;
 }
 
 /**
@@ -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;
-			goto out;
-		}
+		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/online transitions and a PLOGI
 		 */
-- 
2.1.0

Re: [PATCH 5/6] cxlflash: Refactor WWPN setup

[Uma Krishnan]

On 9/2/2016 3:40 PM, Uma Krishnan wrote:
> From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>
>
> 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 <mrochs@linux.vnet.ibm.com>

Acked-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

[PATCH 6/6] cxlflash: Fix context reference tracking on detach

[Uma Krishnan]

From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>

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 <mrochs@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/superpipe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index c91fe6f..9636970 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -912,7 +912,8 @@ static int _cxlflash_disk_detach(struct scsi_device *sdev,
 	 * Release the context reference and the sdev reference that
 	 * bound this LUN to the context.
 	 */
-	put_ctx = !kref_put(&ctxi->kref, remove_context);
+	if (kref_put(&ctxi->kref, remove_context))
+		put_ctx = false;
 	scsi_device_put(sdev);
 out:
 	if (put_ctx)
-- 
2.1.0

Re: [PATCH 6/6] cxlflash: Fix context reference tracking on detach

[Uma Krishnan]

On 9/2/2016 3:40 PM, Uma Krishnan wrote:
> From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>
>
> 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 <mrochs@linux.vnet.ibm.com>

Acked-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

Re: [PATCH 0/6] cxlflash: Miscellaneous fixes

[Martin K. Petersen]

>>>>> "Uma" == Uma Krishnan <ukrishn@linux.vnet.ibm.com> writes:

Uma> This patch set contains miscellaneous fixes in the device cleanup
Uma> and EEH recovery paths. One of the patches simplifies the WWPN
Uma> assignment routine.

Uma> There are a couple of fixes that resolve regressions introduced by
Uma> Commit 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL
Uma> Flash cards") and Commit 888baf069f49 ("scsi: cxlflash: Add kref to
Uma> context")

Applied patches 1 and 2 to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering