| Message ID | 20220426101241.30100-1-nj.shetty@samsung.com |
|---|---|
| Headers | show |
| Series | Add Copy offload support | expand |
On 4/26/22 19:12, Nitesh Shetty wrote: > The patch series covers the points discussed in November 2021 virtual call > [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. > We have covered the Initial agreed requirements in this patchset. > Patchset borrows Mikulas's token based approach for 2 bdev > implementation. > > Overall series supports – > > 1. Driver > - NVMe Copy command (single NS), including support in nvme-target (for > block and file backend) > > 2. Block layer > - Block-generic copy (REQ_COPY flag), with interface accommodating > two block-devs, and multi-source/destination interface > - Emulation, when offload is natively absent > - dm-linear support (for cases not requiring split) > > 3. User-interface > - new ioctl > - copy_file_range for zonefs > > 4. In-kernel user > - dm-kcopyd > - copy_file_range in zonefs > > For zonefs copy_file_range - Seems we cannot levearge fstest here. Limited > testing is done at this point using a custom application for unit testing. https://github.com/westerndigitalcorporation/zonefs-tools ./configure --with-tests make sudo make install Then run tests/zonefs-tests.sh Adding test case is simple. Just add script files under tests/scripts I just realized that the README file of this project is not documenting this. I will update it. > > Appreciate the inputs on plumbing and how to test this further? > Perhaps some of it can be discussed during LSF/MM too. > > [0] https://lore.kernel.org/linux-nvme/CA+1E3rJ7BZ7LjQXXTdX+-0Edz=zT14mmPGMiVCzUgB33C60tbQ@mail.gmail.com/ > > Changes in v4: > - added copy_file_range support for zonefs > - added documentaion about new sysfs entries > - incorporated review comments on v3 > - minor fixes > > > Arnav Dawn (2): > nvmet: add copy command support for bdev and file ns > fs: add support for copy file range in zonefs > > Nitesh Shetty (7): > block: Introduce queue limits for copy-offload support > block: Add copy offload support infrastructure > block: Introduce a new ioctl for copy > block: add emulation for copy > nvme: add copy offload support > dm: Add support for copy offload. > dm: Enable copy offload for dm-linear target > > SelvaKumar S (1): > dm kcopyd: use copy offload support > > Documentation/ABI/stable/sysfs-block | 83 +++++++ > block/blk-lib.c | 358 +++++++++++++++++++++++++++ > block/blk-map.c | 2 +- > block/blk-settings.c | 59 +++++ > block/blk-sysfs.c | 138 +++++++++++ > block/blk.h | 2 + > block/ioctl.c | 32 +++ > drivers/md/dm-kcopyd.c | 55 +++- > drivers/md/dm-linear.c | 1 + > drivers/md/dm-table.c | 45 ++++ > drivers/md/dm.c | 6 + > drivers/nvme/host/core.c | 116 ++++++++- > drivers/nvme/host/fc.c | 4 + > drivers/nvme/host/nvme.h | 7 + > drivers/nvme/host/pci.c | 25 ++ > drivers/nvme/host/rdma.c | 6 + > drivers/nvme/host/tcp.c | 14 ++ > drivers/nvme/host/trace.c | 19 ++ > drivers/nvme/target/admin-cmd.c | 8 +- > drivers/nvme/target/io-cmd-bdev.c | 65 +++++ > drivers/nvme/target/io-cmd-file.c | 49 ++++ > fs/zonefs/super.c | 178 ++++++++++++- > fs/zonefs/zonefs.h | 1 + > include/linux/blk_types.h | 21 ++ > include/linux/blkdev.h | 17 ++ > include/linux/device-mapper.h | 5 + > include/linux/nvme.h | 43 +++- > include/uapi/linux/fs.h | 23 ++ > 28 files changed, 1367 insertions(+), 15 deletions(-) > > > base-commit: e7d6987e09a328d4a949701db40ef63fbb970670
O Wed, Apr 27, 2022 at 11:19:48AM +0900, Damien Le Moal wrote: > On 4/26/22 19:12, Nitesh Shetty wrote: > > The patch series covers the points discussed in November 2021 virtual call > > [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. > > We have covered the Initial agreed requirements in this patchset. > > Patchset borrows Mikulas's token based approach for 2 bdev > > implementation. > > > > Overall series supports – > > > > 1. Driver > > - NVMe Copy command (single NS), including support in nvme-target (for > > block and file backend) > > It would also be nice to have copy offload emulation in null_blk for testing. > We can plan this in next phase of copy support, once this series settles down. -- Nitesh Shetty
On Wed, Apr 27, 2022 at 10:46:32AM +0900, Damien Le Moal wrote: > On 4/26/22 19:12, Nitesh Shetty wrote: > > The patch series covers the points discussed in November 2021 virtual call > > [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. > > We have covered the Initial agreed requirements in this patchset. > > Patchset borrows Mikulas's token based approach for 2 bdev > > implementation. > > > > Overall series supports – > > > > 1. Driver > > - NVMe Copy command (single NS), including support in nvme-target (for > > block and file backend) > > > > 2. Block layer > > - Block-generic copy (REQ_COPY flag), with interface accommodating > > two block-devs, and multi-source/destination interface > > - Emulation, when offload is natively absent > > - dm-linear support (for cases not requiring split) > > > > 3. User-interface > > - new ioctl > > - copy_file_range for zonefs > > > > 4. In-kernel user > > - dm-kcopyd > > - copy_file_range in zonefs > > > > For zonefs copy_file_range - Seems we cannot levearge fstest here. Limited > > testing is done at this point using a custom application for unit testing. > > https://protect2.fireeye.com/v1/url?k=b14bf8e1-d0361099-b14a73ae-74fe485fffb1-9bd9bbb269af18f9&q=1&e=b9714c29-ea22-4fa5-8a2a-eeb42ca4bdc1&u=https%3A%2F%2Fgithub.com%2Fwesterndigitalcorporation%2Fzonefs-tools > > ./configure --with-tests > make > sudo make install > > Then run tests/zonefs-tests.sh > > Adding test case is simple. Just add script files under tests/scripts > > I just realized that the README file of this project is not documenting > this. I will update it. > Thank you. We will try to use this. Any plans to integrate this testsuite with fstests(xfstest) ? -- Nitesh Shetty
On 4/28/22 00:38, Nitesh Shetty wrote: > On Wed, Apr 27, 2022 at 10:46:32AM +0900, Damien Le Moal wrote: >> On 4/26/22 19:12, Nitesh Shetty wrote: >>> The patch series covers the points discussed in November 2021 virtual call >>> [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. >>> We have covered the Initial agreed requirements in this patchset. >>> Patchset borrows Mikulas's token based approach for 2 bdev >>> implementation. >>> >>> Overall series supports – >>> >>> 1. Driver >>> - NVMe Copy command (single NS), including support in nvme-target (for >>> block and file backend) >>> >>> 2. Block layer >>> - Block-generic copy (REQ_COPY flag), with interface accommodating >>> two block-devs, and multi-source/destination interface >>> - Emulation, when offload is natively absent >>> - dm-linear support (for cases not requiring split) >>> >>> 3. User-interface >>> - new ioctl >>> - copy_file_range for zonefs >>> >>> 4. In-kernel user >>> - dm-kcopyd >>> - copy_file_range in zonefs >>> >>> For zonefs copy_file_range - Seems we cannot levearge fstest here. Limited >>> testing is done at this point using a custom application for unit testing. >> >> https://protect2.fireeye.com/v1/url?k=b14bf8e1-d0361099-b14a73ae-74fe485fffb1-9bd9bbb269af18f9&q=1&e=b9714c29-ea22-4fa5-8a2a-eeb42ca4bdc1&u=https%3A%2F%2Fgithub.com%2Fwesterndigitalcorporation%2Fzonefs-tools >> >> ./configure --with-tests >> make >> sudo make install >> >> Then run tests/zonefs-tests.sh >> >> Adding test case is simple. Just add script files under tests/scripts >> >> I just realized that the README file of this project is not documenting >> this. I will update it. >> > > Thank you. We will try to use this. > Any plans to integrate this testsuite with fstests(xfstest) ? No. It is not a good fit since zonefs cannot pass most of the generic test cases. > > -- > Nitesh Shetty > >
On Thu, Apr 28, 2022 at 07:05:32AM +0900, Damien Le Moal wrote: > On 4/27/22 21:49, Nitesh Shetty wrote: > > O Wed, Apr 27, 2022 at 11:19:48AM +0900, Damien Le Moal wrote: > >> On 4/26/22 19:12, Nitesh Shetty wrote: > >>> The patch series covers the points discussed in November 2021 virtual call > >>> [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. > >>> We have covered the Initial agreed requirements in this patchset. > >>> Patchset borrows Mikulas's token based approach for 2 bdev > >>> implementation. > >>> > >>> Overall series supports – > >>> > >>> 1. Driver > >>> - NVMe Copy command (single NS), including support in nvme-target (for > >>> block and file backend) > >> > >> It would also be nice to have copy offload emulation in null_blk for testing. > >> > > > > We can plan this in next phase of copy support, once this series settles down. > > So how can people test your series ? Not a lot of drives out there with > copy support. > Yeah not many drives at present, Qemu can be used to test NVMe copy. -- Nitesh Shetty
On 4/28/22 16:49, Nitesh Shetty wrote: > On Thu, Apr 28, 2022 at 07:05:32AM +0900, Damien Le Moal wrote: >> On 4/27/22 21:49, Nitesh Shetty wrote: >>> O Wed, Apr 27, 2022 at 11:19:48AM +0900, Damien Le Moal wrote: >>>> On 4/26/22 19:12, Nitesh Shetty wrote: >>>>> The patch series covers the points discussed in November 2021 virtual call >>>>> [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. >>>>> We have covered the Initial agreed requirements in this patchset. >>>>> Patchset borrows Mikulas's token based approach for 2 bdev >>>>> implementation. >>>>> >>>>> Overall series supports – >>>>> >>>>> 1. Driver >>>>> - NVMe Copy command (single NS), including support in nvme-target (for >>>>> block and file backend) >>>> >>>> It would also be nice to have copy offload emulation in null_blk for testing. >>>> >>> >>> We can plan this in next phase of copy support, once this series settles down. >> >> So how can people test your series ? Not a lot of drives out there with >> copy support. >> > > Yeah not many drives at present, Qemu can be used to test NVMe copy. Upstream QEMU ? What is the command line options ? An example would be nice. But I still think null_blk support would be easiest.
On 4/28/22 14:37, Damien Le Moal wrote: > On 4/28/22 16:49, Nitesh Shetty wrote: >> On Thu, Apr 28, 2022 at 07:05:32AM +0900, Damien Le Moal wrote: >>> On 4/27/22 21:49, Nitesh Shetty wrote: >>>> O Wed, Apr 27, 2022 at 11:19:48AM +0900, Damien Le Moal wrote: >>>>> On 4/26/22 19:12, Nitesh Shetty wrote: >>>>>> The patch series covers the points discussed in November 2021 virtual call >>>>>> [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. >>>>>> We have covered the Initial agreed requirements in this patchset. >>>>>> Patchset borrows Mikulas's token based approach for 2 bdev >>>>>> implementation. >>>>>> >>>>>> Overall series supports – >>>>>> >>>>>> 1. Driver >>>>>> - NVMe Copy command (single NS), including support in nvme-target (for >>>>>> block and file backend) >>>>> >>>>> It would also be nice to have copy offload emulation in null_blk for testing. >>>>> >>>> >>>> We can plan this in next phase of copy support, once this series settles down. >>> >>> So how can people test your series ? Not a lot of drives out there with >>> copy support. >>> >> >> Yeah not many drives at present, Qemu can be used to test NVMe copy. > > Upstream QEMU ? What is the command line options ? An example would be > nice. But I still think null_blk support would be easiest. +1 for adding copy offloading support in null_blk. That enables running copy offloading tests without depending on Qemu. Thanks, Bart.
On Wed, Apr 27, 2022 at 06:19:51PM +0530, Nitesh Shetty wrote: > O Wed, Apr 27, 2022 at 11:19:48AM +0900, Damien Le Moal wrote: > > On 4/26/22 19:12, Nitesh Shetty wrote: > > > The patch series covers the points discussed in November 2021 virtual call > > > [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. > > > We have covered the Initial agreed requirements in this patchset. > > > Patchset borrows Mikulas's token based approach for 2 bdev > > > implementation. > > > > > > Overall series supports – > > > > > > 1. Driver > > > - NVMe Copy command (single NS), including support in nvme-target (for > > > block and file backend) > > > > It would also be nice to have copy offload emulation in null_blk for testing. > > > > We can plan this in next phase of copy support, once this series settles down. Why not just hook the loopback driver up to copy_file_range() so that the backend filesystem can just reflink copy the ranges being passed? That would enable testing on btrfs, XFS and NFSv4.2 hosted image files without needing any special block device setup at all... i.e. I think you're doing this compeltely backwards by trying to target non-existent hardware first.... Cheers, Dave.
On 2022/05/02 21:14, Damien Le Moal wrote: > On 2022/04/27 21:49, Nitesh Shetty wrote: >> O Wed, Apr 27, 2022 at 11:19:48AM +0900, Damien Le Moal wrote: >>> On 4/26/22 19:12, Nitesh Shetty wrote: >>>> The patch series covers the points discussed in November 2021 virtual call >>>> [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. >>>> We have covered the Initial agreed requirements in this patchset. >>>> Patchset borrows Mikulas's token based approach for 2 bdev >>>> implementation. >>>> >>>> Overall series supports – >>>> >>>> 1. Driver >>>> - NVMe Copy command (single NS), including support in nvme-target (for >>>> block and file backend) >>> >>> It would also be nice to have copy offload emulation in null_blk for testing. >>> >> >> We can plan this in next phase of copy support, once this series settles down. > > Why ? How do you expect people to test simply without null_blk ? Sutre, you said > QEMU can be used. But if copy offload is not upstream for QEMU either, there is > no easy way to test. > > Adding that support to null_blk would not be hard at all. Sorry. Replied again to an email I already replied to. vger keep sending me multiple copies of the same emails...
On Mon, May 02, 2022 at 09:54:55PM +0900, Damien Le Moal wrote: > On 2022/05/02 13:09, Dave Chinner wrote: > > On Wed, Apr 27, 2022 at 06:19:51PM +0530, Nitesh Shetty wrote: > >> O Wed, Apr 27, 2022 at 11:19:48AM +0900, Damien Le Moal wrote: > >>> On 4/26/22 19:12, Nitesh Shetty wrote: > >>>> The patch series covers the points discussed in November 2021 virtual call > >>>> [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. > >>>> We have covered the Initial agreed requirements in this patchset. > >>>> Patchset borrows Mikulas's token based approach for 2 bdev > >>>> implementation. > >>>> > >>>> Overall series supports – > >>>> > >>>> 1. Driver > >>>> - NVMe Copy command (single NS), including support in nvme-target (for > >>>> block and file backend) > >>> > >>> It would also be nice to have copy offload emulation in null_blk for testing. > >>> > >> > >> We can plan this in next phase of copy support, once this series settles down. > > > > Why not just hook the loopback driver up to copy_file_range() so > > that the backend filesystem can just reflink copy the ranges being > > passed? That would enable testing on btrfs, XFS and NFSv4.2 hosted > > image files without needing any special block device setup at all... > > That is a very good idea ! But that will cover only the non-zoned case. For copy > offload on zoned devices, adding support in null_blk is probably the simplest > thing to do. Sure, but that's a zone device implementation issue, not a "how do applications use this offload" issue. i.e. zonefs support is not necessary to test the bio/block layer interfaces at all. All we need is a block device that can decode the bio-encoded offload packet and execute it to do full block layer testing. We can build dm devices on top of loop devices, etc, so we can test that the oflload support is plumbed, sliced, diced, and regurgitated correctly that way. We don't need actual low level device drivers to test this. And, unlike the nullblk device, using the loopback device w/ copy_file_range() will also allow data integrity testing if a generic copy_file_range() offload implementation is added. That is, we test a non-reflink capable filesystem on the loop device with the image file hosted on a reflink-capable filesystem. The upper filesystem copy then gets offloaded to reflinks in the lower filesystem. We already have copy_file_range() support in fsx, so all the data integrity fsx tests in fstests will exercise this offload path and find all the data corruptions the initial block layer bugs expose... Further, fsstress also has copy_file_range() support, and so all the fstests that generate stress tests or use fstress as load for failure testing will also exercise it. Indeed, this then gives us fine-grained error injection capability within fstests via devices like dm-flakey. What happens when dm-flakey kills the device IO mid-offload? Does everything recover correctly? Do we end up with data corruption? Are partial offload completions when errors occur signalled correctly? Is there -any- test coverage (or even capability for testing) of user driven copy offload failure situations like this in any of the other test suites? I mean, once the loop device has cfr offload, we can use dm-flakey to kill IO in the image file or even do a force shutdown of the image host filesystem. Hence we can actually trash the copy offload operation in mid-flight, not just error it out on full completion. This is trivial to do with the fstests infrastructure - it just relies on having generic copy_file_range() block offload support and a loopback device offload of hardware copy bios back to copy_file_range().... This is what I mean about copy offload being designed the wrong way. We have the high level hooks needed to implement it right though the filesystems and block layer without any specific hardware support, and we can test the whole stack without needing specific hardware support. We already have filesystem level copy offload acceleration, so the last thing we want to see is a block layer offload implementation that is incompatible with the semantics we've already exposed to userspace for copy offloads. As I said: > > i.e. I think you're doing this compeltely backwards by trying to > > target non-existent hardware first.... Rather than tie the block layer offload function/implementation to the specific quirks of a specific target hardware, we should be adding generic support in the block layer for the copy offload semantics we've already exposed to userspace. We already have test coverage and infrastructure for this interface and is already in use by applications. Transparent hardware acceleration of data copies when the hardware supports it is exactly where copy offloads are useful - implementing support based around hardware made of unobtainium and then adding high level user facing API support as an afterthought is putting the cart before the horse. We need to make sure the high level functionality is robust and handles errors correctly before we even worry about what quirks the hardware might bring to the table. Build a reference model first with the loop device and copy-file-range, test it, validate it, make sure it all works. Then hook up the hardware, and fix all the hardware bugs that are exposed before the hardware is released to the general public.... Why haven't we learnt this lesson yet from all the problems we've had with, say, broken discard/trim, zeroing, erase, etc in hardware implementations, incompatible hardware protocol implementations of equivalent functionality, etc? i.e. We haven't defined the OS required behaviour that hardware must support and instead just tried to make whatever has come from the hardware vendor's "standarisation" process work ok? In this case, we already have a functioning model, syscalls and user applications making use of copy offloads at the OS level. Now we need to implement those exact semantics at the block layer to build a validated reference model for the block layer offload behaviour that hardware must comply with. Then hardware offloads in actual hardware can be compared and validated against the reference model behaviour, and any hardware that doesn't match can be quirked/blacklisted until the manufacturer fixes their firmware... Cheers, Dave.
The patch series covers the points discussed in November 2021 virtual call [LSF/MM/BFP TOPIC] Storage: Copy Offload[0]. We have covered the Initial agreed requirements in this patchset. Patchset borrows Mikulas's token based approach for 2 bdev implementation. Overall series supports – 1. Driver - NVMe Copy command (single NS), including support in nvme-target (for block and file backend) 2. Block layer - Block-generic copy (REQ_COPY flag), with interface accommodating two block-devs, and multi-source/destination interface - Emulation, when offload is natively absent - dm-linear support (for cases not requiring split) 3. User-interface - new ioctl - copy_file_range for zonefs 4. In-kernel user - dm-kcopyd - copy_file_range in zonefs For zonefs copy_file_range - Seems we cannot levearge fstest here. Limited testing is done at this point using a custom application for unit testing. Appreciate the inputs on plumbing and how to test this further? Perhaps some of it can be discussed during LSF/MM too. [0] https://lore.kernel.org/linux-nvme/CA+1E3rJ7BZ7LjQXXTdX+-0Edz=zT14mmPGMiVCzUgB33C60tbQ@mail.gmail.com/ Changes in v4: - added copy_file_range support for zonefs - added documentaion about new sysfs entries - incorporated review comments on v3 - minor fixes Arnav Dawn (2): nvmet: add copy command support for bdev and file ns fs: add support for copy file range in zonefs Nitesh Shetty (7): block: Introduce queue limits for copy-offload support block: Add copy offload support infrastructure block: Introduce a new ioctl for copy block: add emulation for copy nvme: add copy offload support dm: Add support for copy offload. dm: Enable copy offload for dm-linear target SelvaKumar S (1): dm kcopyd: use copy offload support Documentation/ABI/stable/sysfs-block | 83 +++++++ block/blk-lib.c | 358 +++++++++++++++++++++++++++ block/blk-map.c | 2 +- block/blk-settings.c | 59 +++++ block/blk-sysfs.c | 138 +++++++++++ block/blk.h | 2 + block/ioctl.c | 32 +++ drivers/md/dm-kcopyd.c | 55 +++- drivers/md/dm-linear.c | 1 + drivers/md/dm-table.c | 45 ++++ drivers/md/dm.c | 6 + drivers/nvme/host/core.c | 116 ++++++++- drivers/nvme/host/fc.c | 4 + drivers/nvme/host/nvme.h | 7 + drivers/nvme/host/pci.c | 25 ++ drivers/nvme/host/rdma.c | 6 + drivers/nvme/host/tcp.c | 14 ++ drivers/nvme/host/trace.c | 19 ++ drivers/nvme/target/admin-cmd.c | 8 +- drivers/nvme/target/io-cmd-bdev.c | 65 +++++ drivers/nvme/target/io-cmd-file.c | 49 ++++ fs/zonefs/super.c | 178 ++++++++++++- fs/zonefs/zonefs.h | 1 + include/linux/blk_types.h | 21 ++ include/linux/blkdev.h | 17 ++ include/linux/device-mapper.h | 5 + include/linux/nvme.h | 43 +++- include/uapi/linux/fs.h | 23 ++ 28 files changed, 1367 insertions(+), 15 deletions(-) base-commit: e7d6987e09a328d4a949701db40ef63fbb970670