nvme-pci: Bounce buffer for interleaved metadata
RFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
NVMe namespace formats allow the possibility for metadata as extended LBAs. These require the memory interleave block and metadata in a single virtually contiguous buffer.
The Linux block layer, however, maintains metadata and data in separate buffers, which is unusable for NVMe drives using interleaved metadata formats.
This patch will enable such formats by allocating a bounce buffer interleaving the block and metadata, copying the everythign into the buffer for writes, or from it for reads.
I dislike this feature intensely. It is incredibly slow and enough memory overhead to make this not very useful for reclaim, but it's possible people will leave me alone if the Linux nvme driver accomodated this format.
On the other hand, I get the impression some people requesting this may think their application will get to access the extended LBAs. The reality is the kernel owns the metadata, so I may just setting myself up to explain why "fdisk" still shows a 512b format instead of 520b...
--- drivers/nvme/host/core.c | 13 +++- drivers/nvme/host/nvme.h | 1 - drivers/nvme/host/pci.c | 170 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 162 insertions(+), 22 deletions(-)
drivers/nvme/host/core.c +11 -2
@@ -1414,11 +1414,20 @@ static void nvme_update_disk_info(struct gendisk *disk,blk_queue_physical_block_size (disk->queue, bs);blk_queue_io_min (disk->queue, bs); -if (ns->ms && !ns->ext && - (ns->ctrl->ops->flags &NVME_F_METADATA_SUPPORTED )) +if (ns->ms && (ns->ctrl->ops->flags &NVME_F_METADATA_SUPPORTED ))nvme_init_integrity (disk, ns->ms, ns->pi_type);if (ns->ms && !nvme_ns_has_pi (ns) && !blk_get_integrity (disk)) capacity =0 ; +else if (ns->ms && ns->ext && ns->ctrl->max_hw_sectors !=UINT_MAX ) { +/* Interleaved metadata counts toward MDTS */ +unsigned sectors = ns->ctrl->max_hw_sectors >> + (ns->lba_shift -9 ); +unsigned new_max = ns->ctrl->max_hw_sectors - +DIV_ROUND_UP (sectors * ns->ms,512 ); + +blk_queue_max_hw_sectors (disk->queue, new_max); + }else +blk_queue_max_hw_sectors (disk->queue, ns->ctrl->max_hw_sectors);set_capacity (disk, capacity);if (ns->ctrl->oncs &NVME_CTRL_ONCS_DSM )
drivers/nvme/host/nvme.h +0 -1
@@ -152,7 +152,6 @@ struct nvme_ctrl {struct opal_dev *opal_dev; -char name[12 ];u16 cntlid;u32 ctrl_config;
drivers/nvme/host/pci.c +151 -19
268 lines not on this page · raw message
Signature · 1 line
2.14.3
Sagi Grimberg
maintainerRFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
Hi Keith,
Quoted 7 lines
NVMe namespace formats allow the possibility for metadata as extended LBAs. These require the memory interleave block and metadata in a single virtually contiguous buffer.
The Linux block layer, however, maintains metadata and data in separate buffers, which is unusable for NVMe drives using interleaved metadata formats.
That's not specific for NVMe, I vaguely recall we had this discussion for passthru scsi devices (in scsi target context) 5 years ago... It makes sense for FC (and few RDMA devices) that already get interleaved metadata from the wire to keep it as is instead of scattering it if the backend nvme device supports interleaved mode...
I would say that this support for this is something that belongs in the block layer. IIRC mkp also expressed interest in using preadv2/pwritev2 to for user-space to use DIF with some accounting on the iovec so maybe we can add a flag for interleaved metadata.
Quoted 8 lines
This patch will enable such formats by allocating a bounce buffer interleaving the block and metadata, copying the everythign into the buffer for writes, or from it for reads.
I dislike this feature intensely. It is incredibly slow and enough memory overhead to make this not very useful for reclaim, but it's possible people will leave me alone if the Linux nvme driver accomodated this format.
Not only that it will be non-useful, but probably unusable. Once upon of time iser did bounce buffering with large contiguous atomic allocations, it just doesn't work... especially with nvme large number of deep queues that can host commands of MDTS bytes each.
If we end up keeping it private to nvme, the first comment I'd give you is to avoid high-order allocations, you'll see lots of bug reports otherwise...
RFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
Quoted 14 lines · Sagi Grimberg
Quoted 7 lines
NVMe namespace formats allow the possibility for metadata as extended LBAs. These require the memory interleave block and metadata in a single virtually contiguous buffer.
The Linux block layer, however, maintains metadata and data in separate buffers, which is unusable for NVMe drives using interleaved metadata formats.
That's not specific for NVMe, I vaguely recall we had this discussion for passthru scsi devices (in scsi target context) 5 years ago... It makes sense for FC (and few RDMA devices) that already get interleaved metadata from the wire to keep it as is instead of scattering it if the backend nvme device supports interleaved mode...
Quoted 4 lines
I would say that this support for this is something that belongs in the block layer. IIRC mkp also expressed interest in using preadv2/pwritev2 to for user-space to use DIF with some accounting on the iovec so maybe we can add a flag for interleaved metadata.
That's an interesting thought. If the buffer from userspace already provides the metadata buffer interleaved with the block data, that would obviate the need for copying, and significantly help performance for such formats.
Quoted 9 lines
Quoted 8 lines
This patch will enable such formats by allocating a bounce buffer interleaving the block and metadata, copying the everythign into the buffer for writes, or from it for reads.
I dislike this feature intensely. It is incredibly slow and enough memory overhead to make this not very useful for reclaim, but it's possible people will leave me alone if the Linux nvme driver accomodated this format.
Quoted 8 lines
Not only that it will be non-useful, but probably unusable. Once upon of time iser did bounce buffering with large contiguous atomic allocations, it just doesn't work... especially with nvme large number of deep queues that can host commands of MDTS bytes each.
If we end up keeping it private to nvme, the first comment I'd give you is to avoid high-order allocations, you'll see lots of bug reports otherwise...
Thanks for bringing up those points. If we do go a route that has nvme double buffer everything, we could cap the max transfer size and/or vmalloc instead of kmalloc to reduce memory pressure. I'm not sure either would be sufficient, though.
RFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
In reply to Sagi GrimbergSagi,
Quoted 3 lines
It makes sense for FC (and few RDMA devices) that already get interleaved metadata from the wire to keep it as is instead of scattering it if the backend nvme device supports interleaved mode...
Yeah, assuming that the PI doesn't have to get translated.
Quoted 4 lines
I would say that this support for this is something that belongs in the block layer. IIRC mkp also expressed interest in using preadv2/pwritev2 to for user-space to use DIF with some accounting on the iovec
Indeed.
Quoted 1 line
so maybe we can add a flag for interleaved metadata.
What would the use case be for this? Userspace target driver? I know lots of widgets that rely on interleaved but they are all using SPDK.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Sagi Grimberg
maintainerRFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
Quoted 7 lines
Sagi,
Quoted 3 lines
It makes sense for FC (and few RDMA devices) that already get interleaved metadata from the wire to keep it as is instead of scattering it if the backend nvme device supports interleaved mode...
Yeah, assuming that the PI doesn't have to get translated.
From what I am aware of, devices can translate the PI and even the block
size and still keep it interleaved on the host memory layout. At least that was the case in the one implementation I was involved with.
Quoted 11 lines
Quoted 4 lines
I would say that this support for this is something that belongs in the block layer. IIRC mkp also expressed interest in using preadv2/pwritev2 to for user-space to use DIF with some accounting on the iovec
Indeed.
Quoted 1 line
so maybe we can add a flag for interleaved metadata.
What would the use case be for this? Userspace target driver? I know lots of widgets that rely on interleaved but they are all using SPDK.
Didn't came from me...
RFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
In reply to Keith BuschKeith,
Quoted 3 lines
This patch will enable such formats by allocating a bounce buffer interleaving the block and metadata, copying the everythign into the buffer for writes, or from it for reads.
Blargh. My eyes are bleeding!
Quoted 4 lines
I dislike this feature intensely. It is incredibly slow and enough memory overhead to make this not very useful for reclaim, but it's possible people will leave me alone if the Linux nvme driver accomodated this format.
*sigh*
Quoted 4 lines
On the other hand, I get the impression some people requesting this may think their application will get to access the extended LBAs. The reality is the kernel owns the metadata, so I may just setting myself up to explain why "fdisk" still shows a 512b format instead of 520b...
The whole point of DIF (over using regular 520 or 528 byte sectors) was to keep the logical block size at 512 and not deal with the PI in the data buffers.
And the point of defining DIX was to avoid having to do what your patch is doing.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
RFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
Quoted 11 lines · Martin K. Petersen
Quoted 4 lines
On the other hand, I get the impression some people requesting this may think their application will get to access the extended LBAs. The reality is the kernel owns the metadata, so I may just setting myself up to explain why "fdisk" still shows a 512b format instead of 520b...
The whole point of DIF (over using regular 520 or 528 byte sectors) was to keep the logical block size at 512 and not deal with the PI in the data buffers.
And the point of defining DIX was to avoid having to do what your patch is doing.
Right, this RFC is just about enabling formats that don't subscribe to the DIX format. It turns out some people believe those extended LBAs are useful for something.
I still think this LBA format is not a good fit for this driver, but I'd like to not push people to use out-of-tree or user space drivers if there is a reasonable way to accommodate here. The driver's existing NVMe IO passthrough makes this format reachable already, but there is resistance to use the ioctl over more standard read/write paths.
Christoph Hellwig
maintainerRFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
Quoted 9 lines · Keith Busch
Right, this RFC is just about enabling formats that don't subscribe to the DIX format. It turns out some people believe those extended LBAs are useful for something.
I still think this LBA format is not a good fit for this driver, but I'd like to not push people to use out-of-tree or user space drivers if there is a reasonable way to accommodate here. The driver's existing NVMe IO passthrough makes this format reachable already, but there is resistance to use the ioctl over more standard read/write paths.
For a good reason. I think these formats are completely bogus for something pretending to be a block device, and your patch just shows how bogus they are.
What is the use case for this silly game?
RFC PATCH 1/1 nvme-pci: Bounce buffer for interleaved metadata
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Keith Busch
Right, this RFC is just about enabling formats that don't subscribe to the DIX format. It turns out some people believe those extended LBAs are useful for something.
I still think this LBA format is not a good fit for this driver, but I'd like to not push people to use out-of-tree or user space drivers if there is a reasonable way to accommodate here. The driver's existing NVMe IO passthrough makes this format reachable already, but there is resistance to use the ioctl over more standard read/write paths.
Quoted 5 lines
For a good reason. I think these formats are completely bogus for something pretending to be a block device, and your patch just shows how bogus they are.
What is the use case for this silly game?
Certainly not for DIF PI. At a high level, the use case is to store block data and its metadata atomically rather than write them separately. RAID journal software might be able to leverage a device like that.