-----Original Message-----
From: Johannes Thumshirn <jthumsh...@suse.de>
Date: Thursday, March 31, 2016 at 12:37 AM
To: Bart Van Assche <bart.vanass...@sandisk.com>
Cc: James Bottomley <j...@linux.vnet.ibm.com>, "Martin K. Petersen" 
<martin.peter...@oracle.com>, Himanshu Madhani <himanshu.madh...@qlogic.com>, 
Quinn Tran <quinn.t...@qlogic.com>, Christoph Hellwig <h...@lst.de>, linux-scsi 
<linux-scsi@vger.kernel.org>, "linux-scsi-ow...@vger.kernel.org" 
<linux-scsi-ow...@vger.kernel.org>
Subject: Re: [PATCH 2/3] qla2xxx: Remove set-but-not-used variables

>On 2016-03-31 01:25, Bart Van Assche wrote:
>> Detected these variables by building the qla2xxx driver with W=1.
>> Note: removing the code that sets BIT_14 is fine since that bit is
>> never tested. The output of git grep -nH -- '->cmd_flags\s*&'
>> drivers/scsi/qla2xxx
>> is as follows:
>> 
>> drivers/scsi/qla2xxx/tcm_qla2xxx.c:285:      WARN_ON(cmd->cmd_flags &  
>> BIT_16);
>> drivers/scsi/qla2xxx/tcm_qla2xxx.c:302:      BUG_ON(cmd->cmd_flags & 
>> BIT_20);
>> drivers/scsi/qla2xxx/tcm_qla2xxx.c:627:      if (cmd->cmd_flags &  BIT_5) {
>> 
>> Signed-off-by: Bart Van Assche <bart.vanass...@sandisk.com>
>> Cc: Quinn Tran <quinn.t...@qlogic.com>
>> Cc: Himanshu Madhani <himanshu.madh...@qlogic.com>
>> Cc: Christoph Hellwig <h...@lst.de>
>> ---
>>  drivers/scsi/qla2xxx/qla_attr.c    | 3 +--
>>  drivers/scsi/qla2xxx/qla_mbx.c     | 2 --
>>  drivers/scsi/qla2xxx/tcm_qla2xxx.c | 7 -------
>>  3 files changed, 1 insertion(+), 11 deletions(-)
>> 
>> diff --git a/drivers/scsi/qla2xxx/qla_attr.c 
>> b/drivers/scsi/qla2xxx/qla_attr.c
>> index 4dc06a13..db85c5c 100644
>> --- a/drivers/scsi/qla2xxx/qla_attr.c
>> +++ b/drivers/scsi/qla2xxx/qla_attr.c
>> @@ -839,7 +839,6 @@ qla2x00_issue_logo(struct file *filp, struct 
>> kobject *kobj,
>>      struct scsi_qla_host *vha = 
>> shost_priv(dev_to_shost(container_of(kobj,
>>          struct device, kobj)));
>>      int type;
>> -    int rval = 0;
>>      port_id_t did;
>> 
>>      type = simple_strtol(buf, NULL, 10);
>> @@ -853,7 +852,7 @@ qla2x00_issue_logo(struct file *filp, struct 
>> kobject *kobj,
>> 
>>      ql_log(ql_log_info, vha, 0x70e4, "%s: %d\n", __func__, type);
>> 
>> -    rval = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did);
>> +    qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did);
>>      return count;
>>  }
>> 
>
>OK, qla24xx_els_dcmd_iocb() is a bit scary. It can return -ENOMEM, but 
>no caller checks it (the 2nd caller _only_ checks the return value in a 
>debug message). Maybe we should think about that.
>
>> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c 
>> b/drivers/scsi/qla2xxx/qla_mbx.c
>> index 968b846..bf2d357 100644
>> --- a/drivers/scsi/qla2xxx/qla_mbx.c
>> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
>> @@ -607,7 +607,6 @@ qla_set_exlogin_mem_cfg(scsi_qla_host_t *vha,
>> dma_addr_t phys_addr)
>>      mbx_cmd_t       mc;
>>      mbx_cmd_t       *mcp = &mc;
>>      struct qla_hw_data *ha = vha->hw;
>> -    int configured_count;
>> 
>>      ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x111a,
>>          "Entered %s.\n", __func__);
>> @@ -630,7 +629,6 @@ qla_set_exlogin_mem_cfg(scsi_qla_host_t *vha,
>> dma_addr_t phys_addr)
>>              /*EMPTY*/
>>              ql_dbg(ql_dbg_mbx, vha, 0x111b, "Failed=%x.\n", rval);
>>      } else {
>> -            configured_count = mcp->mb[11];
>>              ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x118c,
>>                  "Done %s.\n", __func__);
>>      }
>> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> index c1461d2..97fcbf2 100644
>> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> @@ -311,13 +311,6 @@ static void tcm_qla2xxx_free_cmd(struct 
>> qla_tgt_cmd *cmd)
>>   */
>>  static int tcm_qla2xxx_check_stop_free(struct se_cmd *se_cmd)
>>  {
>> -    struct qla_tgt_cmd *cmd;
>> -
>> -    if ((se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) == 0) {
>> -            cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);
>> -            cmd->cmd_flags |= BIT_14;
>> -    }
>> -
>>      return target_put_sess_cmd(se_cmd);
>>  }
>
>Apart from the qla24xx_els_dcmd_iocb() thingy (which has nothing to do 
>with your patch)
>Reviewed-by: Johannes Thumshirn <jthumsh...@suse.de>

QT: We use the cmd_flags portion for debugging crashes.  It’s our form of 
tracing. Please do not delete. Thanks.


>
N�����r��y����b�X��ǧv�^�)޺{.n�+����{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�m��������zZ+�����ݢj"��!�i

Reply via email to