Re: [Open-FCoE] [PATCH] scsi: fix Wunused-but-set-variable buildwarning
On Thu, 2015-05-14 at 20:12 +0200, Nicholas Mc Guire wrote:
> commit "[SCSI] libfc: remove tgt_flags from fc_fcp_pkt struct"
> removed the last usage of rpriv (rpriv->flags) but forgot to remove
> the unused rpriv struct resulting in:
> drivers/scsi/libfc/fc_fcp.c: In function 'fc_queuecommand':
> drivers/scsi/libfc/fc_fcp.c:1795:30: warning: variable 'rpriv' set
> but not used [-Wunused-but-set-variable]
> so simply drop its declaration and setting.
>
> Patch was compile tested with x86_64_defconfig + SCSI_LOWLEVEL=y,
> CONFIG_SCSI_FC_ATTRS=m, CONFIG_LIBFC=m
>
> Patch is against 4.1-rc3 (localversion-next is -next-20150514)
>
> Signed-off-by: Nicholas Mc Guire
> ---
>
> drivers/scsi/libfc/fc_fcp.c |3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
> index c438b81..fee6928 100644
> --- a/drivers/scsi/libfc/fc_fcp.c
> +++ b/drivers/scsi/libfc/fc_fcp.c
> @@ -1792,7 +1792,6 @@ int fc_queuecommand(struct Scsi_Host *shost, struct
> scsi_cmnd *sc_cmd)
> struct fc_lport *lport = shost_priv(shost);
> struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
> struct fc_fcp_pkt *fsp;
> - struct fc_rport_libfc_priv *rpriv;
> int rval;
> int rc = 0;
> struct fc_stats *stats;
> @@ -1814,8 +1813,6 @@ int fc_queuecommand(struct Scsi_Host *shost, struct
> scsi_cmnd *sc_cmd)
> goto out;
> }
>
> - rpriv = rport->dd_data;
> -
> if (!fc_fcp_lport_queue_ready(lport)) {
> if (lport->qfull)
> fc_fcp_can_queue_ramp_down(lport);
Looks good, Thx
Acked-by: Vasu Dev
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Open-FCoE] [PATCH] scsi: match wait_for_completion_timeout return type
On Thu, 2015-05-14 at 20:12 +0200, Nicholas Mc Guire wrote:
> Return type of wait_for_completion_timeout is unsigned long not int.
> An appropriately named unsigned long is added, and the assignments
> as well as error checking fixed up.
>
> API conformance testing for completions with coccinelle spatches are being
> used to locate API usage inconsistencies:
> ./drivers/scsi/libfc/fc_fcp.c:1279
> int return assigned to unsigned long
>
> Patch was compile tested with x86_64_defconfig + SCSI_LOWLEVEL=y,
> CONFIG_SCSI_FC_ATTRS=m, CONFIG_LIBFC=m
>
> Patch is against 4.1-rc3 (localversion-next is -next-20150514)
>
> Signed-off-by: Nicholas Mc Guire
> ---
> drivers/scsi/libfc/fc_fcp.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
> index c679594..c438b81 100644
> --- a/drivers/scsi/libfc/fc_fcp.c
> +++ b/drivers/scsi/libfc/fc_fcp.c
> @@ -1261,7 +1261,7 @@ static void fc_lun_reset_send(unsigned long data)
> static int fc_lun_reset(struct fc_lport *lport, struct fc_fcp_pkt *fsp,
> unsigned int id, unsigned int lun)
> {
> - int rc;
> + unsigned long time_left;
>
> fsp->cdb_cmd.fc_dl = htonl(fsp->data_len);
> fsp->cdb_cmd.fc_tm_flags = FCP_TMF_LUN_RESET;
> @@ -1276,7 +1276,7 @@ static int fc_lun_reset(struct fc_lport *lport, struct
> fc_fcp_pkt *fsp,
>* wait for completion of reset
>* after that make sure all commands are terminated
>*/
> - rc = wait_for_completion_timeout(&fsp->tm_done, FC_SCSI_TM_TOV);
> + time_left = wait_for_completion_timeout(&fsp->tm_done, FC_SCSI_TM_TOV);
>
> spin_lock_bh(&fsp->scsi_pkt_lock);
> fsp->state |= FC_SRB_COMPL;
> @@ -1292,7 +1292,7 @@ static int fc_lun_reset(struct fc_lport *lport, struct
> fc_fcp_pkt *fsp,
> fsp->wait_for_comp = 0;
> spin_unlock_bh(&fsp->scsi_pkt_lock);
>
> - if (!rc) {
> + if (!time_left) {
> FC_SCSI_DBG(lport, "lun reset failed\n");
> return FAILED;
> }
Looks good, Thx
Acked-by: Vasu Dev
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Open-FCoE] [PATCH] scsi: fix Wunused-but-set-variable buildwarning
On Fri, 2015-05-15 at 09:14 +0200, Nicholas Mc Guire wrote: > On Thu, 14 May 2015, Prasad Gondi wrote: > > > It seems like rpriv is used to set the fsp->tgt_flags originally > > > > > fsp->tgt_flags = rpriv->flags > > > > And fsp->tgt_flags are used in "fc_fcp_cmd_send" like this > > > > setup_timer(&fsp->timer, fc_fcp_timeout, (unsigned long)fsp); > > if (rpriv->flags & FC_RP_FLAGS_REC_SUPPORTED) > > fc_fcp_timer_set(fsp, get_fsp_rec_tov(fsp)); > > > > Main purpose of this flags used is to set the correct TimeOut Value for > > fc_fcp_timer. > > > > So is the removal of the "fsp->tgt_flags = rpriv->flags" in > > fc_queuecommand() is intentional? Or by mistake? > > > thats something I can't say - but the commit message indicated that the > removal of tgt_flags was intentional. > It was intentional and good cleanup since now rpriv->flags is used directly with that change instead of storing in fsp->tgt_flags as it was before. > > Once we clear that out we can see whether this change make sense? > > Anycase the patch under discussion is straight forward clean up patch since it just removes a local stack variable which is not used. Thanks, Vasu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
