sparse fixes for nvme/fc

sparse fixes for nvme/fc

[Christoph Hellwig]

[sorry for the double post, the first series got the linux-nvme
 address wrong]

This fixes a couple ?parse warnings in the FC code, missing statics,
missing endianess annotations and incorrect byte swaps.

[PATCH 1/7] nvme/fc: mark two symbols static

[Christoph Hellwig]

Found by sparse.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/fc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 596b3a453b54..63819b12c1da 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1183,7 +1183,7 @@ nvme_fc_exit_aen_ops(struct nvme_fc_ctrl *ctrl)
 	}
 }
 
-void
+static void
 nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 {
 	struct nvme_fc_fcp_op *op = fcp_req_to_fcp_op(req);
@@ -1671,7 +1671,7 @@ __nvme_fc_abort_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_fcp_op *op)
 	return 0;
 }
 
-enum blk_eh_timer_return
+static enum blk_eh_timer_return
 nvme_fc_timeout(struct request *rq, bool reserved)
 {
 	struct nvme_fc_fcp_op *op = blk_mq_rq_to_pdu(rq);
-- 
2.11.0

[PATCH 2/7] nvme/fc: don't byte swap command_id

[Christoph Hellwig]

The command ID is a field for host only use and thus always is in
native endian.

Found by sparse.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/fc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 63819b12c1da..68887818801e 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1274,7 +1274,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 			     be32_to_cpu(op->rsp_iu.xfrd_len) !=
 					freq->transferred_length ||
 			     op->rsp_iu.status_code ||
-			     op->rqno != le16_to_cpu(cqe->command_id))) {
+			     op->rqno != cqe->command_id)) {
 			status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
 			goto done;
 		}
@@ -1830,7 +1830,7 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
 	sqe->rw.dptr.sgl.addr = 0;
 
 	/* odd that we set the command_id - should come from nvme-fabrics */
-	WARN_ON_ONCE(sqe->common.command_id != cpu_to_le16(op->rqno));
+	WARN_ON_ONCE(sqe->common.command_id != op->rqno);
 
 	if (op->rq) {				/* skipped on aens */
 		ret = nvme_fc_map_data(ctrl, op->rq, op);
-- 
2.11.0

[PATCH 3/7] nvmet/fc: mark nvmet_fc_handle_fcp_rqst static

[Christoph Hellwig]

Found by sparse.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/fc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 074bd3743b5f..3626dd8cc8a1 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -2055,7 +2055,7 @@ nvmet_fc_fcp_nvme_cmd_done(struct nvmet_req *nvme_req)
 /*
  * Actual processing routine for received FC-NVME LS Requests from the LLD
  */
-void
+static void
 nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport,
 			struct nvmet_fc_fcp_iod *fod)
 {
-- 
2.11.0

[PATCH 4/7] nvmet/fc: fix endianess annoations for nvmet_fc_format_rsp_hdr

[Christoph Hellwig]

The passed in desc_len is a big endian value, so mark it as such.

Found by sparse.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/fc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 3626dd8cc8a1..d504c52ac9cb 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1058,7 +1058,7 @@ EXPORT_SYMBOL_GPL(nvmet_fc_unregister_targetport);
 
 
 static void
-nvmet_fc_format_rsp_hdr(void *buf, u8 ls_cmd, u32 desc_len, u8 rqst_ls_cmd)
+nvmet_fc_format_rsp_hdr(void *buf, u8 ls_cmd, __be32 desc_len, u8 rqst_ls_cmd)
 {
 	struct fcnvme_ls_acc_hdr *acc = buf;
 
-- 
2.11.0

[PATCH 5/7] nvmet/fc: mark the sqhd field as __le16

[Christoph Hellwig]

That's what it's used as.

Found by sparse.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/fc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index d504c52ac9cb..4392b514725b 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -119,7 +119,7 @@ struct nvmet_fc_tgt_queue {
 	u16				qid;
 	u16				sqsize;
 	u16				ersp_ratio;
-	u16				sqhd;
+	__le16				sqhd;
 	int				cpu;
 	atomic_t			connected;
 	atomic_t			sqtail;
-- 
2.11.0

[PATCH 6/7] nvmet/fc: properly endian swap sq_head

[Christoph Hellwig]

Found by sparse.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/fc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 4392b514725b..62eba29c85fb 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1700,7 +1700,7 @@ nvmet_fc_prep_fcp_rsp(struct nvmet_fc_tgtport *tgtport,
 	    xfr_length != fod->total_length ||
 	    (le16_to_cpu(cqe->status) & 0xFFFE) || cqewd[0] || cqewd[1] ||
 	    (sqe->flags & (NVME_CMD_FUSE_FIRST | NVME_CMD_FUSE_SECOND)) ||
-	    queue_90percent_full(fod->queue, cqe->sq_head))
+	    queue_90percent_full(fod->queue, le16_to_cpu(cqe->sq_head)))
 		send_ersp = true;
 
 	/* re-set the fields */
-- 
2.11.0

[PATCH 7/7] nvmet/fcloop: mark two symbols static

[Christoph Hellwig]

Found by sparse.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/fcloop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index aaa3dbe22bd5..15551ef79c8c 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -666,7 +666,7 @@ fcloop_targetport_delete(struct nvmet_fc_target_port *targetport)
 #define	FCLOOP_SGL_SEGS			256
 #define FCLOOP_DMABOUND_4G		0xFFFFFFFF
 
-struct nvme_fc_port_template fctemplate = {
+static struct nvme_fc_port_template fctemplate = {
 	.localport_delete	= fcloop_localport_delete,
 	.remoteport_delete	= fcloop_remoteport_delete,
 	.create_queue		= fcloop_create_queue,
@@ -686,7 +686,7 @@ struct nvme_fc_port_template fctemplate = {
 	.fcprqst_priv_sz	= sizeof(struct fcloop_ini_fcpreq),
 };
 
-struct nvmet_fc_target_template tgttemplate = {
+static struct nvmet_fc_target_template tgttemplate = {
 	.targetport_delete	= fcloop_targetport_delete,
 	.xmt_ls_rsp		= fcloop_xmt_ls_rsp,
 	.fcp_op			= fcloop_fcp_op,
-- 
2.11.0

[PATCH 1/7] nvme/fc: mark two symbols static

[Johannes Thumshirn]

On Fri, Apr 21, 2017@10:49:59AM +0200, Christoph Hellwig wrote:
> Found by sparse.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 2/7] nvme/fc: don't byte swap command_id

[Johannes Thumshirn]

On Fri, Apr 21, 2017@10:50:00AM +0200, Christoph Hellwig wrote:
> The command ID is a field for host only use and thus always is in
> native endian.
> 
> Found by sparse.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 3/7] nvmet/fc: mark nvmet_fc_handle_fcp_rqst static

[Johannes Thumshirn]

On Fri, Apr 21, 2017@10:50:01AM +0200, Christoph Hellwig wrote:
> Found by sparse.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 4/7] nvmet/fc: fix endianess annoations for nvmet_fc_format_rsp_hdr

[Johannes Thumshirn]

On Fri, Apr 21, 2017@10:50:02AM +0200, Christoph Hellwig wrote:
> The passed in desc_len is a big endian value, so mark it as such.
> 
> Found by sparse.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 5/7] nvmet/fc: mark the sqhd field as __le16

[Johannes Thumshirn]

On Fri, Apr 21, 2017@10:50:03AM +0200, Christoph Hellwig wrote:
> That's what it's used as.
> 
> Found by sparse.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 6/7] nvmet/fc: properly endian swap sq_head

[Johannes Thumshirn]

On Fri, Apr 21, 2017@10:50:04AM +0200, Christoph Hellwig wrote:
> Found by sparse.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 7/7] nvmet/fcloop: mark two symbols static

[Johannes Thumshirn]

On Fri, Apr 21, 2017@10:50:05AM +0200, Christoph Hellwig wrote:
> Found by sparse.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 1/7] nvme/fc: mark two symbols static

[James Smart]

Looks good

Signed-off-by: James Smart <james.smart at broadcom.com>

-- james

[PATCH 2/7] nvme/fc: don't byte swap command_id

[James Smart]

On 4/21/2017 1:50 AM, Christoph Hellwig wrote:
> The command ID is a field for host only use and thus always is in
> native endian.
>
> Found by sparse.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
>   drivers/nvme/host/fc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 63819b12c1da..68887818801e 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -1274,7 +1274,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
>   			     be32_to_cpu(op->rsp_iu.xfrd_len) !=
>   					freq->transferred_length ||
>   			     op->rsp_iu.status_code ||
> -			     op->rqno != le16_to_cpu(cqe->command_id))) {
> +			     op->rqno != cqe->command_id)) {
>   			status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
>   			goto done;
>   		}

This check was replaced in the following patch, obsoleting this change:
http://lists.infradead.org/pipermail/linux-nvme/2017-April/009259.html


> @@ -1830,7 +1830,7 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
>   	sqe->rw.dptr.sgl.addr = 0;
>   
>   	/* odd that we set the command_id - should come from nvme-fabrics */
> -	WARN_ON_ONCE(sqe->common.command_id != cpu_to_le16(op->rqno));
> +	WARN_ON_ONCE(sqe->common.command_id != op->rqno);
>   
>   	if (op->rq) {				/* skipped on aens */
>   		ret = nvme_fc_map_data(ctrl, op->rq, op);

This line was removed by the following patch, obsoleting this change:
http://lists.infradead.org/pipermail/linux-nvme/2017-April/009260.html

-- james

[PATCH 5/7] nvmet/fc: mark the sqhd field as __le16

[Christoph Hellwig]

On Fri, Apr 21, 2017@06:17:08PM -0700, James Smart wrote:
> This is fine, but this also highlights several different areas where sqhd 
> is referenced and endianness wasn't accounted for.  So I'll need to make a 
> complimentary patch.

All the assignments to/from it are already to __le16 values.

[PATCH 2/7] nvme/fc: don't byte swap command_id

[Christoph Hellwig]

On Fri, Apr 21, 2017@06:10:28PM -0700, James Smart wrote:
> On 4/21/2017 1:50 AM, Christoph Hellwig wrote:
>> The command ID is a field for host only use and thus always is in
>> native endian.
>>
>> Found by sparse.
>>
>> Signed-off-by: Christoph Hellwig <hch at lst.de>
>> ---
>>   drivers/nvme/host/fc.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
>> index 63819b12c1da..68887818801e 100644
>> --- a/drivers/nvme/host/fc.c
>> +++ b/drivers/nvme/host/fc.c
>> @@ -1274,7 +1274,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
>>   			     be32_to_cpu(op->rsp_iu.xfrd_len) !=
>>   					freq->transferred_length ||
>>   			     op->rsp_iu.status_code ||
>> -			     op->rqno != le16_to_cpu(cqe->command_id))) {
>> +			     op->rqno != cqe->command_id)) {
>>   			status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
>>   			goto done;
>>   		}
>
> This check was replaced in the following patch, obsoleting this change:
> http://lists.infradead.org/pipermail/linux-nvme/2017-April/009259.html

Can you please resend this and the two patches related to it?  They had
non-trivial conflicts with the error status rework.

[PATCH 2/7] nvme/fc: don't byte swap command_id

[James Smart]

On 4/23/2017 12:43 AM, Christoph Hellwig wrote:
> On Fri, Apr 21, 2017@06:10:28PM -0700, James Smart wrote:
>> On 4/21/2017 1:50 AM, Christoph Hellwig wrote:
>>> The command ID is a field for host only use and thus always is in
>>> native endian.
>>>
>>> Found by sparse.
>>>
>>> Signed-off-by: Christoph Hellwig <hch at lst.de>
>>> ---
>>>    drivers/nvme/host/fc.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
>>> index 63819b12c1da..68887818801e 100644
>>> --- a/drivers/nvme/host/fc.c
>>> +++ b/drivers/nvme/host/fc.c
>>> @@ -1274,7 +1274,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
>>>    			     be32_to_cpu(op->rsp_iu.xfrd_len) !=
>>>    					freq->transferred_length ||
>>>    			     op->rsp_iu.status_code ||
>>> -			     op->rqno != le16_to_cpu(cqe->command_id))) {
>>> +			     op->rqno != cqe->command_id)) {
>>>    			status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
>>>    			goto done;
>>>    		}
>> This check was replaced in the following patch, obsoleting this change:
>> http://lists.infradead.org/pipermail/linux-nvme/2017-April/009259.html
> Can you please resend this and the two patches related to it?  They had
> non-trivial conflicts with the error status rework.

Resent: As your patch had been merged, the new patches layer on top.

-- james