Compliment of the day,
Compliment of the day, I am Mr.Philippine.Kabore I Have a Business Proposal of $5.3 million For You. I am aware of the unsafe nature of the internet, and was compelled to use this medium due to the nature of this project. I have access to very vital information that can be used to transfer this huge amount of money, which may culminate into the investment of the said funds into your company or any lucrative venture in your country. If you will like to assist me as a partner then indicate your interest, after which we shall both discuss the modalities and the sharing percentage. Upon receipt of your reply on your expression of Interest I will give you full details, on how the business will be executed I am open for negotiation. You should forward your reply to this private email id (mrphilippines...@yahoo.com) Thanks for your anticipated cooperation. Note you might receive this message in your inbox or spam or junk folder, depends on your web host or server network. Thanks’ Best Regards Mr.Philippine.Kabore,
Re: [PATCH 16/23] zfcp: use enum zfcp_erp_steps for struct zfcp_erp_action.step
On 11/16/18 3:19 PM, Steffen Maier wrote:
On 11/16/2018 12:17 PM, Hannes Reinecke wrote:
On 11/8/18 3:44 PM, Steffen Maier wrote:
Use the already defined enum for this purpose to get at least some build
checking (even though an enum is type equivalent to an int in C).
v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup code in zfcp_erp.c")
introduced the enum which was cpp defines previously.
Since struct zfcp_erp_action type is embedded into other structures
living in zfcp_def.h, we have to move enum zfcp_erp_act_type from
its private definition in zfcp_erp.c to the zfcp-global zfcp_def.h
Silence some false -Wswitch compiler warning cases with individual
NOP cases. When adding more enum values and building with W=1 we
would get compiler warnings about missed new cases.
Add missing break statements in some of the above switch cases.
No functional change, but making it future-proof.
I think all of these should have had a break statement ever since,
even if these switch cases happened to be the last ones in the switch
statement body.
"Fall through" in the context of switch case usually means not to have a
break and fall through to the subsequent switch case. However, I think
this old comment meant that here we do not have an _early return_ in the
switch case but the code path continues after the switch case body.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_def.h | 16 +++-
drivers/s390/scsi/zfcp_erp.c | 35
+--
2 files changed, 40 insertions(+), 11 deletions(-)
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -107,6 +107,20 @@ enum zfcp_erp_act_type {
ZFCP_ERP_ACTION_REOPEN_ADAPTER = 4,
};
+/*
+ * Values must fit into u16 because of code dependencies:
+ * zfcp_dbf_rec_run_lvl(), zfcp_dbf_rec_run(), zfcp_dbf_rec_run_wka(),
+ * _dbf_rec_running.rec_step.
+ */
+enum zfcp_erp_steps {
+ ZFCP_ERP_STEP_UNINITIALIZED = 0x,
+ ZFCP_ERP_STEP_PHYS_PORT_CLOSING = 0x0010,
+ ZFCP_ERP_STEP_PORT_CLOSING = 0x0100,
+ ZFCP_ERP_STEP_PORT_OPENING = 0x0800,
+ ZFCP_ERP_STEP_LUN_CLOSING = 0x1000,
+ ZFCP_ERP_STEP_LUN_OPENING = 0x2000,
+};
+
struct zfcp_erp_action {
struct list_head list;
enum zfcp_erp_act_type type; /* requested action code */
@@ -114,7 +128,7 @@ struct zfcp_erp_action {
struct zfcp_port *port;
struct scsi_device *sdev;
u32 status; /* recovery status */
- u32 step; /* active step of this erp action */
+ enum zfcp_erp_steps step; /* active step of this erp
action */
unsigned long fsf_req_id;
struct timer_list timer;
};
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -24,15 +24,6 @@ enum zfcp_erp_act_flags {
ZFCP_STATUS_ERP_NO_REF = 0x0080,
};
-enum zfcp_erp_steps {
- ZFCP_ERP_STEP_UNINITIALIZED = 0x,
- ZFCP_ERP_STEP_PHYS_PORT_CLOSING = 0x0010,
- ZFCP_ERP_STEP_PORT_CLOSING = 0x0100,
- ZFCP_ERP_STEP_PORT_OPENING = 0x0800,
- ZFCP_ERP_STEP_LUN_CLOSING = 0x1000,
- ZFCP_ERP_STEP_LUN_OPENING = 0x2000,
-};
-
/*
* Eyecatcher pseudo flag to bitwise or-combine with enum
zfcp_erp_act_type.
* Used to indicate that an ERP action could not be set up despite
a detected
@@ -900,6 +891,13 @@ static int zfcp_erp_port_forced_strategy
case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
if (!(status & ZFCP_STATUS_PORT_PHYS_OPEN))
return ZFCP_ERP_SUCCEEDED;
+ break;
+ case ZFCP_ERP_STEP_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
return ZFCP_ERP_FAILED;
}
@@ -974,7 +972,12 @@ static int zfcp_erp_port_strategy_open_c
port->d_id = 0;
return ZFCP_ERP_FAILED;
}
- /* fall through otherwise */
+ /* no early return otherwise, continue after switch case */
+ break;
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
return ZFCP_ERP_FAILED;
}
@@ -998,6 +1001,12 @@ static int zfcp_erp_port_strategy(struct
if (p_status & ZFCP_STATUS_COMMON_OPEN)
return ZFCP_ERP_FAILED;
break;
+ case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
close_init_done:
@@ -1058,6 +1067,12 @@ static int zfcp_erp_lun_strategy(struct
case ZFCP_ERP_STEP_LUN_OPENING:
if (atomic_read(_sdev->status) & ZFCP_STATUS_COMMON_OPEN)
return ZFCP_ERP_SUCCEEDED;
+ break;
+ case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ /* NOP */
+
Re: [PATCH v2] megaraid_sas: Add support for MegaRAID Aero controllers
On 11/9/18 6:47 PM, Shivasharan S wrote: This patch adds support for MegaRAID Aero controller PCI IDs. Throw a message when a Configurable secure type controller is encountered. Signed-off-by: Shivasharan S --- Changes in v2: Change dev_warn to dev_info. drivers/scsi/megaraid/megaraid_sas.h | 4 drivers/scsi/megaraid/megaraid_sas_base.c | 15 +++ 2 files changed, 19 insertions(+) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [RFC PATCH 0/3] target: remove some unused stats
Hi Mike
On Tue, 30 Oct 2018 10:54:07 -0500, Mike Christie wrote:
> >> This patchset removes a couple of unused error stat counters and a
> >> redundant cumulative counter.
> >> I've tagged this patchset RFC, as it may be considered a kernel<->user
> >> (configfs) API change.
> >
> > Ping, any thoughts on this patchset?
> >
>
> I think these stats were supposed to match the iSCSI MIB definitions. It
> was not clear why we didn't just fix them so they report the correct values?
I finally got a chance to look through rfc#4544. It does indeed look
like these counters correspond to:
IscsiInstanceSsnErrorStatsEntry ::= SEQUENCE {
iscsiInstSsnDigestErrors Counter32,
iscsiInstSsnCxnTimeoutErrors Counter32,
iscsiInstSsnFormatErrors Counter32
}
There doesn't appear to be anything stopping us from plumbing them into
the corresponding error paths, so I'll withdraw this patchset.
Cheers, David
Re: [PATCH 12/23] zfcp: update kernel message for invalid FCP_CMND length, it's not the CDB
On 11/16/2018 12:13 PM, Hannes Reinecke wrote:
On 11/8/18 3:44 PM, Steffen Maier wrote:
The CDB is just a part inside of FCP_CMND, see zfcp_fc_scsi_to_fcp().
While at it, fix the device driver reaction: adapter not LUN shutdown.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_fsf.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index c949c65ffc6a..0bdbc596da97 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2090,11 +2090,8 @@ static void zfcp_fsf_fcp_handler_common(struct
zfcp_fsf_req *req,
break;
case FSF_CMND_LENGTH_NOT_VALID:
dev_err(>adapter->ccw_device->dev,
- "Incorrect CDB length %d, LUN 0x%016Lx on "
- "port 0x%016Lx closed\n",
- req->qtcb->bottom.io.fcp_cmnd_length,
- (unsigned long long)zfcp_scsi_dev_lun(sdev),
- (unsigned long long)zfcp_sdev->port->wwpn);
+ "Incorrect FCP_CMND length %d, FCP device closed\n",
+ req->qtcb->bottom.io.fcp_cmnd_length);
zfcp_erp_adapter_shutdown(req->adapter, 0, "fssfch4");
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
break;
Really? You're only fixing the message, not the adapter behaviour.
Care to clarify the commit message?
This is one of few cases in zfcp where we shutdown the entire adapter.
If we would ever get this, it would be very likely a zfcp bug in
hardcoded parts affecting all IO paths to all our LUNs. Also, retrying
would likely repeat the error.
IIRC, it's always been an adapter shutdown in git history. It's not that
the type of recovery has changed at some point. A previous version of
the message even had the correct recovery description part
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=553448f6c4838a1e4bed2bc9301c748278d7d9ce
v2.6.27 ("[SCSI] zfcp: Message cleanup")
but broke with
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=ff3b24fa5370a7ca618f212284d9b36fcedb9c0e
v2.6.28 ("[SCSI] zfcp: Update message with input from review")
Is this what you would like to see as clarification?
--
Mit freundlichen Gruessen / Kind regards
Steffen Maier
Linux on IBM Z Development
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH 16/23] zfcp: use enum zfcp_erp_steps for struct zfcp_erp_action.step
On 11/16/2018 12:17 PM, Hannes Reinecke wrote:
On 11/8/18 3:44 PM, Steffen Maier wrote:
Use the already defined enum for this purpose to get at least some build
checking (even though an enum is type equivalent to an int in C).
v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup code in zfcp_erp.c")
introduced the enum which was cpp defines previously.
Since struct zfcp_erp_action type is embedded into other structures
living in zfcp_def.h, we have to move enum zfcp_erp_act_type from
its private definition in zfcp_erp.c to the zfcp-global zfcp_def.h
Silence some false -Wswitch compiler warning cases with individual
NOP cases. When adding more enum values and building with W=1 we
would get compiler warnings about missed new cases.
Add missing break statements in some of the above switch cases.
No functional change, but making it future-proof.
I think all of these should have had a break statement ever since,
even if these switch cases happened to be the last ones in the switch
statement body.
"Fall through" in the context of switch case usually means not to have a
break and fall through to the subsequent switch case. However, I think
this old comment meant that here we do not have an _early return_ in the
switch case but the code path continues after the switch case body.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_def.h | 16 +++-
drivers/s390/scsi/zfcp_erp.c | 35 +--
2 files changed, 40 insertions(+), 11 deletions(-)
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -107,6 +107,20 @@ enum zfcp_erp_act_type {
ZFCP_ERP_ACTION_REOPEN_ADAPTER = 4,
};
+/*
+ * Values must fit into u16 because of code dependencies:
+ * zfcp_dbf_rec_run_lvl(), zfcp_dbf_rec_run(), zfcp_dbf_rec_run_wka(),
+ * _dbf_rec_running.rec_step.
+ */
+enum zfcp_erp_steps {
+ ZFCP_ERP_STEP_UNINITIALIZED = 0x,
+ ZFCP_ERP_STEP_PHYS_PORT_CLOSING = 0x0010,
+ ZFCP_ERP_STEP_PORT_CLOSING = 0x0100,
+ ZFCP_ERP_STEP_PORT_OPENING = 0x0800,
+ ZFCP_ERP_STEP_LUN_CLOSING = 0x1000,
+ ZFCP_ERP_STEP_LUN_OPENING = 0x2000,
+};
+
struct zfcp_erp_action {
struct list_head list;
enum zfcp_erp_act_type type; /* requested action code */
@@ -114,7 +128,7 @@ struct zfcp_erp_action {
struct zfcp_port *port;
struct scsi_device *sdev;
u32 status; /* recovery status */
- u32 step; /* active step of this erp action */
+ enum zfcp_erp_steps step; /* active step of this erp action */
unsigned long fsf_req_id;
struct timer_list timer;
};
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -24,15 +24,6 @@ enum zfcp_erp_act_flags {
ZFCP_STATUS_ERP_NO_REF = 0x0080,
};
-enum zfcp_erp_steps {
- ZFCP_ERP_STEP_UNINITIALIZED = 0x,
- ZFCP_ERP_STEP_PHYS_PORT_CLOSING = 0x0010,
- ZFCP_ERP_STEP_PORT_CLOSING = 0x0100,
- ZFCP_ERP_STEP_PORT_OPENING = 0x0800,
- ZFCP_ERP_STEP_LUN_CLOSING = 0x1000,
- ZFCP_ERP_STEP_LUN_OPENING = 0x2000,
-};
-
/*
* Eyecatcher pseudo flag to bitwise or-combine with enum
zfcp_erp_act_type.
* Used to indicate that an ERP action could not be set up despite a
detected
@@ -900,6 +891,13 @@ static int zfcp_erp_port_forced_strategy
case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
if (!(status & ZFCP_STATUS_PORT_PHYS_OPEN))
return ZFCP_ERP_SUCCEEDED;
+ break;
+ case ZFCP_ERP_STEP_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
return ZFCP_ERP_FAILED;
}
@@ -974,7 +972,12 @@ static int zfcp_erp_port_strategy_open_c
port->d_id = 0;
return ZFCP_ERP_FAILED;
}
- /* fall through otherwise */
+ /* no early return otherwise, continue after switch case */
+ break;
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
return ZFCP_ERP_FAILED;
}
@@ -998,6 +1001,12 @@ static int zfcp_erp_port_strategy(struct
if (p_status & ZFCP_STATUS_COMMON_OPEN)
return ZFCP_ERP_FAILED;
break;
+ case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
close_init_done:
@@ -1058,6 +1067,12 @@ static int zfcp_erp_lun_strategy(struct
case ZFCP_ERP_STEP_LUN_OPENING:
if (atomic_read(_sdev->status) & ZFCP_STATUS_COMMON_OPEN)
return ZFCP_ERP_SUCCEEDED;
+ break;
+ case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ /* NOP */
+ break;
}
return ZFCP_ERP_FAILED;
}
Re: [PATCH 23/23] zfcp: drop old default switch case which might paper over missing case
On 11/8/18 3:44 PM, Steffen Maier wrote:
This was introduced with v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup
code in zfcp_erp.c") but would now suppress helpful -Wswitch compiler
warnings when building with W=1 such as the following forced example:
drivers/s390/scsi/zfcp_erp.c: In function 'zfcp_erp_setup_act':
drivers/s390/scsi/zfcp_erp.c:220:2: warning: enumeration value
'ZFCP_ERP_ACTION_REOPEN_PORT' not handled in switch [-Wswitch]
switch (need) {
^~
But then again, only with W=1 we would notice unhandled enum cases.
Without the default cases and a missed unhandled enum case, the code
might perform unforeseen things we might not want...
As of today, we never run through the removed default case,
so removing it is no functional change.
In the future, we never should run through a default case but
introduce the necessary specific case(s) to handle new functionality.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_erp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c
index 9345fed3bb37..744a64680d5b 100644
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -257,9 +257,6 @@ static struct zfcp_erp_action *zfcp_erp_setup_act(enum
zfcp_erp_act_type need,
ZFCP_STATUS_COMMON_RUNNING))
act_status |= ZFCP_STATUS_ERP_CLOSE_ONLY;
break;
-
- default:
- return NULL;
}
WARN_ON_ONCE(erp_action->adapter != adapter);
Same here.
If you don't expect the code to run into this case, please do add a
WARN_ON() or something so that it can be tracked down.
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 22/23] zfcp: drop default switch case which might paper over missing case
On 11/8/18 3:44 PM, Steffen Maier wrote:
This was introduced with v4.18 commit 8c3d20aada70 ("scsi: zfcp: fix
missing REC trigger trace for all objects in ERP_FAILED") but would now
suppress helpful -Wswitch compiler warnings when building with W=1 such as
the following forced example:
drivers/s390/scsi/zfcp_erp.c: In function 'zfcp_erp_handle_failed':
drivers/s390/scsi/zfcp_erp.c:126:2: warning: enumeration value
'ZFCP_ERP_ACTION_REOPEN_PORT_FORCED' not handled in switch [-Wswitch]
switch (want) {
^~
But then again, only with W=1 we would notice unhandled enum cases.
Without the default cases and a missed unhandled enum case, the code
might perform unforeseen things we might not want...
As of today, we never run through the removed default case,
so removing it is no functional change.
In the future, we never should run through a default case but
introduce the necessary specific case(s) to handle new functionality.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_erp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c
index b2845c5b8106..9345fed3bb37 100644
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -151,9 +151,6 @@ static enum zfcp_erp_act_type zfcp_erp_handle_failed(
adapter, ZFCP_STATUS_COMMON_ERP_FAILED);
}
break;
- default:
- need = 0;
- break;
}
return need;
If you 'should' not run through this code path, doesn't it warrant a
WARN_ON() or something?
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 21/23] zfcp: silence -Wimplicit-fallthrough in zfcp_erp_lun_strategy()
On 11/8/18 3:44 PM, Steffen Maier wrote: For some reason the already existing substring "fall through" in the comment is not sufficient for GCC to silence -Wimplicit-fallthrough. CC [M] drivers/s390/scsi/zfcp_erp.o drivers/s390/scsi/zfcp_erp.c: In function 'zfcp_erp_lun_strategy': drivers/s390/scsi/zfcp_erp.c:1065:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (atomic_read(_sdev->status) & ZFCP_STATUS_COMMON_OPEN) ^ drivers/s390/scsi/zfcp_erp.c:1068:2: note: here case ZFCP_ERP_STEP_LUN_CLOSING: ^~~~ Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_erp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c index 8e5f01f5be81..b2845c5b8106 100644 --- a/drivers/s390/scsi/zfcp_erp.c +++ b/drivers/s390/scsi/zfcp_erp.c @@ -1070,7 +1070,8 @@ static enum zfcp_erp_act_result zfcp_erp_lun_strategy( zfcp_erp_lun_strategy_clearstati(sdev); if (atomic_read(_sdev->status) & ZFCP_STATUS_COMMON_OPEN) return zfcp_erp_lun_strategy_close(erp_action); - /* already closed, fall through */ + /* already closed */ + /* fall through */ case ZFCP_ERP_STEP_LUN_CLOSING: if (atomic_read(_sdev->status) & ZFCP_STATUS_COMMON_OPEN) return ZFCP_ERP_FAILED; Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 19/23] zfcp: silence all W=1 build warnings for existing kdoc
On 11/8/18 3:44 PM, Steffen Maier wrote: While at it also improve some copy kdoc mistakes. Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_dbf.c | 13 - drivers/s390/scsi/zfcp_erp.c | 6 +++--- drivers/s390/scsi/zfcp_fc.c | 2 +- drivers/s390/scsi/zfcp_fsf.c | 14 +- drivers/s390/scsi/zfcp_qdio.c | 3 +-- 5 files changed, 22 insertions(+), 16 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 20/23] zfcp: silence remaining kdoc warnings in header files
On 11/8/18 3:44 PM, Steffen Maier wrote: Improve whatever the following simple invocation reported: $ ./scripts/kernel-doc -none drivers/s390/scsi/*.h While at it, improve some related kdoc, including struct zfcp_fsf_ct_els in zfcp_fsf.h. Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_dbf.h | 10 +- drivers/s390/scsi/zfcp_fc.h | 21 ++--- drivers/s390/scsi/zfcp_fsf.h | 4 ++-- drivers/s390/scsi/zfcp_qdio.h| 9 +++-- drivers/s390/scsi/zfcp_reqlist.h | 2 +- 5 files changed, 37 insertions(+), 9 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 18/23] zfcp: properly format LUN (and WWPN) for LUN sharing violation kmsg
On 11/8/18 3:44 PM, Steffen Maier wrote: zfcp: : LUN 0x0 on port 0x5005076. ... zfcp: : LUN 0x1 on port 0x5005076. ... should be zfcp: : LUN 0x on port 0x5005076. ... zfcp: : LUN 0x0001 on port 0x5005076. is already in use by CSS., MIF Image ID . Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_fsf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c index 0bdbc596da97..b83d249d07dc 100644 --- a/drivers/s390/scsi/zfcp_fsf.c +++ b/drivers/s390/scsi/zfcp_fsf.c @@ -1811,7 +1811,7 @@ static void zfcp_fsf_open_lun_handler(struct zfcp_fsf_req *req) case FSF_LUN_SHARING_VIOLATION: if (qual->word[0]) dev_warn(_sdev->port->adapter->ccw_device->dev, -"LUN 0x%Lx on port 0x%Lx is already in " +"LUN 0x%016Lx on port 0x%016Lx is already in " "use by CSS%d, MIF Image ID %x\n", zfcp_scsi_dev_lun(sdev), (unsigned long long)zfcp_sdev->port->wwpn, Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 17/23] zfcp: use enum zfcp_erp_act_result for argument/return of affected functions
On 11/8/18 3:44 PM, Steffen Maier wrote:
With that instead of just "int" it becomes clear which functions return
this type and which ones also accept it as argument they just pass through
in some cases or modify in other cases.
v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup code in zfcp_erp.c")
introduced the enum which was cpp defines previously.
Silence some false -Wswitch compiler warning cases with individual
NOP cases. When adding more enum values and building with W=1 we
would get compiler warnings about missed new cases.
Consistently use the variable name "result", so change "retval" in
zfcp_erp_strategy() to "result". This avoids confusion with other
compile unit variables "retval" having different semantics and type.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_erp.c | 124 +--
1 file changed, 84 insertions(+), 40 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c
index 3da870e55ab5..5c7fb64111fe 100644
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -713,7 +713,8 @@ static void zfcp_erp_enqueue_ptp_port(struct zfcp_adapter
*adapter)
_zfcp_erp_port_reopen(port, 0, "ereptp1");
}
-static int zfcp_erp_adapter_strat_fsf_xconf(struct zfcp_erp_action *erp_action)
+static enum zfcp_erp_act_result zfcp_erp_adapter_strat_fsf_xconf(
+ struct zfcp_erp_action *erp_action)
{
int retries;
int sleep = 1;
@@ -758,7 +759,8 @@ static int zfcp_erp_adapter_strat_fsf_xconf(struct
zfcp_erp_action *erp_action)
return ZFCP_ERP_SUCCEEDED;
}
-static int zfcp_erp_adapter_strategy_open_fsf_xport(struct zfcp_erp_action *act)
+static enum zfcp_erp_act_result zfcp_erp_adapter_strategy_open_fsf_xport(
+ struct zfcp_erp_action *act)
{
int ret;
struct zfcp_adapter *adapter = act->adapter;
@@ -783,7 +785,8 @@ static int zfcp_erp_adapter_strategy_open_fsf_xport(struct
zfcp_erp_action *act)
return ZFCP_ERP_SUCCEEDED;
}
-static int zfcp_erp_adapter_strategy_open_fsf(struct zfcp_erp_action *act)
+static enum zfcp_erp_act_result zfcp_erp_adapter_strategy_open_fsf(
+ struct zfcp_erp_action *act)
{
if (zfcp_erp_adapter_strat_fsf_xconf(act) == ZFCP_ERP_FAILED)
return ZFCP_ERP_FAILED;
@@ -822,7 +825,8 @@ static void zfcp_erp_adapter_strategy_close(struct
zfcp_erp_action *act)
ZFCP_STATUS_ADAPTER_LINK_UNPLUGGED, >status);
}
-static int zfcp_erp_adapter_strategy_open(struct zfcp_erp_action *act)
+static enum zfcp_erp_act_result zfcp_erp_adapter_strategy_open(
+ struct zfcp_erp_action *act)
{
struct zfcp_adapter *adapter = act->adapter;
@@ -843,7 +847,8 @@ static int zfcp_erp_adapter_strategy_open(struct zfcp_erp_action *act)
return ZFCP_ERP_SUCCEEDED;
}
-static int zfcp_erp_adapter_strategy(struct zfcp_erp_action *act)
+static enum zfcp_erp_act_result zfcp_erp_adapter_strategy(
+ struct zfcp_erp_action *act)
{
struct zfcp_adapter *adapter = act->adapter;
@@ -861,7 +866,8 @@ static int zfcp_erp_adapter_strategy(struct zfcp_erp_action *act)
return ZFCP_ERP_SUCCEEDED;
}
-static int zfcp_erp_port_forced_strategy_close(struct zfcp_erp_action *act)
+static enum zfcp_erp_act_result zfcp_erp_port_forced_strategy_close(
+ struct zfcp_erp_action *act)
{
int retval;
@@ -875,7 +881,8 @@ static int zfcp_erp_port_forced_strategy_close(struct zfcp_erp_action *act)
return ZFCP_ERP_CONTINUES;
}
-static int zfcp_erp_port_forced_strategy(struct zfcp_erp_action *erp_action)
+static enum zfcp_erp_act_result zfcp_erp_port_forced_strategy(
+ struct zfcp_erp_action *erp_action)
{
struct zfcp_port *port = erp_action->port;
int status = atomic_read(>status);
@@ -902,7 +909,8 @@ static int zfcp_erp_port_forced_strategy(struct
zfcp_erp_action *erp_action)
return ZFCP_ERP_FAILED;
}
-static int zfcp_erp_port_strategy_close(struct zfcp_erp_action *erp_action)
+static enum zfcp_erp_act_result zfcp_erp_port_strategy_close(
+ struct zfcp_erp_action *erp_action)
{
int retval;
@@ -915,7 +923,8 @@ static int zfcp_erp_port_strategy_close(struct zfcp_erp_action *erp_action)
return ZFCP_ERP_CONTINUES;
}
-static int zfcp_erp_port_strategy_open_port(struct zfcp_erp_action *erp_action)
+static enum zfcp_erp_act_result zfcp_erp_port_strategy_open_port(
+ struct zfcp_erp_action *erp_action)
{
int retval;
@@ -941,7 +950,8 @@ static int zfcp_erp_open_ptp_port(struct zfcp_erp_action *act)
return zfcp_erp_port_strategy_open_port(act);
}
-static int zfcp_erp_port_strategy_open_common(struct zfcp_erp_action *act)
+static enum zfcp_erp_act_result zfcp_erp_port_strategy_open_common(
+ struct zfcp_erp_action *act)
{
struct zfcp_adapter *adapter = act->adapter;
Re: [PATCH 16/23] zfcp: use enum zfcp_erp_steps for struct zfcp_erp_action.step
On 11/8/18 3:44 PM, Steffen Maier wrote:
Use the already defined enum for this purpose to get at least some build
checking (even though an enum is type equivalent to an int in C).
v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup code in zfcp_erp.c")
introduced the enum which was cpp defines previously.
Since struct zfcp_erp_action type is embedded into other structures
living in zfcp_def.h, we have to move enum zfcp_erp_act_type from
its private definition in zfcp_erp.c to the zfcp-global zfcp_def.h
Silence some false -Wswitch compiler warning cases with individual
NOP cases. When adding more enum values and building with W=1 we
would get compiler warnings about missed new cases.
Add missing break statements in some of the above switch cases.
No functional change, but making it future-proof.
I think all of these should have had a break statement ever since,
even if these switch cases happened to be the last ones in the switch
statement body.
"Fall through" in the context of switch case usually means not to have a
break and fall through to the subsequent switch case. However, I think
this old comment meant that here we do not have an _early return_ in the
switch case but the code path continues after the switch case body.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_def.h | 16 +++-
drivers/s390/scsi/zfcp_erp.c | 35 +--
2 files changed, 40 insertions(+), 11 deletions(-)
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -107,6 +107,20 @@ enum zfcp_erp_act_type {
ZFCP_ERP_ACTION_REOPEN_ADAPTER = 4,
};
+/*
+ * Values must fit into u16 because of code dependencies:
+ * zfcp_dbf_rec_run_lvl(), zfcp_dbf_rec_run(), zfcp_dbf_rec_run_wka(),
+ * _dbf_rec_running.rec_step.
+ */
+enum zfcp_erp_steps {
+ ZFCP_ERP_STEP_UNINITIALIZED = 0x,
+ ZFCP_ERP_STEP_PHYS_PORT_CLOSING = 0x0010,
+ ZFCP_ERP_STEP_PORT_CLOSING = 0x0100,
+ ZFCP_ERP_STEP_PORT_OPENING = 0x0800,
+ ZFCP_ERP_STEP_LUN_CLOSING = 0x1000,
+ ZFCP_ERP_STEP_LUN_OPENING = 0x2000,
+};
+
struct zfcp_erp_action {
struct list_head list;
enum zfcp_erp_act_type type; /* requested action code */
@@ -114,7 +128,7 @@ struct zfcp_erp_action {
struct zfcp_port *port;
struct scsi_device *sdev;
u32 status; /* recovery status */
- u32 step; /* active step of this erp action */
+ enum zfcp_erp_steps step; /* active step of this erp action */
unsigned long fsf_req_id;
struct timer_list timer;
};
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -24,15 +24,6 @@ enum zfcp_erp_act_flags {
ZFCP_STATUS_ERP_NO_REF = 0x0080,
};
-enum zfcp_erp_steps {
- ZFCP_ERP_STEP_UNINITIALIZED = 0x,
- ZFCP_ERP_STEP_PHYS_PORT_CLOSING = 0x0010,
- ZFCP_ERP_STEP_PORT_CLOSING = 0x0100,
- ZFCP_ERP_STEP_PORT_OPENING = 0x0800,
- ZFCP_ERP_STEP_LUN_CLOSING = 0x1000,
- ZFCP_ERP_STEP_LUN_OPENING = 0x2000,
-};
-
/*
* Eyecatcher pseudo flag to bitwise or-combine with enum zfcp_erp_act_type.
* Used to indicate that an ERP action could not be set up despite a detected
@@ -900,6 +891,13 @@ static int zfcp_erp_port_forced_strategy
case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
if (!(status & ZFCP_STATUS_PORT_PHYS_OPEN))
return ZFCP_ERP_SUCCEEDED;
+ break;
+ case ZFCP_ERP_STEP_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
return ZFCP_ERP_FAILED;
}
@@ -974,7 +972,12 @@ static int zfcp_erp_port_strategy_open_c
port->d_id = 0;
return ZFCP_ERP_FAILED;
}
- /* fall through otherwise */
+ /* no early return otherwise, continue after switch case */
+ break;
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
return ZFCP_ERP_FAILED;
}
@@ -998,6 +1001,12 @@ static int zfcp_erp_port_strategy(struct
if (p_status & ZFCP_STATUS_COMMON_OPEN)
return ZFCP_ERP_FAILED;
break;
+ case ZFCP_ERP_STEP_PHYS_PORT_CLOSING:
+ case ZFCP_ERP_STEP_PORT_OPENING:
+ case ZFCP_ERP_STEP_LUN_CLOSING:
+ case ZFCP_ERP_STEP_LUN_OPENING:
+ /* NOP */
+ break;
}
close_init_done:
@@ -1058,6 +1067,12 @@ static int zfcp_erp_lun_strategy(struct
case ZFCP_ERP_STEP_LUN_OPENING:
if (atomic_read(_sdev->status) & ZFCP_STATUS_COMMON_OPEN)
Re: [PATCH 15/23] zfcp: the action field of zfcp_erp_action is actually the type
On 11/8/18 3:44 PM, Steffen Maier wrote:
_erp_action.action ==> _erp_action.type
While at it, make use of the already defined enum for this purpose
to get at least some build checking (even though an enum is type equivalent
to an int in C). v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup code
in zfcp_erp.c") introduced the enum which was cpp defines previously.
To prevent compiler warnings with the switch(act->type), we have to
separate the recently added eyecatchers from enum zfcp_erp_act_type.
Since struct zfcp_erp_action type is embedded into other structures
living in zfcp_def.h, we have to move enum zfcp_erp_act_type from
its private definition in zfcp_erp.c to the zfcp-global zfcp_def.h.
Silence one false -Wswitch compiler warning case: LUNs as the leaves in our
object tree do not have any follow-up success recovery.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_dbf.c | 2 +-
drivers/s390/scsi/zfcp_def.h | 20 +++-
drivers/s390/scsi/zfcp_erp.c | 77 +---
3 files changed, 56 insertions(+), 43 deletions(-)
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 14/23] zfcp: clarify function argument name for trace tag string
On 11/8/18 3:44 PM, Steffen Maier wrote:
v2.6.30 commit 5ffd51a5e495 ("[SCSI] zfcp: replace current ERP logging
with a more convenient version") changed trace record distinguishing from a
numerical ID to a 7 character string called "trace tag". While starting to
use function arguments with different type and semantics, it did not change
the argument name accordingly.
v2.6.38 commit ae0904f60fab ("[SCSI] zfcp: Redesign of the debug tracing
for recovery actions.") renamed variable names "id" into "tag" but only
within zfcp_dbf.*, not within zfcp_erp.c.
This was a bit confusing since the remainder of zfcp does use the term
"trace tag". Also "id" is quite generic and it's not obvious for what.
Just unify it consistently and use the "dbf" prefix to relate the arguments
to the code in zfcp_dbf.*.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_erp.c | 92 ++-
drivers/s390/scsi/zfcp_ext.h | 8 ++--
drivers/s390/scsi/zfcp_qdio.c | 8 ++--
3 files changed, 57 insertions(+), 51 deletions(-)
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 13/23] zfcp: ERP thread setup kdoc update
On 11/8/18 3:44 PM, Steffen Maier wrote:
zfcp_erp_thread_setup() update complements v2.6.32 commit 347c6a965dc1
("[SCSI] zfcp: Use kthread API for zfcp erp thread").
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_erp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c
index e7e6b63905e2..f6a2d66eef57 100644
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -1489,7 +1489,7 @@ static int zfcp_erp_thread(void *data)
* zfcp_erp_thread_setup - Start ERP thread for adapter
* @adapter: Adapter to start the ERP thread for
*
- * Returns 0 on success or error code from kernel_thread()
+ * Return: 0 on success, or error code from kthread_run().
*/
int zfcp_erp_thread_setup(struct zfcp_adapter *adapter)
{
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 12/23] zfcp: update kernel message for invalid FCP_CMND length, it's not the CDB
On 11/8/18 3:44 PM, Steffen Maier wrote: The CDB is just a part inside of FCP_CMND, see zfcp_fc_scsi_to_fcp(). While at it, fix the device driver reaction: adapter not LUN shutdown. Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_fsf.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c index c949c65ffc6a..0bdbc596da97 100644 --- a/drivers/s390/scsi/zfcp_fsf.c +++ b/drivers/s390/scsi/zfcp_fsf.c @@ -2090,11 +2090,8 @@ static void zfcp_fsf_fcp_handler_common(struct zfcp_fsf_req *req, break; case FSF_CMND_LENGTH_NOT_VALID: dev_err(>adapter->ccw_device->dev, - "Incorrect CDB length %d, LUN 0x%016Lx on " - "port 0x%016Lx closed\n", - req->qtcb->bottom.io.fcp_cmnd_length, - (unsigned long long)zfcp_scsi_dev_lun(sdev), - (unsigned long long)zfcp_sdev->port->wwpn); + "Incorrect FCP_CMND length %d, FCP device closed\n", + req->qtcb->bottom.io.fcp_cmnd_length); zfcp_erp_adapter_shutdown(req->adapter, 0, "fssfch4"); req->status |= ZFCP_STATUS_FSFREQ_ERROR; break; Really? You're only fixing the message, not the adapter behaviour. Care to clarify the commit message? Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 11/23] zfcp: drop duplicate seq_no from zfcp_fsf_req which is also in QTCB header
On 11/8/18 3:44 PM, Steffen Maier wrote: There is no point for double bookkeeping especially just for tracing. The trace can take it from the QTCB which always exists for non-SRB responses traced with zfcp_dbf_hba_fsf_res(). As a side effect, this removes an alignment hole and reduces the size of struct zfcp_fsf_req, and thus of each pending request, by 8 bytes. Before: $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko ... struct fsf_qtcb * qtcb; /* 144 8 */ u32seq_no; /* 152 4 */ /* XXX 4 bytes hole, try to pack */ void * data; /* 160 8 */ ... /* size: 296, cachelines: 2, members: 14 */ /* sum members: 288, holes: 2, sum holes: 8 */ /* last cacheline: 40 bytes */ After: $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko ... struct fsf_qtcb * qtcb; /* 144 8 */ void * data; /* 152 8 */ ... /* size: 288, cachelines: 2, members: 13 */ /* sum members: 284, holes: 1, sum holes: 4 */ Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_dbf.c | 2 +- drivers/s390/scsi/zfcp_def.h | 2 -- drivers/s390/scsi/zfcp_fsf.c | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) Makes one wonder what we need the sequence number for ... Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 10/23] zfcp: drop duplicate fsf_command from zfcp_fsf_req which is also in QTCB header
On 11/8/18 3:44 PM, Steffen Maier wrote: Status read buffers (SRBs, unsolicited notifications) never use a QTCB [zfcp_fsf_req_create()]. zfcp_fsf_req_send() already uses this to distinguish SRBs from other FSF request types. We can re-use this method in zfcp_fsf_req_complete(). Introduce a helper function to make the check for req->qtcb less magic. SRBs always are FSF_QTCB_UNSOLICITED_STATUS, so we can hard-code this for the two trace functions dealing with SRBs. All other FSF request types have a QTCB and we can get the fsf_command from there. zfcp_dbf_hba_fsf_response() and thus zfcp_dbf_hba_fsf_res() are only called for non-SRB requests so it's safe to dereference the QTCB [zfcp_fsf_req_complete() returns early on SRB, else calls zfcp_fsf_protstatus_eval() which calls zfcp_dbf_hba_fsf_response()]. In zfcp_scsi_forget_cmnd() we guard the QTCB dereference with a preceding NULL check and rely on boolean shortcut evaluation. As a side effect, this causes an alignment hole which we can close in a later patch after having cleaned up all fields of struct zfcp_fsf_req. Before: $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko ... u32status; /* 136 4 */ u32fsf_command; /* 140 4 */ struct fsf_qtcb * qtcb; /* 144 8 */ ... After: $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko ... u32status; /* 136 4 */ /* XXX 4 bytes hole, try to pack */ struct fsf_qtcb * qtcb; /* 144 8 */ ... Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_dbf.c | 8 drivers/s390/scsi/zfcp_dbf.h | 4 ++-- drivers/s390/scsi/zfcp_def.h | 7 +-- drivers/s390/scsi/zfcp_fsf.c | 14 ++ drivers/s390/scsi/zfcp_scsi.c | 4 +++- 5 files changed, 20 insertions(+), 17 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 09/23] zfcp: drop unnecessary forward prototype for struct zfcp_fsf_req
On 11/8/18 3:44 PM, Steffen Maier wrote:
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_def.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h
index 31b3e2bb3b42..572debf2f528 100644
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -89,8 +89,6 @@
/* STRUCTURE DEFINITIONS */
-struct zfcp_fsf_req;
-
struct zfcp_erp_action {
struct list_head list;
int action; /* requested action code */
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 07/23] zfcp: namespace prefix for internal latency data structures
On 11/8/18 3:44 PM, Steffen Maier wrote: In contrast to struct fsf_qual_latency_info, the ones here are not FSF but software defined zfcp-internal. Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_def.h | 14 +++--- drivers/s390/scsi/zfcp_fsf.c | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 08/23] zfcp: group sort internal structure definitions for proximity
On 11/8/18 3:44 PM, Steffen Maier wrote: Have structures just before the structures that use them (without disrupting sequences of using structures such as zfcp_unit and zfcp_scsi_dev): - zfcp_adapter_mempool embedded in zfcp_adapter, - zfcp_latenc... embedded in zfcp_scsi_dev. Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_def.h | 58 ++-- 1 file changed, 29 insertions(+), 29 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 06/23] zfcp: update width in comment for ZFCP_COMMON_FLAGS mask
On 11/8/18 3:44 PM, Steffen Maier wrote:
v2.6.10 history commit 4062e12b2ba2 ("[PATCH] s390: zfcp act enhancements")
extended this mask by one nibble with the introduction of
ZFCP_STATUS_COMMON_ACCESS_DENIED == 0x0080 for ACT
(access control table).
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_def.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h
index 87a1fef5568e..13bfc13eb42d 100644
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -49,8 +49,8 @@
/*** ADAPTER/PORT/UNIT AND FSF_REQ STATUS FLAGS
**/
/*
- * Note, the leftmost status byte is common among adapter, port
- * and unit
+ * Note, the leftmost 12 status bits (3 nibbles) are common among adapter, port
+ * and unit. This is a mask for bitwise 'and' with status values.
*/
#define ZFCP_COMMON_FLAGS 0xfff0
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 05/23] zfcp: move scsi_eh & non-ERP timeout defines owned by and local to zfcp_fsf.c
On 11/8/18 3:44 PM, Steffen Maier wrote: Also clarify namespace prefix for the timeout used for FSF requests on behalf of SCSI error recovery: It is zfcp_fsf_ not zfcp_scsi_. Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block --- drivers/s390/scsi/zfcp_def.h | 6 -- drivers/s390/scsi/zfcp_fsf.c | 9 +++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h index 1b6d64eb66b7..87a1fef5568e 100644 --- a/drivers/s390/scsi/zfcp_def.h +++ b/drivers/s390/scsi/zfcp_def.h @@ -41,17 +41,11 @@ #include "zfcp_fc.h" #include "zfcp_qdio.h" -/* SCSI SPECIFIC DEFINES */ -#define ZFCP_SCSI_ER_TIMEOUT(10*HZ) - /* FSF SPECIFIC DEFINES */ /* ATTENTION: value must not be used by hardware */ #define FSF_QTCB_UNSOLICITED_STATUS 0x6305 -/* timeout value for "default timer" for fsf requests */ -#define ZFCP_FSF_REQUEST_TIMEOUT (60*HZ) - /*** ADAPTER/PORT/UNIT AND FSF_REQ STATUS FLAGS **/ /* diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c index 3c86e27f094d..095ab7fdcf4b 100644 --- a/drivers/s390/scsi/zfcp_fsf.c +++ b/drivers/s390/scsi/zfcp_fsf.c @@ -19,6 +19,11 @@ #include "zfcp_qdio.h" #include "zfcp_reqlist.h" +/* timeout for FSF requests sent during scsi_eh: abort or FCP TMF */ +#define ZFCP_FSF_SCSI_ER_TIMEOUT (10*HZ) +/* timeout for: exchange config/port data outside ERP, or open/close WKA port */ +#define ZFCP_FSF_REQUEST_TIMEOUT (60*HZ) + struct kmem_cache *zfcp_fsf_qtcb_cache; static void zfcp_fsf_request_timeout_handler(struct timer_list *t) @@ -912,7 +917,7 @@ struct zfcp_fsf_req *zfcp_fsf_abort_fcp_cmnd(struct scsi_cmnd *scmnd) req->qtcb->header.port_handle = zfcp_sdev->port->handle; req->qtcb->bottom.support.req_handle = (u64) old_req_id; - zfcp_fsf_start_timer(req, ZFCP_SCSI_ER_TIMEOUT); + zfcp_fsf_start_timer(req, ZFCP_FSF_SCSI_ER_TIMEOUT); if (!zfcp_fsf_req_send(req)) goto out; @@ -2369,7 +2374,7 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_device *sdev, fcp_cmnd = >qtcb->bottom.io.fcp_cmnd.iu; zfcp_fc_fcp_tm(fcp_cmnd, sdev, tm_flags); - zfcp_fsf_start_timer(req, ZFCP_SCSI_ER_TIMEOUT); + zfcp_fsf_start_timer(req, ZFCP_FSF_SCSI_ER_TIMEOUT); if (!zfcp_fsf_req_send(req)) goto out; Actually, I would love to see ZFCP_FSF_SCSI_ER_TIMEOUT be initialized by scsi_device->eh_timeout ... But that's probably something for a bigger review affecting all drivers. Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 04/23] zfcp: drop unnecessary forward prototype for struct zfcp_reqlist
On 11/8/18 3:44 PM, Steffen Maier wrote:
While struct zfcp_adapter contains a pointer to zfcp_reqlist,
the pointer field does not need to know the structure or even a prototype.
The prototype was introduced with v2.6.34 commit b6bd2fb92a7b ("[SCSI]
zfcp: Move FSF request tracking code to new file").
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_def.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h
index 3396a47721a7..1b6d64eb66b7 100644
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -4,7 +4,7 @@
*
* Global definitions for the zfcp device driver.
*
- * Copyright IBM Corp. 2002, 2010
+ * Copyright IBM Corp. 2002, 2017
*/
#ifndef ZFCP_DEF_H
@@ -41,8 +41,6 @@
#include "zfcp_fc.h"
#include "zfcp_qdio.h"
-struct zfcp_reqlist;
-
/* SCSI SPECIFIC DEFINES
*/
#define ZFCP_SCSI_ER_TIMEOUT(10*HZ)
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 03/23] zfcp: move SG table helper from aux to fc and make them static
On 11/8/18 3:44 PM, Steffen Maier wrote:
Since commit 663e0890e31c ("[SCSI] zfcp: remove access control tables
interface") these helper functions are only used for auto port scan in
zfcp_fc.c. Also change them to the corresponding namespace prefix.
This is a small cleanup for the miscellaneous catchall compile unit
zfcp_aux.c.
Signed-off-by: Steffen Maier
Reviewed-by: Benjamin Block
---
drivers/s390/scsi/zfcp_aux.c | 44 +-
drivers/s390/scsi/zfcp_fc.c | 46 ++--
2 files changed, 45 insertions(+), 45 deletions(-)
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH 02/22] zfcp: remove unnecessary null pointer check before mempool_destroy
On 11/8/18 3:44 PM, Steffen Maier wrote:
From: zhong jiang
mempool_destroy has taken null pointer check into account. so remove the
redundant check.
Signed-off-by: zhong jiang
Acked-by: Benjamin Block
[ma...@linux.ibm.com: depends on v4.3 4e3ca3e033d1 ("mm/mempool: allow NULL `pool'
pointer in mempool_destroy()")]
Signed-off-by: Steffen Maier
---
drivers/s390/scsi/zfcp_aux.c | 21 +++--
1 file changed, 7 insertions(+), 14 deletions(-)
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH] scsi: lpfc: fix block guard enablement on SLI3 adapters
On 11/12/18 9:58 AM, Martin Wilck wrote:
Since f44ac12f1dcc, BG enablement is tracked with the
LPFC_SLI3_BG_ENABLED bit, which is set in lpfc_get_cfgparam
before lpfc_sli_config_sli_port() is called. The bit
shouldn't be cleared before checking the feature.
Based on problem analysis by David Bond.
Fixes: f44ac12f1dcc "scsi: lpfc: Memory allocation error during driver start-up on
power8"
Tested-by: David Bond
Signed-off-by: Martin Wilck
Cc: sta...@vger.kernel.org # 4.17.x
Cc: sta...@vger.kernel.org # 4.18.x
Cc: sta...@vger.kernel.org # 4.19.x
---
drivers/scsi/lpfc/lpfc_init.c | 6 +-
drivers/scsi/lpfc/lpfc_sli.c | 1 -
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 323a32e..6b61cae 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -167,7 +167,11 @@ lpfc_config_port_prep(struct lpfc_hba *phba)
sizeof(phba->wwpn));
}
- phba->sli3_options = 0x0;
+ /*
+* Clear all option bits except LPFC_SLI3_BG_ENABLED,
+* which was already set in lpfc_get_cfgparam()
+*/
+ phba->sli3_options &= (uint32_t)LPFC_SLI3_BG_ENABLED;
/* Setup and issue mailbox READ REV command */
lpfc_read_rev(phba, pmb);
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 783a154..b9e5cd7 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -4965,7 +4965,6 @@ lpfc_sli_config_port(struct lpfc_hba *phba, int sli_mode)
phba->sli3_options &= ~(LPFC_SLI3_NPIV_ENABLED |
LPFC_SLI3_HBQ_ENABLED |
LPFC_SLI3_CRP_ENABLED |
- LPFC_SLI3_BG_ENABLED |
LPFC_SLI3_DSS_ENABLED);
if (rc != MBX_SUCCESS) {
lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Re: [PATCH -next] scsi: libfc: Remove set but not used variable 'disc'
Looks good, Reviewed-by: Johannes Thumshirn
