| Message ID | 20210214230240.301275-1-its@irrelevant.dk (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/block/nvme: metadata and end-to-end data protection support | expand |
On Mon, Feb 15, 2021 at 12:02:28AM +0100, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > This is RFC v3 of a series that adds support for metadata and end-to-end data > protection. > > First, on the subject of metadata, in v1, support was restricted to > extended logical blocks, which was pretty trivial to implement, but > required special initialization and broke DULBE. In v2, metadata is > always stored continuously at the end of the underlying block device. > This has the advantage of not breaking DULBE since the data blocks > remains aligned and allows bdrv_block_status to be used to determinate > allocation status. It comes at the expense of complicating the extended > LBA emulation, but on the other hand it also gains support for metadata > transfered as a separate buffer. > > The end-to-end data protection support blew up in terms of required > changes. This is due to the fact that a bunch of new commands has been > added to the device since v1 (zone append, compare, copy), and they all > require various special handling for protection information. If > potential reviewers would like it split up into multiple patches, each > adding pi support to one command, shout out. > > The core of the series (metadata and eedp) is preceeded by a set of > patches that refactors mapping (yes, again) and tries to deal with the > qsg/iov duality mess (maybe also again?). > > Support fro metadata and end-to-end data protection is all joint work > with Gollu Appalanaidu. Patches 1 - 8 look good to me: Reviewed-by: Keith Busch <kbusch@kernel.org> I like the LBA format and protection info support too, but might need some minor changes. The verify implementation looked fine, but lacking a generic backing for it sounds to me the use cases aren't there to justify taking on this feature.
On Feb 16 16:19, Keith Busch wrote: > On Mon, Feb 15, 2021 at 12:02:28AM +0100, Klaus Jensen wrote: > > From: Klaus Jensen <k.jensen@samsung.com> > > > > This is RFC v3 of a series that adds support for metadata and end-to-end data > > protection. > > > > First, on the subject of metadata, in v1, support was restricted to > > extended logical blocks, which was pretty trivial to implement, but > > required special initialization and broke DULBE. In v2, metadata is > > always stored continuously at the end of the underlying block device. > > This has the advantage of not breaking DULBE since the data blocks > > remains aligned and allows bdrv_block_status to be used to determinate > > allocation status. It comes at the expense of complicating the extended > > LBA emulation, but on the other hand it also gains support for metadata > > transfered as a separate buffer. > > > > The end-to-end data protection support blew up in terms of required > > changes. This is due to the fact that a bunch of new commands has been > > added to the device since v1 (zone append, compare, copy), and they all > > require various special handling for protection information. If > > potential reviewers would like it split up into multiple patches, each > > adding pi support to one command, shout out. > > > > The core of the series (metadata and eedp) is preceeded by a set of > > patches that refactors mapping (yes, again) and tries to deal with the > > qsg/iov duality mess (maybe also again?). > > > > Support fro metadata and end-to-end data protection is all joint work > > with Gollu Appalanaidu. > > Patches 1 - 8 look good to me: > > Reviewed-by: Keith Busch <kbusch@kernel.org> > > I like the LBA format and protection info support too, but might need > some minor changes. > Cool, thanks for the reviews Keith! > The verify implementation looked fine, but lacking a generic backing for > it sounds to me the use cases aren't there to justify taking on this > feature. Please check my reply on the verify patch - can you elaborate on "generic backing"? I'm not sure I understand what you have in mind, API-wise.
On Wed, Feb 17, 2021 at 10:06:49AM +0100, Klaus Jensen wrote: > On Feb 16 16:19, Keith Busch wrote: > > The verify implementation looked fine, but lacking a generic backing for > > it sounds to me the use cases aren't there to justify taking on this > > feature. > > Please check my reply on the verify patch - can you elaborate on > "generic backing"? I'm not sure I understand what you have in mind, > API-wise. I meant it'd be nice if qemu block api provided a function like "blk_aio_pverify()" to handle the details that you're implementing in the nvme device. As you mentioned though, handling the protection information part is problematic.
From: Klaus Jensen <k.jensen@samsung.com> This is RFC v3 of a series that adds support for metadata and end-to-end data protection. First, on the subject of metadata, in v1, support was restricted to extended logical blocks, which was pretty trivial to implement, but required special initialization and broke DULBE. In v2, metadata is always stored continuously at the end of the underlying block device. This has the advantage of not breaking DULBE since the data blocks remains aligned and allows bdrv_block_status to be used to determinate allocation status. It comes at the expense of complicating the extended LBA emulation, but on the other hand it also gains support for metadata transfered as a separate buffer. The end-to-end data protection support blew up in terms of required changes. This is due to the fact that a bunch of new commands has been added to the device since v1 (zone append, compare, copy), and they all require various special handling for protection information. If potential reviewers would like it split up into multiple patches, each adding pi support to one command, shout out. The core of the series (metadata and eedp) is preceeded by a set of patches that refactors mapping (yes, again) and tries to deal with the qsg/iov duality mess (maybe also again?). Support fro metadata and end-to-end data protection is all joint work with Gollu Appalanaidu. v3: * added patch with Verify command * added patches for multiple LBA formats and Format NVM * changed NvmeSG to be a union (Keith) Gollu Appalanaidu (1): hw/block/nvme: add verify command Klaus Jensen (9): hw/block/nvme: remove redundant len member in compare context hw/block/nvme: remove block accounting for write zeroes hw/block/nvme: fix strerror printing hw/block/nvme: try to deal with the iov/qsg duality hw/block/nvme: remove the req dependency in map functions hw/block/nvme: refactor nvme_dma hw/block/nvme: add metadata support hw/block/nvme: end-to-end data protection hw/block/nvme: add non-mdts command size limit for verify Minwoo Im (2): hw/block/nvme: support multiple lba formats hw/block/nvme: add support for the format nvm command hw/block/nvme-ns.h | 47 +- hw/block/nvme.h | 56 +- include/block/nvme.h | 34 +- hw/block/nvme-ns.c | 90 +- hw/block/nvme.c | 2063 +++++++++++++++++++++++++++++++++++------ hw/block/trace-events | 25 +- 6 files changed, 2027 insertions(+), 288 deletions(-)