two minor nvme endianess patches

two minor nvme endianess patches

[Christoph Hellwig]

-

[PATCH 1/2] nvme: remove spurious use of *_to_cpup helpers

[Christoph Hellwig]

Switch to the normal endianess helpers that take an integer instead of
the pointer to it.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/block/nvme-core.c | 28 ++++++++++++++--------------
 drivers/block/nvme-scsi.c |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 666e994..8a27893 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -281,13 +281,13 @@ static void special_completion(struct nvme_queue *nvmeq, void *ctx,
 	if (ctx == CMD_CTX_COMPLETED) {
 		dev_warn(nvmeq->q_dmadev,
 				"completed id %d twice on queue %d\n",
-				cqe->command_id, le16_to_cpup(&cqe->sq_id));
+				cqe->command_id, le16_to_cpu(cqe->sq_id));
 		return;
 	}
 	if (ctx == CMD_CTX_INVALID) {
 		dev_warn(nvmeq->q_dmadev,
 				"invalid id %d completed on queue %d\n",
-				cqe->command_id, le16_to_cpup(&cqe->sq_id));
+				cqe->command_id, le16_to_cpu(cqe->sq_id));
 		return;
 	}
 	dev_warn(nvmeq->q_dmadev, "Unknown special completion %p\n", ctx);
@@ -308,8 +308,8 @@ static void *cancel_cmd_info(struct nvme_cmd_info *cmd, nvme_completion_fn *fn)
 static void async_req_completion(struct nvme_queue *nvmeq, void *ctx,
 						struct nvme_completion *cqe)
 {
-	u32 result = le32_to_cpup(&cqe->result);
-	u16 status = le16_to_cpup(&cqe->status) >> 1;
+	u32 result = le32_to_cpu(cqe->result);
+	u16 status = le16_to_cpu(cqe->status) >> 1;
 
 	if (status == NVME_SC_SUCCESS || status == NVME_SC_ABORT_REQ)
 		++nvmeq->dev->event_limit;
@@ -330,8 +330,8 @@ static void abort_completion(struct nvme_queue *nvmeq, void *ctx,
 {
 	struct request *req = ctx;
 
-	u16 status = le16_to_cpup(&cqe->status) >> 1;
-	u32 result = le32_to_cpup(&cqe->result);
+	u16 status = le16_to_cpu(cqe->status) >> 1;
+	u32 result = le32_to_cpu(cqe->result);
 
 	blk_mq_free_request(req);
 
@@ -343,8 +343,8 @@ static void async_completion(struct nvme_queue *nvmeq, void *ctx,
 						struct nvme_completion *cqe)
 {
 	struct async_cmd_info *cmdinfo = ctx;
-	cmdinfo->result = le32_to_cpup(&cqe->result);
-	cmdinfo->status = le16_to_cpup(&cqe->status) >> 1;
+	cmdinfo->result = le32_to_cpu(cqe->result);
+	cmdinfo->status = le16_to_cpu(cqe->status) >> 1;
 	queue_kthread_work(cmdinfo->worker, &cmdinfo->work);
 	blk_mq_free_request(cmdinfo->req);
 }
@@ -607,7 +607,7 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx,
 	struct request *req = iod_get_private(iod);
 	struct nvme_cmd_info *cmd_rq = blk_mq_rq_to_pdu(req);
 
-	u16 status = le16_to_cpup(&cqe->status) >> 1;
+	u16 status = le16_to_cpu(cqe->status) >> 1;
 
 	if (unlikely(status)) {
 		if (!(status & NVME_SC_DNR || blk_noretry_request(req))
@@ -632,7 +632,7 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx,
 	} else
 		req->errors = 0;
 	if (req->cmd_type == REQ_TYPE_DRV_PRIV) {
-		u32 result = le32_to_cpup(&cqe->result);
+		u32 result = le32_to_cpu(cqe->result);
 		req->special = (void *)(uintptr_t)result;
 	}
 
@@ -2031,7 +2031,7 @@ static int nvme_revalidate_disk(struct gendisk *disk)
 	if (ns->ms && !blk_get_integrity(disk))
 		set_capacity(disk, 0);
 	else
-		set_capacity(disk, le64_to_cpup(&id->nsze) << (ns->lba_shift - 9));
+		set_capacity(disk, le64_to_cpu(id->nsze) << (ns->lba_shift - 9));
 
 	if (dev->oncs & NVME_CTRL_ONCS_DSM)
 		nvme_config_discard(ns);
@@ -2430,7 +2430,7 @@ static void nvme_dev_scan(struct work_struct *work)
 		return;
 	if (nvme_identify_ctrl(dev, &ctrl))
 		return;
-	nvme_scan_namespaces(dev, le32_to_cpup(&ctrl->nn));
+	nvme_scan_namespaces(dev, le32_to_cpu(ctrl->nn));
 	kfree(ctrl);
 }
 
@@ -2454,8 +2454,8 @@ static int nvme_dev_add(struct nvme_dev *dev)
 		return -EIO;
 	}
 
-	nn = le32_to_cpup(&ctrl->nn);
-	dev->oncs = le16_to_cpup(&ctrl->oncs);
+	nn = le32_to_cpu(ctrl->nn);
+	dev->oncs = le16_to_cpu(ctrl->oncs);
 	dev->abort_limit = ctrl->acl + 1;
 	dev->vwc = ctrl->vwc;
 	memcpy(dev->serial, ctrl->sn, sizeof(ctrl->sn));
diff --git a/drivers/block/nvme-scsi.c b/drivers/block/nvme-scsi.c
index e5a63f0..800427b 100644
--- a/drivers/block/nvme-scsi.c
+++ b/drivers/block/nvme-scsi.c
@@ -1178,7 +1178,7 @@ static void nvme_trans_fill_read_cap(u8 *response, struct nvme_id_ns *id_ns,
 
 	flbas = (id_ns->flbas) & 0x0F;
 	lba_length = (1 << (id_ns->lbaf[flbas].ds));
-	rlba = le64_to_cpup(&id_ns->nsze) - 1;
+	rlba = le64_to_cpu(id_ns->nsze) - 1;
 	(id_ns->dps) ? (prot_en = 0x01) : (prot_en = 0);
 
 	if (!cdb16) {
-- 
1.9.1

[PATCH 2/2] nvme.h: add missing endianess annotations

[Christoph Hellwig]

Doesn't matter for the current NVMe driver, but we might as well get it in
order for future software controller implementations.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 include/uapi/linux/nvme.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/nvme.h b/include/uapi/linux/nvme.h
index 732b32e..d1b644b 100644
--- a/include/uapi/linux/nvme.h
+++ b/include/uapi/linux/nvme.h
@@ -50,8 +50,8 @@ struct nvme_id_ctrl {
 	__u8			ieee[3];
 	__u8			mic;
 	__u8			mdts;
-	__u16			cntlid;
-	__u32			ver;
+	__le16			cntlid;
+	__le32			ver;
 	__u8			rsvd84[172];
 	__le16			oacs;
 	__u8			acl;
-- 
1.9.1

[PATCH 1/2] nvme: remove spurious use of *_to_cpup helpers

[Matthew Wilcox]

On Mon, Aug 17, 2015@09:09:39PM +0200, Christoph Hellwig wrote:
> Switch to the normal endianess helpers that take an integer instead of
> the pointer to it.

Why?  Some CPUs have a 'load-reversed-endian' instruction, which can be
used in the _to_cpup() cases, but not in the _to_cpu() cases.

> -				cqe->command_id, le16_to_cpup(&cqe->sq_id));
> +				cqe->command_id, le16_to_cpu(cqe->sq_id));

[PATCH 1/2] nvme: remove spurious use of *_to_cpup helpers

[Christoph Hellwig]

On Mon, Aug 17, 2015@04:12:37PM -0400, Matthew Wilcox wrote:
> On Mon, Aug 17, 2015@09:09:39PM +0200, Christoph Hellwig wrote:
> > Switch to the normal endianess helpers that take an integer instead of
> > the pointer to it.
> 
> Why?  Some CPUs have a 'load-reversed-endian' instruction, which can be
> used in the _to_cpup() cases, but not in the _to_cpu() cases.

They can and will be used in both cases.

Powerpc examples listings below, as you can see they produce identical code.

------------------ le32_to_cpu ------------------
test.o:     file format elf32-powerpc


Disassembly of section .text:

00000000 <foor>:
};

int foor(struct foo *foo)
{
	return le32_to_cpu(foo->foo);
}
   0:	7c 60 1c 2c 	lwbrx   r3,0,r3
   4:	4e 80 00 20 	blr

------------------ le32_to_cpup ------------------
test.o:     file format elf32-powerpc


Disassembly of section .text:

00000000 <foor>:
};

int foor(struct foo *foo)
{
	return le32_to_cpup(&foo->foo);
}
   0:	7c 60 1c 2c 	lwbrx   r3,0,r3
   4:	4e 80 00 20 	blr

[PATCH 1/2] nvme: remove spurious use of *_to_cpup helpers

[Jens Axboe]

On 08/17/2015 02:12 PM, Matthew Wilcox wrote:
> On Mon, Aug 17, 2015@09:09:39PM +0200, Christoph Hellwig wrote:
>> Switch to the normal endianess helpers that take an integer instead of
>> the pointer to it.
>
> Why?  Some CPUs have a 'load-reversed-endian' instruction, which can be
> used in the _to_cpup() cases, but not in the _to_cpu() cases.
>
>> -				cqe->command_id, le16_to_cpup(&cqe->sq_id));
>> +				cqe->command_id, le16_to_cpu(cqe->sq_id));

All those nvme equipped sparc and s390's? :-)

Joke aside, do we really have that 'load-reversed-endian' addition to 
the endianness conversion API just because of two rather esoteric 
platforms? Seems silly.

-- 
Jens Axboe

[PATCH 1/2] nvme: remove spurious use of *_to_cpup helpers

[Christoph Hellwig]

On Tue, Aug 18, 2015@11:51:46AM -0600, Jens Axboe wrote:
> All those nvme equipped sparc and s390's? :-)

I'm pretty sure there are some power systems that have them.

> Joke aside, do we really have that 'load-reversed-endian' addition to the 
> endianness conversion API just because of two rather esoteric platforms? 
> Seems silly.

As mentioned before BE platforms don't need it, compilers have handled
it fine every since the invention of peephole optimization in the 60s.

The cpup helpers are more useful if you iterate over an array of
descriptors:

fs/nfs/blocklayout/dev.c:       b->type = be32_to_cpup(p++);
fs/nfs/blocklayout/dev.c:               b->simple.nr_sigs = be32_to_cpup(p++);
fs/nfs/blocklayout/dev.c:                       b->simple.sigs[i].sig_len = be32_to_cpup(p++);
fs/nfs/blocklayout/dev.c:               b->slice.volume = be32_to_cpup(p++);
fs/nfs/blocklayout/dev.c:               b->concat.volumes_count = be32_to_cpup(p++);
fs/nfs/blocklayout/dev.c:                       b->concat.volumes[i] = be32_to_cpup(p++);
fs/nfs/blocklayout/dev.c:               b->stripe.volumes_count = be32_to_cpup(p++);
fs/nfs/blocklayout/dev.c:                       b->stripe.volumes[i] = be32_to_cpup(p++);

XDR decoding is full of them for example.