* [PATCH] target_core_alua: silence GCC warning
[not found] <1392714172-2712-1-git-send-email-geert@linux-m68k.org>
@ 2014-02-19 9:52 ` Paul Bolle
2014-02-19 9:59 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2014-02-19 9:52 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Geert Uytterhoeven, linux-scsi, target-devel, linux-kernel
On Tue, 2014-02-18 at 10:02 +0100, Geert Uytterhoeven wrote:
> *** WARNINGS ***
>
> 188 regressions:
> [...]
> + /scratch/kisskb/src/drivers/target/target_core_alua.c: warning: 'alua_ascq' may be used uninitialized in this function [-Wuninitialized]: => 773:18
This one popped up on my (Fedora 20 based) builds on 32 and 64 bits x86.
Geert seems to have seen this on v3.12 already. I first saw it in
v3.14-rc1. Not clear why that is. Anyhow, this is what I came up with to
make this warning go away. (Compile tested only. I don't use this driver
myself.)
-------->8--------
From: Paul Bolle <pebolle@tiscali.nl>
Building target_core_alua.o triggers a GCC warning:
drivers/target/target_core_alua.c: In function ‘target_alua_state_check’:
drivers/target/target_core_alua.c:773:18: warning: ‘alua_ascq’ may be used uninitialized in this function [-Wmaybe-uninitialized]
cmd->scsi_ascq = alua_ascq;
^
This is a false positive. A little trial and error shows it is
apparently caused by core_alua_state_lba_dependent(). It must be hard
for GCC to track the branches of switch statement, inside a
list_for_each_entry loop, inside a while loop.
But if we add a small (inline) helper function we can reorganize the
code a bit. That also allows to drop alua_ascq which, obviously, gets
rid of this warning.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
drivers/target/target_core_alua.c | 94 ++++++++++++++++++---------------------
1 file changed, 44 insertions(+), 50 deletions(-)
diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index c3d9df6..4111f43 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -455,11 +455,26 @@ out:
return rc;
}
+static inline void set_ascq(struct se_cmd *cmd, u8 alua_ascq)
+{
+ /*
+ * Set SCSI additional sense code (ASC) to 'LUN Not Accessible';
+ * The ALUA additional sense code qualifier (ASCQ) is determined
+ * by the ALUA primary or secondary access state..
+ */
+ pr_debug("[%s]: ALUA TG Port not available, "
+ "SenseKey: NOT_READY, ASC/ASCQ: "
+ "0x04/0x%02x\n",
+ cmd->se_tfo->get_fabric_name(), alua_ascq);
+
+ cmd->scsi_asc = 0x04;
+ cmd->scsi_ascq = alua_ascq;
+}
+
static inline int core_alua_state_nonoptimized(
struct se_cmd *cmd,
unsigned char *cdb,
- int nonop_delay_msecs,
- u8 *alua_ascq)
+ int nonop_delay_msecs)
{
/*
* Set SCF_ALUA_NON_OPTIMIZED here, this value will be checked
@@ -471,10 +486,9 @@ static inline int core_alua_state_nonoptimized(
return 0;
}
-static inline int core_alua_state_lba_dependent(
+static noinline int core_alua_state_lba_dependent(
struct se_cmd *cmd,
- struct t10_alua_tg_pt_gp *tg_pt_gp,
- u8 *alua_ascq)
+ struct t10_alua_tg_pt_gp *tg_pt_gp)
{
struct se_device *dev = cmd->se_dev;
u64 segment_size, segment_mult, sectors, lba;
@@ -520,7 +534,7 @@ static inline int core_alua_state_lba_dependent(
}
if (!cur_map) {
spin_unlock(&dev->t10_alua.lba_map_lock);
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
list_for_each_entry(map_mem, &cur_map->lba_map_mem_list,
@@ -531,11 +545,11 @@ static inline int core_alua_state_lba_dependent(
switch(map_mem->lba_map_mem_alua_state) {
case ALUA_ACCESS_STATE_STANDBY:
spin_unlock(&dev->t10_alua.lba_map_lock);
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
case ALUA_ACCESS_STATE_UNAVAILABLE:
spin_unlock(&dev->t10_alua.lba_map_lock);
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
default:
break;
@@ -548,8 +562,7 @@ static inline int core_alua_state_lba_dependent(
static inline int core_alua_state_standby(
struct se_cmd *cmd,
- unsigned char *cdb,
- u8 *alua_ascq)
+ unsigned char *cdb)
{
/*
* Allowed CDBs for ALUA_ACCESS_STATE_STANDBY as defined by
@@ -570,7 +583,7 @@ static inline int core_alua_state_standby(
case MI_REPORT_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
}
case MAINTENANCE_OUT:
@@ -578,7 +591,7 @@ static inline int core_alua_state_standby(
case MO_SET_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
}
case REQUEST_SENSE:
@@ -588,7 +601,7 @@ static inline int core_alua_state_standby(
case WRITE_BUFFER:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
}
@@ -597,8 +610,7 @@ static inline int core_alua_state_standby(
static inline int core_alua_state_unavailable(
struct se_cmd *cmd,
- unsigned char *cdb,
- u8 *alua_ascq)
+ unsigned char *cdb)
{
/*
* Allowed CDBs for ALUA_ACCESS_STATE_UNAVAILABLE as defined by
@@ -613,7 +625,7 @@ static inline int core_alua_state_unavailable(
case MI_REPORT_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
case MAINTENANCE_OUT:
@@ -621,7 +633,7 @@ static inline int core_alua_state_unavailable(
case MO_SET_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
case REQUEST_SENSE:
@@ -629,7 +641,7 @@ static inline int core_alua_state_unavailable(
case WRITE_BUFFER:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
@@ -638,8 +650,7 @@ static inline int core_alua_state_unavailable(
static inline int core_alua_state_transition(
struct se_cmd *cmd,
- unsigned char *cdb,
- u8 *alua_ascq)
+ unsigned char *cdb)
{
/*
* Allowed CDBs for ALUA_ACCESS_STATE_TRANSITION as defined by
@@ -654,7 +665,7 @@ static inline int core_alua_state_transition(
case MI_REPORT_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_STATE_TRANSITION;
+ set_ascq(cmd, ASCQ_04H_ALUA_STATE_TRANSITION);
return 1;
}
case REQUEST_SENSE:
@@ -662,7 +673,7 @@ static inline int core_alua_state_transition(
case WRITE_BUFFER:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_STATE_TRANSITION;
+ set_ascq(cmd, ASCQ_04H_ALUA_STATE_TRANSITION);
return 1;
}
@@ -684,8 +695,6 @@ target_alua_state_check(struct se_cmd *cmd)
struct t10_alua_tg_pt_gp *tg_pt_gp;
struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem;
int out_alua_state, nonop_delay_msecs;
- u8 alua_ascq;
- int ret;
if (dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE)
return 0;
@@ -701,9 +710,8 @@ target_alua_state_check(struct se_cmd *cmd)
if (atomic_read(&port->sep_tg_pt_secondary_offline)) {
pr_debug("ALUA: Got secondary offline status for local"
" target port\n");
- alua_ascq = ASCQ_04H_ALUA_OFFLINE;
- ret = 1;
- goto out;
+ set_ascq(cmd, ASCQ_04H_ALUA_OFFLINE);
+ return TCM_CHECK_CONDITION_NOT_READY;
}
/*
* Second, obtain the struct t10_alua_tg_pt_gp_member pointer to the
@@ -731,20 +739,23 @@ target_alua_state_check(struct se_cmd *cmd)
switch (out_alua_state) {
case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
- ret = core_alua_state_nonoptimized(cmd, cdb,
- nonop_delay_msecs, &alua_ascq);
+ core_alua_state_nonoptimized(cmd, cdb, nonop_delay_msecs);
break;
case ALUA_ACCESS_STATE_STANDBY:
- ret = core_alua_state_standby(cmd, cdb, &alua_ascq);
+ if (core_alua_state_standby(cmd, cdb))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
case ALUA_ACCESS_STATE_UNAVAILABLE:
- ret = core_alua_state_unavailable(cmd, cdb, &alua_ascq);
+ if (core_alua_state_unavailable(cmd, cdb))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
case ALUA_ACCESS_STATE_TRANSITION:
- ret = core_alua_state_transition(cmd, cdb, &alua_ascq);
+ if (core_alua_state_transition(cmd, cdb))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
case ALUA_ACCESS_STATE_LBA_DEPENDENT:
- ret = core_alua_state_lba_dependent(cmd, tg_pt_gp, &alua_ascq);
+ if (core_alua_state_lba_dependent(cmd, tg_pt_gp))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
/*
* OFFLINE is a secondary ALUA target port group access state, that is
@@ -757,23 +768,6 @@ target_alua_state_check(struct se_cmd *cmd)
return TCM_INVALID_CDB_FIELD;
}
-out:
- if (ret > 0) {
- /*
- * Set SCSI additional sense code (ASC) to 'LUN Not Accessible';
- * The ALUA additional sense code qualifier (ASCQ) is determined
- * by the ALUA primary or secondary access state..
- */
- pr_debug("[%s]: ALUA TG Port not available, "
- "SenseKey: NOT_READY, ASC/ASCQ: "
- "0x04/0x%02x\n",
- cmd->se_tfo->get_fabric_name(), alua_ascq);
-
- cmd->scsi_asc = 0x04;
- cmd->scsi_ascq = alua_ascq;
- return TCM_CHECK_CONDITION_NOT_READY;
- }
-
return 0;
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] target_core_alua: silence GCC warning
2014-02-19 9:52 ` [PATCH] target_core_alua: silence GCC warning Paul Bolle
@ 2014-02-19 9:59 ` Geert Uytterhoeven
2014-02-19 10:05 ` Paul Bolle
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-02-19 9:59 UTC (permalink / raw)
To: Paul Bolle
Cc: Nicholas A. Bellinger, scsi, target-devel,
linux-kernel@vger.kernel.org
On Wed, Feb 19, 2014 at 10:52 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> - ret = core_alua_state_nonoptimized(cmd, cdb,
> - nonop_delay_msecs, &alua_ascq);
> + core_alua_state_nonoptimized(cmd, cdb, nonop_delay_msecs);
I suggest making core_alua_state_nonoptimized() return void, too.
Currently it always returns zero.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target_core_alua: silence GCC warning
2014-02-19 9:59 ` Geert Uytterhoeven
@ 2014-02-19 10:05 ` Paul Bolle
2014-02-19 22:59 ` Nicholas A. Bellinger
0 siblings, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2014-02-19 10:05 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Nicholas A. Bellinger, scsi, target-devel,
linux-kernel@vger.kernel.org
On Wed, 2014-02-19 at 10:59 +0100, Geert Uytterhoeven wrote:
> On Wed, Feb 19, 2014 at 10:52 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> > - ret = core_alua_state_nonoptimized(cmd, cdb,
> > - nonop_delay_msecs, &alua_ascq);
> > + core_alua_state_nonoptimized(cmd, cdb, nonop_delay_msecs);
>
> I suggest making core_alua_state_nonoptimized() return void, too.
> Currently it always returns zero.
Good catch. Not sure how I missed that, since I deliberately ignore its
return value here.
I'll wait with sending a v2 until after Nicholas or the other target
developers have given their feedback.
Paul Bolle
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target_core_alua: silence GCC warning
2014-02-19 10:05 ` Paul Bolle
@ 2014-02-19 22:59 ` Nicholas A. Bellinger
2014-02-20 8:07 ` [PATCH v2] " Paul Bolle
0 siblings, 1 reply; 6+ messages in thread
From: Nicholas A. Bellinger @ 2014-02-19 22:59 UTC (permalink / raw)
To: Paul Bolle
Cc: Geert Uytterhoeven, scsi, target-devel,
linux-kernel@vger.kernel.org
On Wed, 2014-02-19 at 11:05 +0100, Paul Bolle wrote:
> On Wed, 2014-02-19 at 10:59 +0100, Geert Uytterhoeven wrote:
> > On Wed, Feb 19, 2014 at 10:52 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> > > - ret = core_alua_state_nonoptimized(cmd, cdb,
> > > - nonop_delay_msecs, &alua_ascq);
> > > + core_alua_state_nonoptimized(cmd, cdb, nonop_delay_msecs);
> >
> > I suggest making core_alua_state_nonoptimized() return void, too.
> > Currently it always returns zero.
>
> Good catch. Not sure how I missed that, since I deliberately ignore its
> return value here.
>
> I'll wait with sending a v2 until after Nicholas or the other target
> developers have given their feedback.
>
>
Hi Paul,
The patch is fine with me.. Feel free to post a -v2.
Thanks,
--nab
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] target_core_alua: silence GCC warning
2014-02-19 22:59 ` Nicholas A. Bellinger
@ 2014-02-20 8:07 ` Paul Bolle
2014-02-20 18:33 ` Nicholas A. Bellinger
0 siblings, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2014-02-20 8:07 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Geert Uytterhoeven, linux-scsi, target-devel, linux-kernel
Building target_core_alua.o triggers a GCC warning:
drivers/target/target_core_alua.c: In function ‘target_alua_state_check’:
drivers/target/target_core_alua.c:773:18: warning: ‘alua_ascq’ may be used uninitialized in this function [-Wmaybe-uninitialized]
cmd->scsi_ascq = alua_ascq;
^
This is a false positive. A little trial and error shows it is
apparently caused by core_alua_state_lba_dependent(). It must be hard
for GCC to track the branches of a switch statement, inside a
list_for_each_entry loop, inside a while loop.
But if we add a small (inline) helper function we can reorganize the
code a bit. That also allows to drop alua_ascq which, obviously, gets
rid of this warning.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
v2: Make core_alua_state_nonoptimized() return void, as Geert
suggested.
Also keep core_alua_state_lba_dependent() inline. Setting that function
noinline was just a leftover from the trial and error fase, and isn't
needed to make the warning go away. Ie, I was sloppy in v1!
Still compile tested only.
drivers/target/target_core_alua.c | 95 ++++++++++++++++++---------------------
1 file changed, 44 insertions(+), 51 deletions(-)
diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index c3d9df6..fcbe612 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -455,11 +455,26 @@ out:
return rc;
}
-static inline int core_alua_state_nonoptimized(
+static inline void set_ascq(struct se_cmd *cmd, u8 alua_ascq)
+{
+ /*
+ * Set SCSI additional sense code (ASC) to 'LUN Not Accessible';
+ * The ALUA additional sense code qualifier (ASCQ) is determined
+ * by the ALUA primary or secondary access state..
+ */
+ pr_debug("[%s]: ALUA TG Port not available, "
+ "SenseKey: NOT_READY, ASC/ASCQ: "
+ "0x04/0x%02x\n",
+ cmd->se_tfo->get_fabric_name(), alua_ascq);
+
+ cmd->scsi_asc = 0x04;
+ cmd->scsi_ascq = alua_ascq;
+}
+
+static inline void core_alua_state_nonoptimized(
struct se_cmd *cmd,
unsigned char *cdb,
- int nonop_delay_msecs,
- u8 *alua_ascq)
+ int nonop_delay_msecs)
{
/*
* Set SCF_ALUA_NON_OPTIMIZED here, this value will be checked
@@ -468,13 +483,11 @@ static inline int core_alua_state_nonoptimized(
*/
cmd->se_cmd_flags |= SCF_ALUA_NON_OPTIMIZED;
cmd->alua_nonop_delay = nonop_delay_msecs;
- return 0;
}
static inline int core_alua_state_lba_dependent(
struct se_cmd *cmd,
- struct t10_alua_tg_pt_gp *tg_pt_gp,
- u8 *alua_ascq)
+ struct t10_alua_tg_pt_gp *tg_pt_gp)
{
struct se_device *dev = cmd->se_dev;
u64 segment_size, segment_mult, sectors, lba;
@@ -520,7 +533,7 @@ static inline int core_alua_state_lba_dependent(
}
if (!cur_map) {
spin_unlock(&dev->t10_alua.lba_map_lock);
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
list_for_each_entry(map_mem, &cur_map->lba_map_mem_list,
@@ -531,11 +544,11 @@ static inline int core_alua_state_lba_dependent(
switch(map_mem->lba_map_mem_alua_state) {
case ALUA_ACCESS_STATE_STANDBY:
spin_unlock(&dev->t10_alua.lba_map_lock);
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
case ALUA_ACCESS_STATE_UNAVAILABLE:
spin_unlock(&dev->t10_alua.lba_map_lock);
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
default:
break;
@@ -548,8 +561,7 @@ static inline int core_alua_state_lba_dependent(
static inline int core_alua_state_standby(
struct se_cmd *cmd,
- unsigned char *cdb,
- u8 *alua_ascq)
+ unsigned char *cdb)
{
/*
* Allowed CDBs for ALUA_ACCESS_STATE_STANDBY as defined by
@@ -570,7 +582,7 @@ static inline int core_alua_state_standby(
case MI_REPORT_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
}
case MAINTENANCE_OUT:
@@ -578,7 +590,7 @@ static inline int core_alua_state_standby(
case MO_SET_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
}
case REQUEST_SENSE:
@@ -588,7 +600,7 @@ static inline int core_alua_state_standby(
case WRITE_BUFFER:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
}
@@ -597,8 +609,7 @@ static inline int core_alua_state_standby(
static inline int core_alua_state_unavailable(
struct se_cmd *cmd,
- unsigned char *cdb,
- u8 *alua_ascq)
+ unsigned char *cdb)
{
/*
* Allowed CDBs for ALUA_ACCESS_STATE_UNAVAILABLE as defined by
@@ -613,7 +624,7 @@ static inline int core_alua_state_unavailable(
case MI_REPORT_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
case MAINTENANCE_OUT:
@@ -621,7 +632,7 @@ static inline int core_alua_state_unavailable(
case MO_SET_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
case REQUEST_SENSE:
@@ -629,7 +640,7 @@ static inline int core_alua_state_unavailable(
case WRITE_BUFFER:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+ set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
@@ -638,8 +649,7 @@ static inline int core_alua_state_unavailable(
static inline int core_alua_state_transition(
struct se_cmd *cmd,
- unsigned char *cdb,
- u8 *alua_ascq)
+ unsigned char *cdb)
{
/*
* Allowed CDBs for ALUA_ACCESS_STATE_TRANSITION as defined by
@@ -654,7 +664,7 @@ static inline int core_alua_state_transition(
case MI_REPORT_TARGET_PGS:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_STATE_TRANSITION;
+ set_ascq(cmd, ASCQ_04H_ALUA_STATE_TRANSITION);
return 1;
}
case REQUEST_SENSE:
@@ -662,7 +672,7 @@ static inline int core_alua_state_transition(
case WRITE_BUFFER:
return 0;
default:
- *alua_ascq = ASCQ_04H_ALUA_STATE_TRANSITION;
+ set_ascq(cmd, ASCQ_04H_ALUA_STATE_TRANSITION);
return 1;
}
@@ -684,8 +694,6 @@ target_alua_state_check(struct se_cmd *cmd)
struct t10_alua_tg_pt_gp *tg_pt_gp;
struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem;
int out_alua_state, nonop_delay_msecs;
- u8 alua_ascq;
- int ret;
if (dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE)
return 0;
@@ -701,9 +709,8 @@ target_alua_state_check(struct se_cmd *cmd)
if (atomic_read(&port->sep_tg_pt_secondary_offline)) {
pr_debug("ALUA: Got secondary offline status for local"
" target port\n");
- alua_ascq = ASCQ_04H_ALUA_OFFLINE;
- ret = 1;
- goto out;
+ set_ascq(cmd, ASCQ_04H_ALUA_OFFLINE);
+ return TCM_CHECK_CONDITION_NOT_READY;
}
/*
* Second, obtain the struct t10_alua_tg_pt_gp_member pointer to the
@@ -731,20 +738,23 @@ target_alua_state_check(struct se_cmd *cmd)
switch (out_alua_state) {
case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
- ret = core_alua_state_nonoptimized(cmd, cdb,
- nonop_delay_msecs, &alua_ascq);
+ core_alua_state_nonoptimized(cmd, cdb, nonop_delay_msecs);
break;
case ALUA_ACCESS_STATE_STANDBY:
- ret = core_alua_state_standby(cmd, cdb, &alua_ascq);
+ if (core_alua_state_standby(cmd, cdb))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
case ALUA_ACCESS_STATE_UNAVAILABLE:
- ret = core_alua_state_unavailable(cmd, cdb, &alua_ascq);
+ if (core_alua_state_unavailable(cmd, cdb))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
case ALUA_ACCESS_STATE_TRANSITION:
- ret = core_alua_state_transition(cmd, cdb, &alua_ascq);
+ if (core_alua_state_transition(cmd, cdb))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
case ALUA_ACCESS_STATE_LBA_DEPENDENT:
- ret = core_alua_state_lba_dependent(cmd, tg_pt_gp, &alua_ascq);
+ if (core_alua_state_lba_dependent(cmd, tg_pt_gp))
+ return TCM_CHECK_CONDITION_NOT_READY;
break;
/*
* OFFLINE is a secondary ALUA target port group access state, that is
@@ -757,23 +767,6 @@ target_alua_state_check(struct se_cmd *cmd)
return TCM_INVALID_CDB_FIELD;
}
-out:
- if (ret > 0) {
- /*
- * Set SCSI additional sense code (ASC) to 'LUN Not Accessible';
- * The ALUA additional sense code qualifier (ASCQ) is determined
- * by the ALUA primary or secondary access state..
- */
- pr_debug("[%s]: ALUA TG Port not available, "
- "SenseKey: NOT_READY, ASC/ASCQ: "
- "0x04/0x%02x\n",
- cmd->se_tfo->get_fabric_name(), alua_ascq);
-
- cmd->scsi_asc = 0x04;
- cmd->scsi_ascq = alua_ascq;
- return TCM_CHECK_CONDITION_NOT_READY;
- }
-
return 0;
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] target_core_alua: silence GCC warning
2014-02-20 8:07 ` [PATCH v2] " Paul Bolle
@ 2014-02-20 18:33 ` Nicholas A. Bellinger
0 siblings, 0 replies; 6+ messages in thread
From: Nicholas A. Bellinger @ 2014-02-20 18:33 UTC (permalink / raw)
To: Paul Bolle; +Cc: Geert Uytterhoeven, linux-scsi, target-devel, linux-kernel
On Thu, 2014-02-20 at 09:07 +0100, Paul Bolle wrote:
> Building target_core_alua.o triggers a GCC warning:
> drivers/target/target_core_alua.c: In function ‘target_alua_state_check’:
> drivers/target/target_core_alua.c:773:18: warning: ‘alua_ascq’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> cmd->scsi_ascq = alua_ascq;
> ^
>
> This is a false positive. A little trial and error shows it is
> apparently caused by core_alua_state_lba_dependent(). It must be hard
> for GCC to track the branches of a switch statement, inside a
> list_for_each_entry loop, inside a while loop.
>
> But if we add a small (inline) helper function we can reorganize the
> code a bit. That also allows to drop alua_ascq which, obviously, gets
> rid of this warning.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> v2: Make core_alua_state_nonoptimized() return void, as Geert
> suggested.
>
> Also keep core_alua_state_lba_dependent() inline. Setting that function
> noinline was just a leftover from the trial and error fase, and isn't
> needed to make the warning go away. Ie, I was sloppy in v1!
>
> Still compile tested only.
>
> drivers/target/target_core_alua.c | 95 ++++++++++++++++++---------------------
> 1 file changed, 44 insertions(+), 51 deletions(-)
>
Applied to target-pending/for-next.
Thanks Paul!
--nab
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-20 18:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1392714172-2712-1-git-send-email-geert@linux-m68k.org>
2014-02-19 9:52 ` [PATCH] target_core_alua: silence GCC warning Paul Bolle
2014-02-19 9:59 ` Geert Uytterhoeven
2014-02-19 10:05 ` Paul Bolle
2014-02-19 22:59 ` Nicholas A. Bellinger
2014-02-20 8:07 ` [PATCH v2] " Paul Bolle
2014-02-20 18:33 ` Nicholas A. Bellinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox