sparse fixes

PATCH New Christoph Hellwig 2 participants 10 messages Dec 13, 2018 – Dec 18, 2018

Subsystems (2)

NVMe NVMe target

lore

Share & tools

lore

Christoph Hellwig

author maintainer

2018-12-13 08:51 UTC #0 lore raw

Mostly endianess annoations in the new TCP code, but nvme_poll_irqdisable didn't make sparse happy, so a small fix there as well.

Christoph Hellwig

author maintainer

2018-12-13 08:51 UTC #1 lore raw

PATCH 1/3 nvmet-tcp: fix endianess annotations

Signed-off-by Christoph Hellwig <hch at lst.de>

---
 drivers/nvme/target/tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/nvme/target/tcp.c +2 -2

index d31bec260160..44b37b202e39 100644
@@ -758,7 +758,7 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
 
 	if (icreq->maxr2t != 0) {
 		pr_err("queue %d: unsupported maxr2t %d\n", queue->idx,
-			le16_to_cpu(icreq->maxr2t) + 1);
+			le32_to_cpu(icreq->maxr2t) + 1);
 		return -EPROTO;
 	}

@@ -776,7 +776,7 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
 	icresp->hdr.pdo = 0;
 	icresp->hdr.plen = cpu_to_le32(icresp->hdr.hlen);
 	icresp->pfv = cpu_to_le16(NVME_TCP_PFV_1_0);
-	icresp->maxdata = 0xffff; /* FIXME: support r2t */
+	icresp->maxdata = cpu_to_le32(0xffff); /* FIXME: support r2t */
 	icresp->cpda = 0;
 	if (queue->hdr_digest)
 		icresp->digest |= NVME_TCP_HDR_DIGEST_ENABLE;

Signature · 1 line

2.19.2

Sagi Grimberg

maintainer

2018-12-13 16:42 UTC #2 lore raw

PATCH 1/3 nvmet-tcp: fix endianess annotations

drivers/nvme/target/tcp.c static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue) · line 782

Quoted 10 lines

drivers/nvme/target/tcp.c +1 -1

@@ -776,7 +776,7 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
  	icresp->hdr.pdo = 0;
  	icresp->hdr.plen = cpu_to_le32(icresp->hdr.hlen);
  	icresp->pfv = cpu_to_le16(NVME_TCP_PFV_1_0);
-	icresp->maxdata = 0xffff; /* FIXME: support r2t */
+	icresp->maxdata = cpu_to_le32(0xffff); /* FIXME: support r2t */
  	icresp->cpda = 0;
  	if (queue->hdr_digest)
  		icresp->digest |= NVME_TCP_HDR_DIGEST_ENABLE;

This needs an actual fix, we need to send 0xffffffff as we don't have this any limitation..

Christoph Hellwig

author maintainer

2018-12-13 20:10 UTC #3 lore raw

PATCH 1/3 nvmet-tcp: fix endianess annotations

Quoted 13 lines · Sagi Grimberg

drivers/nvme/target/tcp.c static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue) · line 782
Quoted 10 lines
drivers/nvme/target/tcp.c +1 -1
@@ -776,7 +776,7 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
  	icresp->hdr.pdo = 0;
  	icresp->hdr.plen = cpu_to_le32(icresp->hdr.hlen);
  	icresp->pfv = cpu_to_le16(NVME_TCP_PFV_1_0);
-	icresp->maxdata = 0xffff; /* FIXME: support r2t */
+	icresp->maxdata = cpu_to_le32(0xffff); /* FIXME: support r2t */
  	icresp->cpda = 0;
  	if (queue->hdr_digest)
  		icresp->digest |= NVME_TCP_HDR_DIGEST_ENABLE;
This needs an actual fix, we need to send 0xffffffff as we don't have this any limitation..

Please either send a fix on top of this, or if you want separately and I'll rebase.

Christoph Hellwig

author maintainer

2018-12-16 16:24 UTC #4 lore raw

PATCH 1/3 nvmet-tcp: fix endianess annotations

In reply to Sagi Grimberg

Quoted 13 lines · Sagi Grimberg

drivers/nvme/target/tcp.c static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue) · line 782
Quoted 10 lines
drivers/nvme/target/tcp.c +1 -1
@@ -776,7 +776,7 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
  	icresp->hdr.pdo = 0;
  	icresp->hdr.plen = cpu_to_le32(icresp->hdr.hlen);
  	icresp->pfv = cpu_to_le16(NVME_TCP_PFV_1_0);
-	icresp->maxdata = 0xffff; /* FIXME: support r2t */
+	icresp->maxdata = cpu_to_le32(0xffff); /* FIXME: support r2t */
  	icresp->cpda = 0;
  	if (queue->hdr_digest)
  		icresp->digest |= NVME_TCP_HDR_DIGEST_ENABLE;
This needs an actual fix, we need to send 0xffffffff as we don't have this any limitation..

Can I get an ACK for this for now? I can also send the proper fix on top then..

Sagi Grimberg

maintainer

2018-12-18 07:24 UTC #5 lore raw

PATCH 1/3 nvmet-tcp: fix endianess annotations

Quoted 3 lines

Can I get an ACK for this for now? I can also send the proper fix on top then..

Acked-by Sagi Grimberg <sagi at grimberg.me>

Christoph Hellwig

author maintainer

2018-12-13 08:51 UTC #6 lore raw

PATCH 2/3 nvmet-tcp: fix endianess annotations

In reply to Christoph Hellwig

Signed-off-by Christoph Hellwig <hch at lst.de>

---
 drivers/nvme/host/tcp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

drivers/nvme/host/tcp.c +5 -4

index 15543358e245..5b64051671ca 100644
@@ -35,7 +35,7 @@ struct nvme_tcp_request {
 	u32			pdu_sent;
 	u16			ttag;
 	struct list_head	entry;
-	u32			ddgst;
+	__le32			ddgst;
 
 	struct bio		*curr_bio;
 	struct iov_iter		iter;

@@ -272,7 +272,8 @@ nvme_tcp_fetch_request(struct nvme_tcp_queue *queue)
 	return req;
 }
 
-static inline void nvme_tcp_ddgst_final(struct ahash_request *hash, u32 *dgst)
+static inline void nvme_tcp_ddgst_final(struct ahash_request *hash,
+		__le32 *dgst)
 {
 	ahash_request_set_crypt(hash, NULL, (u8 *)dgst, 0);
 	crypto_ahash_final(hash);

@@ -817,7 +818,7 @@ static void nvme_tcp_fail_request(struct nvme_tcp_request *req)
 	union nvme_result res = {};
 
 	nvme_end_request(blk_mq_rq_from_pdu(req),
-		NVME_SC_DATA_XFER_ERROR, res);
+		cpu_to_le16(NVME_SC_DATA_XFER_ERROR), res);
 }
 
 static int nvme_tcp_try_send_data(struct nvme_tcp_request *req)

@@ -1950,7 +1951,7 @@ nvme_tcp_timeout(struct request *rq, bool reserved)
 		union nvme_result res = {};
 
 		nvme_req(rq)->flags |= NVME_REQ_CANCELLED;
-		nvme_end_request(rq, NVME_SC_ABORT_REQ, res);
+		nvme_end_request(rq, cpu_to_le16(NVME_SC_ABORT_REQ), res);
 		return BLK_EH_DONE;
 	}

Signature · 1 line

2.19.2

Sagi Grimberg

maintainer

2018-12-13 16:42 UTC #7 lore raw

PATCH 2/3 nvmet-tcp: fix endianess annotations

Reviewed-by Sagi Grimberg <sagi at grimberg.me>

Christoph Hellwig

author maintainer

2018-12-13 08:51 UTC #8 lore raw

PATCH 3/3 nvme-pci: refactor nvme_poll_irqdisable to make sparse happy

In reply to Christoph Hellwig

By duplicating the nvme_process_cq in both branches we keep the sparse lock context checking happy, so do it.

Signed-off-by Christoph Hellwig <hch at lst.de>

---
 drivers/nvme/host/pci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/nvme/host/pci.c +6 -6

index fb9d8270f32c..c0f66d2f6e80 100644
@@ -1093,15 +1093,15 @@ static int nvme_poll_irqdisable(struct nvme_queue *nvmeq, unsigned int tag)
 	 * using the CQ lock.  For normal interrupt driven threads we have
 	 * to disable the interrupt to avoid racing with it.
 	 */
-	if (nvmeq->cq_vector == -1)
+	if (nvmeq->cq_vector == -1) {
 		spin_lock(&nvmeq->cq_poll_lock);
-	else
-		disable_irq(pci_irq_vector(pdev, nvmeq->cq_vector));
-	found = nvme_process_cq(nvmeq, &start, &end, tag);
-	if (nvmeq->cq_vector == -1)
+		found = nvme_process_cq(nvmeq, &start, &end, tag);
 		spin_unlock(&nvmeq->cq_poll_lock);
-	else
+	} else {
+		disable_irq(pci_irq_vector(pdev, nvmeq->cq_vector));
+		found = nvme_process_cq(nvmeq, &start, &end, tag);
 		enable_irq(pci_irq_vector(pdev, nvmeq->cq_vector));
+	}
 
 	nvme_complete_cqes(nvmeq, start, end);
 	return found;

Signature · 1 line

2.19.2

Sagi Grimberg

maintainer

2018-12-13 16:43 UTC #9 lore raw

PATCH 3/3 nvme-pci: refactor nvme_poll_irqdisable to make sparse happy

Reviewed-by Sagi Grimberg <sagi at grimberg.me>