linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/22] zfcp fixes and cleanups
@ 2017-07-28 10:30 Benjamin Block
  2017-07-28 10:30 ` [PATCH 01/22] zfcp: replace zfcp_qdio_sbale_count by sg_nents Benjamin Block
                   ` (22 more replies)
  0 siblings, 23 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Benjamin Block, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390

Hi all,

here is a series of (important) fixes and some additional cleanups for the
zfcp driver. Our fixes are marked for stable accordingly.

Patches 01 - 04 are cleanups and external patches (also cleanups) that we
      & 13 - 22     had queued for quite some time now

Patches 05 - 12 are driver fixes that are all marked for stable as well

The set applies to the fixes branch of James' scsi.git.


                                                Beste Grüße / Best regards,
                                                  - Benjamin Block

Benjamin Block (2):
  zfcp: convert bool-definitions to use 'true' instead of '1'
  zfcp: add handling for FCP_RESID_OVER to the fcp ingress path

Corentin Labbe (1):
  zfcp: Remove unneeded linux/miscdevice.h include

LABBE Corentin (1):
  zfcp: replace zfcp_qdio_sbale_count by sg_nents

Lukáš Korenčik (1):
  zfcp: use setup_timer instead of init_timer

Martin Peschke (4):
  zfcp: clean up redundant code with fall through in link down SRB
    switch case
  zfcp: clean up a member of struct zfcp_qdio that was assigned but
    never used
  zfcp: clean up unnecessary module_param_named() with
    no_auto_port_rescan
  zfcp: early returns for traces disabled via level

Steffen Maier (13):
  zfcp: fix queuecommand for scsi_eh commands when DIX enabled
  zfcp: fix capping of unsuccessful GPN_FT SAN response trace records
  zfcp: fix passing fsf_req to SCSI trace on TMF to correlate with HBA
  zfcp: fix missing trace records for early returns in TMF eh handlers
  zfcp: fix payload with full FCP_RSP IU in SCSI trace records
  zfcp: trace HBA FSF response by default on dismiss or timedout late
    response
  zfcp: trace high part of "new" 64 bit SCSI LUN
  zfcp: more fitting constant for fc_ct_hdr.ct_reason on port scan
    response
  zfcp: clarify that we don't need "link" test on failed open port
  zfcp: use common code fcp_cmnd and fcp_resp with union in
    fsf_qtcb_bottom_io
  zfcp: use endianness conversions with common FC(P) struct fields
  zfcp: fix kernel doc comment typos for struct zfcp_dbf_scsi
  zfcp: clean up no longer existent prototype from zfcp API header

 drivers/s390/scsi/zfcp_aux.c  |  1 -
 drivers/s390/scsi/zfcp_dbf.c  | 95 +++++++++++++++++++++++++++++++++++--------
 drivers/s390/scsi/zfcp_dbf.h  | 25 ++++++++----
 drivers/s390/scsi/zfcp_erp.c  |  5 +--
 drivers/s390/scsi/zfcp_ext.h  |  1 -
 drivers/s390/scsi/zfcp_fc.c   | 52 +++++++++++------------
 drivers/s390/scsi/zfcp_fc.h   | 25 ++++++++----
 drivers/s390/scsi/zfcp_fsf.c  | 35 ++++++++--------
 drivers/s390/scsi/zfcp_fsf.h  | 12 ++++--
 drivers/s390/scsi/zfcp_qdio.c |  2 +-
 drivers/s390/scsi/zfcp_qdio.h | 17 --------
 drivers/s390/scsi/zfcp_scsi.c | 18 ++++----
 12 files changed, 178 insertions(+), 110 deletions(-)

--
2.11.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 01/22] zfcp: replace zfcp_qdio_sbale_count by sg_nents
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 02/22] zfcp: use setup_timer instead of init_timer Benjamin Block
                   ` (21 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: LABBE Corentin, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390, Benjamin Block

From: LABBE Corentin <clabbe.montjoie@gmail.com>

The zfcp_qdio_sbale_count function do the same work than sg_nents().
So replace it by sg_nents() for removing duplicate code.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fsf.c  |  3 +--
 drivers/s390/scsi/zfcp_qdio.h | 15 ---------------
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 27ff38f839fc..3b69ec5e69ed 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -991,8 +991,7 @@ static int zfcp_fsf_setup_ct_els_sbals(struct zfcp_fsf_req *req,
 		qtcb->bottom.support.resp_buf_length =
 			zfcp_qdio_real_bytes(sg_resp);
 
-		zfcp_qdio_set_data_div(qdio, &req->qdio_req,
-					zfcp_qdio_sbale_count(sg_req));
+		zfcp_qdio_set_data_div(qdio, &req->qdio_req, sg_nents(sg_req));
 		zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
 		zfcp_qdio_set_scount(qdio, &req->qdio_req);
 		return 0;
diff --git a/drivers/s390/scsi/zfcp_qdio.h b/drivers/s390/scsi/zfcp_qdio.h
index 497cd379b0d1..85cdb82127e8 100644
--- a/drivers/s390/scsi/zfcp_qdio.h
+++ b/drivers/s390/scsi/zfcp_qdio.h
@@ -225,21 +225,6 @@ void zfcp_qdio_set_data_div(struct zfcp_qdio *qdio,
 }
 
 /**
- * zfcp_qdio_sbale_count - count sbale used
- * @sg: pointer to struct scatterlist
- */
-static inline
-unsigned int zfcp_qdio_sbale_count(struct scatterlist *sg)
-{
-	unsigned int count = 0;
-
-	for (; sg; sg = sg_next(sg))
-		count++;
-
-	return count;
-}
-
-/**
  * zfcp_qdio_real_bytes - count bytes used
  * @sg: pointer to struct scatterlist
  */
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 02/22] zfcp: use setup_timer instead of init_timer
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
  2017-07-28 10:30 ` [PATCH 01/22] zfcp: replace zfcp_qdio_sbale_count by sg_nents Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 03/22] zfcp: Remove unneeded linux/miscdevice.h include Benjamin Block
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Lukáš Korenčik, Steffen Maier, Martin Schwidefsky,
	Heiko Carstens, linux-scsi, linux-s390, Jiri Slaby,
	Benjamin Block

From: Lukáš Korenčik <xkorenc1@fi.muni.cz>

Use initialization with setup_timer function instead of using
init_timer function and data fields. It improves readability.

Signed-off-by: Lukáš Korenčik <xkorenc1@fi.muni.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_erp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c
index 7ccfce559034..37408f5f81ce 100644
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -572,9 +572,8 @@ static void zfcp_erp_memwait_handler(unsigned long data)
 
 static void zfcp_erp_strategy_memwait(struct zfcp_erp_action *erp_action)
 {
-	init_timer(&erp_action->timer);
-	erp_action->timer.function = zfcp_erp_memwait_handler;
-	erp_action->timer.data = (unsigned long) erp_action;
+	setup_timer(&erp_action->timer, zfcp_erp_memwait_handler,
+		    (unsigned long) erp_action);
 	erp_action->timer.expires = jiffies + HZ;
 	add_timer(&erp_action->timer);
 }
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 03/22] zfcp: Remove unneeded linux/miscdevice.h include
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
  2017-07-28 10:30 ` [PATCH 01/22] zfcp: replace zfcp_qdio_sbale_count by sg_nents Benjamin Block
  2017-07-28 10:30 ` [PATCH 02/22] zfcp: use setup_timer instead of init_timer Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 04/22] zfcp: convert bool-definitions to use 'true' instead of '1' Benjamin Block
                   ` (19 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Corentin Labbe, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390, Benjamin Block

From: Corentin Labbe <clabbe.montjoie@gmail.com>

drivers/s390/scsi/zfcp_aux.c does not contain any miscdevice so the
inclusion of linux/miscdevice.h is unnecessary.

[maier@linux.vnet.ibm.com: just for the records, this is in fact a
 minor missing code cleanup of the following older "feature"
 which also dropped the only former use of a misc device in zfcp:
 commit 663e0890e31c ("[SCSI] zfcp: remove access control tables
				    interface")
 commit b5dc3c4800cc ("[SCSI] zfcp: remove access control tables
				    interface (keep sysfs files)")
 commit 1b33ef23946a ("zfcp: remove access control tables interface
			     (port leftovers)")]

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_aux.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c
index bcc8f3dfd4c4..82ac331d9125 100644
--- a/drivers/s390/scsi/zfcp_aux.c
+++ b/drivers/s390/scsi/zfcp_aux.c
@@ -29,7 +29,6 @@
 #define KMSG_COMPONENT "zfcp"
 #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
 
-#include <linux/miscdevice.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/module.h>
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 04/22] zfcp: convert bool-definitions to use 'true' instead of '1'
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (2 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 03/22] zfcp: Remove unneeded linux/miscdevice.h include Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 05/22] zfcp: fix queuecommand for scsi_eh commands when DIX enabled Benjamin Block
                   ` (18 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Benjamin Block, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390

Better form and cleans remaining warnings.

Found with scripts/coccinelle/misc/boolinit.cocci.

Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_qdio.c | 2 +-
 drivers/s390/scsi/zfcp_scsi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_qdio.c b/drivers/s390/scsi/zfcp_qdio.c
index dbf2b54703f7..9e358fc04b78 100644
--- a/drivers/s390/scsi/zfcp_qdio.c
+++ b/drivers/s390/scsi/zfcp_qdio.c
@@ -14,7 +14,7 @@
 #include "zfcp_ext.h"
 #include "zfcp_qdio.h"
 
-static bool enable_multibuffer = 1;
+static bool enable_multibuffer = true;
 module_param_named(datarouter, enable_multibuffer, bool, 0400);
 MODULE_PARM_DESC(datarouter, "Enable hardware data router support (default on)");
 
diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 0678cf714c0e..d99da4e1e27a 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -28,7 +28,7 @@ static bool enable_dif;
 module_param_named(dif, enable_dif, bool, 0400);
 MODULE_PARM_DESC(dif, "Enable DIF/DIX data integrity support");
 
-static bool allow_lun_scan = 1;
+static bool allow_lun_scan = true;
 module_param(allow_lun_scan, bool, 0600);
 MODULE_PARM_DESC(allow_lun_scan, "For NPIV, scan and attach all storage LUNs");
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 05/22] zfcp: fix queuecommand for scsi_eh commands when DIX enabled
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (3 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 04/22] zfcp: convert bool-definitions to use 'true' instead of '1' Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 06/22] zfcp: add handling for FCP_RESID_OVER to the fcp ingress path Benjamin Block
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, stable, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

Since commit db007fc5e20c ("[SCSI] Command protection operation"),
scsi_eh_prep_cmnd() saves scmd->prot_op and temporarily resets it to
SCSI_PROT_NORMAL.
Other FCP LLDDs such as qla2xxx and lpfc shield their queuecommand()
to only access any of scsi_prot_sg...() if
(scsi_get_prot_op(cmd) != SCSI_PROT_NORMAL).

Do the same thing for zfcp, which introduced DIX support with
commit ef3eb71d8ba4 ("[SCSI] zfcp: Introduce experimental support for
DIF/DIX").

Otherwise, TUR SCSI commands as part of scsi_eh likely fail in zfcp,
because the regular SCSI command with DIX protection data, that scsi_eh
re-uses in scsi_send_eh_cmnd(), of course still has
(scsi_prot_sg_count() != 0) and so zfcp sends down bogus requests to the
FCP channel hardware.

This causes scsi_eh_test_devices() to have (finish_cmds == 0)
[not SCSI device is online or not scsi_eh_tur() failed]
so regular SCSI commands, that caused / were affected by scsi_eh,
are moved to work_q and scsi_eh_test_devices() itself returns false.
In turn, it unnecessarily escalates in our case in scsi_eh_ready_devs()
beyond host reset to finally scsi_eh_offline_sdevs()
which sets affected SCSI devices offline with the following kernel message:

"kernel: sd H:0:T:L: Device offlined - not ready after error recovery"

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: ef3eb71d8ba4 ("[SCSI] zfcp: Introduce experimental support for DIF/DIX")
Cc: <stable@vger.kernel.org> #2.6.36+
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fsf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 3b69ec5e69ed..e894ec92076c 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2257,7 +2257,8 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
 	fcp_cmnd = (struct fcp_cmnd *) &req->qtcb->bottom.io.fcp_cmnd;
 	zfcp_fc_scsi_to_fcp(fcp_cmnd, scsi_cmnd, 0);
 
-	if (scsi_prot_sg_count(scsi_cmnd)) {
+	if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
+	    scsi_prot_sg_count(scsi_cmnd)) {
 		zfcp_qdio_set_data_div(qdio, &req->qdio_req,
 				       scsi_prot_sg_count(scsi_cmnd));
 		retval = zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req,
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 06/22] zfcp: add handling for FCP_RESID_OVER to the fcp ingress path
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (4 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 05/22] zfcp: fix queuecommand for scsi_eh commands when DIX enabled Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 07/22] zfcp: fix capping of unsuccessful GPN_FT SAN response trace records Benjamin Block
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Benjamin Block, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390, stable

Up until now zfcp would just ignore the FCP_RESID_OVER flag in the FCP
response IU. When this flag is set, it is possible, in regards to the
FCP standard, that the storage-server processes the command normally, up
to the point where data is missing and simply ignores those.

In this case no CHECK CONDITION would be set, and because we ignored the
FCP_RESID_OVER flag we resulted in at least a data loss or even
-corruption as a follow-up error, depending on how the
applications/layers on top behave. To prevent this, we now set the
host-byte of the corresponding scsi_cmnd to DID_ERROR.

Other storage-behaviors, where the same condition results in a CHECK
CONDITION set in the answer, don't need to be changed as they are
handled in the mid-layer already.

Following is an example trace record decoded with zfcpdbf from the
s390-tools package. We forcefully injected a fc_dl which is one byte too
small:

Timestamp      : ...
Area           : SCSI
Subarea        : 00
Level          : 3
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : rsl_err
Request ID     : 0x...
SCSI ID        : 0x...
SCSI LUN       : 0x...
SCSI result    : 0x00070000
                     ^^DID_ERROR
SCSI retries   : 0x..
SCSI allowed   : 0x..
SCSI scribble  : 0x...
SCSI opcode    : 2a000000 00000000 08000000 00000000
FCP rsp inf cod: 0x00
FCP rsp IU     : 00000000 00000000 00000400 00000001
                                       ^^fr_flags==FCP_RESID_OVER
                                         ^^fr_status==SAM_STAT_GOOD
                                            ^^^^^^^^fr_resid
                 00000000 00000000

As of now, we don't actively handle to possibility that a response IU
has both flags - FCP_RESID_OVER and FCP_RESID_UNDER - set at once.

Reported-by: Luke M. Hopkins <lmhopkin@us.ibm.com>
Reviewed-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: 553448f6c483 ("[SCSI] zfcp: Message cleanup")
Fixes: ea127f975424 ("[PATCH] s390 (7/7): zfcp host adapter.") (tglx/history.git)
Cc: <stable@vger.kernel.org> #2.6.33+
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fc.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_fc.h b/drivers/s390/scsi/zfcp_fc.h
index df2b541c8287..a2275825186f 100644
--- a/drivers/s390/scsi/zfcp_fc.h
+++ b/drivers/s390/scsi/zfcp_fc.h
@@ -4,7 +4,7 @@
  * Fibre Channel related definitions and inline functions for the zfcp
  * device driver
  *
- * Copyright IBM Corp. 2009
+ * Copyright IBM Corp. 2009, 2017
  */
 
 #ifndef ZFCP_FC_H
@@ -279,6 +279,10 @@ void zfcp_fc_eval_fcp_rsp(struct fcp_resp_with_ext *fcp_rsp,
 		     !(rsp_flags & FCP_SNS_LEN_VAL) &&
 		     fcp_rsp->resp.fr_status == SAM_STAT_GOOD)
 			set_host_byte(scsi, DID_ERROR);
+	} else if (unlikely(rsp_flags & FCP_RESID_OVER)) {
+		/* FCP_DL was not sufficient for SCSI data length */
+		if (fcp_rsp->resp.fr_status == SAM_STAT_GOOD)
+			set_host_byte(scsi, DID_ERROR);
 	}
 }
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 07/22] zfcp: fix capping of unsuccessful GPN_FT SAN response trace records
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (5 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 06/22] zfcp: add handling for FCP_RESID_OVER to the fcp ingress path Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 08/22] zfcp: fix passing fsf_req to SCSI trace on TMF to correlate with HBA Benjamin Block
                   ` (15 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, stable, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

v4.9 commit aceeffbb59bb ("zfcp: trace full payload of all SAN records
(req,resp,iels)") fixed trace data loss of 2.6.38 commit 2c55b750a884
("[SCSI] zfcp: Redesign of the debug tracing for SAN records.")
necessary for problem determination, e.g. to see the
currently active zone set during automatic port scan.

While it already saves space by not dumping any empty residual entries
of the large successful GPN_FT response (4 pages), there are seldom cases
where the GPN_FT response is unsuccessful and likely does not have
FC_NS_FID_LAST set in fp_flags so we did not cap the trace record.
We typically see such case for an initiator WWPN, which is not in any zone.

Cap unsuccessful responses to at least the actual basic CT_IU response
plus whatever fits the SAN trace record built-in "payload" buffer
just in case there's trailing information
of which we would at least see the existence and its beginning.

In order not to erroneously cap successful responses, we need to swap
calling the trace function and setting the CT / ELS status to success (0).

Example trace record pair formatted with zfcpdbf:

Timestamp      : ...
Area           : SAN
Subarea        : 00
Level          : 1
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : fssct_1
Request ID     : 0x<request_id>
Destination ID : 0x00fffffc
SAN req short  : 01000000 fc020000 01720ffc 00000000
                 00000008
SAN req length : 20
|
Timestamp      : ...
Area           : SAN
Subarea        : 00
Level          : 1
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 2
Tag            : fsscth2
Request ID     : 0x<request_id>
Destination ID : 0x00fffffc
SAN resp short : 01000000 fc020000 80010000 00090700
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
SAN resp length: 16384
San resp info  : 01000000 fc020000 80010000 00090700
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]
                 00000000 00000000 00000000 00000000 [trailing info]

The fix saves all but one of the previously associated 64 PAYload trace
record chunks of size 256 bytes each.

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: aceeffbb59bb ("zfcp: trace full payload of all SAN records (req,resp,iels)")
Fixes: 2c55b750a884 ("[SCSI] zfcp: Redesign of the debug tracing for SAN records.")
Cc: <stable@vger.kernel.org> #2.6.38+
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c | 10 +++++++++-
 drivers/s390/scsi/zfcp_fsf.c |  4 ++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index d5bf36ec8a75..31d62ea5fdcd 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -3,7 +3,7 @@
  *
  * Debug traces for zfcp.
  *
- * Copyright IBM Corp. 2002, 2016
+ * Copyright IBM Corp. 2002, 2017
  */
 
 #define KMSG_COMPONENT "zfcp"
@@ -447,6 +447,7 @@ static u16 zfcp_dbf_san_res_cap_len_if_gpn_ft(char *tag,
 	struct fc_ct_hdr *reqh = sg_virt(ct_els->req);
 	struct fc_ns_gid_ft *reqn = (struct fc_ns_gid_ft *)(reqh + 1);
 	struct scatterlist *resp_entry = ct_els->resp;
+	struct fc_ct_hdr *resph;
 	struct fc_gpn_ft_resp *acc;
 	int max_entries, x, last = 0;
 
@@ -473,6 +474,13 @@ static u16 zfcp_dbf_san_res_cap_len_if_gpn_ft(char *tag,
 		return len; /* not GPN_FT response so do not cap */
 
 	acc = sg_virt(resp_entry);
+
+	/* cap all but accept CT responses to at least the CT header */
+	resph = (struct fc_ct_hdr *)acc;
+	if ((ct_els->status) ||
+	    (resph->ct_cmd != cpu_to_be16(FC_FS_ACC)))
+		return max(FC_CT_HDR_LEN, ZFCP_DBF_SAN_MAX_PAYLOAD);
+
 	max_entries = (reqh->ct_mr_size * 4 / sizeof(struct fc_gpn_ft_resp))
 		+ 1 /* zfcp_fc_scan_ports: bytes correct, entries off-by-one
 		     * to account for header as 1st pseudo "entry" */;
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index e894ec92076c..c10b4b9f1574 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -928,8 +928,8 @@ static void zfcp_fsf_send_ct_handler(struct zfcp_fsf_req *req)
 
 	switch (header->fsf_status) {
         case FSF_GOOD:
-		zfcp_dbf_san_res("fsscth2", req);
 		ct->status = 0;
+		zfcp_dbf_san_res("fsscth2", req);
 		break;
         case FSF_SERVICE_CLASS_NOT_SUPPORTED:
 		zfcp_fsf_class_not_supp(req);
@@ -1108,8 +1108,8 @@ static void zfcp_fsf_send_els_handler(struct zfcp_fsf_req *req)
 
 	switch (header->fsf_status) {
 	case FSF_GOOD:
-		zfcp_dbf_san_res("fsselh1", req);
 		send_els->status = 0;
+		zfcp_dbf_san_res("fsselh1", req);
 		break;
 	case FSF_SERVICE_CLASS_NOT_SUPPORTED:
 		zfcp_fsf_class_not_supp(req);
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 08/22] zfcp: fix passing fsf_req to SCSI trace on TMF to correlate with HBA
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (6 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 07/22] zfcp: fix capping of unsuccessful GPN_FT SAN response trace records Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 09/22] zfcp: fix missing trace records for early returns in TMF eh handlers Benjamin Block
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, stable, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

Without this fix we get SCSI trace records on task management functions
which cannot be correlated to HBA trace records because all fields
related to the FSF request are empty (zero).
Also, the FCP_RSP_IU is missing as well as any sense data if available.

This was caused by v2.6.14 commit 8a36e4532ea1 ("[SCSI] zfcp: enhancement
of zfcp debug features") introducing trace records for TMFs but
hard coding NULL for a possibly existing TMF FSF request.
The scsi_cmnd scribble is also zero or unrelated for the TMF request
so it also could not lookup a suitable FSF request from there.

A broken example trace record formatted with zfcpdbf from the s390-tools
package:

Timestamp      : ...
Area           : SCSI
Subarea        : 00
Level          : 1
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : lr_fail
Request ID     : 0x0000000000000000
                   ^^^^^^^^^^^^^^^^ no correlation to HBA record
SCSI ID        : 0x<scsitarget>
SCSI LUN       : 0x<scsilun>
SCSI result    : 0x000e0000
SCSI retries   : 0x00
SCSI allowed   : 0x05
SCSI scribble  : 0x0000000000000000
SCSI opcode    : 2a000017 3bb80000 08000000 00000000
FCP rsp inf cod: 0x00
                   ^^ no TMF response
FCP rsp IU     : 00000000 00000000 00000000 00000000
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 00000000 00000000
                 ^^^^^^^^^^^^^^^^^ no interesting FCP_RSP_IU
Sense len      : ...
^^^^^^^^^^^^^^^^^^^^ no sense data length
Sense info     : ...
^^^^^^^^^^^^^^^^^^^^ no sense data content, even if present

There are some true cases where we really do not have an FSF request:
"rsl_fai" from zfcp_dbf_scsi_fail_send() called for early
returns / completions in zfcp_scsi_queuecommand(),
"abrt_or", "abrt_bl", "abrt_ru", "abrt_ar" from
zfcp_scsi_eh_abort_handler() where we did not get as far,
"lr_nres", "tr_nres" from zfcp_task_mgmt_function() where we're
successful and do not need to do anything because adapter stopped.
For these cases it's correct to pass NULL for fsf_req to _zfcp_dbf_scsi().

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: 8a36e4532ea1 ("[SCSI] zfcp: enhancement of zfcp debug features")
Cc: <stable@vger.kernel.org> #2.6.38+
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.h  | 7 ++++---
 drivers/s390/scsi/zfcp_scsi.c | 8 ++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index db186d44cfaf..776d1ac125ff 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -2,7 +2,7 @@
  * zfcp device driver
  * debug feature declarations
  *
- * Copyright IBM Corp. 2008, 2016
+ * Copyright IBM Corp. 2008, 2017
  */
 
 #ifndef ZFCP_DBF_H
@@ -401,7 +401,8 @@ void zfcp_dbf_scsi_abort(char *tag, struct scsi_cmnd *scmd,
  * @flag: indicates type of reset (Target Reset, Logical Unit Reset)
  */
 static inline
-void zfcp_dbf_scsi_devreset(char *tag, struct scsi_cmnd *scmnd, u8 flag)
+void zfcp_dbf_scsi_devreset(char *tag, struct scsi_cmnd *scmnd, u8 flag,
+			    struct zfcp_fsf_req *fsf_req)
 {
 	char tmp_tag[ZFCP_DBF_TAG_LEN];
 
@@ -411,7 +412,7 @@ void zfcp_dbf_scsi_devreset(char *tag, struct scsi_cmnd *scmnd, u8 flag)
 		memcpy(tmp_tag, "lr_", 3);
 
 	memcpy(&tmp_tag[3], tag, 4);
-	_zfcp_dbf_scsi(tmp_tag, 1, scmnd, NULL);
+	_zfcp_dbf_scsi(tmp_tag, 1, scmnd, fsf_req);
 }
 
 /**
diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index d99da4e1e27a..4c9ac55d053d 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -3,7 +3,7 @@
  *
  * Interface to Linux SCSI midlayer.
  *
- * Copyright IBM Corp. 2002, 2016
+ * Copyright IBM Corp. 2002, 2017
  */
 
 #define KMSG_COMPONENT "zfcp"
@@ -278,7 +278,7 @@ static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags)
 
 		if (!(atomic_read(&adapter->status) &
 		      ZFCP_STATUS_COMMON_RUNNING)) {
-			zfcp_dbf_scsi_devreset("nres", scpnt, tm_flags);
+			zfcp_dbf_scsi_devreset("nres", scpnt, tm_flags, NULL);
 			return SUCCESS;
 		}
 	}
@@ -288,10 +288,10 @@ static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags)
 	wait_for_completion(&fsf_req->completion);
 
 	if (fsf_req->status & ZFCP_STATUS_FSFREQ_TMFUNCFAILED) {
-		zfcp_dbf_scsi_devreset("fail", scpnt, tm_flags);
+		zfcp_dbf_scsi_devreset("fail", scpnt, tm_flags, fsf_req);
 		retval = FAILED;
 	} else {
-		zfcp_dbf_scsi_devreset("okay", scpnt, tm_flags);
+		zfcp_dbf_scsi_devreset("okay", scpnt, tm_flags, fsf_req);
 		zfcp_scsi_forget_cmnds(zfcp_sdev, tm_flags);
 	}
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 09/22] zfcp: fix missing trace records for early returns in TMF eh handlers
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (7 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 08/22] zfcp: fix passing fsf_req to SCSI trace on TMF to correlate with HBA Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 10/22] zfcp: fix payload with full FCP_RSP IU in SCSI trace records Benjamin Block
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, stable, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

For problem determination we need to see that we were in scsi_eh
as well as whether and why we were successful or not.

The following commits introduced new early returns without adding
a trace record:

v2.6.35 commit a1dbfddd02d2
("[SCSI] zfcp: Pass return code from fc_block_scsi_eh to scsi eh")
on fc_block_scsi_eh() returning != 0 which is FAST_IO_FAIL,

v2.6.30 commit 63caf367e1c9
("[SCSI] zfcp: Improve reliability of SCSI eh handlers in zfcp")
on not having gotten an FSF request after the maximum number of retry
attempts and thus could not issue a TMF and has to return FAILED.

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: a1dbfddd02d2 ("[SCSI] zfcp: Pass return code from fc_block_scsi_eh to scsi eh")
Fixes: 63caf367e1c9 ("[SCSI] zfcp: Improve reliability of SCSI eh handlers in zfcp")
Cc: <stable@vger.kernel.org> #2.6.38+
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_scsi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 4c9ac55d053d..ec3ddd1d31d5 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -273,8 +273,10 @@ static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags)
 
 		zfcp_erp_wait(adapter);
 		ret = fc_block_scsi_eh(scpnt);
-		if (ret)
+		if (ret) {
+			zfcp_dbf_scsi_devreset("fiof", scpnt, tm_flags, NULL);
 			return ret;
+		}
 
 		if (!(atomic_read(&adapter->status) &
 		      ZFCP_STATUS_COMMON_RUNNING)) {
@@ -282,8 +284,10 @@ static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags)
 			return SUCCESS;
 		}
 	}
-	if (!fsf_req)
+	if (!fsf_req) {
+		zfcp_dbf_scsi_devreset("reqf", scpnt, tm_flags, NULL);
 		return FAILED;
+	}
 
 	wait_for_completion(&fsf_req->completion);
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 10/22] zfcp: fix payload with full FCP_RSP IU in SCSI trace records
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (8 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 09/22] zfcp: fix missing trace records for early returns in TMF eh handlers Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 11/22] zfcp: trace HBA FSF response by default on dismiss or timedout late response Benjamin Block
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, stable, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

If the FCP_RSP UI has optional parts (FCP_SNS_INFO or FCP_RSP_INFO) and
thus does not fit into the fsp_rsp field built into a SCSI trace record,
trace the full FCP_RSP UI with all optional parts as payload record
instead of just FCP_SNS_INFO as payload and
a 1 byte RSP_INFO_CODE part of FCP_RSP_INFO built into the SCSI record.

That way we would also get the full FCP_SNS_INFO in case a
target would ever send more than
min(SCSI_SENSE_BUFFERSIZE==96, ZFCP_DBF_PAY_MAX_REC==256)==96.

The mandatory part of FCP_RSP IU is only 24 bytes.
PAYload costs at least one full PAY record of 256 bytes anyway.
We cap to the hardware response size which is only FSF_FCP_RSP_SIZE==128.
So we can just put the whole FCP_RSP IU with any optional parts into
PAYload similarly as we do for SAN PAY since v4.9 commit aceeffbb59bb
("zfcp: trace full payload of all SAN records (req,resp,iels)").
This does not cause any additional trace records wasting memory.

Decoded trace records were confusing because they showed a hard-coded
sense data length of 96 even if the FCP_RSP_IU field FCP_SNS_LEN showed
actually less.

Since the same commit, we set pl_len for SAN traces to the full length of a
request/response even if we cap the corresponding trace.
In contrast, here for SCSI traces we set pl_len to the pre-computed
length of FCP_RSP IU considering SNS_LEN or RSP_LEN if valid.
Nonetheless we trace a hardcoded payload of length FSF_FCP_RSP_SIZE==128
if there were optional parts.
This makes it easier for the zfcpdbf tool to format only the relevant
part of the long FCP_RSP UI buffer. And any trailing information is still
available in the payload trace record just in case.

Rename the payload record tag from "fcp_sns" to "fcp_riu" to make the new
content explicit to zfcpdbf which can then pick a suitable field name such
as "FCP rsp IU all:" instead of "Sense info :"
Also, the same zfcpdbf can still be backwards compatible with "fcp_sns".

Old example trace record before this fix, formatted with the tool zfcpdbf
from s390-tools:

Timestamp      : ...
Area           : SCSI
Subarea        : 00
Level          : 3
Exception      : -
CPU id         : ..
Caller         : 0x...
Record id      : 1
Tag            : rsl_err
Request id     : 0x<request_id>
SCSI ID        : 0x...
SCSI LUN       : 0x...
SCSI result    : 0x00000002
SCSI retries   : 0x00
SCSI allowed   : 0x05
SCSI scribble  : 0x<request_id>
SCSI opcode    : 00000000 00000000 00000000 00000000
FCP rsp inf cod: 0x00
FCP rsp IU     : 00000000 00000000 00000202 00000000
                                       ^^==FCP_SNS_LEN_VALID
                 00000020 00000000
                 ^^^^^^^^==FCP_SNS_LEN==32
Sense len      : 96 <==min(SCSI_SENSE_BUFFERSIZE,ZFCP_DBF_PAY_MAX_REC)
Sense info     : 70000600 00000018 00000000 29000000
                 00000400 00000000 00000000 00000000
                 00000000 00000000 00000000 00000000<==superfluous
                 00000000 00000000 00000000 00000000<==superfluous
                 00000000 00000000 00000000 00000000<==superfluous
                 00000000 00000000 00000000 00000000<==superfluous

New example trace records with this fix:

Timestamp      : ...
Area           : SCSI
Subarea        : 00
Level          : 3
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : rsl_err
Request ID     : 0x<request_id>
SCSI ID        : 0x...
SCSI LUN       : 0x...
SCSI result    : 0x00000002
SCSI retries   : 0x00
SCSI allowed   : 0x03
SCSI scribble  : 0x<request_id>
SCSI opcode    : a30c0112 00000000 02000000 00000000
FCP rsp inf cod: 0x00
FCP rsp IU     : 00000000 00000000 00000a02 00000200
                 00000020 00000000
FCP rsp IU len : 56
FCP rsp IU all : 00000000 00000000 00000a02 00000200
                                       ^^=FCP_RESID_UNDER|FCP_SNS_LEN_VALID
                 00000020 00000000 70000500 00000018
                 ^^^^^^^^==FCP_SNS_LEN
                                   ^^^^^^^^^^^^^^^^^
                 00000000 240000cb 00011100 00000000
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 00000000 00000000
                 ^^^^^^^^^^^^^^^^^==FCP_SNS_INFO

Timestamp      : ...
Area           : SCSI
Subarea        : 00
Level          : 1
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : lr_okay
Request ID     : 0x<request_id>
SCSI ID        : 0x...
SCSI LUN       : 0x...
SCSI result    : 0x00000000
SCSI retries   : 0x00
SCSI allowed   : 0x05
SCSI scribble  : 0x<request_id>
SCSI opcode    : <CDB of unrelated SCSI command passed to eh handler>
FCP rsp inf cod: 0x00
FCP rsp IU     : 00000000 00000000 00000100 00000000
                 00000000 00000008
FCP rsp IU len : 32
FCP rsp IU all : 00000000 00000000 00000100 00000000
                                       ^^==FCP_RSP_LEN_VALID
                 00000000 00000008 00000000 00000000
                          ^^^^^^^^==FCP_RSP_LEN
                                   ^^^^^^^^^^^^^^^^^==FCP_RSP_INFO

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: 250a1352b95e ("[SCSI] zfcp: Redesign of the debug tracing for SCSI records.")
Cc: <stable@vger.kernel.org> #2.6.38+
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 31d62ea5fdcd..c801f9782cb2 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -572,19 +572,32 @@ void zfcp_dbf_scsi(char *tag, int level, struct scsi_cmnd *sc,
 
 	if (fsf) {
 		rec->fsf_req_id = fsf->req_id;
+		rec->pl_len = FCP_RESP_WITH_EXT;
 		fcp_rsp = (struct fcp_resp_with_ext *)
 				&(fsf->qtcb->bottom.io.fcp_rsp);
+		/* mandatory parts of FCP_RSP IU in this SCSI record */
 		memcpy(&rec->fcp_rsp, fcp_rsp, FCP_RESP_WITH_EXT);
 		if (fcp_rsp->resp.fr_flags & FCP_RSP_LEN_VAL) {
 			fcp_rsp_info = (struct fcp_resp_rsp_info *) &fcp_rsp[1];
 			rec->fcp_rsp_info = fcp_rsp_info->rsp_code;
+			rec->pl_len += be32_to_cpu(fcp_rsp->ext.fr_rsp_len);
 		}
 		if (fcp_rsp->resp.fr_flags & FCP_SNS_LEN_VAL) {
-			rec->pl_len = min((u16)SCSI_SENSE_BUFFERSIZE,
-					  (u16)ZFCP_DBF_PAY_MAX_REC);
-			zfcp_dbf_pl_write(dbf, sc->sense_buffer, rec->pl_len,
-					  "fcp_sns", fsf->req_id);
+			rec->pl_len += be32_to_cpu(fcp_rsp->ext.fr_sns_len);
 		}
+		/* complete FCP_RSP IU in associated PAYload record
+		 * but only if there are optional parts
+		 */
+		if (fcp_rsp->resp.fr_flags != 0)
+			zfcp_dbf_pl_write(
+				dbf, fcp_rsp,
+				/* at least one full PAY record
+				 * but not beyond hardware response field
+				 */
+				min_t(u16, max_t(u16, rec->pl_len,
+						 ZFCP_DBF_PAY_MAX_REC),
+				      FSF_FCP_RSP_SIZE),
+				"fcp_riu", fsf->req_id);
 	}
 
 	debug_event(dbf->scsi, level, rec, sizeof(*rec));
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 11/22] zfcp: trace HBA FSF response by default on dismiss or timedout late response
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (9 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 10/22] zfcp: fix payload with full FCP_RSP IU in SCSI trace records Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 12/22] zfcp: trace high part of "new" 64 bit SCSI LUN Benjamin Block
                   ` (11 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, stable, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

At the default trace level, we only trace unsuccessful events including
FSF responses.

zfcp_dbf_hba_fsf_response() only used protocol status and FSF status to
decide on an unsuccessful response. However, this is only one of multiple
possible sources determining a failed struct zfcp_fsf_req.

An FSF request can also "fail" if its response runs into an ERP timeout
or if it gets dismissed because a higher level recovery was triggered
[trace tags "erscf_1" or "erscf_2" in zfcp_erp_strategy_check_fsfreq()].
FSF requests with ERP timeout are:
FSF_QTCB_EXCHANGE_CONFIG_DATA, FSF_QTCB_EXCHANGE_PORT_DATA,
FSF_QTCB_OPEN_PORT_WITH_DID or FSF_QTCB_CLOSE_PORT or
FSF_QTCB_CLOSE_PHYSICAL_PORT for target ports,
FSF_QTCB_OPEN_LUN, FSF_QTCB_CLOSE_LUN.
One example is slow queue processing which can cause follow-on errors,
e.g. FSF_PORT_ALREADY_OPEN after FSF_QTCB_OPEN_PORT_WITH_DID timed out.
In order to see the root cause, we need to see late responses even if the
channel presented them successfully with FSF_PROT_GOOD and FSF_GOOD.
Example trace records formatted with zfcpdbf from the s390-tools package:

Timestamp      : ...
Area           : REC
Subarea        : 00
Level          : 1
Exception      : -
CPU ID         : ..
Caller         : ...
Record ID      : 1
Tag            : fcegpf1
LUN            : 0xffffffffffffffff
WWPN           : 0x<WWPN>
D_ID           : 0x00<D_ID>
Adapter status : 0x5400050b
Port status    : 0x41200000
LUN status     : 0x00000000
Ready count    : 0x00000001
Running count  : 0x...
ERP want       : 0x02				ZFCP_ERP_ACTION_REOPEN_PORT
ERP need       : 0x02				ZFCP_ERP_ACTION_REOPEN_PORT
|
Timestamp      : ...				30 seconds later
Area           : REC
Subarea        : 00
Level          : 1
Exception      : -
CPU ID         : ..
Caller         : ...
Record ID      : 2
Tag            : erscf_2
LUN            : 0xffffffffffffffff
WWPN           : 0x<WWPN>
D_ID           : 0x00<D_ID>
Adapter status : 0x5400050b
Port status    : 0x41200000
LUN status     : 0x00000000
Request ID     : 0x<request_ID>
ERP status     : 0x10000000			ZFCP_STATUS_ERP_TIMEDOUT
ERP step       : 0x0800				ZFCP_ERP_STEP_PORT_OPENING
ERP action     : 0x02				ZFCP_ERP_ACTION_REOPEN_PORT
ERP count      : 0x00
|
Timestamp      : ...				later than previous record
Area           : HBA
Subarea        : 00
Level          : 5	> default level		=> 3	<= default level
Exception      : -
CPU ID         : 00
Caller         : ...
Record ID      : 1
Tag            : fs_qtcb			=> fs_rerr
Request ID     : 0x<request_ID>
Request status : 0x00001010			ZFCP_STATUS_FSFREQ_DISMISSED
						| ZFCP_STATUS_FSFREQ_CLEANUP
FSF cmnd       : 0x00000005
FSF sequence no: 0x...
FSF issued     : ...				> 30 seconds ago
FSF stat       : 0x00000000			FSF_GOOD
FSF stat qual  : 00000000 00000000 00000000 00000000
Prot stat      : 0x00000001			FSF_PROT_GOOD
Prot stat qual : 00000000 00000000 00000000 00000000
Port handle    : 0x...
LUN handle     : 0x00000000
QTCB log length: ...
QTCB log info  : ...

In case of problems detecting that new responses are waiting on the input
queue, we sooner or later trigger adapter recovery due to an FSF request
timeout (trace tag "fsrth_1").
FSF requests with FSF request timeout are:
typically FSF_QTCB_ABORT_FCP_CMND; but theoretically also
FSF_QTCB_EXCHANGE_CONFIG_DATA or FSF_QTCB_EXCHANGE_PORT_DATA via sysfs,
FSF_QTCB_OPEN_PORT_WITH_DID or FSF_QTCB_CLOSE_PORT for WKA ports,
FSF_QTCB_FCP_CMND for task management function (LUN / target reset).
One or more pending requests can meanwhile have FSF_PROT_GOOD and FSF_GOOD
because the channel filled in the response via DMA into the request's QTCB.

In a theroretical case, inject code can create an erroneous FSF request
on purpose. If data router is enabled, it uses deferred error reporting.
A READ SCSI command can succeed with FSF_PROT_GOOD, FSF_GOOD, and
SAM_STAT_GOOD. But on writing the read data to host memory via DMA,
it can still fail, e.g. if an intentionally wrong scatter list does not
provide enough space. Rather than getting an unsuccessful response,
we get a QDIO activate check which in turn triggers adapter recovery.
One or more pending requests can meanwhile have FSF_PROT_GOOD and FSF_GOOD
because the channel filled in the response via DMA into the request's QTCB.
Example trace records formatted with zfcpdbf from the s390-tools package:

Timestamp      : ...
Area           : HBA
Subarea        : 00
Level          : 6	> default level		=> 3	<= default level
Exception      : -
CPU ID         : ..
Caller         : ...
Record ID      : 1
Tag            : fs_norm			=> fs_rerr
Request ID     : 0x<request_ID2>
Request status : 0x00001010			ZFCP_STATUS_FSFREQ_DISMISSED
						| ZFCP_STATUS_FSFREQ_CLEANUP
FSF cmnd       : 0x00000001
FSF sequence no: 0x...
FSF issued     : ...
FSF stat       : 0x00000000			FSF_GOOD
FSF stat qual  : 00000000 00000000 00000000 00000000
Prot stat      : 0x00000001			FSF_PROT_GOOD
Prot stat qual : ........ ........ 00000000 00000000
Port handle    : 0x...
LUN handle     : 0x...
|
Timestamp      : ...
Area           : SCSI
Subarea        : 00
Level          : 3
Exception      : -
CPU ID         : ..
Caller         : ...
Record ID      : 1
Tag            : rsl_err
Request ID     : 0x<request_ID2>
SCSI ID        : 0x...
SCSI LUN       : 0x...
SCSI result    : 0x000e0000			DID_TRANSPORT_DISRUPTED
SCSI retries   : 0x00
SCSI allowed   : 0x05
SCSI scribble  : 0x<request_ID2>
SCSI opcode    : 28...				Read(10)
FCP rsp inf cod: 0x00
FCP rsp IU     : 00000000 00000000 00000000 00000000
                                         ^^	SAM_STAT_GOOD
                 00000000 00000000

Only with luck in both above cases, we could see a follow-on trace record
of an unsuccesful event following a successful but late FSF response with
FSF_PROT_GOOD and FSF_GOOD. Typically this was the case for I/O requests
resulting in a SCSI trace record "rsl_err" with DID_TRANSPORT_DISRUPTED
[On ZFCP_STATUS_FSFREQ_DISMISSED, zfcp_fsf_protstatus_eval() sets
ZFCP_STATUS_FSFREQ_ERROR seen by the request handler functions as failure].
However, the reason for this follow-on trace was invisible because the
corresponding HBA trace record was missing at the default trace level
(by default hidden records with tags "fs_norm", "fs_qtcb", or "fs_open").

On adapter recovery, after we had shut down the QDIO queues, we perform
unsuccessful pseudo completions with flag ZFCP_STATUS_FSFREQ_DISMISSED
for each pending FSF request in zfcp_fsf_req_dismiss_all().
In order to find the root cause, we need to see all pseudo responses even
if the channel presented them successfully with FSF_PROT_GOOD and FSF_GOOD.

Therefore, check zfcp_fsf_req.status for ZFCP_STATUS_FSFREQ_DISMISSED
or ZFCP_STATUS_FSFREQ_ERROR and trace with a new tag "fs_rerr".

It does not matter that there are numerous places which set
ZFCP_STATUS_FSFREQ_ERROR after the location where we trace an FSF response
early. These cases are based on protocol status != FSF_PROT_GOOD or
== FSF_PROT_FSF_STATUS_PRESENTED and are thus already traced by default
as trace tag "fs_perr" or "fs_ferr" respectively.

NB: The trace record with tag "fssrh_1" for status read buffers on dismiss
all remains. zfcp_fsf_req_complete() handles this and returns early.
All other FSF request types are handled separately and as described above.

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: 8a36e4532ea1 ("[SCSI] zfcp: enhancement of zfcp debug features")
Fixes: 2e261af84cdb ("[SCSI] zfcp: Only collect FSF/HBA debug data for matching trace levels")
Cc: <stable@vger.kernel.org> #2.6.38+
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index 776d1ac125ff..8e7f8e6037d2 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -323,7 +323,11 @@ void zfcp_dbf_hba_fsf_response(struct zfcp_fsf_req *req)
 {
 	struct fsf_qtcb *qtcb = req->qtcb;
 
-	if ((qtcb->prefix.prot_status != FSF_PROT_GOOD) &&
+	if (unlikely(req->status & (ZFCP_STATUS_FSFREQ_DISMISSED |
+				    ZFCP_STATUS_FSFREQ_ERROR))) {
+		zfcp_dbf_hba_fsf_resp("fs_rerr", 3, req);
+
+	} else if ((qtcb->prefix.prot_status != FSF_PROT_GOOD) &&
 	    (qtcb->prefix.prot_status != FSF_PROT_FSF_STATUS_PRESENTED)) {
 		zfcp_dbf_hba_fsf_resp("fs_perr", 1, req);
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 12/22] zfcp: trace high part of "new" 64 bit SCSI LUN
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (10 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 11/22] zfcp: trace HBA FSF response by default on dismiss or timedout late response Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:30 ` [PATCH 13/22] zfcp: more fitting constant for fc_ct_hdr.ct_reason on port scan response Benjamin Block
                   ` (10 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, stable, Jens Remus, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

Complements debugging aspects of the otherwise functionally complete
v3.17 commit 9cb78c16f5da ("scsi: use 64-bit LUNs").

While I don't have access to a target exporting 3 or 4 level LUNs,
I did test it by explicitly attaching a non-existent fake 4 level LUN
by means of zfcp sysfs attribute "unit_add".
In order to see corresponding trace records of otherwise successful
events, we had to increase the trace level of area SCSI and HBA to 6.

$ echo 6 > /sys/kernel/debug/s390dbf/zfcp_0.0.1880_scsi/level
$ echo 6 > /sys/kernel/debug/s390dbf/zfcp_0.0.1880_hba/level

$ echo 0x4011402240334044 > \
  /sys/bus/ccw/drivers/zfcp/0.0.1880/0x50050763031bd327/unit_add

Example output formatted by an updated zfcpdbf from the s390-tools
package interspersed with kernel messages at scsi_logging_level=4605:

Timestamp      : ...
Area           : REC
Subarea        : 00
Level          : 1
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : scsla_1
LUN            : 0x4011402240334044
WWPN           : 0x50050763031bd327
D_ID           : 0x00......
Adapter status : 0x5400050b
Port status    : 0x54000001
LUN status     : 0x41000000
Ready count    : 0x00000001
Running count  : 0x00000000
ERP want       : 0x01
ERP need       : 0x01

scsi 2:0:0:4630896905707208721: scsi scan: INQUIRY pass 1 length 36
scsi 2:0:0:4630896905707208721: scsi scan: INQUIRY successful with code 0x0

Timestamp      : ...
Area           : HBA
Subarea        : 00
Level          : 6
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : fs_norm
Request ID     : 0x<inquiry2-req-id>
Request status : 0x00000010
FSF cmnd       : 0x00000001
FSF sequence no: 0x...
FSF issued     : ...
FSF stat       : 0x00000000
FSF stat qual  : 00000000 00000000 00000000 00000000
Prot stat      : 0x00000001
Prot stat qual : ........ ........ 00000000 00000000
Port handle    : 0x...
LUN handle     : 0x...
|
Timestamp      : ...
Area           : SCSI
Subarea        : 00
Level          : 6
Exception      : -
CPU ID         : ..
Caller         : 0x...
Record ID      : 1
Tag            : rsl_nor
Request ID     : 0x<inquiry2-req-id>
SCSI ID        : 0x00000000
SCSI LUN       : 0x40224011
SCSI LUN high  : 0x40444033 <=======================
SCSI result    : 0x00000000
SCSI retries   : 0x00
SCSI allowed   : 0x03
SCSI scribble  : 0x<inquiry2-req-id>
SCSI opcode    : 12000000 a4000000 00000000 00000000
FCP rsp inf cod: 0x00
FCP rsp IU     : 00000000 00000000 00000000 00000000
                 00000000 00000000

scsi 2:0:0:4630896905707208721: scsi scan: INQUIRY pass 2 length 164
scsi 2:0:0:4630896905707208721: scsi scan: INQUIRY successful with code 0x0
scsi 2:0:0:4630896905707208721: scsi scan: peripheral device type of 31, \
no device added

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Fixes: 9cb78c16f5da ("scsi: use 64-bit LUNs")
Cc: <stable@vger.kernel.org> #3.17+
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Reviewed-by: Jens Remus <jremus@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c | 2 +-
 drivers/s390/scsi/zfcp_dbf.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index c801f9782cb2..34367d172961 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -563,8 +563,8 @@ void zfcp_dbf_scsi(char *tag, int level, struct scsi_cmnd *sc,
 	rec->scsi_retries = sc->retries;
 	rec->scsi_allowed = sc->allowed;
 	rec->scsi_id = sc->device->id;
-	/* struct zfcp_dbf_scsi needs to be updated to handle 64bit LUNs */
 	rec->scsi_lun = (u32)sc->device->lun;
+	rec->scsi_lun_64_hi = (u32)(sc->device->lun >> 32);
 	rec->host_scribble = (unsigned long)sc->host_scribble;
 
 	memcpy(rec->scsi_opcode, sc->cmnd,
diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index 8e7f8e6037d2..b60667c145fd 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -204,7 +204,7 @@ enum zfcp_dbf_scsi_id {
  * @id: unique number of recovery record type
  * @tag: identifier string specifying the location of initiation
  * @scsi_id: scsi device id
- * @scsi_lun: scsi device logical unit number
+ * @scsi_lun: scsi device logical unit number, low part of 64 bit, old 32 bit
  * @scsi_result: scsi result
  * @scsi_retries: current retry number of scsi request
  * @scsi_allowed: allowed retries
@@ -214,6 +214,7 @@ enum zfcp_dbf_scsi_id {
  * @host_scribble: LLD specific data attached to SCSI request
  * @pl_len: length of paload stored as zfcp_dbf_pay
  * @fsf_rsp: response for fsf request
+ * @scsi_lun_64_hi: scsi device logical unit number, high part of 64 bit
  */
 struct zfcp_dbf_scsi {
 	u8 id;
@@ -230,6 +231,7 @@ struct zfcp_dbf_scsi {
 	u64 host_scribble;
 	u16 pl_len;
 	struct fcp_resp_with_ext fcp_rsp;
+	u32 scsi_lun_64_hi;
 } __packed;
 
 /**
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 13/22] zfcp: more fitting constant for fc_ct_hdr.ct_reason on port scan response
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (11 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 12/22] zfcp: trace high part of "new" 64 bit SCSI LUN Benjamin Block
@ 2017-07-28 10:30 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 14/22] zfcp: clarify that we don't need "link" test on failed open port Benjamin Block
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:30 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

v2.6.33 commit dbf5dfe9dbce ("[SCSI] zfcp: Use common code definitions for
FC CT structs") replaced own definitions with common code definitions.
While FC_BA_RJT_UNABLE happens to be defined with the same value 9 as
FC_FS_RJT_UNABL and thus also works, here we should use the latter from
fc_gs.h.
See also its use in libfc's fc_disc_gpn_ft_resp().

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index 7331eea67435..cc3f378782b2 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -3,7 +3,7 @@
  *
  * Fibre Channel related functions for the zfcp device driver.
  *
- * Copyright IBM Corp. 2008, 2010
+ * Copyright IBM Corp. 2008, 2017
  */
 
 #define KMSG_COMPONENT "zfcp"
@@ -667,7 +667,7 @@ static int zfcp_fc_eval_gpn_ft(struct zfcp_fc_req *fc_req,
 		return -EIO;
 
 	if (hdr->ct_cmd != FC_FS_ACC) {
-		if (hdr->ct_reason == FC_BA_RJT_UNABLE)
+		if (hdr->ct_reason == FC_FS_RJT_UNABL)
 			return -EAGAIN; /* might be a temporary condition */
 		return -EIO;
 	}
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 14/22] zfcp: clarify that we don't need "link" test on failed open port
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (12 preceding siblings ...)
  2017-07-28 10:30 ` [PATCH 13/22] zfcp: more fitting constant for fc_ct_hdr.ct_reason on port scan response Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 15/22] zfcp: use common code fcp_cmnd and fcp_resp with union in fsf_qtcb_bottom_io Benjamin Block
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fsf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index c10b4b9f1574..9f73b8fc7f3b 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -3,7 +3,7 @@
  *
  * Implementation of FSF commands.
  *
- * Copyright IBM Corp. 2002, 2015
+ * Copyright IBM Corp. 2002, 2017
  */
 
 #define KMSG_COMPONENT "zfcp"
@@ -1393,6 +1393,8 @@ static void zfcp_fsf_open_port_handler(struct zfcp_fsf_req *req)
 	case FSF_ADAPTER_STATUS_AVAILABLE:
 		switch (header->fsf_status_qual.word[0]) {
 		case FSF_SQ_INVOKE_LINK_TEST_PROCEDURE:
+			/* no zfcp_fc_test_link() with failed open port */
+			/* fall through */
 		case FSF_SQ_ULP_DEPENDENT_ERP_REQUIRED:
 		case FSF_SQ_NO_RETRY_POSSIBLE:
 			req->status |= ZFCP_STATUS_FSFREQ_ERROR;
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 15/22] zfcp: use common code fcp_cmnd and fcp_resp with union in fsf_qtcb_bottom_io
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (13 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 14/22] zfcp: clarify that we don't need "link" test on failed open port Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 16/22] zfcp: use endianness conversions with common FC(P) struct fields Benjamin Block
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

This eases crash dump analysis by automatically dissecting these
protocol headers at least somewhat rather than getting a string
interpretation of large unstructured character array buffer fields.

Also, we can get rid of some unnecessary and error-prone type casts.

This change is possible since v2.6.33 commit 4318e08c84e4
("[SCSI] zfcp: Update FCP protocol related code").

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c |  3 +--
 drivers/s390/scsi/zfcp_dbf.h |  2 +-
 drivers/s390/scsi/zfcp_fsf.c | 10 ++++++----
 drivers/s390/scsi/zfcp_fsf.h | 12 +++++++++---
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 34367d172961..225e60d5d3fc 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -573,8 +573,7 @@ void zfcp_dbf_scsi(char *tag, int level, struct scsi_cmnd *sc,
 	if (fsf) {
 		rec->fsf_req_id = fsf->req_id;
 		rec->pl_len = FCP_RESP_WITH_EXT;
-		fcp_rsp = (struct fcp_resp_with_ext *)
-				&(fsf->qtcb->bottom.io.fcp_rsp);
+		fcp_rsp = &(fsf->qtcb->bottom.io.fcp_rsp.iu);
 		/* mandatory parts of FCP_RSP IU in this SCSI record */
 		memcpy(&rec->fcp_rsp, fcp_rsp, FCP_RESP_WITH_EXT);
 		if (fcp_rsp->resp.fr_flags & FCP_RSP_LEN_VAL) {
diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index b60667c145fd..33ccf15b63e0 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -301,7 +301,7 @@ bool zfcp_dbf_hba_fsf_resp_suppress(struct zfcp_fsf_req *req)
 
 	if (qtcb->prefix.qtcb_type != FSF_IO_COMMAND)
 		return false; /* not an FCP response */
-	fcp_rsp = (struct fcp_resp *)&qtcb->bottom.io.fcp_rsp;
+	fcp_rsp = &qtcb->bottom.io.fcp_rsp.iu.resp;
 	rsp_flags = fcp_rsp->fr_flags;
 	fr_status = fcp_rsp->fr_status;
 	return (fsf_stat == FSF_FCP_RSP_AVAILABLE) &&
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 9f73b8fc7f3b..6ddaee5f3701 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2143,7 +2143,8 @@ static void zfcp_fsf_fcp_cmnd_handler(struct zfcp_fsf_req *req)
 		zfcp_scsi_dif_sense_error(scpnt, 0x3);
 		goto skip_fsfstatus;
 	}
-	fcp_rsp = (struct fcp_resp_with_ext *) &req->qtcb->bottom.io.fcp_rsp;
+	BUILD_BUG_ON(sizeof(struct fcp_resp_with_ext) > FSF_FCP_RSP_SIZE);
+	fcp_rsp = &req->qtcb->bottom.io.fcp_rsp.iu;
 	zfcp_fc_eval_fcp_rsp(fcp_rsp, scpnt);
 
 skip_fsfstatus:
@@ -2256,7 +2257,8 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
 	if (zfcp_fsf_set_data_dir(scsi_cmnd, &io->data_direction))
 		goto failed_scsi_cmnd;
 
-	fcp_cmnd = (struct fcp_cmnd *) &req->qtcb->bottom.io.fcp_cmnd;
+	BUILD_BUG_ON(sizeof(struct fcp_cmnd) > FSF_FCP_CMND_SIZE);
+	fcp_cmnd = &req->qtcb->bottom.io.fcp_cmnd.iu;
 	zfcp_fc_scsi_to_fcp(fcp_cmnd, scsi_cmnd, 0);
 
 	if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
@@ -2301,7 +2303,7 @@ static void zfcp_fsf_fcp_task_mgmt_handler(struct zfcp_fsf_req *req)
 
 	zfcp_fsf_fcp_handler_common(req);
 
-	fcp_rsp = (struct fcp_resp_with_ext *) &req->qtcb->bottom.io.fcp_rsp;
+	fcp_rsp = &req->qtcb->bottom.io.fcp_rsp.iu;
 	rsp_info = (struct fcp_resp_rsp_info *) &fcp_rsp[1];
 
 	if ((rsp_info->rsp_code != FCP_TMF_CMPL) ||
@@ -2350,7 +2352,7 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_cmnd *scmnd,
 
 	zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
 
-	fcp_cmnd = (struct fcp_cmnd *) &req->qtcb->bottom.io.fcp_cmnd;
+	fcp_cmnd = &req->qtcb->bottom.io.fcp_cmnd.iu;
 	zfcp_fc_scsi_to_fcp(fcp_cmnd, scmnd, tm_flags);
 
 	zfcp_fsf_start_timer(req, ZFCP_SCSI_ER_TIMEOUT);
diff --git a/drivers/s390/scsi/zfcp_fsf.h b/drivers/s390/scsi/zfcp_fsf.h
index ea3c76ac0de1..88feba5bfda4 100644
--- a/drivers/s390/scsi/zfcp_fsf.h
+++ b/drivers/s390/scsi/zfcp_fsf.h
@@ -3,7 +3,7 @@
  *
  * Interface to the FSF support functions.
  *
- * Copyright IBM Corp. 2002, 2016
+ * Copyright IBM Corp. 2002, 2017
  */
 
 #ifndef FSF_H
@@ -312,8 +312,14 @@ struct fsf_qtcb_bottom_io {
 	u32 data_block_length;
 	u32 prot_data_length;
 	u8  res2[4];
-	u8  fcp_cmnd[FSF_FCP_CMND_SIZE];
-	u8  fcp_rsp[FSF_FCP_RSP_SIZE];
+	union {
+		u8		byte[FSF_FCP_CMND_SIZE];
+		struct fcp_cmnd iu;
+	}   fcp_cmnd;
+	union {
+		u8			 byte[FSF_FCP_RSP_SIZE];
+		struct fcp_resp_with_ext iu;
+	}   fcp_rsp;
 	u8  res3[64];
 } __attribute__ ((packed));
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 16/22] zfcp: use endianness conversions with common FC(P) struct fields
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (14 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 15/22] zfcp: use common code fcp_cmnd and fcp_resp with union in fsf_qtcb_bottom_io Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 17/22] zfcp: fix kernel doc comment typos for struct zfcp_dbf_scsi Benjamin Block
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

Just to silence sparse. Since zfcp only exists for s390 and
s390 is big endian, this has been working correctly without conversions
and all the new conversions are NOPs so no performance impact.

Nonetheless, use the conversion on the constant expression where possible.

NB: N_Port-IDs have always been handled with hton24 or ntoh24 conversions
because they also convert to / from character array.

Affected common code structs and .fields are:

HOT I/O PATH:
fcp_cmnd .fc_dl
   FCP command: regular SCSI I/O, including DIX case

SEMI-HOT I/O PATH:
fcp_cmnd .fc_dl
   recovery FCP command: task management function (LUN / target reset)
fcp_resp_ext
   FCP response having FCP_SNS_LEN_VAL with .fr_rsp_len .fr_sns_len
   FCP response having FCP_RESID_UNDER with .fr_resid

RECOVERY / DISCOVERY PATHS:
fc_ct_hdr .ct_cmd .ct_mr_size
   zfcp auto port scan [GPN_FT] with fc_gpn_ft_resp.fp_wwpn,
   recovery for returned port [GID_PN] with fc_ns_gid_pn.fn_wwpn,
   get symbolic port name [GSPN],
   register symbolic port name [RSPN] (NPIV only).
fc_els_rscn .rscn_plen
   incoming ELS (RSCN).
fc_els_flogi .fl_wwpn .fl_wwnn
   incoming ELS (PLOGI),
   port open response with .fl_csp.sp_bb_data .fl_cssp[0..3].cp_class,
   FCP channel physical port,
   point-to-point peer (P2P only).
fc_els_logo .fl_n_port_wwn
   incoming ELS (LOGO).
fc_els_adisc .adisc_wwnn .adisc_wwpn
   path test after RSCN for gone target port.

Since v4.10 commit 05de97003c77 ("linux/types.h: enable endian checks for
all sparse builds"), below sparse endianness reports appear by default.
Previously, one needed to pass argument CF="-D__CHECK_ENDIAN__" to make
as in: $ make C=1 CF="-D__CHECK_ENDIAN__" M=drivers/s390/scsi.

Silenced sparse warnings and one error:

$ make C=1 M=drivers/s390/scsi
...
  CHECK   drivers/s390/scsi/zfcp_dbf.c
drivers/s390/scsi/zfcp_dbf.c:463:22: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_dbf.c:476:28: warning: restricted __be16 degrades to integer
  CC      drivers/s390/scsi/zfcp_dbf.o
...
  CHECK   drivers/s390/scsi/zfcp_fc.c
drivers/s390/scsi/zfcp_fc.c:263:26: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_fc.c:299:41: warning: incorrect type in argument 2 (different base types)
drivers/s390/scsi/zfcp_fc.c:299:41:    expected unsigned long long [unsigned] [usertype] wwpn
drivers/s390/scsi/zfcp_fc.c:299:41:    got restricted __be64 [usertype] fl_wwpn
drivers/s390/scsi/zfcp_fc.c:309:40: warning: incorrect type in argument 2 (different base types)
drivers/s390/scsi/zfcp_fc.c:309:40:    expected unsigned long long [unsigned] [usertype] wwpn
drivers/s390/scsi/zfcp_fc.c:309:40:    got restricted __be64 [usertype] fl_n_port_wwn
drivers/s390/scsi/zfcp_fc.c:338:31: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_fc.c:355:24: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:355:24:    expected restricted __be16 [usertype] ct_cmd
drivers/s390/scsi/zfcp_fc.c:355:24:    got unsigned short [unsigned] [usertype] cmd
drivers/s390/scsi/zfcp_fc.c:356:28: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:356:28:    expected restricted __be16 [usertype] ct_mr_size
drivers/s390/scsi/zfcp_fc.c:356:28:    got int
drivers/s390/scsi/zfcp_fc.c:379:36: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:379:36:    expected restricted __be64 [usertype] fn_wwpn
drivers/s390/scsi/zfcp_fc.c:379:36:    got unsigned long long [unsigned] [usertype] wwpn
drivers/s390/scsi/zfcp_fc.c:463:18: warning: restricted __be64 degrades to integer
drivers/s390/scsi/zfcp_fc.c:465:17: warning: cast from restricted __be64
drivers/s390/scsi/zfcp_fc.c:473:20: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:473:20:    expected unsigned long long [unsigned] [usertype] wwnn
drivers/s390/scsi/zfcp_fc.c:473:20:    got restricted __be64 [usertype] fl_wwnn
drivers/s390/scsi/zfcp_fc.c:474:29: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:474:29:    expected unsigned int [unsigned] [usertype] maxframe_size
drivers/s390/scsi/zfcp_fc.c:474:29:    got restricted __be16 [usertype] sp_bb_data
drivers/s390/scsi/zfcp_fc.c:476:30: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_fc.c:478:30: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_fc.c:480:30: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_fc.c:482:30: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_fc.c:500:28: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:500:28:    expected unsigned long long [unsigned] [usertype] wwnn
drivers/s390/scsi/zfcp_fc.c:500:28:    got restricted __be64 [usertype] adisc_wwnn
drivers/s390/scsi/zfcp_fc.c:502:38: warning: restricted __be64 degrades to integer
drivers/s390/scsi/zfcp_fc.c:541:40: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:541:40:    expected restricted __be64 [usertype] adisc_wwpn
drivers/s390/scsi/zfcp_fc.c:541:40:    got unsigned long long [unsigned] [usertype] port_name
drivers/s390/scsi/zfcp_fc.c:542:40: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fc.c:542:40:    expected restricted __be64 [usertype] adisc_wwnn
drivers/s390/scsi/zfcp_fc.c:542:40:    got unsigned long long [unsigned] [usertype] node_name
drivers/s390/scsi/zfcp_fc.c:669:16: warning: restricted __be16 degrades to integer
drivers/s390/scsi/zfcp_fc.c:696:24: warning: restricted __be64 degrades to integer
drivers/s390/scsi/zfcp_fc.c:699:54: warning: incorrect type in argument 2 (different base types)
drivers/s390/scsi/zfcp_fc.c:699:54:    expected unsigned long long [unsigned] [usertype] <noident>
drivers/s390/scsi/zfcp_fc.c:699:54:    got restricted __be64 [usertype] fp_wwpn
  CC      drivers/s390/scsi/zfcp_fc.o
  CHECK   drivers/s390/scsi/zfcp_fsf.c
drivers/s390/scsi/zfcp_fsf.c:479:34: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fsf.c:479:34:    expected unsigned long long [unsigned] [usertype] port_name
drivers/s390/scsi/zfcp_fsf.c:479:34:    got restricted __be64 [usertype] fl_wwpn
drivers/s390/scsi/zfcp_fsf.c:480:34: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fsf.c:480:34:    expected unsigned long long [unsigned] [usertype] node_name
drivers/s390/scsi/zfcp_fsf.c:480:34:    got restricted __be64 [usertype] fl_wwnn
drivers/s390/scsi/zfcp_fsf.c:506:36: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fsf.c:506:36:    expected unsigned long long [unsigned] [usertype] peer_wwpn
drivers/s390/scsi/zfcp_fsf.c:506:36:    got restricted __be64 [usertype] fl_wwpn
drivers/s390/scsi/zfcp_fsf.c:507:36: warning: incorrect type in assignment (different base types)
drivers/s390/scsi/zfcp_fsf.c:507:36:    expected unsigned long long [unsigned] [usertype] peer_wwnn
drivers/s390/scsi/zfcp_fsf.c:507:36:    got restricted __be64 [usertype] fl_wwnn
drivers/s390/scsi/zfcp_fc.h:269:46: warning: restricted __be32 degrades to integer
drivers/s390/scsi/zfcp_fc.h:270:29: error: incompatible types in comparison expression (different base types)

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c |  5 +++--
 drivers/s390/scsi/zfcp_fc.c  | 46 +++++++++++++++++++++++---------------------
 drivers/s390/scsi/zfcp_fc.h  | 19 +++++++++++-------
 drivers/s390/scsi/zfcp_fsf.c |  8 ++++----
 4 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 225e60d5d3fc..484da0b2d678 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -461,7 +461,7 @@ static u16 zfcp_dbf_san_res_cap_len_if_gpn_ft(char *tag,
 	      && reqh->ct_fs_subtype == FC_NS_SUBTYPE
 	      && reqh->ct_options == 0
 	      && reqh->_ct_resvd1 == 0
-	      && reqh->ct_cmd == FC_NS_GPN_FT
+	      && reqh->ct_cmd == cpu_to_be16(FC_NS_GPN_FT)
 	      /* reqh->ct_mr_size can vary so do not match but read below */
 	      && reqh->_ct_resvd2 == 0
 	      && reqh->ct_reason == 0
@@ -481,7 +481,8 @@ static u16 zfcp_dbf_san_res_cap_len_if_gpn_ft(char *tag,
 	    (resph->ct_cmd != cpu_to_be16(FC_FS_ACC)))
 		return max(FC_CT_HDR_LEN, ZFCP_DBF_SAN_MAX_PAYLOAD);
 
-	max_entries = (reqh->ct_mr_size * 4 / sizeof(struct fc_gpn_ft_resp))
+	max_entries = (be16_to_cpu(reqh->ct_mr_size) * 4 /
+		       sizeof(struct fc_gpn_ft_resp))
 		+ 1 /* zfcp_fc_scan_ports: bytes correct, entries off-by-one
 		     * to account for header as 1st pseudo "entry" */;
 
diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index cc3f378782b2..3e715597b739 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -260,7 +260,8 @@ static void zfcp_fc_incoming_rscn(struct zfcp_fsf_req *fsf_req)
 	page = (struct fc_els_rscn_page *) head;
 
 	/* see FC-FS */
-	no_entries = head->rscn_plen / sizeof(struct fc_els_rscn_page);
+	no_entries = be16_to_cpu(head->rscn_plen) /
+		sizeof(struct fc_els_rscn_page);
 
 	for (i = 1; i < no_entries; i++) {
 		/* skip head and start with 1st element */
@@ -296,7 +297,7 @@ static void zfcp_fc_incoming_plogi(struct zfcp_fsf_req *req)
 
 	status_buffer = (struct fsf_status_read_buffer *) req->data;
 	plogi = (struct fc_els_flogi *) status_buffer->payload.data;
-	zfcp_fc_incoming_wwpn(req, plogi->fl_wwpn);
+	zfcp_fc_incoming_wwpn(req, be64_to_cpu(plogi->fl_wwpn));
 }
 
 static void zfcp_fc_incoming_logo(struct zfcp_fsf_req *req)
@@ -306,7 +307,7 @@ static void zfcp_fc_incoming_logo(struct zfcp_fsf_req *req)
 	struct fc_els_logo *logo =
 		(struct fc_els_logo *) status_buffer->payload.data;
 
-	zfcp_fc_incoming_wwpn(req, logo->fl_n_port_wwn);
+	zfcp_fc_incoming_wwpn(req, be64_to_cpu(logo->fl_n_port_wwn));
 }
 
 /**
@@ -335,7 +336,7 @@ static void zfcp_fc_ns_gid_pn_eval(struct zfcp_fc_req *fc_req)
 
 	if (ct_els->status)
 		return;
-	if (gid_pn_rsp->ct_hdr.ct_cmd != FC_FS_ACC)
+	if (gid_pn_rsp->ct_hdr.ct_cmd != cpu_to_be16(FC_FS_ACC))
 		return;
 
 	/* looks like a valid d_id */
@@ -352,8 +353,8 @@ static void zfcp_fc_ct_ns_init(struct fc_ct_hdr *ct_hdr, u16 cmd, u16 mr_size)
 	ct_hdr->ct_rev = FC_CT_REV;
 	ct_hdr->ct_fs_type = FC_FST_DIR;
 	ct_hdr->ct_fs_subtype = FC_NS_SUBTYPE;
-	ct_hdr->ct_cmd = cmd;
-	ct_hdr->ct_mr_size = mr_size / 4;
+	ct_hdr->ct_cmd = cpu_to_be16(cmd);
+	ct_hdr->ct_mr_size = cpu_to_be16(mr_size / 4);
 }
 
 static int zfcp_fc_ns_gid_pn_request(struct zfcp_port *port,
@@ -376,7 +377,7 @@ static int zfcp_fc_ns_gid_pn_request(struct zfcp_port *port,
 
 	zfcp_fc_ct_ns_init(&gid_pn_req->ct_hdr,
 			   FC_NS_GID_PN, ZFCP_FC_CT_SIZE_PAGE);
-	gid_pn_req->gid_pn.fn_wwpn = port->wwpn;
+	gid_pn_req->gid_pn.fn_wwpn = cpu_to_be64(port->wwpn);
 
 	ret = zfcp_fsf_send_ct(&adapter->gs->ds, &fc_req->ct_els,
 			       adapter->pool.gid_pn_req,
@@ -460,26 +461,26 @@ void zfcp_fc_trigger_did_lookup(struct zfcp_port *port)
  */
 void zfcp_fc_plogi_evaluate(struct zfcp_port *port, struct fc_els_flogi *plogi)
 {
-	if (plogi->fl_wwpn != port->wwpn) {
+	if (be64_to_cpu(plogi->fl_wwpn) != port->wwpn) {
 		port->d_id = 0;
 		dev_warn(&port->adapter->ccw_device->dev,
 			 "A port opened with WWPN 0x%016Lx returned data that "
 			 "identifies it as WWPN 0x%016Lx\n",
 			 (unsigned long long) port->wwpn,
-			 (unsigned long long) plogi->fl_wwpn);
+			 (unsigned long long) be64_to_cpu(plogi->fl_wwpn));
 		return;
 	}
 
-	port->wwnn = plogi->fl_wwnn;
-	port->maxframe_size = plogi->fl_csp.sp_bb_data;
+	port->wwnn = be64_to_cpu(plogi->fl_wwnn);
+	port->maxframe_size = be16_to_cpu(plogi->fl_csp.sp_bb_data);
 
-	if (plogi->fl_cssp[0].cp_class & FC_CPC_VALID)
+	if (plogi->fl_cssp[0].cp_class & cpu_to_be16(FC_CPC_VALID))
 		port->supported_classes |= FC_COS_CLASS1;
-	if (plogi->fl_cssp[1].cp_class & FC_CPC_VALID)
+	if (plogi->fl_cssp[1].cp_class & cpu_to_be16(FC_CPC_VALID))
 		port->supported_classes |= FC_COS_CLASS2;
-	if (plogi->fl_cssp[2].cp_class & FC_CPC_VALID)
+	if (plogi->fl_cssp[2].cp_class & cpu_to_be16(FC_CPC_VALID))
 		port->supported_classes |= FC_COS_CLASS3;
-	if (plogi->fl_cssp[3].cp_class & FC_CPC_VALID)
+	if (plogi->fl_cssp[3].cp_class & cpu_to_be16(FC_CPC_VALID))
 		port->supported_classes |= FC_COS_CLASS4;
 }
 
@@ -497,9 +498,9 @@ static void zfcp_fc_adisc_handler(void *data)
 	}
 
 	if (!port->wwnn)
-		port->wwnn = adisc_resp->adisc_wwnn;
+		port->wwnn = be64_to_cpu(adisc_resp->adisc_wwnn);
 
-	if ((port->wwpn != adisc_resp->adisc_wwpn) ||
+	if ((port->wwpn != be64_to_cpu(adisc_resp->adisc_wwpn)) ||
 	    !(atomic_read(&port->status) & ZFCP_STATUS_COMMON_OPEN)) {
 		zfcp_erp_port_reopen(port, ZFCP_STATUS_COMMON_ERP_FAILED,
 				     "fcadh_2");
@@ -538,8 +539,8 @@ static int zfcp_fc_adisc(struct zfcp_port *port)
 
 	/* acc. to FC-FS, hard_nport_id in ADISC should not be set for ports
 	   without FC-AL-2 capability, so we don't set it */
-	fc_req->u.adisc.req.adisc_wwpn = fc_host_port_name(shost);
-	fc_req->u.adisc.req.adisc_wwnn = fc_host_node_name(shost);
+	fc_req->u.adisc.req.adisc_wwpn = cpu_to_be64(fc_host_port_name(shost));
+	fc_req->u.adisc.req.adisc_wwnn = cpu_to_be64(fc_host_node_name(shost));
 	fc_req->u.adisc.req.adisc_cmd = ELS_ADISC;
 	hton24(fc_req->u.adisc.req.adisc_port_id, fc_host_port_id(shost));
 
@@ -666,7 +667,7 @@ static int zfcp_fc_eval_gpn_ft(struct zfcp_fc_req *fc_req,
 	if (ct_els->status)
 		return -EIO;
 
-	if (hdr->ct_cmd != FC_FS_ACC) {
+	if (hdr->ct_cmd != cpu_to_be16(FC_FS_ACC)) {
 		if (hdr->ct_reason == FC_FS_RJT_UNABL)
 			return -EAGAIN; /* might be a temporary condition */
 		return -EIO;
@@ -693,10 +694,11 @@ static int zfcp_fc_eval_gpn_ft(struct zfcp_fc_req *fc_req,
 		if (d_id >= FC_FID_WELL_KNOWN_BASE)
 			continue;
 		/* skip the adapter's port and known remote ports */
-		if (acc->fp_wwpn == fc_host_port_name(adapter->scsi_host))
+		if (be64_to_cpu(acc->fp_wwpn) ==
+		    fc_host_port_name(adapter->scsi_host))
 			continue;
 
-		port = zfcp_port_enqueue(adapter, acc->fp_wwpn,
+		port = zfcp_port_enqueue(adapter, be64_to_cpu(acc->fp_wwpn),
 					 ZFCP_STATUS_COMMON_NOESC, d_id);
 		if (!IS_ERR(port))
 			zfcp_erp_port_reopen(port, 0, "fcegpf1");
diff --git a/drivers/s390/scsi/zfcp_fc.h b/drivers/s390/scsi/zfcp_fc.h
index a2275825186f..41f22d3dc6d1 100644
--- a/drivers/s390/scsi/zfcp_fc.h
+++ b/drivers/s390/scsi/zfcp_fc.h
@@ -212,6 +212,8 @@ static inline
 void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct scsi_cmnd *scsi,
 			 u8 tm_flags)
 {
+	u32 datalen;
+
 	int_to_scsilun(scsi->device->lun, (struct scsi_lun *) &fcp->fc_lun);
 
 	if (unlikely(tm_flags)) {
@@ -228,10 +230,13 @@ void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct scsi_cmnd *scsi,
 
 	memcpy(fcp->fc_cdb, scsi->cmnd, scsi->cmd_len);
 
-	fcp->fc_dl = scsi_bufflen(scsi);
+	datalen = scsi_bufflen(scsi);
+	fcp->fc_dl = cpu_to_be32(datalen);
 
-	if (scsi_get_prot_type(scsi) == SCSI_PROT_DIF_TYPE1)
-		fcp->fc_dl += fcp->fc_dl / scsi->device->sector_size * 8;
+	if (scsi_get_prot_type(scsi) == SCSI_PROT_DIF_TYPE1) {
+		datalen += datalen / scsi->device->sector_size * 8;
+		fcp->fc_dl = cpu_to_be32(datalen);
+	}
 }
 
 /**
@@ -266,14 +271,14 @@ void zfcp_fc_eval_fcp_rsp(struct fcp_resp_with_ext *fcp_rsp,
 	if (unlikely(rsp_flags & FCP_SNS_LEN_VAL)) {
 		sense = (char *) &fcp_rsp[1];
 		if (rsp_flags & FCP_RSP_LEN_VAL)
-			sense += fcp_rsp->ext.fr_rsp_len;
-		sense_len = min(fcp_rsp->ext.fr_sns_len,
-				(u32) SCSI_SENSE_BUFFERSIZE);
+			sense += be32_to_cpu(fcp_rsp->ext.fr_rsp_len);
+		sense_len = min_t(u32, be32_to_cpu(fcp_rsp->ext.fr_sns_len),
+				  SCSI_SENSE_BUFFERSIZE);
 		memcpy(scsi->sense_buffer, sense, sense_len);
 	}
 
 	if (unlikely(rsp_flags & FCP_RESID_UNDER)) {
-		resid = fcp_rsp->ext.fr_resid;
+		resid = be32_to_cpu(fcp_rsp->ext.fr_resid);
 		scsi_set_resid(scsi, resid);
 		if (scsi_bufflen(scsi) - resid < scsi->underflow &&
 		     !(rsp_flags & FCP_SNS_LEN_VAL) &&
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 6ddaee5f3701..cc923c71a0fa 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -476,8 +476,8 @@ static int zfcp_fsf_exchange_config_evaluate(struct zfcp_fsf_req *req)
 	if (req->data)
 		memcpy(req->data, bottom, sizeof(*bottom));
 
-	fc_host_port_name(shost) = nsp->fl_wwpn;
-	fc_host_node_name(shost) = nsp->fl_wwnn;
+	fc_host_port_name(shost) = be64_to_cpu(nsp->fl_wwpn);
+	fc_host_node_name(shost) = be64_to_cpu(nsp->fl_wwnn);
 	fc_host_supported_classes(shost) = FC_COS_CLASS2 | FC_COS_CLASS3;
 
 	adapter->timer_ticks = bottom->timer_interval & ZFCP_FSF_TIMER_INT_MASK;
@@ -503,8 +503,8 @@ static int zfcp_fsf_exchange_config_evaluate(struct zfcp_fsf_req *req)
 	switch (bottom->fc_topology) {
 	case FSF_TOPO_P2P:
 		adapter->peer_d_id = ntoh24(bottom->peer_d_id);
-		adapter->peer_wwpn = plogi->fl_wwpn;
-		adapter->peer_wwnn = plogi->fl_wwnn;
+		adapter->peer_wwpn = be64_to_cpu(plogi->fl_wwpn);
+		adapter->peer_wwnn = be64_to_cpu(plogi->fl_wwnn);
 		fc_host_port_type(shost) = FC_PORTTYPE_PTP;
 		break;
 	case FSF_TOPO_FABRIC:
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 17/22] zfcp: fix kernel doc comment typos for struct zfcp_dbf_scsi
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (15 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 16/22] zfcp: use endianness conversions with common FC(P) struct fields Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 18/22] zfcp: clean up redundant code with fall through in link down SRB switch case Benjamin Block
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

Improves commit 250a1352b95e ("[SCSI] zfcp: Redesign of the debug tracing
for SCSI records.")

Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index 33ccf15b63e0..3508c00458f4 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -208,12 +208,12 @@ enum zfcp_dbf_scsi_id {
  * @scsi_result: scsi result
  * @scsi_retries: current retry number of scsi request
  * @scsi_allowed: allowed retries
- * @fcp_rsp_info: FCP response info
+ * @fcp_rsp_info: FCP response info code
  * @scsi_opcode: scsi opcode
  * @fsf_req_id: request id of fsf request
  * @host_scribble: LLD specific data attached to SCSI request
- * @pl_len: length of paload stored as zfcp_dbf_pay
- * @fsf_rsp: response for fsf request
+ * @pl_len: length of payload stored as zfcp_dbf_pay
+ * @fcp_rsp: response for FCP request
  * @scsi_lun_64_hi: scsi device logical unit number, high part of 64 bit
  */
 struct zfcp_dbf_scsi {
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 18/22] zfcp: clean up redundant code with fall through in link down SRB switch case
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (16 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 17/22] zfcp: fix kernel doc comment typos for struct zfcp_dbf_scsi Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 19/22] zfcp: clean up no longer existent prototype from zfcp API header Benjamin Block
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Martin Peschke, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390, Benjamin Block

From: Martin Peschke <mpeschke@linux.vnet.ibm.com>

Signed-off-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>
[maier@linux.vnet.ibm.com: re-worded short description for more details]
Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fsf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index cc923c71a0fa..eefb474a9e42 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -197,8 +197,6 @@ static void zfcp_fsf_status_read_link_down(struct zfcp_fsf_req *req)
 
 	switch (sr_buf->status_subtype) {
 	case FSF_STATUS_READ_SUB_NO_PHYSICAL_LINK:
-		zfcp_fsf_link_down_info_eval(req, ldi);
-		break;
 	case FSF_STATUS_READ_SUB_FDISC_FAILED:
 		zfcp_fsf_link_down_info_eval(req, ldi);
 		break;
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 19/22] zfcp: clean up no longer existent prototype from zfcp API header
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (17 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 18/22] zfcp: clean up redundant code with fall through in link down SRB switch case Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 20/22] zfcp: clean up a member of struct zfcp_qdio that was assigned but never used Benjamin Block
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Steffen Maier, Martin Schwidefsky, Heiko Carstens, linux-scsi,
	linux-s390, Martin Peschke, Benjamin Block

From: Steffen Maier <maier@linux.vnet.ibm.com>

Commit a54ca0f62f95 ("[SCSI] zfcp: Redesign of the debug tracing for HBA
records.") refactored zfcp_dbf_hba_berr into zfcp_dbf_hba_bit_err
but added the prototype for the latter without removing it for the former.

Suggested-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>
Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_ext.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h
index 9afdbc32b23f..a9e968717dd9 100644
--- a/drivers/s390/scsi/zfcp_ext.h
+++ b/drivers/s390/scsi/zfcp_ext.h
@@ -41,7 +41,6 @@ extern void zfcp_dbf_rec_run_wka(char *, struct zfcp_fc_wka_port *, u64);
 extern void zfcp_dbf_hba_fsf_uss(char *, struct zfcp_fsf_req *);
 extern void zfcp_dbf_hba_fsf_res(char *, int, struct zfcp_fsf_req *);
 extern void zfcp_dbf_hba_bit_err(char *, struct zfcp_fsf_req *);
-extern void zfcp_dbf_hba_berr(struct zfcp_dbf *, struct zfcp_fsf_req *);
 extern void zfcp_dbf_hba_def_err(struct zfcp_adapter *, u64, u16, void **);
 extern void zfcp_dbf_hba_basic(char *, struct zfcp_adapter *);
 extern void zfcp_dbf_san_req(char *, struct zfcp_fsf_req *, u32);
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 20/22] zfcp: clean up a member of struct zfcp_qdio that was assigned but never used
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (18 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 19/22] zfcp: clean up no longer existent prototype from zfcp API header Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 21/22] zfcp: clean up unnecessary module_param_named() with no_auto_port_rescan Benjamin Block
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Martin Peschke, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390, Benjamin Block

From: Martin Peschke <mpeschke@linux.vnet.ibm.com>

v2.6.38 commit a54ca0f62f95 ("[SCSI] zfcp: Redesign of the debug tracing
for HBA records.")
dropped trace information previously introduced with
v2.6.27 commit c3baa9a26c5a ("[SCSI] zfcp: Add information about interrupt
to trace.")
but kept and needlessly assigned a now no longer used struct field.

Signed-off-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>
[maier@linux.vnet.ibm.com: reword, added git history]
Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fsf.c  | 1 -
 drivers/s390/scsi/zfcp_qdio.h | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index eefb474a9e42..69d1dc3ec79d 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2394,7 +2394,6 @@ void zfcp_fsf_reqid_check(struct zfcp_qdio *qdio, int sbal_idx)
 			      req_id, dev_name(&adapter->ccw_device->dev));
 		}
 
-		fsf_req->qdio_req.sbal_response = sbal_idx;
 		zfcp_fsf_req_complete(fsf_req);
 
 		if (likely(sbale->eflags & SBAL_EFLAGS_LAST_ENTRY))
diff --git a/drivers/s390/scsi/zfcp_qdio.h b/drivers/s390/scsi/zfcp_qdio.h
index 85cdb82127e8..7f647a90c750 100644
--- a/drivers/s390/scsi/zfcp_qdio.h
+++ b/drivers/s390/scsi/zfcp_qdio.h
@@ -54,7 +54,6 @@ struct zfcp_qdio {
  * @sbal_last: last sbal for this request
  * @sbal_limit: last possible sbal for this request
  * @sbale_curr: current sbale at creation of this request
- * @sbal_response: sbal used in interrupt
  * @qdio_outb_usage: usage of outbound queue
  */
 struct zfcp_qdio_req {
@@ -64,7 +63,6 @@ struct zfcp_qdio_req {
 	u8	sbal_last;
 	u8	sbal_limit;
 	u8	sbale_curr;
-	u8	sbal_response;
 	u16	qdio_outb_usage;
 };
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 21/22] zfcp: clean up unnecessary module_param_named() with no_auto_port_rescan
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (19 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 20/22] zfcp: clean up a member of struct zfcp_qdio that was assigned but never used Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-07-28 10:31 ` [PATCH 22/22] zfcp: early returns for traces disabled via level Benjamin Block
  2017-08-07 17:19 ` [PATCH 00/22] zfcp fixes and cleanups Martin K. Petersen
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Martin Peschke, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390, Benjamin Block

From: Martin Peschke <mpeschke@linux.vnet.ibm.com>

Improves commit 43f60cbd56c4 ("[SCSI] zfcp: No automatic port_rescan on
events")

Signed-off-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>
[maier@linux.vnet.ibm.com: reword, underscore in description to match sysfs]
Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index 3e715597b739..8210645c2111 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -29,7 +29,7 @@ static u32 zfcp_fc_rscn_range_mask[] = {
 };
 
 static bool no_auto_port_rescan;
-module_param_named(no_auto_port_rescan, no_auto_port_rescan, bool, 0600);
+module_param(no_auto_port_rescan, bool, 0600);
 MODULE_PARM_DESC(no_auto_port_rescan,
 		 "no automatic port_rescan (default off)");
 
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 22/22] zfcp: early returns for traces disabled via level
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (20 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 21/22] zfcp: clean up unnecessary module_param_named() with no_auto_port_rescan Benjamin Block
@ 2017-07-28 10:31 ` Benjamin Block
  2017-08-07 17:19 ` [PATCH 00/22] zfcp fixes and cleanups Martin K. Petersen
  22 siblings, 0 replies; 26+ messages in thread
From: Benjamin Block @ 2017-07-28 10:31 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen
  Cc: Martin Peschke, Steffen Maier, Martin Schwidefsky, Heiko Carstens,
	linux-scsi, linux-s390, Benjamin Block

From: Martin Peschke <mpeschke@linux.vnet.ibm.com>

This patch adds early checks to avoid burning CPU cycles on
the assembly of trace entries which would be skipped anyway.

Introduce a static const variable to keep the trace level to check with
debug_level_enabled() in sync with the actual trace emit with
debug_event(). In order not to refactor the SAN tracing too much,
simply use a define instead.

This change is only for the non / semi hot paths,
while the actual (I/O) hot path was already improved earlier:
zfcp_dbf_scsi() is already guarded by its only caller _zfcp_dbf_scsi()
since commit dcd20e2316cd ("[SCSI] zfcp: Only collect SCSI debug data for
matching trace levels").
zfcp_dbf_hba_fsf_res() is already guarded by its only caller
zfcp_dbf_hba_fsf_response() since commit 2e261af84cdb ("[SCSI] zfcp: Only
collect FSF/HBA debug data for matching trace levels").

Signed-off-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>
[maier@linux.vnet.ibm.com: rebase, reword, default level 3 branch prediction]
Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c | 54 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 484da0b2d678..8227076c9cbb 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -113,8 +113,12 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req)
 	struct zfcp_dbf *dbf = req->adapter->dbf;
 	struct fsf_status_read_buffer *srb = req->data;
 	struct zfcp_dbf_hba *rec = &dbf->hba_buf;
+	static int const level = 2;
 	unsigned long flags;
 
+	if (unlikely(!debug_level_enabled(dbf->hba, level)))
+		return;
+
 	spin_lock_irqsave(&dbf->hba_lock, flags);
 	memset(rec, 0, sizeof(*rec));
 
@@ -142,7 +146,7 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req)
 		zfcp_dbf_pl_write(dbf, srb->payload.data, rec->pl_len,
 				  "fsf_uss", req->req_id);
 log:
-	debug_event(dbf->hba, 2, rec, sizeof(*rec));
+	debug_event(dbf->hba, level, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->hba_lock, flags);
 }
 
@@ -156,8 +160,12 @@ void zfcp_dbf_hba_bit_err(char *tag, struct zfcp_fsf_req *req)
 	struct zfcp_dbf *dbf = req->adapter->dbf;
 	struct zfcp_dbf_hba *rec = &dbf->hba_buf;
 	struct fsf_status_read_buffer *sr_buf = req->data;
+	static int const level = 1;
 	unsigned long flags;
 
+	if (unlikely(!debug_level_enabled(dbf->hba, level)))
+		return;
+
 	spin_lock_irqsave(&dbf->hba_lock, flags);
 	memset(rec, 0, sizeof(*rec));
 
@@ -169,7 +177,7 @@ void zfcp_dbf_hba_bit_err(char *tag, struct zfcp_fsf_req *req)
 	memcpy(&rec->u.be, &sr_buf->payload.bit_error,
 	       sizeof(struct fsf_bit_error_payload));
 
-	debug_event(dbf->hba, 1, rec, sizeof(*rec));
+	debug_event(dbf->hba, level, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->hba_lock, flags);
 }
 
@@ -186,8 +194,12 @@ void zfcp_dbf_hba_def_err(struct zfcp_adapter *adapter, u64 req_id, u16 scount,
 	struct zfcp_dbf *dbf = adapter->dbf;
 	struct zfcp_dbf_pay *payload = &dbf->pay_buf;
 	unsigned long flags;
+	static int const level = 1;
 	u16 length;
 
+	if (unlikely(!debug_level_enabled(dbf->pay, level)))
+		return;
+
 	if (!pl)
 		return;
 
@@ -202,7 +214,7 @@ void zfcp_dbf_hba_def_err(struct zfcp_adapter *adapter, u64 req_id, u16 scount,
 
 	while (payload->counter < scount && (char *)pl[payload->counter]) {
 		memcpy(payload->data, (char *)pl[payload->counter], length);
-		debug_event(dbf->pay, 1, payload, zfcp_dbf_plen(length));
+		debug_event(dbf->pay, level, payload, zfcp_dbf_plen(length));
 		payload->counter++;
 	}
 
@@ -217,15 +229,19 @@ void zfcp_dbf_hba_basic(char *tag, struct zfcp_adapter *adapter)
 {
 	struct zfcp_dbf *dbf = adapter->dbf;
 	struct zfcp_dbf_hba *rec = &dbf->hba_buf;
+	static int const level = 1;
 	unsigned long flags;
 
+	if (unlikely(!debug_level_enabled(dbf->hba, level)))
+		return;
+
 	spin_lock_irqsave(&dbf->hba_lock, flags);
 	memset(rec, 0, sizeof(*rec));
 
 	memcpy(rec->tag, tag, ZFCP_DBF_TAG_LEN);
 	rec->id = ZFCP_DBF_HBA_BASIC;
 
-	debug_event(dbf->hba, 1, rec, sizeof(*rec));
+	debug_event(dbf->hba, level, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->hba_lock, flags);
 }
 
@@ -264,9 +280,13 @@ void zfcp_dbf_rec_trig(char *tag, struct zfcp_adapter *adapter,
 {
 	struct zfcp_dbf *dbf = adapter->dbf;
 	struct zfcp_dbf_rec *rec = &dbf->rec_buf;
+	static int const level = 1;
 	struct list_head *entry;
 	unsigned long flags;
 
+	if (unlikely(!debug_level_enabled(dbf->rec, level)))
+		return;
+
 	spin_lock_irqsave(&dbf->rec_lock, flags);
 	memset(rec, 0, sizeof(*rec));
 
@@ -283,7 +303,7 @@ void zfcp_dbf_rec_trig(char *tag, struct zfcp_adapter *adapter,
 	rec->u.trig.want = want;
 	rec->u.trig.need = need;
 
-	debug_event(dbf->rec, 1, rec, sizeof(*rec));
+	debug_event(dbf->rec, level, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->rec_lock, flags);
 }
 
@@ -300,6 +320,9 @@ void zfcp_dbf_rec_run_lvl(int level, char *tag, struct zfcp_erp_action *erp)
 	struct zfcp_dbf_rec *rec = &dbf->rec_buf;
 	unsigned long flags;
 
+	if (!debug_level_enabled(dbf->rec, level))
+		return;
+
 	spin_lock_irqsave(&dbf->rec_lock, flags);
 	memset(rec, 0, sizeof(*rec));
 
@@ -345,8 +368,12 @@ void zfcp_dbf_rec_run_wka(char *tag, struct zfcp_fc_wka_port *wka_port,
 {
 	struct zfcp_dbf *dbf = wka_port->adapter->dbf;
 	struct zfcp_dbf_rec *rec = &dbf->rec_buf;
+	static int const level = 1;
 	unsigned long flags;
 
+	if (unlikely(!debug_level_enabled(dbf->rec, level)))
+		return;
+
 	spin_lock_irqsave(&dbf->rec_lock, flags);
 	memset(rec, 0, sizeof(*rec));
 
@@ -362,10 +389,12 @@ void zfcp_dbf_rec_run_wka(char *tag, struct zfcp_fc_wka_port *wka_port,
 	rec->u.run.rec_action = ~0;
 	rec->u.run.rec_count = ~0;
 
-	debug_event(dbf->rec, 1, rec, sizeof(*rec));
+	debug_event(dbf->rec, level, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->rec_lock, flags);
 }
 
+#define ZFCP_DBF_SAN_LEVEL 1
+
 static inline
 void zfcp_dbf_san(char *tag, struct zfcp_dbf *dbf,
 		  char *paytag, struct scatterlist *sg, u8 id, u16 len,
@@ -408,7 +437,7 @@ void zfcp_dbf_san(char *tag, struct zfcp_dbf *dbf,
 				      (u16)(sg->length - offset));
 			/* cap_len <= pay_sum < cap_len+ZFCP_DBF_PAY_MAX_REC */
 			memcpy(payload->data, sg_virt(sg) + offset, pay_len);
-			debug_event(dbf->pay, 1, payload,
+			debug_event(dbf->pay, ZFCP_DBF_SAN_LEVEL, payload,
 				    zfcp_dbf_plen(pay_len));
 			payload->counter++;
 			offset += pay_len;
@@ -418,7 +447,7 @@ void zfcp_dbf_san(char *tag, struct zfcp_dbf *dbf,
 	spin_unlock(&dbf->pay_lock);
 
 out:
-	debug_event(dbf->san, 1, rec, sizeof(*rec));
+	debug_event(dbf->san, ZFCP_DBF_SAN_LEVEL, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->san_lock, flags);
 }
 
@@ -434,6 +463,9 @@ void zfcp_dbf_san_req(char *tag, struct zfcp_fsf_req *fsf, u32 d_id)
 	struct zfcp_fsf_ct_els *ct_els = fsf->data;
 	u16 length;
 
+	if (unlikely(!debug_level_enabled(dbf->san, ZFCP_DBF_SAN_LEVEL)))
+		return;
+
 	length = (u16)zfcp_qdio_real_bytes(ct_els->req);
 	zfcp_dbf_san(tag, dbf, "san_req", ct_els->req, ZFCP_DBF_SAN_REQ,
 		     length, fsf->req_id, d_id, length);
@@ -512,6 +544,9 @@ void zfcp_dbf_san_res(char *tag, struct zfcp_fsf_req *fsf)
 	struct zfcp_fsf_ct_els *ct_els = fsf->data;
 	u16 length;
 
+	if (unlikely(!debug_level_enabled(dbf->san, ZFCP_DBF_SAN_LEVEL)))
+		return;
+
 	length = (u16)zfcp_qdio_real_bytes(ct_els->resp);
 	zfcp_dbf_san(tag, dbf, "san_res", ct_els->resp, ZFCP_DBF_SAN_RES,
 		     length, fsf->req_id, ct_els->d_id,
@@ -531,6 +566,9 @@ void zfcp_dbf_san_in_els(char *tag, struct zfcp_fsf_req *fsf)
 	u16 length;
 	struct scatterlist sg;
 
+	if (unlikely(!debug_level_enabled(dbf->san, ZFCP_DBF_SAN_LEVEL)))
+		return;
+
 	length = (u16)(srb->length -
 			offsetof(struct fsf_status_read_buffer, payload));
 	sg_init_one(&sg, srb->payload.data, length);
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/22] zfcp fixes and cleanups
  2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
                   ` (21 preceding siblings ...)
  2017-07-28 10:31 ` [PATCH 22/22] zfcp: early returns for traces disabled via level Benjamin Block
@ 2017-08-07 17:19 ` Martin K. Petersen
  2017-08-08 10:02   ` Benjamin Block
  22 siblings, 1 reply; 26+ messages in thread
From: Martin K. Petersen @ 2017-08-07 17:19 UTC (permalink / raw)
  To: Benjamin Block
  Cc: James E . J . Bottomley, Martin K . Petersen, Steffen Maier,
	Martin Schwidefsky, Heiko Carstens, linux-scsi, linux-s390


Hi Benjamin,

> here is a series of (important) fixes and some additional cleanups for
> the zfcp driver. Our fixes are marked for stable accordingly.
>
> Patches 01 - 04 are cleanups and external patches (also cleanups) that we
>       & 13 - 22     had queued for quite some time now
>
> Patches 05 - 12 are driver fixes that are all marked for stable as well

Given the ordering, it's a bit unclear how you want me to apply these.

There are a bunch of fixes you labeled as "important" but they are not
at the beginning of the series. And I am not sure if these have
dependencies on the preceding cleanup patches?

If you have fixes you would like to get into 4.13, please post those
first as a separate series. And clearly indicate that that's the branch
you are aiming for.

If you are OK with everything going into 4.14, then let me know. In that
case you don't have to shuffle the patches around.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/22] zfcp fixes and cleanups
  2017-08-07 17:19 ` [PATCH 00/22] zfcp fixes and cleanups Martin K. Petersen
@ 2017-08-08 10:02   ` Benjamin Block
  2017-08-08 16:07     ` Martin K. Petersen
  0 siblings, 1 reply; 26+ messages in thread
From: Benjamin Block @ 2017-08-08 10:02 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: James E . J . Bottomley, Steffen Maier, Martin Schwidefsky,
	Heiko Carstens, linux-scsi, linux-s390

On Mon, Aug 07, 2017 at 01:19:11PM -0400, Martin K. Petersen wrote:
> > here is a series of (important) fixes and some additional cleanups for
> > the zfcp driver. Our fixes are marked for stable accordingly.
> >
> > Patches 01 - 04 are cleanups and external patches (also cleanups) that we
> >       & 13 - 22     had queued for quite some time now
> >
> > Patches 05 - 12 are driver fixes that are all marked for stable as well
> 
> Given the ordering, it's a bit unclear how you want me to apply these.
> 
> There are a bunch of fixes you labeled as "important" but they are not
> at the beginning of the series. And I am not sure if these have
> dependencies on the preceding cleanup patches?
> 
> If you have fixes you would like to get into 4.13, please post those
> first as a separate series. And clearly indicate that that's the branch
> you are aiming for.
> 
> If you are OK with everything going into 4.14, then let me know. In that
> case you don't have to shuffle the patches around.
> 

I think it is OK for us, if this goes into 4.14 as one series. So if it
is OK with you, just apply the complete series for that.

Thanks.



                                                    Beste Grüße / Best regards,
                                                      - Benjamin Block
-- 
Linux on z Systems Development         /         IBM Systems & Technology Group
		  IBM Deutschland Research & Development GmbH 
Vorsitz. AufsR.: Martina Koederitz     /        Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/22] zfcp fixes and cleanups
  2017-08-08 10:02   ` Benjamin Block
@ 2017-08-08 16:07     ` Martin K. Petersen
  0 siblings, 0 replies; 26+ messages in thread
From: Martin K. Petersen @ 2017-08-08 16:07 UTC (permalink / raw)
  To: Benjamin Block
  Cc: Martin K. Petersen, James E . J . Bottomley, Steffen Maier,
	Martin Schwidefsky, Heiko Carstens, linux-scsi, linux-s390


Benjamin,

>> If you are OK with everything going into 4.14, then let me know. In
>> that case you don't have to shuffle the patches around.

> I think it is OK for us, if this goes into 4.14 as one series. So if
> it is OK with you, just apply the complete series for that.

OK. Applied to 4.14/scsi-queue. Thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2017-08-08 16:08 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-28 10:30 [PATCH 00/22] zfcp fixes and cleanups Benjamin Block
2017-07-28 10:30 ` [PATCH 01/22] zfcp: replace zfcp_qdio_sbale_count by sg_nents Benjamin Block
2017-07-28 10:30 ` [PATCH 02/22] zfcp: use setup_timer instead of init_timer Benjamin Block
2017-07-28 10:30 ` [PATCH 03/22] zfcp: Remove unneeded linux/miscdevice.h include Benjamin Block
2017-07-28 10:30 ` [PATCH 04/22] zfcp: convert bool-definitions to use 'true' instead of '1' Benjamin Block
2017-07-28 10:30 ` [PATCH 05/22] zfcp: fix queuecommand for scsi_eh commands when DIX enabled Benjamin Block
2017-07-28 10:30 ` [PATCH 06/22] zfcp: add handling for FCP_RESID_OVER to the fcp ingress path Benjamin Block
2017-07-28 10:30 ` [PATCH 07/22] zfcp: fix capping of unsuccessful GPN_FT SAN response trace records Benjamin Block
2017-07-28 10:30 ` [PATCH 08/22] zfcp: fix passing fsf_req to SCSI trace on TMF to correlate with HBA Benjamin Block
2017-07-28 10:30 ` [PATCH 09/22] zfcp: fix missing trace records for early returns in TMF eh handlers Benjamin Block
2017-07-28 10:30 ` [PATCH 10/22] zfcp: fix payload with full FCP_RSP IU in SCSI trace records Benjamin Block
2017-07-28 10:30 ` [PATCH 11/22] zfcp: trace HBA FSF response by default on dismiss or timedout late response Benjamin Block
2017-07-28 10:30 ` [PATCH 12/22] zfcp: trace high part of "new" 64 bit SCSI LUN Benjamin Block
2017-07-28 10:30 ` [PATCH 13/22] zfcp: more fitting constant for fc_ct_hdr.ct_reason on port scan response Benjamin Block
2017-07-28 10:31 ` [PATCH 14/22] zfcp: clarify that we don't need "link" test on failed open port Benjamin Block
2017-07-28 10:31 ` [PATCH 15/22] zfcp: use common code fcp_cmnd and fcp_resp with union in fsf_qtcb_bottom_io Benjamin Block
2017-07-28 10:31 ` [PATCH 16/22] zfcp: use endianness conversions with common FC(P) struct fields Benjamin Block
2017-07-28 10:31 ` [PATCH 17/22] zfcp: fix kernel doc comment typos for struct zfcp_dbf_scsi Benjamin Block
2017-07-28 10:31 ` [PATCH 18/22] zfcp: clean up redundant code with fall through in link down SRB switch case Benjamin Block
2017-07-28 10:31 ` [PATCH 19/22] zfcp: clean up no longer existent prototype from zfcp API header Benjamin Block
2017-07-28 10:31 ` [PATCH 20/22] zfcp: clean up a member of struct zfcp_qdio that was assigned but never used Benjamin Block
2017-07-28 10:31 ` [PATCH 21/22] zfcp: clean up unnecessary module_param_named() with no_auto_port_rescan Benjamin Block
2017-07-28 10:31 ` [PATCH 22/22] zfcp: early returns for traces disabled via level Benjamin Block
2017-08-07 17:19 ` [PATCH 00/22] zfcp fixes and cleanups Martin K. Petersen
2017-08-08 10:02   ` Benjamin Block
2017-08-08 16:07     ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).