* [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