[PATCHv3] NVMe: Spliting large i/o in the ioctl path

[PATCHv3] NVMe: Spliting large i/o in the ioctl path

[Sathyavathi M]

From: Sathyavathi M <sathya.m@samsung.com>

The NVME_IOCTL_SUBMIT_IO allows arbitrarily large i/o cmds if mdts is zero.
If the mdts has a limit, there is no check to verify if the cmd sent to device
is within the max transfer limit. This patch splits arbitrarily  large size i/o
to max_hw_sectors before submitting to device. If metadata (extended and
separate) is present then the i/o is split by considering both data+metadata.

Also addresses an issue where 128KB chunk i/o is split into 2(124KB+4KB)
when mdts is 0.

This also merges the previously submitted "Check for Extended metadata in ioctl
path" patch and addresses the comments for that patch.

This patch is generated against for-4.1 branch.

v1->v2
use of BLK_DEF_MAX_SECTORS macro is replaced with value.
v2->v3
Adressed the comments from Keith Busch

Signed-off-by: Sathyavathi M <sathya.m at samsung.com>
---
 drivers/block/nvme-core.c | 132 +++++++++++++++++++++++++++-------------------
 1 file changed, 79 insertions(+), 53 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 94832e1..0992a96 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1746,85 +1746,108 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 	struct nvme_dev *dev = ns->dev;
 	struct nvme_user_io io;
 	struct nvme_command c;
-	unsigned length, meta_len;
-	int status, write;
-	struct nvme_iod *iod;
-	dma_addr_t meta_dma = 0;
+	unsigned tfr_len, meta_len = 0, split_length;
+	int status = 0, write, err = 0, count = 0, blk_sz;
+	u32 tfr_sectors;
+	u32 max_tfr_sectors, num_split_cmds, offset = 0;
+	u64 nxt_map_addr, nxt_meta_map_addr = 0;
+	struct nvme_iod *iod = NULL;
 	void *meta = NULL;
+	dma_addr_t meta_dma = 0;
 
 	if (copy_from_user(&io, uio, sizeof(io)))
 		return -EFAULT;
-	length = (io.nblocks + 1) << ns->lba_shift;
-	meta_len = (io.nblocks + 1) * ns->ms;
-
-	if (meta_len && ((io.metadata & 3) || !io.metadata) && !ns->ext)
+	max_tfr_sectors = dev->max_hw_sectors;
+	if (ns->ms && ((io.metadata & 3) || !io.metadata) && !ns->ext)
 		return -EINVAL;
-	else if (meta_len && ns->ext) {
-		length += meta_len;
-		meta_len = 0;
-	}
-
 	write = io.opcode & 1;
 
 	switch (io.opcode) {
 	case nvme_cmd_write:
 	case nvme_cmd_read:
 	case nvme_cmd_compare:
-		iod = nvme_map_user_pages(dev, write, io.addr, length);
 		break;
 	default:
 		return -EINVAL;
 	}
+	blk_sz = (1 << ns->lba_shift) + ((ns->ext && ns->ms) ? ns->ms : 0);
+	tfr_sectors = (max_tfr_sectors * 512) / blk_sz;
+
+	num_split_cmds = DIV_ROUND_UP((io.nblocks + 1), tfr_sectors);
 
-	if (IS_ERR(iod))
-		return PTR_ERR(iod);
+	for (count = 0; count < num_split_cmds; count++) {
+		tfr_sectors = min(tfr_sectors, (io.nblocks + 1) - offset);
+		tfr_len = tfr_sectors * blk_sz;
+		nxt_map_addr = io.addr + (blk_sz * offset);
+		if (ns->ms && !ns->ext) {
+			nxt_meta_map_addr = io.metadata + (ns->ms * offset);
+			meta_len = tfr_sectors * ns->ms;
+		}
+		iod = nvme_map_user_pages(dev, write, nxt_map_addr, tfr_len);
+
+		if (IS_ERR(iod))
+			return PTR_ERR(iod);
 		
-	length = nvme_setup_prps(dev, iod, length, GFP_KERNEL);
-	if (length != (io.nblocks + 1) << ns->lba_shift) {
-		status = -ENOMEM;
-		goto unmap;
-	}
-	if (meta_len) {
-		meta = dma_alloc_coherent(&dev->pci_dev->dev, meta_len,
-						&meta_dma, GFP_KERNEL);
-		if (!meta) {
+		split_length = nvme_setup_prps(dev, iod, tfr_len, GFP_KERNEL);
+		if (split_length != tfr_len) {
 			status = -ENOMEM;
+			err = true;
 			goto unmap;
 		}
-		if (write) {
-			if (copy_from_user(meta, (void __user *)io.metadata,
-								meta_len)) {
-				status = -EFAULT;
+		/* Mapping not needed for Extenede LBA */
+		if (ns->ms && !ns->ext) {
+			meta = dma_alloc_coherent(&dev->pci_dev->dev, meta_len,
+						&meta_dma, GFP_KERNEL);
+			if (!meta) {
+				status = -ENOMEM;
+				err = true;
 				goto unmap;
 			}
+			if (write) {
+				if (copy_from_user(meta,
+					(void __user *)nxt_meta_map_addr,
+								meta_len)) {
+					status = -EFAULT;
+					err = true;
+					goto unmap;
+				}
+			}
 		}
-	}
 
-	memset(&c, 0, sizeof(c));
-	c.rw.opcode = io.opcode;
-	c.rw.flags = io.flags;
-	c.rw.nsid = cpu_to_le32(ns->ns_id);
-	c.rw.slba = cpu_to_le64(io.slba);
-	c.rw.length = cpu_to_le16(io.nblocks);
-	c.rw.control = cpu_to_le16(io.control);
-	c.rw.dsmgmt = cpu_to_le32(io.dsmgmt);
-	c.rw.reftag = cpu_to_le32(io.reftag);
-	c.rw.apptag = cpu_to_le16(io.apptag);
-	c.rw.appmask = cpu_to_le16(io.appmask);
-	c.rw.prp1 = cpu_to_le64(sg_dma_address(iod->sg));
-	c.rw.prp2 = cpu_to_le64(iod->first_dma);
-	c.rw.metadata = cpu_to_le64(meta_dma);
-	status = nvme_submit_io_cmd(dev, ns, &c, NULL);
+		memset(&c, 0, sizeof(c));
+		c.rw.opcode = io.opcode;
+		c.rw.flags = io.flags;
+		c.rw.nsid = cpu_to_le32(ns->ns_id);
+		c.rw.slba = cpu_to_le64(io.slba + offset);
+		c.rw.length = cpu_to_le16(tfr_sectors - 1);
+		c.rw.control = cpu_to_le16(io.control);
+		c.rw.dsmgmt = cpu_to_le32(io.dsmgmt);
+		if (ns->pi_type) {
+			c.rw.reftag = cpu_to_le32(io.reftag + offset);
+			c.rw.apptag = cpu_to_le16(io.apptag);
+			c.rw.appmask = cpu_to_le16(io.appmask);
+		}
+		c.rw.prp1 = cpu_to_le64(sg_dma_address(iod->sg));
+		c.rw.prp2 = cpu_to_le64(iod->first_dma);
+		c.rw.metadata = cpu_to_le64(meta_dma);
+		status = nvme_submit_io_cmd(dev, ns, &c, NULL);
+		if (status != NVME_SC_SUCCESS)
+			err = true;
  unmap:
-	nvme_unmap_user_pages(dev, write, iod);
-	nvme_free_iod(dev, iod);
-	if (meta) {
-		if (status == NVME_SC_SUCCESS && !write) {
-			if (copy_to_user((void __user *)io.metadata, meta,
-								meta_len))
-				status = -EFAULT;
+		nvme_unmap_user_pages(dev, write, iod);
+		nvme_free_iod(dev, iod);
+		if (meta) {
+			if (status == NVME_SC_SUCCESS && !write) {
+				if (copy_to_user((void __user *)io.metadata +
+					(ns->ms * offset), meta, meta_len))
+					status = -EFAULT;
+			}
+			dma_free_coherent(&dev->pci_dev->dev, meta_len, meta,
+								meta_dma);
 		}
-		dma_free_coherent(&dev->pci_dev->dev, meta_len, meta, meta_dma);
+		offset += tfr_sectors;
+		if (err)
+			break;
 	}
 	return status;
 }
@@ -2310,8 +2333,11 @@ static int nvme_dev_add(struct nvme_dev *dev)
 	memcpy(dev->serial, ctrl->sn, sizeof(ctrl->sn));
 	memcpy(dev->model, ctrl->mn, sizeof(ctrl->mn));
 	memcpy(dev->firmware_rev, ctrl->fr, sizeof(ctrl->fr));
+	/* Increase max transfer size if device reports MDTS=0 */
 	if (ctrl->mdts)
 		dev->max_hw_sectors = 1 << (ctrl->mdts + shift - 9);
+	else
+		dev->max_hw_sectors = UINT_MAX;
 	if ((pdev->vendor == PCI_VENDOR_ID_INTEL) &&
 			(pdev->device == 0x0953) && ctrl->vs[3]) {
 		unsigned int max_hw_sectors;
-- 
1.8.4.2

[PATCHv3] NVMe: Spliting large i/o in the ioctl path

[sathyavathi]

Keith,

Any update on this patch.

Regards,
Sathya
?
------- Original Message -------
Sender : Sathyavathi M<sathya.m at samsung.com> Lead Engineer/SRI-Bangalore-SSD
Solutions/Samsung Electronics
Date : Apr 02, 2015 15:06 (GMT+09:00)
Title : [PATCHv3] NVMe: Spliting large i/o in the ioctl path
?
From: Sathyavathi M 

The NVME_IOCTL_SUBMIT_IO allows arbitrarily large i/o cmds if mdts is zero.
If the mdts has a limit, there is no check to verify if the cmd sent to
device
is within the max transfer limit. This patch splits arbitrarily??large size
i/o
to max_hw_sectors before submitting to device. If metadata (extended and
separate) is present then the i/o is split by considering both
data+metadata.

Also addresses an issue where 128KB chunk i/o is split into 2(124KB+4KB)
when mdts is 0.

This also merges the previously submitted "Check for Extended metadata in
ioctl
path" patch and addresses the comments for that patch.

This patch is generated against for-4.1 branch.

v1->v2
use of BLK_DEF_MAX_SECTORS macro is replaced with value.
v2->v3
Adressed the comments from Keith Busch

Signed-off-by: Sathyavathi M 
---
drivers/block/nvme-core.c | 132
+++++++++++++++++++++++++++-------------------
1 file changed, 79 insertions(+), 53 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 94832e1..0992a96 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1746,85 +1746,108 @@ static int nvme_submit_io(struct nvme_ns *ns,
struct nvme_user_io __user *uio)
struct nvme_dev *dev = ns->dev;
struct nvme_user_io io;
struct nvme_command c;
- unsigned length, meta_len;
- int status, write;
- struct nvme_iod *iod;
- dma_addr_t meta_dma = 0;
+ unsigned tfr_len, meta_len = 0, split_length;
+ int status = 0, write, err = 0, count = 0, blk_sz;
+ u32 tfr_sectors;
+ u32 max_tfr_sectors, num_split_cmds, offset = 0;
+ u64 nxt_map_addr, nxt_meta_map_addr = 0;
+ struct nvme_iod *iod = NULL;
void *meta = NULL;
+ dma_addr_t meta_dma = 0;

if (copy_from_user(&io, uio, sizeof(io)))
return -EFAULT;
- length = (io.nblocks + 1) << ns->lba_shift;
- meta_len = (io.nblocks + 1) * ns->ms;
-
- if (meta_len && ((io.metadata & 3) || !io.metadata) && !ns->ext)
+ max_tfr_sectors = dev->max_hw_sectors;
+ if (ns->ms && ((io.metadata & 3) || !io.metadata) && !ns->ext)
return -EINVAL;
- else if (meta_len && ns->ext) {
- length += meta_len;
- meta_len = 0;
- }
-
write = io.opcode & 1;

switch (io.opcode) {
case nvme_cmd_write:
case nvme_cmd_read:
case nvme_cmd_compare:
- iod = nvme_map_user_pages(dev, write, io.addr, length);
break;
default:
return -EINVAL;
}
+ blk_sz = (1 << ns->lba_shift) + ((ns->ext && ns->ms) ? ns->ms : 0);
+ tfr_sectors = (max_tfr_sectors * 512) / blk_sz;
+
+ num_split_cmds = DIV_ROUND_UP((io.nblocks + 1), tfr_sectors);

- if (IS_ERR(iod))
- return PTR_ERR(iod);
+ for (count = 0; count < num_split_cmds; count++) {
+ tfr_sectors = min(tfr_sectors, (io.nblocks + 1) - offset);
+ tfr_len = tfr_sectors * blk_sz;
+ nxt_map_addr = io.addr + (blk_sz * offset);
+ if (ns->ms && !ns->ext) {
+ nxt_meta_map_addr = io.metadata + (ns->ms * offset);
+ meta_len = tfr_sectors * ns->ms;
+ }
+ iod = nvme_map_user_pages(dev, write, nxt_map_addr, tfr_len);
+
+ if (IS_ERR(iod))
+ return PTR_ERR(iod);

- length = nvme_setup_prps(dev, iod, length, GFP_KERNEL);
- if (length != (io.nblocks + 1) << ns->lba_shift) {
- status = -ENOMEM;
- goto unmap;
- }
- if (meta_len) {
- meta = dma_alloc_coherent(&dev->pci_dev->dev, meta_len,
- &meta_dma, GFP_KERNEL);
- if (!meta) {
+ split_length = nvme_setup_prps(dev, iod, tfr_len, GFP_KERNEL);
+ if (split_length != tfr_len) {
status = -ENOMEM;
+ err = true;
goto unmap;
}
- if (write) {
- if (copy_from_user(meta, (void __user *)io.metadata,
- meta_len)) {
- status = -EFAULT;
+ /* Mapping not needed for Extenede LBA */
+ if (ns->ms && !ns->ext) {
+ meta = dma_alloc_coherent(&dev->pci_dev->dev, meta_len,
+ &meta_dma, GFP_KERNEL);
+ if (!meta) {
+ status = -ENOMEM;
+ err = true;
goto unmap;
}
+ if (write) {
+ if (copy_from_user(meta,
+ (void __user *)nxt_meta_map_addr,
+ meta_len)) {
+ status = -EFAULT;
+ err = true;
+ goto unmap;
+ }
+ }
}
- }

- memset(&c, 0, sizeof(c));
- c.rw.opcode = io.opcode;
- c.rw.flags = io.flags;
- c.rw.nsid = cpu_to_le32(ns->ns_id);
- c.rw.slba = cpu_to_le64(io.slba);
- c.rw.length = cpu_to_le16(io.nblocks);
- c.rw.control = cpu_to_le16(io.control);
- c.rw.dsmgmt = cpu_to_le32(io.dsmgmt);
- c.rw.reftag = cpu_to_le32(io.reftag);
- c.rw.apptag = cpu_to_le16(io.apptag);
- c.rw.appmask = cpu_to_le16(io.appmask);
- c.rw.prp1 = cpu_to_le64(sg_dma_address(iod->sg));
- c.rw.prp2 = cpu_to_le64(iod->first_dma);
- c.rw.metadata = cpu_to_le64(meta_dma);
- status = nvme_submit_io_cmd(dev, ns, &c, NULL);
+ memset(&c, 0, sizeof(c));
+ c.rw.opcode = io.opcode;
+ c.rw.flags = io.flags;
+ c.rw.nsid = cpu_to_le32(ns->ns_id);
+ c.rw.slba = cpu_to_le64(io.slba + offset);
+ c.rw.length = cpu_to_le16(tfr_sectors - 1);
+ c.rw.control = cpu_to_le16(io.control);
+ c.rw.dsmgmt = cpu_to_le32(io.dsmgmt);
+ if (ns->pi_type) {
+ c.rw.reftag = cpu_to_le32(io.reftag + offset);
+ c.rw.apptag = cpu_to_le16(io.apptag);
+ c.rw.appmask = cpu_to_le16(io.appmask);
+ }
+ c.rw.prp1 = cpu_to_le64(sg_dma_address(iod->sg));
+ c.rw.prp2 = cpu_to_le64(iod->first_dma);
+ c.rw.metadata = cpu_to_le64(meta_dma);
+ status = nvme_submit_io_cmd(dev, ns, &c, NULL);
+ if (status != NVME_SC_SUCCESS)
+ err = true;
??unmap:
- nvme_unmap_user_pages(dev, write, iod);
- nvme_free_iod(dev, iod);
- if (meta) {
- if (status == NVME_SC_SUCCESS && !write) {
- if (copy_to_user((void __user *)io.metadata, meta,
- meta_len))
- status = -EFAULT;
+ nvme_unmap_user_pages(dev, write, iod);
+ nvme_free_iod(dev, iod);
+ if (meta) {
+ if (status == NVME_SC_SUCCESS && !write) {
+ if (copy_to_user((void __user *)io.metadata +
+ (ns->ms * offset), meta, meta_len))
+ status = -EFAULT;
+ }
+ dma_free_coherent(&dev->pci_dev->dev, meta_len, meta,
+ meta_dma);
}
- dma_free_coherent(&dev->pci_dev->dev, meta_len, meta, meta_dma);
+ offset += tfr_sectors;
+ if (err)
+ break;
}
return status;
}
@@ -2310,8 +2333,11 @@ static int nvme_dev_add(struct nvme_dev *dev)
memcpy(dev->serial, ctrl->sn, sizeof(ctrl->sn));
memcpy(dev->model, ctrl->mn, sizeof(ctrl->mn));
memcpy(dev->firmware_rev, ctrl->fr, sizeof(ctrl->fr));
+ /* Increase max transfer size if device reports MDTS=0 */
if (ctrl->mdts)
dev->max_hw_sectors = 1 << (ctrl->mdts + shift - 9);
+ else
+ dev->max_hw_sectors = UINT_MAX;
if ((pdev->vendor == PCI_VENDOR_ID_INTEL) &&
(pdev->device == 0x0953) && ctrl->vs[3]) {
unsigned int max_hw_sectors;
-- 
1.8.4.2
?
Regards,
Sathya
?

[PATCHv3] NVMe: Spliting large i/o in the ioctl path

[Keith Busch]

On Mon, 27 Apr 2015, sathyavathi wrote:
> Keith,
>
> Any update on this patch.

No new comments your implementation. This ioctl is not a critical or
fast path though, so I'm not sure why we'd force the kernel driver to
correct a malformed passthru.

Basically, I'm on the side that if your app misuses the ioctl, you get
to keep both pieces. Your patch is available if a maintainer disagrees.

> Regards,
> Sathya
> ?
> From: Sathyavathi M
>
> The NVME_IOCTL_SUBMIT_IO allows arbitrarily large i/o cmds if mdts is zero.
> If the mdts has a limit, there is no check to verify if the cmd sent to
> device
> is within the max transfer limit. This patch splits arbitrarily??large size
> i/o
> to max_hw_sectors before submitting to device. If metadata (extended and
> separate) is present then the i/o is split by considering both
> data+metadata.
>
> Also addresses an issue where 128KB chunk i/o is split into 2(124KB+4KB)
> when mdts is 0.
>
> This also merges the previously submitted "Check for Extended metadata in
> ioctl
> path" patch and addresses the comments for that patch.
>
> This patch is generated against for-4.1 branch.
>
> v1->v2
> use of BLK_DEF_MAX_SECTORS macro is replaced with value.
> v2->v3
> Adressed the comments from Keith Busch
>
> Signed-off-by: Sathyavathi M
> ---
> drivers/block/nvme-core.c | 132
> +++++++++++++++++++++++++++-------------------
> 1 file changed, 79 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index 94832e1..0992a96 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -1746,85 +1746,108 @@ static int nvme_submit_io(struct nvme_ns *ns,
> struct nvme_user_io __user *uio)
> struct nvme_dev *dev = ns->dev;
> struct nvme_user_io io;
> struct nvme_command c;
> - unsigned length, meta_len;
> - int status, write;
> - struct nvme_iod *iod;
> - dma_addr_t meta_dma = 0;
> + unsigned tfr_len, meta_len = 0, split_length;
> + int status = 0, write, err = 0, count = 0, blk_sz;
> + u32 tfr_sectors;
> + u32 max_tfr_sectors, num_split_cmds, offset = 0;
> + u64 nxt_map_addr, nxt_meta_map_addr = 0;
> + struct nvme_iod *iod = NULL;
> void *meta = NULL;
> + dma_addr_t meta_dma = 0;
>
> if (copy_from_user(&io, uio, sizeof(io)))
> return -EFAULT;
> - length = (io.nblocks + 1) << ns->lba_shift;
> - meta_len = (io.nblocks + 1) * ns->ms;
> -
> - if (meta_len && ((io.metadata & 3) || !io.metadata) && !ns->ext)
> + max_tfr_sectors = dev->max_hw_sectors;
> + if (ns->ms && ((io.metadata & 3) || !io.metadata) && !ns->ext)
> return -EINVAL;
> - else if (meta_len && ns->ext) {
> - length += meta_len;
> - meta_len = 0;
> - }
> -
> write = io.opcode & 1;
>
> switch (io.opcode) {
> case nvme_cmd_write:
> case nvme_cmd_read:
> case nvme_cmd_compare:
> - iod = nvme_map_user_pages(dev, write, io.addr, length);
> break;
> default:
> return -EINVAL;
> }
> + blk_sz = (1 << ns->lba_shift) + ((ns->ext && ns->ms) ? ns->ms : 0);
> + tfr_sectors = (max_tfr_sectors * 512) / blk_sz;
> +
> + num_split_cmds = DIV_ROUND_UP((io.nblocks + 1), tfr_sectors);
>
> - if (IS_ERR(iod))
> - return PTR_ERR(iod);
> + for (count = 0; count < num_split_cmds; count++) {
> + tfr_sectors = min(tfr_sectors, (io.nblocks + 1) - offset);
> + tfr_len = tfr_sectors * blk_sz;
> + nxt_map_addr = io.addr + (blk_sz * offset);
> + if (ns->ms && !ns->ext) {
> + nxt_meta_map_addr = io.metadata + (ns->ms * offset);
> + meta_len = tfr_sectors * ns->ms;
> + }
> + iod = nvme_map_user_pages(dev, write, nxt_map_addr, tfr_len);
> +
> + if (IS_ERR(iod))
> + return PTR_ERR(iod);
>
> - length = nvme_setup_prps(dev, iod, length, GFP_KERNEL);
> - if (length != (io.nblocks + 1) << ns->lba_shift) {
> - status = -ENOMEM;
> - goto unmap;
> - }
> - if (meta_len) {
> - meta = dma_alloc_coherent(&dev->pci_dev->dev, meta_len,
> - &meta_dma, GFP_KERNEL);
> - if (!meta) {
> + split_length = nvme_setup_prps(dev, iod, tfr_len, GFP_KERNEL);
> + if (split_length != tfr_len) {
> status = -ENOMEM;
> + err = true;
> goto unmap;
> }
> - if (write) {
> - if (copy_from_user(meta, (void __user *)io.metadata,
> - meta_len)) {
> - status = -EFAULT;
> + /* Mapping not needed for Extenede LBA */
> + if (ns->ms && !ns->ext) {
> + meta = dma_alloc_coherent(&dev->pci_dev->dev, meta_len,
> + &meta_dma, GFP_KERNEL);
> + if (!meta) {
> + status = -ENOMEM;
> + err = true;
> goto unmap;
> }
> + if (write) {
> + if (copy_from_user(meta,
> + (void __user *)nxt_meta_map_addr,
> + meta_len)) {
> + status = -EFAULT;
> + err = true;
> + goto unmap;
> + }
> + }
> }
> - }
>
> - memset(&c, 0, sizeof(c));
> - c.rw.opcode = io.opcode;
> - c.rw.flags = io.flags;
> - c.rw.nsid = cpu_to_le32(ns->ns_id);
> - c.rw.slba = cpu_to_le64(io.slba);
> - c.rw.length = cpu_to_le16(io.nblocks);
> - c.rw.control = cpu_to_le16(io.control);
> - c.rw.dsmgmt = cpu_to_le32(io.dsmgmt);
> - c.rw.reftag = cpu_to_le32(io.reftag);
> - c.rw.apptag = cpu_to_le16(io.apptag);
> - c.rw.appmask = cpu_to_le16(io.appmask);
> - c.rw.prp1 = cpu_to_le64(sg_dma_address(iod->sg));
> - c.rw.prp2 = cpu_to_le64(iod->first_dma);
> - c.rw.metadata = cpu_to_le64(meta_dma);
> - status = nvme_submit_io_cmd(dev, ns, &c, NULL);
> + memset(&c, 0, sizeof(c));
> + c.rw.opcode = io.opcode;
> + c.rw.flags = io.flags;
> + c.rw.nsid = cpu_to_le32(ns->ns_id);
> + c.rw.slba = cpu_to_le64(io.slba + offset);
> + c.rw.length = cpu_to_le16(tfr_sectors - 1);
> + c.rw.control = cpu_to_le16(io.control);
> + c.rw.dsmgmt = cpu_to_le32(io.dsmgmt);
> + if (ns->pi_type) {
> + c.rw.reftag = cpu_to_le32(io.reftag + offset);
> + c.rw.apptag = cpu_to_le16(io.apptag);
> + c.rw.appmask = cpu_to_le16(io.appmask);
> + }
> + c.rw.prp1 = cpu_to_le64(sg_dma_address(iod->sg));
> + c.rw.prp2 = cpu_to_le64(iod->first_dma);
> + c.rw.metadata = cpu_to_le64(meta_dma);
> + status = nvme_submit_io_cmd(dev, ns, &c, NULL);
> + if (status != NVME_SC_SUCCESS)
> + err = true;
> ??unmap:
> - nvme_unmap_user_pages(dev, write, iod);
> - nvme_free_iod(dev, iod);
> - if (meta) {
> - if (status == NVME_SC_SUCCESS && !write) {
> - if (copy_to_user((void __user *)io.metadata, meta,
> - meta_len))
> - status = -EFAULT;
> + nvme_unmap_user_pages(dev, write, iod);
> + nvme_free_iod(dev, iod);
> + if (meta) {
> + if (status == NVME_SC_SUCCESS && !write) {
> + if (copy_to_user((void __user *)io.metadata +
> + (ns->ms * offset), meta, meta_len))
> + status = -EFAULT;
> + }
> + dma_free_coherent(&dev->pci_dev->dev, meta_len, meta,
> + meta_dma);
> }
> - dma_free_coherent(&dev->pci_dev->dev, meta_len, meta, meta_dma);
> + offset += tfr_sectors;
> + if (err)
> + break;
> }
> return status;
> }
> @@ -2310,8 +2333,11 @@ static int nvme_dev_add(struct nvme_dev *dev)
> memcpy(dev->serial, ctrl->sn, sizeof(ctrl->sn));
> memcpy(dev->model, ctrl->mn, sizeof(ctrl->mn));
> memcpy(dev->firmware_rev, ctrl->fr, sizeof(ctrl->fr));
> + /* Increase max transfer size if device reports MDTS=0 */
> if (ctrl->mdts)
> dev->max_hw_sectors = 1 << (ctrl->mdts + shift - 9);
> + else
> + dev->max_hw_sectors = UINT_MAX;
> if ((pdev->vendor == PCI_VENDOR_ID_INTEL) &&
> (pdev->device == 0x0953) && ctrl->vs[3]) {
> unsigned int max_hw_sectors;
> -- 
> 1.8.4.2

[PATCHv3] NVMe: Spliting large i/o in the ioctl path

[Christoph Hellwig]

On Mon, Apr 27, 2015@07:22:29PM +0000, Keith Busch wrote:
> On Mon, 27 Apr 2015, sathyavathi wrote:
> >Keith,
> >
> >Any update on this patch.
> 
> No new comments your implementation. This ioctl is not a critical or
> fast path though, so I'm not sure why we'd force the kernel driver to
> correct a malformed passthru.
> 
> Basically, I'm on the side that if your app misuses the ioctl, you get
> to keep both pieces. Your patch is available if a maintainer disagrees.

Btw, I'll send a patch soon will make the ioctl and SCSI code use the block
layer S/G mapping, which will give us this feature for free.  It might make
sense to hold out for it.  I'll prioritize finishing this one so I can
get it out ASAP.