All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NVMe: Fix IO for extended metadata formats
@ 2015-06-19 17:07 Keith Busch
  2015-06-19 19:16 ` Jens Axboe
  2015-06-22  5:34 ` Christoph Hellwig
  0 siblings, 2 replies; 5+ messages in thread
From: Keith Busch @ 2015-06-19 17:07 UTC (permalink / raw


This fixes io submit ioctl handling when using extended metadata
formats. When these formats are used, the user provides a single virtually
contiguous buffer containing both the block and metadata interleaved,
so the metadata size needs to be added to the total length and not mapped
as a separate transfer.

The command is also driver generated, so this patch does not enforce
blk-integrity extensions provide the metadata buffer.

Reported-by: Marcin Dziegielewski <marcin.dziegielewski at intel.com>
Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-core.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index a230b90..f225da7 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -862,7 +862,8 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
 	 * stripped/generated by the controller with PRACT=1.
 	 */
 	if (ns && ns->ms && !blk_integrity_rq(req)) {
-		if (!(ns->pi_type && ns->ms == 8)) {
+		if (!(ns->pi_type && ns->ms == 8) &&
+					req->cmd_type != REQ_TYPE_DRV_PRIV) {
 			req->errors = -EFAULT;
 			blk_mq_complete_request(req);
 			return BLK_MQ_RQ_QUEUE_OK;
@@ -1759,15 +1760,14 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 	meta_len = (io.nblocks + 1) * ns->ms;
 	write = io.opcode & 1;
 
+	if (ns->ext) {
+		length += meta_len;
+		meta_len = 0;
+	}
 	if (meta_len) {
 		if (((io.metadata & 3) || !io.metadata) && !ns->ext)
 			return -EINVAL;
 
-		if (ns->ext) {
-			length += meta_len;
-			meta_len = 0;
-		}
-
 		meta = dma_alloc_coherent(dev->dev, meta_len,
 						&meta_dma, GFP_KERNEL);
 		if (!meta) {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] NVMe: Fix IO for extended metadata formats
  2015-06-19 17:07 [PATCH] NVMe: Fix IO for extended metadata formats Keith Busch
@ 2015-06-19 19:16 ` Jens Axboe
  2015-06-22  5:34 ` Christoph Hellwig
  1 sibling, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2015-06-19 19:16 UTC (permalink / raw


On 06/19/2015 11:07 AM, Keith Busch wrote:
> This fixes io submit ioctl handling when using extended metadata
> formats. When these formats are used, the user provides a single virtually
> contiguous buffer containing both the block and metadata interleaved,
> so the metadata size needs to be added to the total length and not mapped
> as a separate transfer.
>
> The command is also driver generated, so this patch does not enforce
> blk-integrity extensions provide the metadata buffer.

Applied for 4.2.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] NVMe: Fix IO for extended metadata formats
  2015-06-19 17:07 [PATCH] NVMe: Fix IO for extended metadata formats Keith Busch
  2015-06-19 19:16 ` Jens Axboe
@ 2015-06-22  5:34 ` Christoph Hellwig
  2015-06-22 15:17   ` Busch, Keith
  1 sibling, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2015-06-22  5:34 UTC (permalink / raw


On Fri, Jun 19, 2015@11:07:30AM -0600, Keith Busch wrote:
> This fixes io submit ioctl handling when using extended metadata
> formats. When these formats are used, the user provides a single virtually
> contiguous buffer containing both the block and metadata interleaved,
> so the metadata size needs to be added to the total length and not mapped
> as a separate transfer.
> 
> The command is also driver generated, so this patch does not enforce
> blk-integrity extensions provide the metadata buffer.

Does this actually work properly?  I'd assume the block layer might
not be too happy if we have weird arbitrary sizes for this additional
metadata.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] NVMe: Fix IO for extended metadata formats
  2015-06-22  5:34 ` Christoph Hellwig
@ 2015-06-22 15:17   ` Busch, Keith
  2015-06-22 15:39     ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Busch, Keith @ 2015-06-22 15:17 UTC (permalink / raw


> On Fri, Jun 19, 2015@11:07:30AM -0600, Keith Busch wrote:
> > This fixes io submit ioctl handling when using extended metadata
> > formats. When these formats are used, the user provides a single virtually
> > contiguous buffer containing both the block and metadata interleaved,
> > so the metadata size needs to be added to the total length and not mapped
> > as a separate transfer.
> >
> > The command is also driver generated, so this patch does not enforce
> > blk-integrity extensions provide the metadata buffer.
> 
> Does this actually work properly?  I'd assume the block layer might
> not be too happy if we have weird arbitrary sizes for this additional
> metadata.

Similar to SG_IO, we can submit arbitrary length commands using the nvme
passthrough. It seems to work. Here's a test I did using the "nvme-cli"
program.

First showing the namespace is formatted using a metadata size 8,
capable of only extended settings.

  # nvme id-ns /dev/nvme0n1
  ...
  mc      : 0x1
  ...
  lbaf  1 : ms:8   ds:9  rp:0x2 (in use)
  ...

Create a random 520 byte file (512 bytes for data + 8 for metadata):

  # dd if=/dev/urandom of=rand.file.out bs=520 count=1

Write that 520 bytes using the write passthru:

  # nvme write /dev/nvme0n1 --data-size=520 --data=rand.file.out
  write success

Read it back:

  # nvme read /dev/nvme0n1 --data-size=520 --data=rand.file.in
  read success

Verify the two files are the same:

  # diff rand.file.in rand.file.out
  # echo $?
  0

Similar test using 8 blocks instead of 1:

  # dd if=/dev/urandom of=rand.file.out bs=520 count=8
  # nvme write /dev/nvme0n1 --block-count=7 --data-size=4160 --data=rand.file.out
  # nvme read /dev/nvme0n1 --block-count=7 --data-size=4160 --data=rand.file.in
  # diff rand.file.out rand.file.in
  #

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] NVMe: Fix IO for extended metadata formats
  2015-06-22 15:17   ` Busch, Keith
@ 2015-06-22 15:39     ` Christoph Hellwig
  0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2015-06-22 15:39 UTC (permalink / raw


On Mon, Jun 22, 2015@03:17:07PM +0000, Busch, Keith wrote:
> Similar to SG_IO, we can submit arbitrary length commands using the nvme
> passthrough. It seems to work. Here's a test I did using the "nvme-cli"
> program.

Yeah, I was mostly worried about the request mapping code tripping
up on these unaligned sized.  I don't have a device that supports it
so I couldn't test it myself.

Btw, given your test below:  I had thought for a while to add unit
tests to the nvme-cli package for cases like the one you had below.

The use case would be mostly to verify the driver, less the hardware
unless the two are tangled up or the hardware specific cases are simple
enough.  I've had good succeed with the cunit based SCSI testsuite
in libiscsi, so I could come up with a skeleton for it.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-06-22 15:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-19 17:07 [PATCH] NVMe: Fix IO for extended metadata formats Keith Busch
2015-06-19 19:16 ` Jens Axboe
2015-06-22  5:34 ` Christoph Hellwig
2015-06-22 15:17   ` Busch, Keith
2015-06-22 15:39     ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.