* [PATCH V7 0/6] nvmet: add ZBD backend support
@ 2020-12-15 6:02 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:02 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: sagi, hch, damien.lemoal, Chaitanya Kulkarni
Hi,
NVMeOF Host is capable of handling the NVMe Protocol based Zoned Block
Devices (ZBD) in the Zoned Namespaces (ZNS) mode with the passthru
backend. There is no support for a generic block device backend to
handle the ZBD devices which are not NVMe protocol compliant.
This adds support to export the ZBDs (which are not NVMe drives) to host
the from target via NVMeOF using the host side ZNS interface.
The patch series is generated in bottom-top manner where, it first adds
prep patch and ZNS command-specific handlers on the top of genblk and
updates the data structures, then one by one it wires up the admin cmds
in the order host calls them in namespace initializing sequence. Once
everything is ready, it wires-up the I/O command handlers. See below for
patch-series overview.
All the testcases are passing for the nvme blktests with non ZBD target
device and with ZoneFS where ZBD exported with NVMeOF backed by
null_blk ZBD and null_blk ZBD without NVMeOF. Adding test result below.
Note: This patch-series is based on the earlier posted patch series :-
[PATCH 0/4] nvmet: admin-cmd related cleanups and a fix
http://lists.infradead.org/pipermail/linux-nvme/2020-December/021501.html
Regards,
Chaitanya
Changes from V6:-
1. Instead of calling report zones to find conventional zones in the
loop use the loop inside LLD report zones that also simplifies the
report zone callback for zbd backend.
2. Use -EOPNOTSUPP in the nvmet_bdev_validate_zns_zones_cb() to avoid
checkpatch warning when conventional zone is found.
2. Fix the bug in nvmet_bdev_has_conv_zones().
3. Remove conditional operators in the nvmet_bdev_execute_zone_append().
Changes from V5:-
1. Use bio->bi_iter.bi_sector for result of the REQ_OP_ZONE_APPEND
command.
2. Add endianness to the helper nvmet_sect_to_lba().
3. Make bufsize u32 in zone mgmt recv command handler.
4. Add __GFP_ZERO for report zone data buffer to return clean buffer.
Changes from V4:-
1. Don't use bio_iov_iter_get_pages() instead add a patch to export
bio_add_hw_page() and call it directly for zone append.
2. Add inline vector optimization for append bio.
3. Update the commit logs for the patches.
4. Remove ZNS related identify data structures, use individual members.
5. Add a comment for macro NVMET_MPSMIN_SHIFT.
6. Remove nvmet_bdev() helper.
7. Move the command set identifier code into common code.
8. Use IS_ENABLED() and move helpers fomr zns.c into common code.
9. Add a patch to support Command Set identifiers.
10. Open code nvmet_bdev_validate_zns_zones().
11. Remove the per namespace min zasl calculation and don't allow
namespaces with zasl value > the first ns zasl value.
12. Move the stubs into the header file.
13. Add lba to/from sector conversion helpers and update the
io-cmd-bdev.c to avoid the code duplication.
14. Add everything into one patch for zns command handlers and
respective calls from the target code.
15. Remove the trim ns-desclist admin callback patch from this series.
16. Add bio get and put helpers patches to reduce the duplicate code in
generic bdev, passthru, and generic zns backend.
Changes from V3:-
1. Get rid of the bio_max_zasl check.
2. Remove extra lines.
4. Remove the block layer api export patch.
5. Remove the bvec check in the bio_iov_iter_get_pages() for
REQ_OP_ZONE_APPEND so that we can reuse the code.
Changes from V2:-
1. Move conventional zone bitmap check into
nvmet_bdev_validate_zns_zones().
2. Don't use report zones call to check the runt zone.
3. Trim nvmet_zasl() helper.
4. Fix typo in the nvmet_zns_update_zasl().
5. Remove the comment and fix the mdts calculation in
nvmet_execute_identify_cns_cs_ctrl().
6. Use u64 for bufsize in nvmet_bdev_execute_zone_mgmt_recv().
7. Remove nvmet_zones_to_desc_size() and fix the nr_zones
calculation.
8. Remove the op variable in nvmet_bdev_execute_zone_append().
9. Fix the nr_zones calculation nvmet_bdev_execute_zone_mgmt_recv().
10. Update cover letter subject.
Changes from V1:-
1. Remove the nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o.
2. Mark helpers inline.
3. Fix typos in the comments and update the comments.
4. Get rid of the curly brackets.
5. Don't allow drives with last smaller zones.
6. Calculate the zasl as a function of ax_zone_append_sectors,
bio_max_pages so we don't have to split the bio.
7. Add global subsys->zasl and update the zasl when new namespace
is enabled.
8. Rmove the loop in the nvmet_bdev_execute_zone_mgmt_recv() and
move functionality in to the report zone callback.
9. Add goto for default case in nvmet_bdev_execute_zone_mgmt_send().
10. Allocate the zones buffer with zones size instead of bdev nr_zones.
Chaitanya Kulkarni (6):
block: export bio_add_hw_pages()
nvmet: add lba to sect conversion helpers
nvmet: add NVM command set identifier support
nvmet: add ZBD over ZNS backend support
nvmet: add bio get helper for different backends
nvmet: add bio put helper for different backends
block/bio.c | 1 +
block/blk.h | 4 -
drivers/nvme/target/Makefile | 1 +
drivers/nvme/target/admin-cmd.c | 59 ++++--
drivers/nvme/target/core.c | 14 +-
drivers/nvme/target/io-cmd-bdev.c | 51 +++--
drivers/nvme/target/nvmet.h | 71 +++++++
drivers/nvme/target/passthru.c | 11 +-
drivers/nvme/target/zns.c | 335 ++++++++++++++++++++++++++++++
include/linux/blkdev.h | 4 +
10 files changed, 503 insertions(+), 48 deletions(-)
create mode 100644 drivers/nvme/target/zns.c
Without CONFIG_BLK_DEV_ZONED nvme tests :-
# grep -i blk_dev_zoned .config
# CONFIG_BLK_DEV_ZONED is not set
# gitlog -6
2dc9be4cc588 (HEAD -> nvme-5.11) nvmet: add bio put helper for different backends
72836658a6cf nvmet: add bio get helper for different backends
c1c1819074b6 nvmet: add ZBD over ZNS backend support
428548cd91ff nvmet: add NVM command set identifier support
fba6fbaf6e2a nvmet: add lba to sect conversion helpers
70c9a4c94d5a block: export bio_add_hw_pages()
# makej M=drivers/nvme/ clean
CLEAN drivers/nvme//Module.symvers
# makej M=drivers/nvme/
CC [M] drivers/nvme//host/core.o
CC [M] drivers/nvme//host/trace.o
CC [M] drivers/nvme//host/lightnvm.o
CC [M] drivers/nvme//target/core.o
CC [M] drivers/nvme//host/hwmon.o
CC [M] drivers/nvme//target/configfs.o
CC [M] drivers/nvme//host/pci.o
CC [M] drivers/nvme//target/admin-cmd.o
CC [M] drivers/nvme//host/fabrics.o
CC [M] drivers/nvme//host/rdma.o
CC [M] drivers/nvme//target/fabrics-cmd.o
CC [M] drivers/nvme//target/discovery.o
CC [M] drivers/nvme//host/fc.o
CC [M] drivers/nvme//target/io-cmd-file.o
CC [M] drivers/nvme//host/tcp.o
CC [M] drivers/nvme//target/io-cmd-bdev.o
CC [M] drivers/nvme//target/passthru.o
CC [M] drivers/nvme//target/trace.o
CC [M] drivers/nvme//target/loop.o
CC [M] drivers/nvme//target/rdma.o
CC [M] drivers/nvme//target/fc.o
CC [M] drivers/nvme//target/fcloop.o
CC [M] drivers/nvme//target/tcp.o
LD [M] drivers/nvme//target/nvme-loop.o
LD [M] drivers/nvme//target/nvme-fcloop.o
LD [M] drivers/nvme//target/nvmet-tcp.o
LD [M] drivers/nvme//host/nvme-fabrics.o
LD [M] drivers/nvme//host/nvme.o
LD [M] drivers/nvme//host/nvme-rdma.o
LD [M] drivers/nvme//target/nvmet-rdma.o
LD [M] drivers/nvme//target/nvmet.o
LD [M] drivers/nvme//target/nvmet-fc.o
LD [M] drivers/nvme//host/nvme-tcp.o
LD [M] drivers/nvme//host/nvme-fc.o
LD [M] drivers/nvme//host/nvme-core.o
MODPOST drivers/nvme//Module.symvers
CC [M] drivers/nvme//host/nvme-core.mod.o
CC [M] drivers/nvme//host/nvme-fabrics.mod.o
CC [M] drivers/nvme//host/nvme-fc.mod.o
CC [M] drivers/nvme//host/nvme-rdma.mod.o
CC [M] drivers/nvme//host/nvme-tcp.mod.o
CC [M] drivers/nvme//host/nvme.mod.o
CC [M] drivers/nvme//target/nvme-fcloop.mod.o
CC [M] drivers/nvme//target/nvme-loop.mod.o
CC [M] drivers/nvme//target/nvmet-fc.mod.o
CC [M] drivers/nvme//target/nvmet-rdma.mod.o
CC [M] drivers/nvme//target/nvmet-tcp.mod.o
CC [M] drivers/nvme//target/nvmet.mod.o
LD [M] drivers/nvme//target/nvme-fcloop.ko
LD [M] drivers/nvme//host/nvme-tcp.ko
LD [M] drivers/nvme//host/nvme-core.ko
LD [M] drivers/nvme//target/nvmet-tcp.ko
LD [M] drivers/nvme//target/nvme-loop.ko
LD [M] drivers/nvme//target/nvmet-fc.ko
LD [M] drivers/nvme//host/nvme-fabrics.ko
LD [M] drivers/nvme//host/nvme-fc.ko
LD [M] drivers/nvme//target/nvmet-rdma.ko
LD [M] drivers/nvme//host/nvme-rdma.ko
LD [M] drivers/nvme//host/nvme.ko
LD [M] drivers/nvme//target/nvmet.ko
# cdblktests
# ./check tests/nvme/
nvme/002 (create many subsystems and test discovery) [passed]
runtime 24.923s ... 24.625s
nvme/003 (test if we're sending keep-alives to a discovery controller) [passed]
runtime 10.141s ... 10.143s
nvme/004 (test nvme and nvmet UUID NS descriptors) [passed]
runtime 2.446s ... 2.444s
nvme/005 (reset local loopback target) [not run]
nvme_core module does not have parameter multipath
nvme/006 (create an NVMeOF target with a block device-backed ns) [passed]
runtime 0.113s ... 0.103s
nvme/007 (create an NVMeOF target with a file-backed ns) [passed]
runtime 0.068s ... 0.066s
nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
runtime 2.477s ... 2.477s
nvme/009 (create an NVMeOF host with a file-backed ns) [passed]
runtime 2.447s ... 2.428s
nvme/010 (run data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 21.695s ... 27.905s
nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
runtime 11.413s ... 252.685s
nvme/012 (run mkfs and data verification fio job on NVMeOF block device-backed ns) [passed]
runtime ... 50.439s
nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed ns) [passed]
runtime 268.947s ... 275.912s
nvme/014 (flush a NVMeOF block device-backed ns) [passed]
runtime 21.977s ... 22.190s
nvme/015 (unit test for NVMe flush for file backed ns) [passed]
runtime 20.816s ... 21.174s
nvme/016 (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
runtime 13.469s ... 13.774s
nvme/017 (create/delete many file-ns and test discovery) [passed]
runtime 14.086s ... 13.420s
nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
runtime 2.418s ... 2.416s
nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
runtime 2.477s ... 2.474s
nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
runtime 2.410s ... 2.403s
nvme/021 (test NVMe list command on NVMeOF file-backed ns) [passed]
runtime 2.412s ... 2.404s
nvme/022 (test NVMe reset command on NVMeOF file-backed ns) [passed]
runtime 2.861s ... 2.863s
nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
runtime 2.486s ... 2.471s
nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
runtime 2.430s ... 2.411s
nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
runtime 2.412s ... 2.395s
nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
runtime 2.450s ... 2.420s
nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
runtime 2.416s ... 2.401s
nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
runtime 2.418s ... 2.405s
nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
runtime 2.759s ... 2.732s
nvme/030 (ensure the discovery generation counter is updated appropriately) [passed]
runtime 0.347s ... 0.345s
nvme/031 (test deletion of NVMeOF controllers immediately after setup) [passed]
runtime 13.480s ... 13.432s
nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
runtime 0.039s ... 0.039s
With CONFIG_BLK_DEV_ZONED nvme and zonefs tests :-
# grep -i blk_dev_zoned .config
CONFIG_BLK_DEV_ZONED=y
# make M=drivers/nvme/ clean
CLEAN drivers/nvme//Module.symvers
# make M=drivers/nvme/
CC [M] drivers/nvme//host/core.o
CC [M] drivers/nvme//host/trace.o
CC [M] drivers/nvme//host/lightnvm.o
CC [M] drivers/nvme//host/zns.o
CC [M] drivers/nvme//host/hwmon.o
LD [M] drivers/nvme//host/nvme-core.o
CC [M] drivers/nvme//host/pci.o
LD [M] drivers/nvme//host/nvme.o
CC [M] drivers/nvme//host/fabrics.o
LD [M] drivers/nvme//host/nvme-fabrics.o
CC [M] drivers/nvme//host/rdma.o
LD [M] drivers/nvme//host/nvme-rdma.o
CC [M] drivers/nvme//host/fc.o
LD [M] drivers/nvme//host/nvme-fc.o
CC [M] drivers/nvme//host/tcp.o
LD [M] drivers/nvme//host/nvme-tcp.o
CC [M] drivers/nvme//target/core.o
CC [M] drivers/nvme//target/configfs.o
CC [M] drivers/nvme//target/admin-cmd.o
CC [M] drivers/nvme//target/fabrics-cmd.o
CC [M] drivers/nvme//target/discovery.o
CC [M] drivers/nvme//target/io-cmd-file.o
CC [M] drivers/nvme//target/io-cmd-bdev.o
CC [M] drivers/nvme//target/passthru.o
CC [M] drivers/nvme//target/zns.o
CC [M] drivers/nvme//target/trace.o
LD [M] drivers/nvme//target/nvmet.o
CC [M] drivers/nvme//target/loop.o
LD [M] drivers/nvme//target/nvme-loop.o
CC [M] drivers/nvme//target/rdma.o
LD [M] drivers/nvme//target/nvmet-rdma.o
CC [M] drivers/nvme//target/fc.o
LD [M] drivers/nvme//target/nvmet-fc.o
CC [M] drivers/nvme//target/fcloop.o
LD [M] drivers/nvme//target/nvme-fcloop.o
CC [M] drivers/nvme//target/tcp.o
LD [M] drivers/nvme//target/nvmet-tcp.o
MODPOST drivers/nvme//Module.symvers
CC [M] drivers/nvme//host/nvme-core.mod.o
LD [M] drivers/nvme//host/nvme-core.ko
CC [M] drivers/nvme//host/nvme-fabrics.mod.o
LD [M] drivers/nvme//host/nvme-fabrics.ko
CC [M] drivers/nvme//host/nvme-fc.mod.o
LD [M] drivers/nvme//host/nvme-fc.ko
CC [M] drivers/nvme//host/nvme-rdma.mod.o
LD [M] drivers/nvme//host/nvme-rdma.ko
CC [M] drivers/nvme//host/nvme-tcp.mod.o
LD [M] drivers/nvme//host/nvme-tcp.ko
CC [M] drivers/nvme//host/nvme.mod.o
LD [M] drivers/nvme//host/nvme.ko
CC [M] drivers/nvme//target/nvme-fcloop.mod.o
LD [M] drivers/nvme//target/nvme-fcloop.ko
CC [M] drivers/nvme//target/nvme-loop.mod.o
LD [M] drivers/nvme//target/nvme-loop.ko
CC [M] drivers/nvme//target/nvmet-fc.mod.o
LD [M] drivers/nvme//target/nvmet-fc.ko
CC [M] drivers/nvme//target/nvmet-rdma.mod.o
LD [M] drivers/nvme//target/nvmet-rdma.ko
CC [M] drivers/nvme//target/nvmet-tcp.mod.o
LD [M] drivers/nvme//target/nvmet-tcp.ko
CC [M] drivers/nvme//target/nvmet.mod.o
LD [M] drivers/nvme//target/nvmet.ko
#
# cdblktests
# ./check tests/nvme/
nvme/002 (create many subsystems and test discovery) [passed]
runtime 24.378s ... 24.636s
nvme/003 (test if we're sending keep-alives to a discovery controller) [passed]
runtime 10.133s ... 10.152s
nvme/004 (test nvme and nvmet UUID NS descriptors) [passed]
runtime 2.463s ... 2.478s
nvme/005 (reset local loopback target) [not run]
nvme_core module does not have parameter multipath
nvme/006 (create an NVMeOF target with a block device-backed ns) [passed]
runtime 0.095s ... 0.122s
nvme/007 (create an NVMeOF target with a file-backed ns) [passed]
runtime 0.065s ... 0.079s
nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
runtime 2.473s ... 2.501s
nvme/009 (create an NVMeOF host with a file-backed ns) [passed]
runtime 2.460s ... 2.424s
nvme/010 (run data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 24.526s ... 28.015s
nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
runtime 265.967s ... 282.717s
nvme/012 (run mkfs and data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 44.665s ... 48.124s
nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed ns) [passed]
runtime 261.739s ... 352.331s
nvme/014 (flush a NVMeOF block device-backed ns) [passed]
runtime 21.268s ... 22.013s
nvme/015 (unit test for NVMe flush for file backed ns) [passed]
runtime 18.820s ... 22.104s
nvme/016 (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
runtime 13.899s ... 14.322s
nvme/017 (create/delete many file-ns and test discovery) [passed]
runtime 14.322s ... 14.031s
nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
runtime 2.450s ... 2.444s
nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
runtime 2.475s ... 2.489s
nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
runtime 2.410s ... 2.448s
nvme/021 (test NVMe list command on NVMeOF file-backed ns) [passed]
runtime 2.441s ... 2.439s
nvme/022 (test NVMe reset command on NVMeOF file-backed ns) [passed]
runtime 2.864s ... 2.863s
nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
runtime 2.465s ... 2.446s
nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
runtime 2.416s ... 2.411s
nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
runtime 2.419s ... 2.748s
nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
runtime 2.422s ... 2.410s
nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
runtime 2.456s ... 2.462s
nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
runtime 2.427s ... 2.429s
nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
runtime 2.751s ... 2.755s
nvme/030 (ensure the discovery generation counter is updated appropriately) [passed]
runtime 0.346s ... 0.357s
nvme/031 (test deletion of NVMeOF controllers immediately after setup) [passed]
runtime 13.601s ... 13.591s
nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
runtime 0.039s ... 0.059s
# cdzonefstest
# cdnvme
# cdzonefstest
# ./zonefs-tests.sh /dev/nvme1n1
Gathering information on /dev/nvme1n1...
zonefs-tests on /dev/nvme1n1:
16 zones (0 conventional zones, 16 sequential zones)
131072 512B sectors zone size (64 MiB)
1 max open zones
Running tests
Test 0010: mkzonefs (options) ... PASS
Test 0011: mkzonefs (force format) ... PASS
Test 0012: mkzonefs (invalid device) ... PASS
Test 0013: mkzonefs (super block zone state) ... PASS
Test 0020: mount (default) ... PASS
Test 0021: mount (invalid device) ... PASS
Test 0022: mount (check mount directory sub-directories) ... PASS
Test 0023: mount (options) ... PASS
Test 0030: Number of files (default) ... PASS
Test 0031: Number of files (aggr_cnv) ... skip
Test 0032: Number of files using stat (default) ... PASS
Test 0033: Number of files using stat (aggr_cnv) ... PASS
Test 0034: Number of blocks using stat (default) ... PASS
Test 0035: Number of blocks using stat (aggr_cnv) ... PASS
Test 0040: Files permissions (default) ... PASS
Test 0041: Files permissions (aggr_cnv) ... skip
Test 0042: Files permissions (set value) ... PASS
Test 0043: Files permissions (set value + aggr_cnv) ... skip
Test 0050: Files owner (default) ... PASS
Test 0051: Files owner (aggr_cnv) ... skip
Test 0052: Files owner (set value) ... PASS
Test 0053: Files owner (set value + aggr_cnv) ... skip
Test 0060: Files size (default) ... PASS
Test 0061: Files size (aggr_cnv) ... skip
Test 0070: Conventional file truncate ... skip
Test 0071: Conventional file truncate (aggr_cnv) ... skip
Test 0072: Conventional file unlink ... skip
Test 0073: Conventional file unlink (aggr_cnv) ... skip
Test 0074: Conventional file random write ... skip
Test 0075: Conventional file random write (direct) ... skip
Test 0076: Conventional file random write (aggr_cnv) ... skip
Test 0077: Conventional file random write (aggr_cnv, direct) ... skip
Test 0078: Conventional file mmap read/write ... skip
Test 0079: Conventional file mmap read/write (aggr_cnv) ... skip
Test 0080: Sequential file truncate ... PASS
Test 0081: Sequential file unlink ... PASS
Test 0082: Sequential file buffered write IO ... PASS
Test 0083: Sequential file overwrite ... PASS
Test 0084: Sequential file unaligned write (sync IO) ... PASS
Test 0085: Sequential file unaligned write (async IO) ... PASS
Test 0086: Sequential file append (sync) ... PASS
Test 0087: Sequential file append (async) ... PASS
Test 0088: Sequential file random read ... PASS
Test 0089: Sequential file mmap read/write ... PASS
Test 0090: sequential file 4K synchronous write ... PASS
Test 0091: Sequential file large synchronous write ... PASS
46 / 46 tests passed
--
2.22.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH V7 0/6] nvmet: add ZBD backend support
@ 2020-12-15 6:02 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:02 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: damien.lemoal, sagi, Chaitanya Kulkarni, hch
Hi,
NVMeOF Host is capable of handling the NVMe Protocol based Zoned Block
Devices (ZBD) in the Zoned Namespaces (ZNS) mode with the passthru
backend. There is no support for a generic block device backend to
handle the ZBD devices which are not NVMe protocol compliant.
This adds support to export the ZBDs (which are not NVMe drives) to host
the from target via NVMeOF using the host side ZNS interface.
The patch series is generated in bottom-top manner where, it first adds
prep patch and ZNS command-specific handlers on the top of genblk and
updates the data structures, then one by one it wires up the admin cmds
in the order host calls them in namespace initializing sequence. Once
everything is ready, it wires-up the I/O command handlers. See below for
patch-series overview.
All the testcases are passing for the nvme blktests with non ZBD target
device and with ZoneFS where ZBD exported with NVMeOF backed by
null_blk ZBD and null_blk ZBD without NVMeOF. Adding test result below.
Note: This patch-series is based on the earlier posted patch series :-
[PATCH 0/4] nvmet: admin-cmd related cleanups and a fix
http://lists.infradead.org/pipermail/linux-nvme/2020-December/021501.html
Regards,
Chaitanya
Changes from V6:-
1. Instead of calling report zones to find conventional zones in the
loop use the loop inside LLD report zones that also simplifies the
report zone callback for zbd backend.
2. Use -EOPNOTSUPP in the nvmet_bdev_validate_zns_zones_cb() to avoid
checkpatch warning when conventional zone is found.
2. Fix the bug in nvmet_bdev_has_conv_zones().
3. Remove conditional operators in the nvmet_bdev_execute_zone_append().
Changes from V5:-
1. Use bio->bi_iter.bi_sector for result of the REQ_OP_ZONE_APPEND
command.
2. Add endianness to the helper nvmet_sect_to_lba().
3. Make bufsize u32 in zone mgmt recv command handler.
4. Add __GFP_ZERO for report zone data buffer to return clean buffer.
Changes from V4:-
1. Don't use bio_iov_iter_get_pages() instead add a patch to export
bio_add_hw_page() and call it directly for zone append.
2. Add inline vector optimization for append bio.
3. Update the commit logs for the patches.
4. Remove ZNS related identify data structures, use individual members.
5. Add a comment for macro NVMET_MPSMIN_SHIFT.
6. Remove nvmet_bdev() helper.
7. Move the command set identifier code into common code.
8. Use IS_ENABLED() and move helpers fomr zns.c into common code.
9. Add a patch to support Command Set identifiers.
10. Open code nvmet_bdev_validate_zns_zones().
11. Remove the per namespace min zasl calculation and don't allow
namespaces with zasl value > the first ns zasl value.
12. Move the stubs into the header file.
13. Add lba to/from sector conversion helpers and update the
io-cmd-bdev.c to avoid the code duplication.
14. Add everything into one patch for zns command handlers and
respective calls from the target code.
15. Remove the trim ns-desclist admin callback patch from this series.
16. Add bio get and put helpers patches to reduce the duplicate code in
generic bdev, passthru, and generic zns backend.
Changes from V3:-
1. Get rid of the bio_max_zasl check.
2. Remove extra lines.
4. Remove the block layer api export patch.
5. Remove the bvec check in the bio_iov_iter_get_pages() for
REQ_OP_ZONE_APPEND so that we can reuse the code.
Changes from V2:-
1. Move conventional zone bitmap check into
nvmet_bdev_validate_zns_zones().
2. Don't use report zones call to check the runt zone.
3. Trim nvmet_zasl() helper.
4. Fix typo in the nvmet_zns_update_zasl().
5. Remove the comment and fix the mdts calculation in
nvmet_execute_identify_cns_cs_ctrl().
6. Use u64 for bufsize in nvmet_bdev_execute_zone_mgmt_recv().
7. Remove nvmet_zones_to_desc_size() and fix the nr_zones
calculation.
8. Remove the op variable in nvmet_bdev_execute_zone_append().
9. Fix the nr_zones calculation nvmet_bdev_execute_zone_mgmt_recv().
10. Update cover letter subject.
Changes from V1:-
1. Remove the nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o.
2. Mark helpers inline.
3. Fix typos in the comments and update the comments.
4. Get rid of the curly brackets.
5. Don't allow drives with last smaller zones.
6. Calculate the zasl as a function of ax_zone_append_sectors,
bio_max_pages so we don't have to split the bio.
7. Add global subsys->zasl and update the zasl when new namespace
is enabled.
8. Rmove the loop in the nvmet_bdev_execute_zone_mgmt_recv() and
move functionality in to the report zone callback.
9. Add goto for default case in nvmet_bdev_execute_zone_mgmt_send().
10. Allocate the zones buffer with zones size instead of bdev nr_zones.
Chaitanya Kulkarni (6):
block: export bio_add_hw_pages()
nvmet: add lba to sect conversion helpers
nvmet: add NVM command set identifier support
nvmet: add ZBD over ZNS backend support
nvmet: add bio get helper for different backends
nvmet: add bio put helper for different backends
block/bio.c | 1 +
block/blk.h | 4 -
drivers/nvme/target/Makefile | 1 +
drivers/nvme/target/admin-cmd.c | 59 ++++--
drivers/nvme/target/core.c | 14 +-
drivers/nvme/target/io-cmd-bdev.c | 51 +++--
drivers/nvme/target/nvmet.h | 71 +++++++
drivers/nvme/target/passthru.c | 11 +-
drivers/nvme/target/zns.c | 335 ++++++++++++++++++++++++++++++
include/linux/blkdev.h | 4 +
10 files changed, 503 insertions(+), 48 deletions(-)
create mode 100644 drivers/nvme/target/zns.c
Without CONFIG_BLK_DEV_ZONED nvme tests :-
# grep -i blk_dev_zoned .config
# CONFIG_BLK_DEV_ZONED is not set
# gitlog -6
2dc9be4cc588 (HEAD -> nvme-5.11) nvmet: add bio put helper for different backends
72836658a6cf nvmet: add bio get helper for different backends
c1c1819074b6 nvmet: add ZBD over ZNS backend support
428548cd91ff nvmet: add NVM command set identifier support
fba6fbaf6e2a nvmet: add lba to sect conversion helpers
70c9a4c94d5a block: export bio_add_hw_pages()
# makej M=drivers/nvme/ clean
CLEAN drivers/nvme//Module.symvers
# makej M=drivers/nvme/
CC [M] drivers/nvme//host/core.o
CC [M] drivers/nvme//host/trace.o
CC [M] drivers/nvme//host/lightnvm.o
CC [M] drivers/nvme//target/core.o
CC [M] drivers/nvme//host/hwmon.o
CC [M] drivers/nvme//target/configfs.o
CC [M] drivers/nvme//host/pci.o
CC [M] drivers/nvme//target/admin-cmd.o
CC [M] drivers/nvme//host/fabrics.o
CC [M] drivers/nvme//host/rdma.o
CC [M] drivers/nvme//target/fabrics-cmd.o
CC [M] drivers/nvme//target/discovery.o
CC [M] drivers/nvme//host/fc.o
CC [M] drivers/nvme//target/io-cmd-file.o
CC [M] drivers/nvme//host/tcp.o
CC [M] drivers/nvme//target/io-cmd-bdev.o
CC [M] drivers/nvme//target/passthru.o
CC [M] drivers/nvme//target/trace.o
CC [M] drivers/nvme//target/loop.o
CC [M] drivers/nvme//target/rdma.o
CC [M] drivers/nvme//target/fc.o
CC [M] drivers/nvme//target/fcloop.o
CC [M] drivers/nvme//target/tcp.o
LD [M] drivers/nvme//target/nvme-loop.o
LD [M] drivers/nvme//target/nvme-fcloop.o
LD [M] drivers/nvme//target/nvmet-tcp.o
LD [M] drivers/nvme//host/nvme-fabrics.o
LD [M] drivers/nvme//host/nvme.o
LD [M] drivers/nvme//host/nvme-rdma.o
LD [M] drivers/nvme//target/nvmet-rdma.o
LD [M] drivers/nvme//target/nvmet.o
LD [M] drivers/nvme//target/nvmet-fc.o
LD [M] drivers/nvme//host/nvme-tcp.o
LD [M] drivers/nvme//host/nvme-fc.o
LD [M] drivers/nvme//host/nvme-core.o
MODPOST drivers/nvme//Module.symvers
CC [M] drivers/nvme//host/nvme-core.mod.o
CC [M] drivers/nvme//host/nvme-fabrics.mod.o
CC [M] drivers/nvme//host/nvme-fc.mod.o
CC [M] drivers/nvme//host/nvme-rdma.mod.o
CC [M] drivers/nvme//host/nvme-tcp.mod.o
CC [M] drivers/nvme//host/nvme.mod.o
CC [M] drivers/nvme//target/nvme-fcloop.mod.o
CC [M] drivers/nvme//target/nvme-loop.mod.o
CC [M] drivers/nvme//target/nvmet-fc.mod.o
CC [M] drivers/nvme//target/nvmet-rdma.mod.o
CC [M] drivers/nvme//target/nvmet-tcp.mod.o
CC [M] drivers/nvme//target/nvmet.mod.o
LD [M] drivers/nvme//target/nvme-fcloop.ko
LD [M] drivers/nvme//host/nvme-tcp.ko
LD [M] drivers/nvme//host/nvme-core.ko
LD [M] drivers/nvme//target/nvmet-tcp.ko
LD [M] drivers/nvme//target/nvme-loop.ko
LD [M] drivers/nvme//target/nvmet-fc.ko
LD [M] drivers/nvme//host/nvme-fabrics.ko
LD [M] drivers/nvme//host/nvme-fc.ko
LD [M] drivers/nvme//target/nvmet-rdma.ko
LD [M] drivers/nvme//host/nvme-rdma.ko
LD [M] drivers/nvme//host/nvme.ko
LD [M] drivers/nvme//target/nvmet.ko
# cdblktests
# ./check tests/nvme/
nvme/002 (create many subsystems and test discovery) [passed]
runtime 24.923s ... 24.625s
nvme/003 (test if we're sending keep-alives to a discovery controller) [passed]
runtime 10.141s ... 10.143s
nvme/004 (test nvme and nvmet UUID NS descriptors) [passed]
runtime 2.446s ... 2.444s
nvme/005 (reset local loopback target) [not run]
nvme_core module does not have parameter multipath
nvme/006 (create an NVMeOF target with a block device-backed ns) [passed]
runtime 0.113s ... 0.103s
nvme/007 (create an NVMeOF target with a file-backed ns) [passed]
runtime 0.068s ... 0.066s
nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
runtime 2.477s ... 2.477s
nvme/009 (create an NVMeOF host with a file-backed ns) [passed]
runtime 2.447s ... 2.428s
nvme/010 (run data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 21.695s ... 27.905s
nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
runtime 11.413s ... 252.685s
nvme/012 (run mkfs and data verification fio job on NVMeOF block device-backed ns) [passed]
runtime ... 50.439s
nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed ns) [passed]
runtime 268.947s ... 275.912s
nvme/014 (flush a NVMeOF block device-backed ns) [passed]
runtime 21.977s ... 22.190s
nvme/015 (unit test for NVMe flush for file backed ns) [passed]
runtime 20.816s ... 21.174s
nvme/016 (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
runtime 13.469s ... 13.774s
nvme/017 (create/delete many file-ns and test discovery) [passed]
runtime 14.086s ... 13.420s
nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
runtime 2.418s ... 2.416s
nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
runtime 2.477s ... 2.474s
nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
runtime 2.410s ... 2.403s
nvme/021 (test NVMe list command on NVMeOF file-backed ns) [passed]
runtime 2.412s ... 2.404s
nvme/022 (test NVMe reset command on NVMeOF file-backed ns) [passed]
runtime 2.861s ... 2.863s
nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
runtime 2.486s ... 2.471s
nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
runtime 2.430s ... 2.411s
nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
runtime 2.412s ... 2.395s
nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
runtime 2.450s ... 2.420s
nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
runtime 2.416s ... 2.401s
nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
runtime 2.418s ... 2.405s
nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
runtime 2.759s ... 2.732s
nvme/030 (ensure the discovery generation counter is updated appropriately) [passed]
runtime 0.347s ... 0.345s
nvme/031 (test deletion of NVMeOF controllers immediately after setup) [passed]
runtime 13.480s ... 13.432s
nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
runtime 0.039s ... 0.039s
With CONFIG_BLK_DEV_ZONED nvme and zonefs tests :-
# grep -i blk_dev_zoned .config
CONFIG_BLK_DEV_ZONED=y
# make M=drivers/nvme/ clean
CLEAN drivers/nvme//Module.symvers
# make M=drivers/nvme/
CC [M] drivers/nvme//host/core.o
CC [M] drivers/nvme//host/trace.o
CC [M] drivers/nvme//host/lightnvm.o
CC [M] drivers/nvme//host/zns.o
CC [M] drivers/nvme//host/hwmon.o
LD [M] drivers/nvme//host/nvme-core.o
CC [M] drivers/nvme//host/pci.o
LD [M] drivers/nvme//host/nvme.o
CC [M] drivers/nvme//host/fabrics.o
LD [M] drivers/nvme//host/nvme-fabrics.o
CC [M] drivers/nvme//host/rdma.o
LD [M] drivers/nvme//host/nvme-rdma.o
CC [M] drivers/nvme//host/fc.o
LD [M] drivers/nvme//host/nvme-fc.o
CC [M] drivers/nvme//host/tcp.o
LD [M] drivers/nvme//host/nvme-tcp.o
CC [M] drivers/nvme//target/core.o
CC [M] drivers/nvme//target/configfs.o
CC [M] drivers/nvme//target/admin-cmd.o
CC [M] drivers/nvme//target/fabrics-cmd.o
CC [M] drivers/nvme//target/discovery.o
CC [M] drivers/nvme//target/io-cmd-file.o
CC [M] drivers/nvme//target/io-cmd-bdev.o
CC [M] drivers/nvme//target/passthru.o
CC [M] drivers/nvme//target/zns.o
CC [M] drivers/nvme//target/trace.o
LD [M] drivers/nvme//target/nvmet.o
CC [M] drivers/nvme//target/loop.o
LD [M] drivers/nvme//target/nvme-loop.o
CC [M] drivers/nvme//target/rdma.o
LD [M] drivers/nvme//target/nvmet-rdma.o
CC [M] drivers/nvme//target/fc.o
LD [M] drivers/nvme//target/nvmet-fc.o
CC [M] drivers/nvme//target/fcloop.o
LD [M] drivers/nvme//target/nvme-fcloop.o
CC [M] drivers/nvme//target/tcp.o
LD [M] drivers/nvme//target/nvmet-tcp.o
MODPOST drivers/nvme//Module.symvers
CC [M] drivers/nvme//host/nvme-core.mod.o
LD [M] drivers/nvme//host/nvme-core.ko
CC [M] drivers/nvme//host/nvme-fabrics.mod.o
LD [M] drivers/nvme//host/nvme-fabrics.ko
CC [M] drivers/nvme//host/nvme-fc.mod.o
LD [M] drivers/nvme//host/nvme-fc.ko
CC [M] drivers/nvme//host/nvme-rdma.mod.o
LD [M] drivers/nvme//host/nvme-rdma.ko
CC [M] drivers/nvme//host/nvme-tcp.mod.o
LD [M] drivers/nvme//host/nvme-tcp.ko
CC [M] drivers/nvme//host/nvme.mod.o
LD [M] drivers/nvme//host/nvme.ko
CC [M] drivers/nvme//target/nvme-fcloop.mod.o
LD [M] drivers/nvme//target/nvme-fcloop.ko
CC [M] drivers/nvme//target/nvme-loop.mod.o
LD [M] drivers/nvme//target/nvme-loop.ko
CC [M] drivers/nvme//target/nvmet-fc.mod.o
LD [M] drivers/nvme//target/nvmet-fc.ko
CC [M] drivers/nvme//target/nvmet-rdma.mod.o
LD [M] drivers/nvme//target/nvmet-rdma.ko
CC [M] drivers/nvme//target/nvmet-tcp.mod.o
LD [M] drivers/nvme//target/nvmet-tcp.ko
CC [M] drivers/nvme//target/nvmet.mod.o
LD [M] drivers/nvme//target/nvmet.ko
#
# cdblktests
# ./check tests/nvme/
nvme/002 (create many subsystems and test discovery) [passed]
runtime 24.378s ... 24.636s
nvme/003 (test if we're sending keep-alives to a discovery controller) [passed]
runtime 10.133s ... 10.152s
nvme/004 (test nvme and nvmet UUID NS descriptors) [passed]
runtime 2.463s ... 2.478s
nvme/005 (reset local loopback target) [not run]
nvme_core module does not have parameter multipath
nvme/006 (create an NVMeOF target with a block device-backed ns) [passed]
runtime 0.095s ... 0.122s
nvme/007 (create an NVMeOF target with a file-backed ns) [passed]
runtime 0.065s ... 0.079s
nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
runtime 2.473s ... 2.501s
nvme/009 (create an NVMeOF host with a file-backed ns) [passed]
runtime 2.460s ... 2.424s
nvme/010 (run data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 24.526s ... 28.015s
nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
runtime 265.967s ... 282.717s
nvme/012 (run mkfs and data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 44.665s ... 48.124s
nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed ns) [passed]
runtime 261.739s ... 352.331s
nvme/014 (flush a NVMeOF block device-backed ns) [passed]
runtime 21.268s ... 22.013s
nvme/015 (unit test for NVMe flush for file backed ns) [passed]
runtime 18.820s ... 22.104s
nvme/016 (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
runtime 13.899s ... 14.322s
nvme/017 (create/delete many file-ns and test discovery) [passed]
runtime 14.322s ... 14.031s
nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
runtime 2.450s ... 2.444s
nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
runtime 2.475s ... 2.489s
nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
runtime 2.410s ... 2.448s
nvme/021 (test NVMe list command on NVMeOF file-backed ns) [passed]
runtime 2.441s ... 2.439s
nvme/022 (test NVMe reset command on NVMeOF file-backed ns) [passed]
runtime 2.864s ... 2.863s
nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
runtime 2.465s ... 2.446s
nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
runtime 2.416s ... 2.411s
nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
runtime 2.419s ... 2.748s
nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
runtime 2.422s ... 2.410s
nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
runtime 2.456s ... 2.462s
nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
runtime 2.427s ... 2.429s
nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
runtime 2.751s ... 2.755s
nvme/030 (ensure the discovery generation counter is updated appropriately) [passed]
runtime 0.346s ... 0.357s
nvme/031 (test deletion of NVMeOF controllers immediately after setup) [passed]
runtime 13.601s ... 13.591s
nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
runtime 0.039s ... 0.059s
# cdzonefstest
# cdnvme
# cdzonefstest
# ./zonefs-tests.sh /dev/nvme1n1
Gathering information on /dev/nvme1n1...
zonefs-tests on /dev/nvme1n1:
16 zones (0 conventional zones, 16 sequential zones)
131072 512B sectors zone size (64 MiB)
1 max open zones
Running tests
Test 0010: mkzonefs (options) ... PASS
Test 0011: mkzonefs (force format) ... PASS
Test 0012: mkzonefs (invalid device) ... PASS
Test 0013: mkzonefs (super block zone state) ... PASS
Test 0020: mount (default) ... PASS
Test 0021: mount (invalid device) ... PASS
Test 0022: mount (check mount directory sub-directories) ... PASS
Test 0023: mount (options) ... PASS
Test 0030: Number of files (default) ... PASS
Test 0031: Number of files (aggr_cnv) ... skip
Test 0032: Number of files using stat (default) ... PASS
Test 0033: Number of files using stat (aggr_cnv) ... PASS
Test 0034: Number of blocks using stat (default) ... PASS
Test 0035: Number of blocks using stat (aggr_cnv) ... PASS
Test 0040: Files permissions (default) ... PASS
Test 0041: Files permissions (aggr_cnv) ... skip
Test 0042: Files permissions (set value) ... PASS
Test 0043: Files permissions (set value + aggr_cnv) ... skip
Test 0050: Files owner (default) ... PASS
Test 0051: Files owner (aggr_cnv) ... skip
Test 0052: Files owner (set value) ... PASS
Test 0053: Files owner (set value + aggr_cnv) ... skip
Test 0060: Files size (default) ... PASS
Test 0061: Files size (aggr_cnv) ... skip
Test 0070: Conventional file truncate ... skip
Test 0071: Conventional file truncate (aggr_cnv) ... skip
Test 0072: Conventional file unlink ... skip
Test 0073: Conventional file unlink (aggr_cnv) ... skip
Test 0074: Conventional file random write ... skip
Test 0075: Conventional file random write (direct) ... skip
Test 0076: Conventional file random write (aggr_cnv) ... skip
Test 0077: Conventional file random write (aggr_cnv, direct) ... skip
Test 0078: Conventional file mmap read/write ... skip
Test 0079: Conventional file mmap read/write (aggr_cnv) ... skip
Test 0080: Sequential file truncate ... PASS
Test 0081: Sequential file unlink ... PASS
Test 0082: Sequential file buffered write IO ... PASS
Test 0083: Sequential file overwrite ... PASS
Test 0084: Sequential file unaligned write (sync IO) ... PASS
Test 0085: Sequential file unaligned write (async IO) ... PASS
Test 0086: Sequential file append (sync) ... PASS
Test 0087: Sequential file append (async) ... PASS
Test 0088: Sequential file random read ... PASS
Test 0089: Sequential file mmap read/write ... PASS
Test 0090: sequential file 4K synchronous write ... PASS
Test 0091: Sequential file large synchronous write ... PASS
46 / 46 tests passed
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH V7 1/6] block: export bio_add_hw_pages()
2020-12-15 6:02 ` Chaitanya Kulkarni
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: sagi, hch, damien.lemoal, Chaitanya Kulkarni
To implement the NVMe Zone Append command on the NVMeOF target side for
generic Zoned Block Devices with NVMe Zoned Namespaces interface, we
need to build the bios with hardware limitations, i.e. we use
bio_add_hw_page() with queue_max_zone_append_sectors() instead of
bio_add_page().
Without this API being exported NVMeOF target will require to use
bio_add_hw_page() caller bio_iov_iter_get_pages(). That results in
extra work which is inefficient.
Export the API so that NVMeOF ZBD over ZNS backend can use it to build
Zone Append bios.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
block/bio.c | 1 +
block/blk.h | 4 ----
include/linux/blkdev.h | 4 ++++
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index fa01bef35bb1..eafd97c6c7fd 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -826,6 +826,7 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
bio->bi_iter.bi_size += len;
return len;
}
+EXPORT_SYMBOL(bio_add_hw_page);
/**
* bio_add_pc_page - attempt to add page to passthrough bio
diff --git a/block/blk.h b/block/blk.h
index e05507a8d1e3..1fdb8d5d8590 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -428,8 +428,4 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
#endif
}
-int bio_add_hw_page(struct request_queue *q, struct bio *bio,
- struct page *page, unsigned int len, unsigned int offset,
- unsigned int max_sectors, bool *same_page);
-
#endif /* BLK_INTERNAL_H */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 05b346a68c2e..2bdaa7cacfa3 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -2023,4 +2023,8 @@ int fsync_bdev(struct block_device *bdev);
struct super_block *freeze_bdev(struct block_device *bdev);
int thaw_bdev(struct block_device *bdev, struct super_block *sb);
+int bio_add_hw_page(struct request_queue *q, struct bio *bio,
+ struct page *page, unsigned int len, unsigned int offset,
+ unsigned int max_sectors, bool *same_page);
+
#endif /* _LINUX_BLKDEV_H */
--
2.22.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 1/6] block: export bio_add_hw_pages()
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: damien.lemoal, sagi, Chaitanya Kulkarni, hch
To implement the NVMe Zone Append command on the NVMeOF target side for
generic Zoned Block Devices with NVMe Zoned Namespaces interface, we
need to build the bios with hardware limitations, i.e. we use
bio_add_hw_page() with queue_max_zone_append_sectors() instead of
bio_add_page().
Without this API being exported NVMeOF target will require to use
bio_add_hw_page() caller bio_iov_iter_get_pages(). That results in
extra work which is inefficient.
Export the API so that NVMeOF ZBD over ZNS backend can use it to build
Zone Append bios.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
block/bio.c | 1 +
block/blk.h | 4 ----
include/linux/blkdev.h | 4 ++++
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index fa01bef35bb1..eafd97c6c7fd 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -826,6 +826,7 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
bio->bi_iter.bi_size += len;
return len;
}
+EXPORT_SYMBOL(bio_add_hw_page);
/**
* bio_add_pc_page - attempt to add page to passthrough bio
diff --git a/block/blk.h b/block/blk.h
index e05507a8d1e3..1fdb8d5d8590 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -428,8 +428,4 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
#endif
}
-int bio_add_hw_page(struct request_queue *q, struct bio *bio,
- struct page *page, unsigned int len, unsigned int offset,
- unsigned int max_sectors, bool *same_page);
-
#endif /* BLK_INTERNAL_H */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 05b346a68c2e..2bdaa7cacfa3 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -2023,4 +2023,8 @@ int fsync_bdev(struct block_device *bdev);
struct super_block *freeze_bdev(struct block_device *bdev);
int thaw_bdev(struct block_device *bdev, struct super_block *sb);
+int bio_add_hw_page(struct request_queue *q, struct bio *bio,
+ struct page *page, unsigned int len, unsigned int offset,
+ unsigned int max_sectors, bool *same_page);
+
#endif /* _LINUX_BLKDEV_H */
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 2/6] nvmet: add lba to sect conversion helpers
2020-12-15 6:02 ` Chaitanya Kulkarni
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: sagi, hch, damien.lemoal, Chaitanya Kulkarni
In this preparation patch we add helpers to convert lbas to sectors and
sectors to lba. This is needed to eliminate code duplication in the ZBD
backend.
Use these helpers in the block device backennd.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/io-cmd-bdev.c | 8 +++-----
drivers/nvme/target/nvmet.h | 10 ++++++++++
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 125dde3f410e..23095bdfce06 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -256,8 +256,7 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req)
if (is_pci_p2pdma_page(sg_page(req->sg)))
op |= REQ_NOMERGE;
- sector = le64_to_cpu(req->cmd->rw.slba);
- sector <<= (req->ns->blksize_shift - 9);
+ sector = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
bio = &req->b.inline_bio;
@@ -345,7 +344,7 @@ static u16 nvmet_bdev_discard_range(struct nvmet_req *req,
int ret;
ret = __blkdev_issue_discard(ns->bdev,
- le64_to_cpu(range->slba) << (ns->blksize_shift - 9),
+ nvmet_lba_to_sect(ns, range->slba),
le32_to_cpu(range->nlb) << (ns->blksize_shift - 9),
GFP_KERNEL, 0, bio);
if (ret && ret != -EOPNOTSUPP) {
@@ -414,8 +413,7 @@ static void nvmet_bdev_execute_write_zeroes(struct nvmet_req *req)
if (!nvmet_check_transfer_len(req, 0))
return;
- sector = le64_to_cpu(write_zeroes->slba) <<
- (req->ns->blksize_shift - 9);
+ sector = nvmet_lba_to_sect(req->ns, write_zeroes->slba);
nr_sector = (((sector_t)le16_to_cpu(write_zeroes->length) + 1) <<
(req->ns->blksize_shift - 9));
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 592763732065..8776dd1a0490 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -603,4 +603,14 @@ static inline bool nvmet_ns_has_pi(struct nvmet_ns *ns)
return ns->pi_type && ns->metadata_size == sizeof(struct t10_pi_tuple);
}
+static inline __le64 nvmet_sect_to_lba(struct nvmet_ns *ns, sector_t sect)
+{
+ return cpu_to_le64(sect >> (ns->blksize_shift - SECTOR_SHIFT));
+}
+
+static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
+{
+ return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
+}
+
#endif /* _NVMET_H */
--
2.22.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 2/6] nvmet: add lba to sect conversion helpers
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: damien.lemoal, sagi, Chaitanya Kulkarni, hch
In this preparation patch we add helpers to convert lbas to sectors and
sectors to lba. This is needed to eliminate code duplication in the ZBD
backend.
Use these helpers in the block device backennd.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/io-cmd-bdev.c | 8 +++-----
drivers/nvme/target/nvmet.h | 10 ++++++++++
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 125dde3f410e..23095bdfce06 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -256,8 +256,7 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req)
if (is_pci_p2pdma_page(sg_page(req->sg)))
op |= REQ_NOMERGE;
- sector = le64_to_cpu(req->cmd->rw.slba);
- sector <<= (req->ns->blksize_shift - 9);
+ sector = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
bio = &req->b.inline_bio;
@@ -345,7 +344,7 @@ static u16 nvmet_bdev_discard_range(struct nvmet_req *req,
int ret;
ret = __blkdev_issue_discard(ns->bdev,
- le64_to_cpu(range->slba) << (ns->blksize_shift - 9),
+ nvmet_lba_to_sect(ns, range->slba),
le32_to_cpu(range->nlb) << (ns->blksize_shift - 9),
GFP_KERNEL, 0, bio);
if (ret && ret != -EOPNOTSUPP) {
@@ -414,8 +413,7 @@ static void nvmet_bdev_execute_write_zeroes(struct nvmet_req *req)
if (!nvmet_check_transfer_len(req, 0))
return;
- sector = le64_to_cpu(write_zeroes->slba) <<
- (req->ns->blksize_shift - 9);
+ sector = nvmet_lba_to_sect(req->ns, write_zeroes->slba);
nr_sector = (((sector_t)le16_to_cpu(write_zeroes->length) + 1) <<
(req->ns->blksize_shift - 9));
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 592763732065..8776dd1a0490 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -603,4 +603,14 @@ static inline bool nvmet_ns_has_pi(struct nvmet_ns *ns)
return ns->pi_type && ns->metadata_size == sizeof(struct t10_pi_tuple);
}
+static inline __le64 nvmet_sect_to_lba(struct nvmet_ns *ns, sector_t sect)
+{
+ return cpu_to_le64(sect >> (ns->blksize_shift - SECTOR_SHIFT));
+}
+
+static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
+{
+ return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
+}
+
#endif /* _NVMET_H */
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 3/6] nvmet: add NVM command set identifier support
2020-12-15 6:02 ` Chaitanya Kulkarni
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: sagi, hch, damien.lemoal, Chaitanya Kulkarni
NVMe TP 4056 allows controller to support different command sets.
NVMeoF target currently only supports namespaces that contain
traditional logical blocks that may be randomly read and written. In
some applications there is value in exposing namespaces that contain
logical blocks that have special access rules (e.g. sequentially write
required namespace such as Zoned Namespace (ZNS)).
In order to support the Zoned Block Devices (ZBD) backend, controller
needs to have support for ZNS Command Set Identifier (CSI).
In this preparation patch we adjust the code such that it can now
support different command sets. We update the namespace data
structure to store the CSI value which defaults to NVME_CSI_NVM
which represents traditional logical blocks namespace type.
The CSI support is required to implement the ZBD backend over NVMe ZNS
interface, since ZNS commands belongs to different command set than
the default one.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/admin-cmd.c | 33 ++++++++++++++++++++-------------
drivers/nvme/target/core.c | 13 ++++++++++++-
drivers/nvme/target/nvmet.h | 1 +
3 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 74620240ac47..f4c0f3aca485 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -176,19 +176,26 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
if (!log)
goto out;
- log->acs[nvme_admin_get_log_page] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_identify] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_abort_cmd] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_set_features] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_get_features] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_async_event] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_keep_alive] = cpu_to_le32(1 << 0);
-
- log->iocs[nvme_cmd_read] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_write] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_flush] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
+ switch (req->cmd->get_log_page.csi) {
+ case NVME_CSI_NVM:
+ log->acs[nvme_admin_get_log_page] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_identify] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_abort_cmd] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_set_features] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_get_features] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_async_event] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_keep_alive] = cpu_to_le32(1 << 0);
+
+ log->iocs[nvme_cmd_read] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_write] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_flush] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
+ break;
+ default:
+ status = NVME_SC_INVALID_LOG_PAGE;
+ break;
+ }
status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 8ce4d59cc9e7..672e4009f8d6 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -681,6 +681,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
uuid_gen(&ns->uuid);
ns->buffered_io = false;
+ ns->csi = NVME_CSI_NVM;
return ns;
}
@@ -1103,6 +1104,16 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
return (cc >> NVME_CC_IOCQES_SHIFT) & 0xf;
}
+static inline bool nvmet_cc_css_check(u8 cc_css)
+{
+ switch (cc_css <<= NVME_CC_CSS_SHIFT) {
+ case NVME_CC_CSS_NVM:
+ return true;
+ default:
+ return false;
+ }
+}
+
static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl)
{
lockdep_assert_held(&ctrl->lock);
@@ -1111,7 +1122,7 @@ static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl)
nvmet_cc_iocqes(ctrl->cc) != NVME_NVM_IOCQES ||
nvmet_cc_mps(ctrl->cc) != 0 ||
nvmet_cc_ams(ctrl->cc) != 0 ||
- nvmet_cc_css(ctrl->cc) != 0) {
+ !nvmet_cc_css_check(nvmet_cc_css(ctrl->cc))) {
ctrl->csts = NVME_CSTS_CFS;
return;
}
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 8776dd1a0490..476b3cd91c65 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -81,6 +81,7 @@ struct nvmet_ns {
struct pci_dev *p2p_dev;
int pi_type;
int metadata_size;
+ u8 csi;
};
static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)
--
2.22.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 3/6] nvmet: add NVM command set identifier support
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: damien.lemoal, sagi, Chaitanya Kulkarni, hch
NVMe TP 4056 allows controller to support different command sets.
NVMeoF target currently only supports namespaces that contain
traditional logical blocks that may be randomly read and written. In
some applications there is value in exposing namespaces that contain
logical blocks that have special access rules (e.g. sequentially write
required namespace such as Zoned Namespace (ZNS)).
In order to support the Zoned Block Devices (ZBD) backend, controller
needs to have support for ZNS Command Set Identifier (CSI).
In this preparation patch we adjust the code such that it can now
support different command sets. We update the namespace data
structure to store the CSI value which defaults to NVME_CSI_NVM
which represents traditional logical blocks namespace type.
The CSI support is required to implement the ZBD backend over NVMe ZNS
interface, since ZNS commands belongs to different command set than
the default one.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/admin-cmd.c | 33 ++++++++++++++++++++-------------
drivers/nvme/target/core.c | 13 ++++++++++++-
drivers/nvme/target/nvmet.h | 1 +
3 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 74620240ac47..f4c0f3aca485 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -176,19 +176,26 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
if (!log)
goto out;
- log->acs[nvme_admin_get_log_page] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_identify] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_abort_cmd] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_set_features] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_get_features] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_async_event] = cpu_to_le32(1 << 0);
- log->acs[nvme_admin_keep_alive] = cpu_to_le32(1 << 0);
-
- log->iocs[nvme_cmd_read] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_write] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_flush] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
- log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
+ switch (req->cmd->get_log_page.csi) {
+ case NVME_CSI_NVM:
+ log->acs[nvme_admin_get_log_page] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_identify] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_abort_cmd] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_set_features] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_get_features] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_async_event] = cpu_to_le32(1 << 0);
+ log->acs[nvme_admin_keep_alive] = cpu_to_le32(1 << 0);
+
+ log->iocs[nvme_cmd_read] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_write] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_flush] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
+ log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
+ break;
+ default:
+ status = NVME_SC_INVALID_LOG_PAGE;
+ break;
+ }
status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 8ce4d59cc9e7..672e4009f8d6 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -681,6 +681,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
uuid_gen(&ns->uuid);
ns->buffered_io = false;
+ ns->csi = NVME_CSI_NVM;
return ns;
}
@@ -1103,6 +1104,16 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
return (cc >> NVME_CC_IOCQES_SHIFT) & 0xf;
}
+static inline bool nvmet_cc_css_check(u8 cc_css)
+{
+ switch (cc_css <<= NVME_CC_CSS_SHIFT) {
+ case NVME_CC_CSS_NVM:
+ return true;
+ default:
+ return false;
+ }
+}
+
static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl)
{
lockdep_assert_held(&ctrl->lock);
@@ -1111,7 +1122,7 @@ static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl)
nvmet_cc_iocqes(ctrl->cc) != NVME_NVM_IOCQES ||
nvmet_cc_mps(ctrl->cc) != 0 ||
nvmet_cc_ams(ctrl->cc) != 0 ||
- nvmet_cc_css(ctrl->cc) != 0) {
+ !nvmet_cc_css_check(nvmet_cc_css(ctrl->cc))) {
ctrl->csts = NVME_CSTS_CFS;
return;
}
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 8776dd1a0490..476b3cd91c65 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -81,6 +81,7 @@ struct nvmet_ns {
struct pci_dev *p2p_dev;
int pi_type;
int metadata_size;
+ u8 csi;
};
static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
2020-12-15 6:02 ` Chaitanya Kulkarni
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: sagi, hch, damien.lemoal, Chaitanya Kulkarni
NVMe TP 4053 – Zoned Namespaces (ZNS) allows host software to
communicate with a non-volatile memory subsystem using zones for
NVMe protocol based controllers. NVMeOF already support the ZNS NVMe
Protocol compliant devices on the target in the passthru mode. There
are Generic zoned block devices like Shingled Magnetic Recording (SMR)
HDDs which are not based on the NVMe protocol.
This patch adds ZNS backend to support the ZBDs for NVMeOF target.
This support includes implementing the new command set NVME_CSI_ZNS,
adding different command handlers for ZNS command set such as
NVMe Identify Controller, NVMe Identify Namespace, NVMe Zone Append,
NVMe Zone Management Send and NVMe Zone Management Receive.
With new command set identifier we also update the target command effects
logs to reflect the ZNS compliant commands.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/Makefile | 1 +
drivers/nvme/target/admin-cmd.c | 26 +++
drivers/nvme/target/core.c | 1 +
drivers/nvme/target/io-cmd-bdev.c | 33 ++-
drivers/nvme/target/nvmet.h | 38 ++++
drivers/nvme/target/zns.c | 342 ++++++++++++++++++++++++++++++
6 files changed, 433 insertions(+), 8 deletions(-)
create mode 100644 drivers/nvme/target/zns.c
diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
index ebf91fc4c72e..9837e580fa7e 100644
--- a/drivers/nvme/target/Makefile
+++ b/drivers/nvme/target/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_NVME_TARGET_TCP) += nvmet-tcp.o
nvmet-y += core.o configfs.o admin-cmd.o fabrics-cmd.o \
discovery.o io-cmd-file.o io-cmd-bdev.o
nvmet-$(CONFIG_NVME_TARGET_PASSTHRU) += passthru.o
+nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o
nvme-loop-y += loop.o
nvmet-rdma-y += rdma.o
nvmet-fc-y += fc.o
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index f4c0f3aca485..6f5279b15aa6 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -192,6 +192,15 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
break;
+ case NVME_CSI_ZNS:
+ if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
+ u32 *iocs = log->iocs;
+
+ iocs[nvme_cmd_zone_append] = cpu_to_le32(1 << 0);
+ iocs[nvme_cmd_zone_mgmt_send] = cpu_to_le32(1 << 0);
+ iocs[nvme_cmd_zone_mgmt_recv] = cpu_to_le32(1 << 0);
+ }
+ break;
default:
status = NVME_SC_INVALID_LOG_PAGE;
break;
@@ -614,6 +623,7 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
static void nvmet_execute_identify_desclist(struct nvmet_req *req)
{
+ u16 nvme_cis_zns = NVME_CSI_ZNS;
u16 status = 0;
off_t off = 0;
@@ -638,6 +648,14 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
if (status)
goto out;
}
+ if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
+ if (req->ns->csi == NVME_CSI_ZNS)
+ status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI,
+ NVME_NIDT_CSI_LEN,
+ &nvme_cis_zns, &off);
+ if (status)
+ goto out;
+ }
if (sg_zero_buffer(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE - off,
off) != NVME_IDENTIFY_DATA_SIZE - off)
@@ -655,8 +673,16 @@ static void nvmet_execute_identify(struct nvmet_req *req)
switch (req->cmd->identify.cns) {
case NVME_ID_CNS_NS:
return nvmet_execute_identify_ns(req);
+ case NVME_ID_CNS_CS_NS:
+ if (req->cmd->identify.csi == NVME_CSI_ZNS)
+ return nvmet_execute_identify_cns_cs_ns(req);
+ break;
case NVME_ID_CNS_CTRL:
return nvmet_execute_identify_ctrl(req);
+ case NVME_ID_CNS_CS_CTRL:
+ if (req->cmd->identify.csi == NVME_CSI_ZNS)
+ return nvmet_execute_identify_cns_cs_ctrl(req);
+ break;
case NVME_ID_CNS_NS_ACTIVE_LIST:
return nvmet_execute_identify_nslist(req);
case NVME_ID_CNS_NS_DESC_LIST:
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 672e4009f8d6..17a99c7134dc 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1107,6 +1107,7 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
static inline bool nvmet_cc_css_check(u8 cc_css)
{
switch (cc_css <<= NVME_CC_CSS_SHIFT) {
+ case NVME_CC_CSS_CSI:
case NVME_CC_CSS_NVM:
return true;
default:
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 23095bdfce06..6178ef643962 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -63,6 +63,14 @@ static void nvmet_bdev_ns_enable_integrity(struct nvmet_ns *ns)
}
}
+void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
+{
+ if (ns->bdev) {
+ blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
+ ns->bdev = NULL;
+ }
+}
+
int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
{
int ret;
@@ -86,15 +94,15 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY_T10))
nvmet_bdev_ns_enable_integrity(ns);
- return 0;
-}
-
-void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
-{
- if (ns->bdev) {
- blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
- ns->bdev = NULL;
+ if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && bdev_is_zoned(ns->bdev)) {
+ if (!nvmet_bdev_zns_enable(ns)) {
+ nvmet_bdev_ns_disable(ns);
+ return -EINVAL;
+ }
+ ns->csi = NVME_CSI_ZNS;
}
+
+ return 0;
}
void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
@@ -448,6 +456,15 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
case nvme_cmd_write_zeroes:
req->execute = nvmet_bdev_execute_write_zeroes;
return 0;
+ case nvme_cmd_zone_append:
+ req->execute = nvmet_bdev_execute_zone_append;
+ return 0;
+ case nvme_cmd_zone_mgmt_recv:
+ req->execute = nvmet_bdev_execute_zone_mgmt_recv;
+ return 0;
+ case nvme_cmd_zone_mgmt_send:
+ req->execute = nvmet_bdev_execute_zone_mgmt_send;
+ return 0;
default:
pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
req->sq->qid);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 476b3cd91c65..7361665585a2 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -252,6 +252,10 @@ struct nvmet_subsys {
unsigned int admin_timeout;
unsigned int io_timeout;
#endif /* CONFIG_NVME_TARGET_PASSTHRU */
+
+#ifdef CONFIG_BLK_DEV_ZONED
+ u8 zasl;
+#endif /* CONFIG_BLK_DEV_ZONED */
};
static inline struct nvmet_subsys *to_subsys(struct config_item *item)
@@ -614,4 +618,38 @@ static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
}
+#ifdef CONFIG_BLK_DEV_ZONED
+bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
+void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req);
+void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req);
+void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req);
+void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req);
+void nvmet_bdev_execute_zone_append(struct nvmet_req *req);
+#else /* CONFIG_BLK_DEV_ZONED */
+static inline bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
+{
+ return false;
+}
+static inline void
+nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_bdev_execute_zone_append(struct nvmet_req *req)
+{
+}
+#endif /* CONFIG_BLK_DEV_ZONED */
+
#endif /* _NVMET_H */
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
new file mode 100644
index 000000000000..3981baa647b2
--- /dev/null
+++ b/drivers/nvme/target/zns.c
@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * NVMe ZNS-ZBD command implementation.
+ * Copyright (c) 2020-2021 HGST, a Western Digital Company.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/nvme.h>
+#include <linux/blkdev.h>
+#include "nvmet.h"
+
+/*
+ * We set the Memory Page Size Minimum (MPSMIN) for target controller to 0
+ * which gets added by 12 in the nvme_enable_ctrl() which results in 2^12 = 4k
+ * as page_shift value. When calculating the ZASL use shift by 12.
+ */
+#define NVMET_MPSMIN_SHIFT 12
+
+static u16 nvmet_bdev_zns_checks(struct nvmet_req *req)
+{
+ u16 status = NVME_SC_SUCCESS;
+
+ if (!bdev_is_zoned(req->ns->bdev)) {
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto out;
+ }
+
+ if (req->cmd->zmr.zra != NVME_ZRA_ZONE_REPORT) {
+ status = NVME_SC_INVALID_FIELD;
+ goto out;
+ }
+
+ if (req->cmd->zmr.zrasf != NVME_ZRASF_ZONE_REPORT_ALL) {
+ status = NVME_SC_INVALID_FIELD;
+ goto out;
+ }
+
+ if (req->cmd->zmr.pr != NVME_REPORT_ZONE_PARTIAL)
+ status = NVME_SC_INVALID_FIELD;
+
+out:
+ return status;
+}
+
+/*
+ * ZNS related command implementation and helpers.
+ */
+
+static inline u8 nvmet_zasl(unsigned int zone_append_sects)
+{
+ /*
+ * Zone Append Size Limit is the value experessed in the units
+ * of minimum memory page size (i.e. 12) and is reported power of 2.
+ */
+ return ilog2((zone_append_sects << 9) >> NVMET_MPSMIN_SHIFT);
+}
+
+static inline bool nvmet_zns_update_zasl(struct nvmet_ns *ns)
+{
+ struct request_queue *q = ns->bdev->bd_disk->queue;
+ u8 zasl = nvmet_zasl(queue_max_zone_append_sectors(q));
+
+ if (ns->subsys->zasl)
+ return ns->subsys->zasl < zasl ? false : true;
+
+ ns->subsys->zasl = zasl;
+ return true;
+}
+
+
+static int nvmet_bdev_validate_zns_zones_cb(struct blk_zone *z,
+ unsigned int idx, void *data)
+{
+ if (z->type == BLK_ZONE_TYPE_CONVENTIONAL)
+ return -EOPNOTSUPP;
+ return 0;
+}
+
+static bool nvmet_bdev_has_conv_zones(struct block_device *bdev)
+{
+ int ret;
+
+ if (bdev->bd_disk->queue->conv_zones_bitmap)
+ return true;
+
+ ret = blkdev_report_zones(bdev, 0, blkdev_nr_zones(bdev->bd_disk),
+ nvmet_bdev_validate_zns_zones_cb, NULL);
+
+ return ret < 0 ? true : false;
+}
+
+bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
+{
+ if (nvmet_bdev_has_conv_zones(ns->bdev))
+ return false;
+
+ /*
+ * For ZBC and ZAC devices, writes into sequential zones must be aligned
+ * to the device physical block size. So use this value as the logical
+ * block size to avoid errors.
+ */
+ ns->blksize_shift = blksize_bits(bdev_physical_block_size(ns->bdev));
+
+ if (!nvmet_zns_update_zasl(ns))
+ return false;
+
+ return !(get_capacity(ns->bdev->bd_disk) &
+ (bdev_zone_sectors(ns->bdev) - 1));
+}
+
+/*
+ * ZNS related Admin and I/O command handlers.
+ */
+void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
+{
+ u8 zasl = req->sq->ctrl->subsys->zasl;
+ struct nvmet_ctrl *ctrl = req->sq->ctrl;
+ struct nvme_id_ctrl_zns *id;
+ u16 status;
+
+ id = kzalloc(sizeof(*id), GFP_KERNEL);
+ if (!id) {
+ status = NVME_SC_INTERNAL;
+ goto out;
+ }
+
+ if (ctrl->ops->get_mdts)
+ id->zasl = min_t(u8, ctrl->ops->get_mdts(ctrl), zasl);
+ else
+ id->zasl = zasl;
+
+ status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
+
+ kfree(id);
+out:
+ nvmet_req_complete(req, status);
+}
+
+void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
+{
+ struct nvme_id_ns_zns *id_zns;
+ u16 status = NVME_SC_SUCCESS;
+ u64 zsze;
+
+ if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
+ req->error_loc = offsetof(struct nvme_identify, nsid);
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto out;
+ }
+
+ id_zns = kzalloc(sizeof(*id_zns), GFP_KERNEL);
+ if (!id_zns) {
+ status = NVME_SC_INTERNAL;
+ goto out;
+ }
+
+ req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+ if (!req->ns) {
+ status = NVME_SC_INTERNAL;
+ goto done;
+ }
+
+ if (!bdev_is_zoned(req->ns->bdev)) {
+ req->error_loc = offsetof(struct nvme_identify, nsid);
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto done;
+ }
+
+ nvmet_ns_revalidate(req->ns);
+ zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
+ req->ns->blksize_shift;
+ id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
+ id_zns->mor = cpu_to_le32(bdev_max_open_zones(req->ns->bdev));
+ id_zns->mar = cpu_to_le32(bdev_max_active_zones(req->ns->bdev));
+
+done:
+ status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
+ kfree(id_zns);
+out:
+ nvmet_req_complete(req, status);
+}
+
+struct nvmet_report_zone_data {
+ struct nvmet_ns *ns;
+ struct nvme_zone_report *rz;
+};
+
+static int nvmet_bdev_report_zone_cb(struct blk_zone *z, unsigned int idx,
+ void *data)
+{
+ struct nvmet_report_zone_data *report_zone_data = data;
+ struct nvme_zone_descriptor *entries = report_zone_data->rz->entries;
+ struct nvmet_ns *ns = report_zone_data->ns;
+
+ entries[idx].zcap = nvmet_sect_to_lba(ns, z->capacity);
+ entries[idx].zslba = nvmet_sect_to_lba(ns, z->start);
+ entries[idx].wp = nvmet_sect_to_lba(ns, z->wp);
+ entries[idx].za = z->reset ? 1 << 2 : 0;
+ entries[idx].zt = z->type;
+ entries[idx].zs = z->cond << 4;
+
+ return 0;
+}
+
+void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
+{
+ sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
+ u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
+ struct nvmet_report_zone_data data = { .ns = req->ns };
+ unsigned int nr_zones;
+ int reported_zones;
+ u16 status;
+
+ nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
+ sizeof(struct nvme_zone_descriptor);
+
+ status = nvmet_bdev_zns_checks(req);
+ if (status)
+ goto out;
+
+ data.rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY | __GFP_ZERO);
+ if (!data.rz) {
+ status = NVME_SC_INTERNAL;
+ goto out;
+ }
+
+ reported_zones = blkdev_report_zones(req->ns->bdev, sect, nr_zones,
+ nvmet_bdev_report_zone_cb,
+ &data);
+ if (reported_zones < 0) {
+ status = NVME_SC_INTERNAL;
+ goto out_free_report_zones;
+ }
+
+ data.rz->nr_zones = cpu_to_le64(reported_zones);
+
+ status = nvmet_copy_to_sgl(req, 0, data.rz, bufsize);
+
+out_free_report_zones:
+ kvfree(data.rz);
+out:
+ nvmet_req_complete(req, status);
+}
+
+void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
+{
+ sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zms.slba);
+ sector_t nr_sect = bdev_zone_sectors(req->ns->bdev);
+ u16 status = NVME_SC_SUCCESS;
+ enum req_opf op;
+ int ret;
+
+ if (req->cmd->zms.select_all)
+ nr_sect = get_capacity(req->ns->bdev->bd_disk);
+
+ switch (req->cmd->zms.zsa) {
+ case NVME_ZONE_OPEN:
+ op = REQ_OP_ZONE_OPEN;
+ break;
+ case NVME_ZONE_CLOSE:
+ op = REQ_OP_ZONE_CLOSE;
+ break;
+ case NVME_ZONE_FINISH:
+ op = REQ_OP_ZONE_FINISH;
+ break;
+ case NVME_ZONE_RESET:
+ op = REQ_OP_ZONE_RESET;
+ break;
+ default:
+ status = NVME_SC_INVALID_FIELD;
+ goto out;
+ }
+
+ ret = blkdev_zone_mgmt(req->ns->bdev, op, sect, nr_sect, GFP_KERNEL);
+ if (ret)
+ status = NVME_SC_INTERNAL;
+out:
+ nvmet_req_complete(req, status);
+}
+
+void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
+{
+ sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
+ struct request_queue *q = req->ns->bdev->bd_disk->queue;
+ unsigned int max_sects = queue_max_zone_append_sectors(q);
+ u16 status = NVME_SC_SUCCESS;
+ unsigned int total_len = 0;
+ struct scatterlist *sg;
+ int ret = 0, sg_cnt;
+ struct bio *bio;
+
+ if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
+ return;
+
+ if (!req->sg_cnt) {
+ nvmet_req_complete(req, 0);
+ return;
+ }
+
+ if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
+ bio = &req->b.inline_bio;
+ bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
+ } else {
+ bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
+ }
+
+ bio_set_dev(bio, req->ns->bdev);
+ bio->bi_iter.bi_sector = sect;
+ bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
+ if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
+ bio->bi_opf |= REQ_FUA;
+
+ for_each_sg(req->sg, sg, req->sg_cnt, sg_cnt) {
+ struct page *p = sg_page(sg);
+ unsigned int l = sg->length;
+ unsigned int o = sg->offset;
+ bool same_page = false;
+
+ ret = bio_add_hw_page(q, bio, p, l, o, max_sects, &same_page);
+ if (ret != sg->length) {
+ status = NVME_SC_INTERNAL;
+ goto out_bio_put;
+ }
+ if (same_page)
+ put_page(p);
+
+ total_len += sg->length;
+ }
+
+ if (total_len != nvmet_rw_data_len(req)) {
+ status = NVME_SC_INTERNAL | NVME_SC_DNR;
+ goto out_bio_put;
+ }
+
+ ret = submit_bio_wait(bio);
+ req->cqe->result.u64 = nvmet_sect_to_lba(req->ns,
+ bio->bi_iter.bi_sector);
+
+out_bio_put:
+ if (bio != &req->b.inline_bio)
+ bio_put(bio);
+ nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
+}
--
2.22.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: damien.lemoal, sagi, Chaitanya Kulkarni, hch
NVMe TP 4053 – Zoned Namespaces (ZNS) allows host software to
communicate with a non-volatile memory subsystem using zones for
NVMe protocol based controllers. NVMeOF already support the ZNS NVMe
Protocol compliant devices on the target in the passthru mode. There
are Generic zoned block devices like Shingled Magnetic Recording (SMR)
HDDs which are not based on the NVMe protocol.
This patch adds ZNS backend to support the ZBDs for NVMeOF target.
This support includes implementing the new command set NVME_CSI_ZNS,
adding different command handlers for ZNS command set such as
NVMe Identify Controller, NVMe Identify Namespace, NVMe Zone Append,
NVMe Zone Management Send and NVMe Zone Management Receive.
With new command set identifier we also update the target command effects
logs to reflect the ZNS compliant commands.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/Makefile | 1 +
drivers/nvme/target/admin-cmd.c | 26 +++
drivers/nvme/target/core.c | 1 +
drivers/nvme/target/io-cmd-bdev.c | 33 ++-
drivers/nvme/target/nvmet.h | 38 ++++
drivers/nvme/target/zns.c | 342 ++++++++++++++++++++++++++++++
6 files changed, 433 insertions(+), 8 deletions(-)
create mode 100644 drivers/nvme/target/zns.c
diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
index ebf91fc4c72e..9837e580fa7e 100644
--- a/drivers/nvme/target/Makefile
+++ b/drivers/nvme/target/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_NVME_TARGET_TCP) += nvmet-tcp.o
nvmet-y += core.o configfs.o admin-cmd.o fabrics-cmd.o \
discovery.o io-cmd-file.o io-cmd-bdev.o
nvmet-$(CONFIG_NVME_TARGET_PASSTHRU) += passthru.o
+nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o
nvme-loop-y += loop.o
nvmet-rdma-y += rdma.o
nvmet-fc-y += fc.o
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index f4c0f3aca485..6f5279b15aa6 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -192,6 +192,15 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
break;
+ case NVME_CSI_ZNS:
+ if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
+ u32 *iocs = log->iocs;
+
+ iocs[nvme_cmd_zone_append] = cpu_to_le32(1 << 0);
+ iocs[nvme_cmd_zone_mgmt_send] = cpu_to_le32(1 << 0);
+ iocs[nvme_cmd_zone_mgmt_recv] = cpu_to_le32(1 << 0);
+ }
+ break;
default:
status = NVME_SC_INVALID_LOG_PAGE;
break;
@@ -614,6 +623,7 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
static void nvmet_execute_identify_desclist(struct nvmet_req *req)
{
+ u16 nvme_cis_zns = NVME_CSI_ZNS;
u16 status = 0;
off_t off = 0;
@@ -638,6 +648,14 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
if (status)
goto out;
}
+ if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
+ if (req->ns->csi == NVME_CSI_ZNS)
+ status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI,
+ NVME_NIDT_CSI_LEN,
+ &nvme_cis_zns, &off);
+ if (status)
+ goto out;
+ }
if (sg_zero_buffer(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE - off,
off) != NVME_IDENTIFY_DATA_SIZE - off)
@@ -655,8 +673,16 @@ static void nvmet_execute_identify(struct nvmet_req *req)
switch (req->cmd->identify.cns) {
case NVME_ID_CNS_NS:
return nvmet_execute_identify_ns(req);
+ case NVME_ID_CNS_CS_NS:
+ if (req->cmd->identify.csi == NVME_CSI_ZNS)
+ return nvmet_execute_identify_cns_cs_ns(req);
+ break;
case NVME_ID_CNS_CTRL:
return nvmet_execute_identify_ctrl(req);
+ case NVME_ID_CNS_CS_CTRL:
+ if (req->cmd->identify.csi == NVME_CSI_ZNS)
+ return nvmet_execute_identify_cns_cs_ctrl(req);
+ break;
case NVME_ID_CNS_NS_ACTIVE_LIST:
return nvmet_execute_identify_nslist(req);
case NVME_ID_CNS_NS_DESC_LIST:
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 672e4009f8d6..17a99c7134dc 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1107,6 +1107,7 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
static inline bool nvmet_cc_css_check(u8 cc_css)
{
switch (cc_css <<= NVME_CC_CSS_SHIFT) {
+ case NVME_CC_CSS_CSI:
case NVME_CC_CSS_NVM:
return true;
default:
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 23095bdfce06..6178ef643962 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -63,6 +63,14 @@ static void nvmet_bdev_ns_enable_integrity(struct nvmet_ns *ns)
}
}
+void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
+{
+ if (ns->bdev) {
+ blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
+ ns->bdev = NULL;
+ }
+}
+
int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
{
int ret;
@@ -86,15 +94,15 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY_T10))
nvmet_bdev_ns_enable_integrity(ns);
- return 0;
-}
-
-void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
-{
- if (ns->bdev) {
- blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
- ns->bdev = NULL;
+ if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && bdev_is_zoned(ns->bdev)) {
+ if (!nvmet_bdev_zns_enable(ns)) {
+ nvmet_bdev_ns_disable(ns);
+ return -EINVAL;
+ }
+ ns->csi = NVME_CSI_ZNS;
}
+
+ return 0;
}
void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
@@ -448,6 +456,15 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
case nvme_cmd_write_zeroes:
req->execute = nvmet_bdev_execute_write_zeroes;
return 0;
+ case nvme_cmd_zone_append:
+ req->execute = nvmet_bdev_execute_zone_append;
+ return 0;
+ case nvme_cmd_zone_mgmt_recv:
+ req->execute = nvmet_bdev_execute_zone_mgmt_recv;
+ return 0;
+ case nvme_cmd_zone_mgmt_send:
+ req->execute = nvmet_bdev_execute_zone_mgmt_send;
+ return 0;
default:
pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
req->sq->qid);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 476b3cd91c65..7361665585a2 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -252,6 +252,10 @@ struct nvmet_subsys {
unsigned int admin_timeout;
unsigned int io_timeout;
#endif /* CONFIG_NVME_TARGET_PASSTHRU */
+
+#ifdef CONFIG_BLK_DEV_ZONED
+ u8 zasl;
+#endif /* CONFIG_BLK_DEV_ZONED */
};
static inline struct nvmet_subsys *to_subsys(struct config_item *item)
@@ -614,4 +618,38 @@ static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
}
+#ifdef CONFIG_BLK_DEV_ZONED
+bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
+void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req);
+void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req);
+void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req);
+void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req);
+void nvmet_bdev_execute_zone_append(struct nvmet_req *req);
+#else /* CONFIG_BLK_DEV_ZONED */
+static inline bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
+{
+ return false;
+}
+static inline void
+nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
+{
+}
+static inline void
+nvmet_bdev_execute_zone_append(struct nvmet_req *req)
+{
+}
+#endif /* CONFIG_BLK_DEV_ZONED */
+
#endif /* _NVMET_H */
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
new file mode 100644
index 000000000000..3981baa647b2
--- /dev/null
+++ b/drivers/nvme/target/zns.c
@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * NVMe ZNS-ZBD command implementation.
+ * Copyright (c) 2020-2021 HGST, a Western Digital Company.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/nvme.h>
+#include <linux/blkdev.h>
+#include "nvmet.h"
+
+/*
+ * We set the Memory Page Size Minimum (MPSMIN) for target controller to 0
+ * which gets added by 12 in the nvme_enable_ctrl() which results in 2^12 = 4k
+ * as page_shift value. When calculating the ZASL use shift by 12.
+ */
+#define NVMET_MPSMIN_SHIFT 12
+
+static u16 nvmet_bdev_zns_checks(struct nvmet_req *req)
+{
+ u16 status = NVME_SC_SUCCESS;
+
+ if (!bdev_is_zoned(req->ns->bdev)) {
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto out;
+ }
+
+ if (req->cmd->zmr.zra != NVME_ZRA_ZONE_REPORT) {
+ status = NVME_SC_INVALID_FIELD;
+ goto out;
+ }
+
+ if (req->cmd->zmr.zrasf != NVME_ZRASF_ZONE_REPORT_ALL) {
+ status = NVME_SC_INVALID_FIELD;
+ goto out;
+ }
+
+ if (req->cmd->zmr.pr != NVME_REPORT_ZONE_PARTIAL)
+ status = NVME_SC_INVALID_FIELD;
+
+out:
+ return status;
+}
+
+/*
+ * ZNS related command implementation and helpers.
+ */
+
+static inline u8 nvmet_zasl(unsigned int zone_append_sects)
+{
+ /*
+ * Zone Append Size Limit is the value experessed in the units
+ * of minimum memory page size (i.e. 12) and is reported power of 2.
+ */
+ return ilog2((zone_append_sects << 9) >> NVMET_MPSMIN_SHIFT);
+}
+
+static inline bool nvmet_zns_update_zasl(struct nvmet_ns *ns)
+{
+ struct request_queue *q = ns->bdev->bd_disk->queue;
+ u8 zasl = nvmet_zasl(queue_max_zone_append_sectors(q));
+
+ if (ns->subsys->zasl)
+ return ns->subsys->zasl < zasl ? false : true;
+
+ ns->subsys->zasl = zasl;
+ return true;
+}
+
+
+static int nvmet_bdev_validate_zns_zones_cb(struct blk_zone *z,
+ unsigned int idx, void *data)
+{
+ if (z->type == BLK_ZONE_TYPE_CONVENTIONAL)
+ return -EOPNOTSUPP;
+ return 0;
+}
+
+static bool nvmet_bdev_has_conv_zones(struct block_device *bdev)
+{
+ int ret;
+
+ if (bdev->bd_disk->queue->conv_zones_bitmap)
+ return true;
+
+ ret = blkdev_report_zones(bdev, 0, blkdev_nr_zones(bdev->bd_disk),
+ nvmet_bdev_validate_zns_zones_cb, NULL);
+
+ return ret < 0 ? true : false;
+}
+
+bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
+{
+ if (nvmet_bdev_has_conv_zones(ns->bdev))
+ return false;
+
+ /*
+ * For ZBC and ZAC devices, writes into sequential zones must be aligned
+ * to the device physical block size. So use this value as the logical
+ * block size to avoid errors.
+ */
+ ns->blksize_shift = blksize_bits(bdev_physical_block_size(ns->bdev));
+
+ if (!nvmet_zns_update_zasl(ns))
+ return false;
+
+ return !(get_capacity(ns->bdev->bd_disk) &
+ (bdev_zone_sectors(ns->bdev) - 1));
+}
+
+/*
+ * ZNS related Admin and I/O command handlers.
+ */
+void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
+{
+ u8 zasl = req->sq->ctrl->subsys->zasl;
+ struct nvmet_ctrl *ctrl = req->sq->ctrl;
+ struct nvme_id_ctrl_zns *id;
+ u16 status;
+
+ id = kzalloc(sizeof(*id), GFP_KERNEL);
+ if (!id) {
+ status = NVME_SC_INTERNAL;
+ goto out;
+ }
+
+ if (ctrl->ops->get_mdts)
+ id->zasl = min_t(u8, ctrl->ops->get_mdts(ctrl), zasl);
+ else
+ id->zasl = zasl;
+
+ status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
+
+ kfree(id);
+out:
+ nvmet_req_complete(req, status);
+}
+
+void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
+{
+ struct nvme_id_ns_zns *id_zns;
+ u16 status = NVME_SC_SUCCESS;
+ u64 zsze;
+
+ if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
+ req->error_loc = offsetof(struct nvme_identify, nsid);
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto out;
+ }
+
+ id_zns = kzalloc(sizeof(*id_zns), GFP_KERNEL);
+ if (!id_zns) {
+ status = NVME_SC_INTERNAL;
+ goto out;
+ }
+
+ req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+ if (!req->ns) {
+ status = NVME_SC_INTERNAL;
+ goto done;
+ }
+
+ if (!bdev_is_zoned(req->ns->bdev)) {
+ req->error_loc = offsetof(struct nvme_identify, nsid);
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto done;
+ }
+
+ nvmet_ns_revalidate(req->ns);
+ zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
+ req->ns->blksize_shift;
+ id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
+ id_zns->mor = cpu_to_le32(bdev_max_open_zones(req->ns->bdev));
+ id_zns->mar = cpu_to_le32(bdev_max_active_zones(req->ns->bdev));
+
+done:
+ status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
+ kfree(id_zns);
+out:
+ nvmet_req_complete(req, status);
+}
+
+struct nvmet_report_zone_data {
+ struct nvmet_ns *ns;
+ struct nvme_zone_report *rz;
+};
+
+static int nvmet_bdev_report_zone_cb(struct blk_zone *z, unsigned int idx,
+ void *data)
+{
+ struct nvmet_report_zone_data *report_zone_data = data;
+ struct nvme_zone_descriptor *entries = report_zone_data->rz->entries;
+ struct nvmet_ns *ns = report_zone_data->ns;
+
+ entries[idx].zcap = nvmet_sect_to_lba(ns, z->capacity);
+ entries[idx].zslba = nvmet_sect_to_lba(ns, z->start);
+ entries[idx].wp = nvmet_sect_to_lba(ns, z->wp);
+ entries[idx].za = z->reset ? 1 << 2 : 0;
+ entries[idx].zt = z->type;
+ entries[idx].zs = z->cond << 4;
+
+ return 0;
+}
+
+void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
+{
+ sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
+ u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
+ struct nvmet_report_zone_data data = { .ns = req->ns };
+ unsigned int nr_zones;
+ int reported_zones;
+ u16 status;
+
+ nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
+ sizeof(struct nvme_zone_descriptor);
+
+ status = nvmet_bdev_zns_checks(req);
+ if (status)
+ goto out;
+
+ data.rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY | __GFP_ZERO);
+ if (!data.rz) {
+ status = NVME_SC_INTERNAL;
+ goto out;
+ }
+
+ reported_zones = blkdev_report_zones(req->ns->bdev, sect, nr_zones,
+ nvmet_bdev_report_zone_cb,
+ &data);
+ if (reported_zones < 0) {
+ status = NVME_SC_INTERNAL;
+ goto out_free_report_zones;
+ }
+
+ data.rz->nr_zones = cpu_to_le64(reported_zones);
+
+ status = nvmet_copy_to_sgl(req, 0, data.rz, bufsize);
+
+out_free_report_zones:
+ kvfree(data.rz);
+out:
+ nvmet_req_complete(req, status);
+}
+
+void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
+{
+ sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zms.slba);
+ sector_t nr_sect = bdev_zone_sectors(req->ns->bdev);
+ u16 status = NVME_SC_SUCCESS;
+ enum req_opf op;
+ int ret;
+
+ if (req->cmd->zms.select_all)
+ nr_sect = get_capacity(req->ns->bdev->bd_disk);
+
+ switch (req->cmd->zms.zsa) {
+ case NVME_ZONE_OPEN:
+ op = REQ_OP_ZONE_OPEN;
+ break;
+ case NVME_ZONE_CLOSE:
+ op = REQ_OP_ZONE_CLOSE;
+ break;
+ case NVME_ZONE_FINISH:
+ op = REQ_OP_ZONE_FINISH;
+ break;
+ case NVME_ZONE_RESET:
+ op = REQ_OP_ZONE_RESET;
+ break;
+ default:
+ status = NVME_SC_INVALID_FIELD;
+ goto out;
+ }
+
+ ret = blkdev_zone_mgmt(req->ns->bdev, op, sect, nr_sect, GFP_KERNEL);
+ if (ret)
+ status = NVME_SC_INTERNAL;
+out:
+ nvmet_req_complete(req, status);
+}
+
+void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
+{
+ sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
+ struct request_queue *q = req->ns->bdev->bd_disk->queue;
+ unsigned int max_sects = queue_max_zone_append_sectors(q);
+ u16 status = NVME_SC_SUCCESS;
+ unsigned int total_len = 0;
+ struct scatterlist *sg;
+ int ret = 0, sg_cnt;
+ struct bio *bio;
+
+ if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
+ return;
+
+ if (!req->sg_cnt) {
+ nvmet_req_complete(req, 0);
+ return;
+ }
+
+ if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
+ bio = &req->b.inline_bio;
+ bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
+ } else {
+ bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
+ }
+
+ bio_set_dev(bio, req->ns->bdev);
+ bio->bi_iter.bi_sector = sect;
+ bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
+ if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
+ bio->bi_opf |= REQ_FUA;
+
+ for_each_sg(req->sg, sg, req->sg_cnt, sg_cnt) {
+ struct page *p = sg_page(sg);
+ unsigned int l = sg->length;
+ unsigned int o = sg->offset;
+ bool same_page = false;
+
+ ret = bio_add_hw_page(q, bio, p, l, o, max_sects, &same_page);
+ if (ret != sg->length) {
+ status = NVME_SC_INTERNAL;
+ goto out_bio_put;
+ }
+ if (same_page)
+ put_page(p);
+
+ total_len += sg->length;
+ }
+
+ if (total_len != nvmet_rw_data_len(req)) {
+ status = NVME_SC_INTERNAL | NVME_SC_DNR;
+ goto out_bio_put;
+ }
+
+ ret = submit_bio_wait(bio);
+ req->cqe->result.u64 = nvmet_sect_to_lba(req->ns,
+ bio->bi_iter.bi_sector);
+
+out_bio_put:
+ if (bio != &req->b.inline_bio)
+ bio_put(bio);
+ nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
+}
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 5/6] nvmet: add bio get helper for different backends
2020-12-15 6:02 ` Chaitanya Kulkarni
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: sagi, hch, damien.lemoal, Chaitanya Kulkarni
With the addition of the zns backend now we have three different
backends with inline bio optimization. That leads to having
duplicate code in for allocating or initializing the bio in all
three backends: generic bdev, passsthru, and generic zns.
Add a helper function to reduce the duplicate code such that helper
function accepts the bi_end_io callback which gets initialize for the
non-inline bio_alloc() case. This is due to the special case needed for
the passthru backend non-inline bio allocation bio_alloc() where we set
the bio->bi_end_io = bio_put, having this parameter avoids the extra
branch in the passthru fast path. For rest of the backends, we set the
same bi_end_io callback for inline and non-inline cases, that is for
generic bdev we set to nvmet_bio_done() and for generic zns we set to
NULL.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/io-cmd-bdev.c | 7 +------
drivers/nvme/target/nvmet.h | 16 ++++++++++++++++
drivers/nvme/target/passthru.c | 8 +-------
drivers/nvme/target/zns.c | 8 +-------
4 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 6178ef643962..72746e29cb0d 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -266,12 +266,7 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req)
sector = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
- if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
- bio = &req->b.inline_bio;
- bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
- } else {
- bio = bio_alloc(GFP_KERNEL, min(sg_cnt, BIO_MAX_PAGES));
- }
+ bio = nvmet_req_bio_get(req, NULL);
bio_set_dev(bio, req->ns->bdev);
bio->bi_iter.bi_sector = sector;
bio->bi_private = req;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 7361665585a2..3fc84f79cce1 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -652,4 +652,20 @@ nvmet_bdev_execute_zone_append(struct nvmet_req *req)
}
#endif /* CONFIG_BLK_DEV_ZONED */
+static inline struct bio *nvmet_req_bio_get(struct nvmet_req *req,
+ bio_end_io_t *bi_end_io)
+{
+ struct bio *bio;
+
+ if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
+ bio = &req->b.inline_bio;
+ bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
+ return bio;
+ }
+
+ bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
+ bio->bi_end_io = bi_end_io;
+ return bio;
+}
+
#endif /* _NVMET_H */
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index b9776fc8f08f..54f765b566ee 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -194,13 +194,7 @@ static int nvmet_passthru_map_sg(struct nvmet_req *req, struct request *rq)
if (req->sg_cnt > BIO_MAX_PAGES)
return -EINVAL;
- if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
- bio = &req->p.inline_bio;
- bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
- } else {
- bio = bio_alloc(GFP_KERNEL, min(req->sg_cnt, BIO_MAX_PAGES));
- bio->bi_end_io = bio_put;
- }
+ bio = nvmet_req_bio_get(req, bio_put);
bio->bi_opf = req_op(rq);
for_each_sg(req->sg, sg, req->sg_cnt, i) {
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 3981baa647b2..8bafab98d076 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -296,13 +296,7 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
return;
}
- if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
- bio = &req->b.inline_bio;
- bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
- } else {
- bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
- }
-
+ bio = nvmet_req_bio_get(req, NULL);
bio_set_dev(bio, req->ns->bdev);
bio->bi_iter.bi_sector = sect;
bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
--
2.22.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 5/6] nvmet: add bio get helper for different backends
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: damien.lemoal, sagi, Chaitanya Kulkarni, hch
With the addition of the zns backend now we have three different
backends with inline bio optimization. That leads to having
duplicate code in for allocating or initializing the bio in all
three backends: generic bdev, passsthru, and generic zns.
Add a helper function to reduce the duplicate code such that helper
function accepts the bi_end_io callback which gets initialize for the
non-inline bio_alloc() case. This is due to the special case needed for
the passthru backend non-inline bio allocation bio_alloc() where we set
the bio->bi_end_io = bio_put, having this parameter avoids the extra
branch in the passthru fast path. For rest of the backends, we set the
same bi_end_io callback for inline and non-inline cases, that is for
generic bdev we set to nvmet_bio_done() and for generic zns we set to
NULL.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/io-cmd-bdev.c | 7 +------
drivers/nvme/target/nvmet.h | 16 ++++++++++++++++
drivers/nvme/target/passthru.c | 8 +-------
drivers/nvme/target/zns.c | 8 +-------
4 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 6178ef643962..72746e29cb0d 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -266,12 +266,7 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req)
sector = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
- if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
- bio = &req->b.inline_bio;
- bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
- } else {
- bio = bio_alloc(GFP_KERNEL, min(sg_cnt, BIO_MAX_PAGES));
- }
+ bio = nvmet_req_bio_get(req, NULL);
bio_set_dev(bio, req->ns->bdev);
bio->bi_iter.bi_sector = sector;
bio->bi_private = req;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 7361665585a2..3fc84f79cce1 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -652,4 +652,20 @@ nvmet_bdev_execute_zone_append(struct nvmet_req *req)
}
#endif /* CONFIG_BLK_DEV_ZONED */
+static inline struct bio *nvmet_req_bio_get(struct nvmet_req *req,
+ bio_end_io_t *bi_end_io)
+{
+ struct bio *bio;
+
+ if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
+ bio = &req->b.inline_bio;
+ bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
+ return bio;
+ }
+
+ bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
+ bio->bi_end_io = bi_end_io;
+ return bio;
+}
+
#endif /* _NVMET_H */
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index b9776fc8f08f..54f765b566ee 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -194,13 +194,7 @@ static int nvmet_passthru_map_sg(struct nvmet_req *req, struct request *rq)
if (req->sg_cnt > BIO_MAX_PAGES)
return -EINVAL;
- if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
- bio = &req->p.inline_bio;
- bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
- } else {
- bio = bio_alloc(GFP_KERNEL, min(req->sg_cnt, BIO_MAX_PAGES));
- bio->bi_end_io = bio_put;
- }
+ bio = nvmet_req_bio_get(req, bio_put);
bio->bi_opf = req_op(rq);
for_each_sg(req->sg, sg, req->sg_cnt, i) {
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 3981baa647b2..8bafab98d076 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -296,13 +296,7 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
return;
}
- if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
- bio = &req->b.inline_bio;
- bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
- } else {
- bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
- }
-
+ bio = nvmet_req_bio_get(req, NULL);
bio_set_dev(bio, req->ns->bdev);
bio->bi_iter.bi_sector = sect;
bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 6/6] nvmet: add bio put helper for different backends
2020-12-15 6:02 ` Chaitanya Kulkarni
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: sagi, hch, damien.lemoal, Chaitanya Kulkarni
With the addition of zns backend now we have three different
backends with inline bio optimization. That leads to having duplicate
code in for freeing the bio in all three backends: generic bdev,
passsthru and generic zns.
Add a helper function to avoid the duplicate code and update the
respective backends.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/io-cmd-bdev.c | 3 +--
drivers/nvme/target/nvmet.h | 6 ++++++
drivers/nvme/target/passthru.c | 3 +--
drivers/nvme/target/zns.c | 3 +--
4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 72746e29cb0d..6ffd84a620e7 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -172,8 +172,7 @@ static void nvmet_bio_done(struct bio *bio)
struct nvmet_req *req = bio->bi_private;
nvmet_req_complete(req, blk_to_nvme_status(req, bio->bi_status));
- if (bio != &req->b.inline_bio)
- bio_put(bio);
+ nvmet_req_bio_put(req, bio);
}
#ifdef CONFIG_BLK_DEV_INTEGRITY
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 3fc84f79cce1..e770086b5890 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -668,4 +668,10 @@ static inline struct bio *nvmet_req_bio_get(struct nvmet_req *req,
return bio;
}
+static inline void nvmet_req_bio_put(struct nvmet_req *req, struct bio *bio)
+{
+ if (bio != &req->b.inline_bio)
+ bio_put(bio);
+}
+
#endif /* _NVMET_H */
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 54f765b566ee..a4a73d64c603 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -200,8 +200,7 @@ static int nvmet_passthru_map_sg(struct nvmet_req *req, struct request *rq)
for_each_sg(req->sg, sg, req->sg_cnt, i) {
if (bio_add_pc_page(rq->q, bio, sg_page(sg), sg->length,
sg->offset) < sg->length) {
- if (bio != &req->p.inline_bio)
- bio_put(bio);
+ nvmet_req_bio_put(req, bio);
return -EINVAL;
}
}
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 8bafab98d076..d6a8310cf672 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -330,7 +330,6 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
bio->bi_iter.bi_sector);
out_bio_put:
- if (bio != &req->b.inline_bio)
- bio_put(bio);
+ nvmet_req_bio_put(req, bio);
nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
}
--
2.22.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V7 6/6] nvmet: add bio put helper for different backends
@ 2020-12-15 6:03 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 6:03 UTC (permalink / raw)
To: linux-block, linux-nvme; +Cc: damien.lemoal, sagi, Chaitanya Kulkarni, hch
With the addition of zns backend now we have three different
backends with inline bio optimization. That leads to having duplicate
code in for freeing the bio in all three backends: generic bdev,
passsthru and generic zns.
Add a helper function to avoid the duplicate code and update the
respective backends.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/io-cmd-bdev.c | 3 +--
drivers/nvme/target/nvmet.h | 6 ++++++
drivers/nvme/target/passthru.c | 3 +--
drivers/nvme/target/zns.c | 3 +--
4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 72746e29cb0d..6ffd84a620e7 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -172,8 +172,7 @@ static void nvmet_bio_done(struct bio *bio)
struct nvmet_req *req = bio->bi_private;
nvmet_req_complete(req, blk_to_nvme_status(req, bio->bi_status));
- if (bio != &req->b.inline_bio)
- bio_put(bio);
+ nvmet_req_bio_put(req, bio);
}
#ifdef CONFIG_BLK_DEV_INTEGRITY
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 3fc84f79cce1..e770086b5890 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -668,4 +668,10 @@ static inline struct bio *nvmet_req_bio_get(struct nvmet_req *req,
return bio;
}
+static inline void nvmet_req_bio_put(struct nvmet_req *req, struct bio *bio)
+{
+ if (bio != &req->b.inline_bio)
+ bio_put(bio);
+}
+
#endif /* _NVMET_H */
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 54f765b566ee..a4a73d64c603 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -200,8 +200,7 @@ static int nvmet_passthru_map_sg(struct nvmet_req *req, struct request *rq)
for_each_sg(req->sg, sg, req->sg_cnt, i) {
if (bio_add_pc_page(rq->q, bio, sg_page(sg), sg->length,
sg->offset) < sg->length) {
- if (bio != &req->p.inline_bio)
- bio_put(bio);
+ nvmet_req_bio_put(req, bio);
return -EINVAL;
}
}
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 8bafab98d076..d6a8310cf672 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -330,7 +330,6 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
bio->bi_iter.bi_sector);
out_bio_put:
- if (bio != &req->b.inline_bio)
- bio_put(bio);
+ nvmet_req_bio_put(req, bio);
nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
}
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
2020-12-15 6:03 ` Chaitanya Kulkarni
@ 2020-12-15 9:06 ` Damien Le Moal
-1 siblings, 0 replies; 26+ messages in thread
From: Damien Le Moal @ 2020-12-15 9:06 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block, linux-nvme; +Cc: sagi, hch
On 2020/12/15 15:03, Chaitanya Kulkarni wrote:
> NVMe TP 4053 – Zoned Namespaces (ZNS) allows host software to
> communicate with a non-volatile memory subsystem using zones for
> NVMe protocol based controllers. NVMeOF already support the ZNS NVMe
> Protocol compliant devices on the target in the passthru mode. There
> are Generic zoned block devices like Shingled Magnetic Recording (SMR)
> HDDs which are not based on the NVMe protocol.
>
> This patch adds ZNS backend to support the ZBDs for NVMeOF target.
>
> This support includes implementing the new command set NVME_CSI_ZNS,
> adding different command handlers for ZNS command set such as
> NVMe Identify Controller, NVMe Identify Namespace, NVMe Zone Append,
> NVMe Zone Management Send and NVMe Zone Management Receive.
>
> With new command set identifier we also update the target command effects
> logs to reflect the ZNS compliant commands.
>
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
> drivers/nvme/target/Makefile | 1 +
> drivers/nvme/target/admin-cmd.c | 26 +++
> drivers/nvme/target/core.c | 1 +
> drivers/nvme/target/io-cmd-bdev.c | 33 ++-
> drivers/nvme/target/nvmet.h | 38 ++++
> drivers/nvme/target/zns.c | 342 ++++++++++++++++++++++++++++++
> 6 files changed, 433 insertions(+), 8 deletions(-)
> create mode 100644 drivers/nvme/target/zns.c
>
> diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
> index ebf91fc4c72e..9837e580fa7e 100644
> --- a/drivers/nvme/target/Makefile
> +++ b/drivers/nvme/target/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_NVME_TARGET_TCP) += nvmet-tcp.o
> nvmet-y += core.o configfs.o admin-cmd.o fabrics-cmd.o \
> discovery.o io-cmd-file.o io-cmd-bdev.o
> nvmet-$(CONFIG_NVME_TARGET_PASSTHRU) += passthru.o
> +nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o
> nvme-loop-y += loop.o
> nvmet-rdma-y += rdma.o
> nvmet-fc-y += fc.o
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index f4c0f3aca485..6f5279b15aa6 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -192,6 +192,15 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
> log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
> log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
> break;
> + case NVME_CSI_ZNS:
> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
> + u32 *iocs = log->iocs;
> +
> + iocs[nvme_cmd_zone_append] = cpu_to_le32(1 << 0);
> + iocs[nvme_cmd_zone_mgmt_send] = cpu_to_le32(1 << 0);
> + iocs[nvme_cmd_zone_mgmt_recv] = cpu_to_le32(1 << 0);
> + }
> + break;
> default:
> status = NVME_SC_INVALID_LOG_PAGE;
> break;
> @@ -614,6 +623,7 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
>
> static void nvmet_execute_identify_desclist(struct nvmet_req *req)
> {
> + u16 nvme_cis_zns = NVME_CSI_ZNS;
> u16 status = 0;
> off_t off = 0;
>
> @@ -638,6 +648,14 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
> if (status)
> goto out;
> }
> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
> + if (req->ns->csi == NVME_CSI_ZNS)
> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI,
> + NVME_NIDT_CSI_LEN,
> + &nvme_cis_zns, &off);
> + if (status)
> + goto out;
> + }
>
> if (sg_zero_buffer(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE - off,
> off) != NVME_IDENTIFY_DATA_SIZE - off)
> @@ -655,8 +673,16 @@ static void nvmet_execute_identify(struct nvmet_req *req)
> switch (req->cmd->identify.cns) {
> case NVME_ID_CNS_NS:
> return nvmet_execute_identify_ns(req);
> + case NVME_ID_CNS_CS_NS:
> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
> + return nvmet_execute_identify_cns_cs_ns(req);
> + break;
> case NVME_ID_CNS_CTRL:
> return nvmet_execute_identify_ctrl(req);
> + case NVME_ID_CNS_CS_CTRL:
> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
> + return nvmet_execute_identify_cns_cs_ctrl(req);
> + break;
> case NVME_ID_CNS_NS_ACTIVE_LIST:
> return nvmet_execute_identify_nslist(req);
> case NVME_ID_CNS_NS_DESC_LIST:
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 672e4009f8d6..17a99c7134dc 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1107,6 +1107,7 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
> static inline bool nvmet_cc_css_check(u8 cc_css)
> {
> switch (cc_css <<= NVME_CC_CSS_SHIFT) {
> + case NVME_CC_CSS_CSI:
> case NVME_CC_CSS_NVM:
> return true;
> default:
> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
> index 23095bdfce06..6178ef643962 100644
> --- a/drivers/nvme/target/io-cmd-bdev.c
> +++ b/drivers/nvme/target/io-cmd-bdev.c
> @@ -63,6 +63,14 @@ static void nvmet_bdev_ns_enable_integrity(struct nvmet_ns *ns)
> }
> }
>
> +void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
> +{
> + if (ns->bdev) {
> + blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
> + ns->bdev = NULL;
> + }
> +}
> +
> int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
> {
> int ret;
> @@ -86,15 +94,15 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
> if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY_T10))
> nvmet_bdev_ns_enable_integrity(ns);
>
> - return 0;
> -}
> -
> -void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
> -{
> - if (ns->bdev) {
> - blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
> - ns->bdev = NULL;
> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && bdev_is_zoned(ns->bdev)) {
> + if (!nvmet_bdev_zns_enable(ns)) {
> + nvmet_bdev_ns_disable(ns);
> + return -EINVAL;
> + }
> + ns->csi = NVME_CSI_ZNS;
> }
> +
> + return 0;
> }
>
> void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
> @@ -448,6 +456,15 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
> case nvme_cmd_write_zeroes:
> req->execute = nvmet_bdev_execute_write_zeroes;
> return 0;
> + case nvme_cmd_zone_append:
> + req->execute = nvmet_bdev_execute_zone_append;
> + return 0;
> + case nvme_cmd_zone_mgmt_recv:
> + req->execute = nvmet_bdev_execute_zone_mgmt_recv;
> + return 0;
> + case nvme_cmd_zone_mgmt_send:
> + req->execute = nvmet_bdev_execute_zone_mgmt_send;
> + return 0;
> default:
> pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
> req->sq->qid);
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 476b3cd91c65..7361665585a2 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -252,6 +252,10 @@ struct nvmet_subsys {
> unsigned int admin_timeout;
> unsigned int io_timeout;
> #endif /* CONFIG_NVME_TARGET_PASSTHRU */
> +
> +#ifdef CONFIG_BLK_DEV_ZONED
> + u8 zasl;
> +#endif /* CONFIG_BLK_DEV_ZONED */
> };
>
> static inline struct nvmet_subsys *to_subsys(struct config_item *item)
> @@ -614,4 +618,38 @@ static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
> return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
> }
>
> +#ifdef CONFIG_BLK_DEV_ZONED
> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req);
> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req);
> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req);
> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req);
> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req);
> +#else /* CONFIG_BLK_DEV_ZONED */
> +static inline bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
> +{
> + return false;
> +}
> +static inline void
> +nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_bdev_execute_zone_append(struct nvmet_req *req)
> +{
> +}
> +#endif /* CONFIG_BLK_DEV_ZONED */
> +
> #endif /* _NVMET_H */
> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
> new file mode 100644
> index 000000000000..3981baa647b2
> --- /dev/null
> +++ b/drivers/nvme/target/zns.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * NVMe ZNS-ZBD command implementation.
> + * Copyright (c) 2020-2021 HGST, a Western Digital Company.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/nvme.h>
> +#include <linux/blkdev.h>
> +#include "nvmet.h"
> +
> +/*
> + * We set the Memory Page Size Minimum (MPSMIN) for target controller to 0
> + * which gets added by 12 in the nvme_enable_ctrl() which results in 2^12 = 4k
> + * as page_shift value. When calculating the ZASL use shift by 12.
> + */
> +#define NVMET_MPSMIN_SHIFT 12
> +
> +static u16 nvmet_bdev_zns_checks(struct nvmet_req *req)
> +{
> + u16 status = NVME_SC_SUCCESS;
> +
> + if (!bdev_is_zoned(req->ns->bdev)) {
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto out;
> + }
> +
> + if (req->cmd->zmr.zra != NVME_ZRA_ZONE_REPORT) {
> + status = NVME_SC_INVALID_FIELD;
> + goto out;
> + }
> +
> + if (req->cmd->zmr.zrasf != NVME_ZRASF_ZONE_REPORT_ALL) {
> + status = NVME_SC_INVALID_FIELD;
> + goto out;
> + }
> +
> + if (req->cmd->zmr.pr != NVME_REPORT_ZONE_PARTIAL)
> + status = NVME_SC_INVALID_FIELD;
> +
> +out:
> + return status;
> +}
> +
> +/*
> + * ZNS related command implementation and helpers.
> + */
> +
> +static inline u8 nvmet_zasl(unsigned int zone_append_sects)
> +{
> + /*
> + * Zone Append Size Limit is the value experessed in the units
> + * of minimum memory page size (i.e. 12) and is reported power of 2.
> + */
> + return ilog2((zone_append_sects << 9) >> NVMET_MPSMIN_SHIFT);
> +}
> +
> +static inline bool nvmet_zns_update_zasl(struct nvmet_ns *ns)
> +{
> + struct request_queue *q = ns->bdev->bd_disk->queue;
> + u8 zasl = nvmet_zasl(queue_max_zone_append_sectors(q));
> +
> + if (ns->subsys->zasl)
> + return ns->subsys->zasl < zasl ? false : true;
> +
> + ns->subsys->zasl = zasl;
> + return true;
> +}
> +
> +
> +static int nvmet_bdev_validate_zns_zones_cb(struct blk_zone *z,
> + unsigned int idx, void *data)
> +{
> + if (z->type == BLK_ZONE_TYPE_CONVENTIONAL)
> + return -EOPNOTSUPP;
> + return 0;
> +}
> +
> +static bool nvmet_bdev_has_conv_zones(struct block_device *bdev)
> +{
> + int ret;
> +
> + if (bdev->bd_disk->queue->conv_zones_bitmap)
> + return true;
> +
> + ret = blkdev_report_zones(bdev, 0, blkdev_nr_zones(bdev->bd_disk),
> + nvmet_bdev_validate_zns_zones_cb, NULL);
> +
> + return ret < 0 ? true : false;
> +}
> +
> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
> +{
> + if (nvmet_bdev_has_conv_zones(ns->bdev))
> + return false;
> +
> + /*
> + * For ZBC and ZAC devices, writes into sequential zones must be aligned
> + * to the device physical block size. So use this value as the logical
> + * block size to avoid errors.
> + */
> + ns->blksize_shift = blksize_bits(bdev_physical_block_size(ns->bdev));
> +
> + if (!nvmet_zns_update_zasl(ns))
> + return false;
> +
> + return !(get_capacity(ns->bdev->bd_disk) &
> + (bdev_zone_sectors(ns->bdev) - 1));
> +}
> +
> +/*
> + * ZNS related Admin and I/O command handlers.
> + */
> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
> +{
> + u8 zasl = req->sq->ctrl->subsys->zasl;
> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
> + struct nvme_id_ctrl_zns *id;
> + u16 status;
> +
> + id = kzalloc(sizeof(*id), GFP_KERNEL);
> + if (!id) {
> + status = NVME_SC_INTERNAL;
> + goto out;
> + }
> +
> + if (ctrl->ops->get_mdts)
> + id->zasl = min_t(u8, ctrl->ops->get_mdts(ctrl), zasl);
> + else
> + id->zasl = zasl;
> +
> + status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
> +
> + kfree(id);
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
> +{
> + struct nvme_id_ns_zns *id_zns;
> + u16 status = NVME_SC_SUCCESS;
> + u64 zsze;
> +
> + if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
> + req->error_loc = offsetof(struct nvme_identify, nsid);
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto out;
> + }
> +
> + id_zns = kzalloc(sizeof(*id_zns), GFP_KERNEL);
> + if (!id_zns) {
> + status = NVME_SC_INTERNAL;
> + goto out;
> + }
> +
> + req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
> + if (!req->ns) {
> + status = NVME_SC_INTERNAL;
> + goto done;
> + }
> +
> + if (!bdev_is_zoned(req->ns->bdev)) {
> + req->error_loc = offsetof(struct nvme_identify, nsid);
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto done;
> + }
> +
> + nvmet_ns_revalidate(req->ns);
> + zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
> + req->ns->blksize_shift;
> + id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
> + id_zns->mor = cpu_to_le32(bdev_max_open_zones(req->ns->bdev));
> + id_zns->mar = cpu_to_le32(bdev_max_active_zones(req->ns->bdev));
> +
> +done:
> + status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
> + kfree(id_zns);
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +struct nvmet_report_zone_data {
> + struct nvmet_ns *ns;
> + struct nvme_zone_report *rz;
> +};
> +
> +static int nvmet_bdev_report_zone_cb(struct blk_zone *z, unsigned int idx,
> + void *data)
> +{
> + struct nvmet_report_zone_data *report_zone_data = data;
> + struct nvme_zone_descriptor *entries = report_zone_data->rz->entries;
> + struct nvmet_ns *ns = report_zone_data->ns;
> +
> + entries[idx].zcap = nvmet_sect_to_lba(ns, z->capacity);
> + entries[idx].zslba = nvmet_sect_to_lba(ns, z->start);
> + entries[idx].wp = nvmet_sect_to_lba(ns, z->wp);
> + entries[idx].za = z->reset ? 1 << 2 : 0;
> + entries[idx].zt = z->type;
> + entries[idx].zs = z->cond << 4;
> +
> + return 0;
> +}
> +
> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
> +{
> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
> + struct nvmet_report_zone_data data = { .ns = req->ns };
> + unsigned int nr_zones;
> + int reported_zones;
> + u16 status;
> +
> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
> + sizeof(struct nvme_zone_descriptor);
You could move this right before the vmalloc call since it is first used there.
> +
> + status = nvmet_bdev_zns_checks(req);
> + if (status)
> + goto out;
> +
> + data.rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY | __GFP_ZERO);
> + if (!data.rz) {
> + status = NVME_SC_INTERNAL;
> + goto out;
> + }
> +
> + reported_zones = blkdev_report_zones(req->ns->bdev, sect, nr_zones,
> + nvmet_bdev_report_zone_cb,
> + &data);
> + if (reported_zones < 0) {
> + status = NVME_SC_INTERNAL;
> + goto out_free_report_zones;
> + }
> +
> + data.rz->nr_zones = cpu_to_le64(reported_zones);
> +
> + status = nvmet_copy_to_sgl(req, 0, data.rz, bufsize);
> +
> +out_free_report_zones:
> + kvfree(data.rz);
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
> +{
> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zms.slba);
> + sector_t nr_sect = bdev_zone_sectors(req->ns->bdev);
> + u16 status = NVME_SC_SUCCESS;
> + enum req_opf op;
> + int ret;
> +
> + if (req->cmd->zms.select_all)
> + nr_sect = get_capacity(req->ns->bdev->bd_disk);
> +
> + switch (req->cmd->zms.zsa) {
> + case NVME_ZONE_OPEN:
> + op = REQ_OP_ZONE_OPEN;
> + break;
> + case NVME_ZONE_CLOSE:
> + op = REQ_OP_ZONE_CLOSE;
> + break;
> + case NVME_ZONE_FINISH:
> + op = REQ_OP_ZONE_FINISH;
> + break;
> + case NVME_ZONE_RESET:
> + op = REQ_OP_ZONE_RESET;
> + break;
> + default:
> + status = NVME_SC_INVALID_FIELD;
> + goto out;
> + }
> +
> + ret = blkdev_zone_mgmt(req->ns->bdev, op, sect, nr_sect, GFP_KERNEL);
> + if (ret)
> + status = NVME_SC_INTERNAL;
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
> +{
> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
> + struct request_queue *q = req->ns->bdev->bd_disk->queue;
> + unsigned int max_sects = queue_max_zone_append_sectors(q);
> + u16 status = NVME_SC_SUCCESS;
> + unsigned int total_len = 0;
> + struct scatterlist *sg;
> + int ret = 0, sg_cnt;
> + struct bio *bio;
> +
> + if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
> + return;
> +
> + if (!req->sg_cnt) {
> + nvmet_req_complete(req, 0);
> + return;
> + }
> +
> + if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
> + bio = &req->b.inline_bio;
> + bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
> + } else {
> + bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
> + }
> +
> + bio_set_dev(bio, req->ns->bdev);
> + bio->bi_iter.bi_sector = sect;
> + bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
> + if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
> + bio->bi_opf |= REQ_FUA;
> +
> + for_each_sg(req->sg, sg, req->sg_cnt, sg_cnt) {
> + struct page *p = sg_page(sg);
> + unsigned int l = sg->length;
> + unsigned int o = sg->offset;
> + bool same_page = false;
> +
> + ret = bio_add_hw_page(q, bio, p, l, o, max_sects, &same_page);
> + if (ret != sg->length) {
> + status = NVME_SC_INTERNAL;
> + goto out_bio_put;
> + }
> + if (same_page)
> + put_page(p);
> +
> + total_len += sg->length;
> + }
> +
> + if (total_len != nvmet_rw_data_len(req)) {
> + status = NVME_SC_INTERNAL | NVME_SC_DNR;
> + goto out_bio_put;
> + }
> +
> + ret = submit_bio_wait(bio);
> + req->cqe->result.u64 = nvmet_sect_to_lba(req->ns,
> + bio->bi_iter.bi_sector);
> +
> +out_bio_put:
> + if (bio != &req->b.inline_bio)
> + bio_put(bio);
> + nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
> +}
Wasn't it the plan to integrate this function into the normal read-write
proccessing ? I remember Christoph commenting on that.
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
@ 2020-12-15 9:06 ` Damien Le Moal
0 siblings, 0 replies; 26+ messages in thread
From: Damien Le Moal @ 2020-12-15 9:06 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block, linux-nvme; +Cc: sagi, hch
On 2020/12/15 15:03, Chaitanya Kulkarni wrote:
> NVMe TP 4053 – Zoned Namespaces (ZNS) allows host software to
> communicate with a non-volatile memory subsystem using zones for
> NVMe protocol based controllers. NVMeOF already support the ZNS NVMe
> Protocol compliant devices on the target in the passthru mode. There
> are Generic zoned block devices like Shingled Magnetic Recording (SMR)
> HDDs which are not based on the NVMe protocol.
>
> This patch adds ZNS backend to support the ZBDs for NVMeOF target.
>
> This support includes implementing the new command set NVME_CSI_ZNS,
> adding different command handlers for ZNS command set such as
> NVMe Identify Controller, NVMe Identify Namespace, NVMe Zone Append,
> NVMe Zone Management Send and NVMe Zone Management Receive.
>
> With new command set identifier we also update the target command effects
> logs to reflect the ZNS compliant commands.
>
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
> drivers/nvme/target/Makefile | 1 +
> drivers/nvme/target/admin-cmd.c | 26 +++
> drivers/nvme/target/core.c | 1 +
> drivers/nvme/target/io-cmd-bdev.c | 33 ++-
> drivers/nvme/target/nvmet.h | 38 ++++
> drivers/nvme/target/zns.c | 342 ++++++++++++++++++++++++++++++
> 6 files changed, 433 insertions(+), 8 deletions(-)
> create mode 100644 drivers/nvme/target/zns.c
>
> diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
> index ebf91fc4c72e..9837e580fa7e 100644
> --- a/drivers/nvme/target/Makefile
> +++ b/drivers/nvme/target/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_NVME_TARGET_TCP) += nvmet-tcp.o
> nvmet-y += core.o configfs.o admin-cmd.o fabrics-cmd.o \
> discovery.o io-cmd-file.o io-cmd-bdev.o
> nvmet-$(CONFIG_NVME_TARGET_PASSTHRU) += passthru.o
> +nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o
> nvme-loop-y += loop.o
> nvmet-rdma-y += rdma.o
> nvmet-fc-y += fc.o
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index f4c0f3aca485..6f5279b15aa6 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -192,6 +192,15 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
> log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
> log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
> break;
> + case NVME_CSI_ZNS:
> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
> + u32 *iocs = log->iocs;
> +
> + iocs[nvme_cmd_zone_append] = cpu_to_le32(1 << 0);
> + iocs[nvme_cmd_zone_mgmt_send] = cpu_to_le32(1 << 0);
> + iocs[nvme_cmd_zone_mgmt_recv] = cpu_to_le32(1 << 0);
> + }
> + break;
> default:
> status = NVME_SC_INVALID_LOG_PAGE;
> break;
> @@ -614,6 +623,7 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
>
> static void nvmet_execute_identify_desclist(struct nvmet_req *req)
> {
> + u16 nvme_cis_zns = NVME_CSI_ZNS;
> u16 status = 0;
> off_t off = 0;
>
> @@ -638,6 +648,14 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
> if (status)
> goto out;
> }
> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
> + if (req->ns->csi == NVME_CSI_ZNS)
> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI,
> + NVME_NIDT_CSI_LEN,
> + &nvme_cis_zns, &off);
> + if (status)
> + goto out;
> + }
>
> if (sg_zero_buffer(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE - off,
> off) != NVME_IDENTIFY_DATA_SIZE - off)
> @@ -655,8 +673,16 @@ static void nvmet_execute_identify(struct nvmet_req *req)
> switch (req->cmd->identify.cns) {
> case NVME_ID_CNS_NS:
> return nvmet_execute_identify_ns(req);
> + case NVME_ID_CNS_CS_NS:
> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
> + return nvmet_execute_identify_cns_cs_ns(req);
> + break;
> case NVME_ID_CNS_CTRL:
> return nvmet_execute_identify_ctrl(req);
> + case NVME_ID_CNS_CS_CTRL:
> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
> + return nvmet_execute_identify_cns_cs_ctrl(req);
> + break;
> case NVME_ID_CNS_NS_ACTIVE_LIST:
> return nvmet_execute_identify_nslist(req);
> case NVME_ID_CNS_NS_DESC_LIST:
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 672e4009f8d6..17a99c7134dc 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1107,6 +1107,7 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
> static inline bool nvmet_cc_css_check(u8 cc_css)
> {
> switch (cc_css <<= NVME_CC_CSS_SHIFT) {
> + case NVME_CC_CSS_CSI:
> case NVME_CC_CSS_NVM:
> return true;
> default:
> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
> index 23095bdfce06..6178ef643962 100644
> --- a/drivers/nvme/target/io-cmd-bdev.c
> +++ b/drivers/nvme/target/io-cmd-bdev.c
> @@ -63,6 +63,14 @@ static void nvmet_bdev_ns_enable_integrity(struct nvmet_ns *ns)
> }
> }
>
> +void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
> +{
> + if (ns->bdev) {
> + blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
> + ns->bdev = NULL;
> + }
> +}
> +
> int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
> {
> int ret;
> @@ -86,15 +94,15 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
> if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY_T10))
> nvmet_bdev_ns_enable_integrity(ns);
>
> - return 0;
> -}
> -
> -void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
> -{
> - if (ns->bdev) {
> - blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
> - ns->bdev = NULL;
> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && bdev_is_zoned(ns->bdev)) {
> + if (!nvmet_bdev_zns_enable(ns)) {
> + nvmet_bdev_ns_disable(ns);
> + return -EINVAL;
> + }
> + ns->csi = NVME_CSI_ZNS;
> }
> +
> + return 0;
> }
>
> void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
> @@ -448,6 +456,15 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
> case nvme_cmd_write_zeroes:
> req->execute = nvmet_bdev_execute_write_zeroes;
> return 0;
> + case nvme_cmd_zone_append:
> + req->execute = nvmet_bdev_execute_zone_append;
> + return 0;
> + case nvme_cmd_zone_mgmt_recv:
> + req->execute = nvmet_bdev_execute_zone_mgmt_recv;
> + return 0;
> + case nvme_cmd_zone_mgmt_send:
> + req->execute = nvmet_bdev_execute_zone_mgmt_send;
> + return 0;
> default:
> pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
> req->sq->qid);
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 476b3cd91c65..7361665585a2 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -252,6 +252,10 @@ struct nvmet_subsys {
> unsigned int admin_timeout;
> unsigned int io_timeout;
> #endif /* CONFIG_NVME_TARGET_PASSTHRU */
> +
> +#ifdef CONFIG_BLK_DEV_ZONED
> + u8 zasl;
> +#endif /* CONFIG_BLK_DEV_ZONED */
> };
>
> static inline struct nvmet_subsys *to_subsys(struct config_item *item)
> @@ -614,4 +618,38 @@ static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
> return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
> }
>
> +#ifdef CONFIG_BLK_DEV_ZONED
> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req);
> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req);
> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req);
> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req);
> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req);
> +#else /* CONFIG_BLK_DEV_ZONED */
> +static inline bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
> +{
> + return false;
> +}
> +static inline void
> +nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
> +{
> +}
> +static inline void
> +nvmet_bdev_execute_zone_append(struct nvmet_req *req)
> +{
> +}
> +#endif /* CONFIG_BLK_DEV_ZONED */
> +
> #endif /* _NVMET_H */
> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
> new file mode 100644
> index 000000000000..3981baa647b2
> --- /dev/null
> +++ b/drivers/nvme/target/zns.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * NVMe ZNS-ZBD command implementation.
> + * Copyright (c) 2020-2021 HGST, a Western Digital Company.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/nvme.h>
> +#include <linux/blkdev.h>
> +#include "nvmet.h"
> +
> +/*
> + * We set the Memory Page Size Minimum (MPSMIN) for target controller to 0
> + * which gets added by 12 in the nvme_enable_ctrl() which results in 2^12 = 4k
> + * as page_shift value. When calculating the ZASL use shift by 12.
> + */
> +#define NVMET_MPSMIN_SHIFT 12
> +
> +static u16 nvmet_bdev_zns_checks(struct nvmet_req *req)
> +{
> + u16 status = NVME_SC_SUCCESS;
> +
> + if (!bdev_is_zoned(req->ns->bdev)) {
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto out;
> + }
> +
> + if (req->cmd->zmr.zra != NVME_ZRA_ZONE_REPORT) {
> + status = NVME_SC_INVALID_FIELD;
> + goto out;
> + }
> +
> + if (req->cmd->zmr.zrasf != NVME_ZRASF_ZONE_REPORT_ALL) {
> + status = NVME_SC_INVALID_FIELD;
> + goto out;
> + }
> +
> + if (req->cmd->zmr.pr != NVME_REPORT_ZONE_PARTIAL)
> + status = NVME_SC_INVALID_FIELD;
> +
> +out:
> + return status;
> +}
> +
> +/*
> + * ZNS related command implementation and helpers.
> + */
> +
> +static inline u8 nvmet_zasl(unsigned int zone_append_sects)
> +{
> + /*
> + * Zone Append Size Limit is the value experessed in the units
> + * of minimum memory page size (i.e. 12) and is reported power of 2.
> + */
> + return ilog2((zone_append_sects << 9) >> NVMET_MPSMIN_SHIFT);
> +}
> +
> +static inline bool nvmet_zns_update_zasl(struct nvmet_ns *ns)
> +{
> + struct request_queue *q = ns->bdev->bd_disk->queue;
> + u8 zasl = nvmet_zasl(queue_max_zone_append_sectors(q));
> +
> + if (ns->subsys->zasl)
> + return ns->subsys->zasl < zasl ? false : true;
> +
> + ns->subsys->zasl = zasl;
> + return true;
> +}
> +
> +
> +static int nvmet_bdev_validate_zns_zones_cb(struct blk_zone *z,
> + unsigned int idx, void *data)
> +{
> + if (z->type == BLK_ZONE_TYPE_CONVENTIONAL)
> + return -EOPNOTSUPP;
> + return 0;
> +}
> +
> +static bool nvmet_bdev_has_conv_zones(struct block_device *bdev)
> +{
> + int ret;
> +
> + if (bdev->bd_disk->queue->conv_zones_bitmap)
> + return true;
> +
> + ret = blkdev_report_zones(bdev, 0, blkdev_nr_zones(bdev->bd_disk),
> + nvmet_bdev_validate_zns_zones_cb, NULL);
> +
> + return ret < 0 ? true : false;
> +}
> +
> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
> +{
> + if (nvmet_bdev_has_conv_zones(ns->bdev))
> + return false;
> +
> + /*
> + * For ZBC and ZAC devices, writes into sequential zones must be aligned
> + * to the device physical block size. So use this value as the logical
> + * block size to avoid errors.
> + */
> + ns->blksize_shift = blksize_bits(bdev_physical_block_size(ns->bdev));
> +
> + if (!nvmet_zns_update_zasl(ns))
> + return false;
> +
> + return !(get_capacity(ns->bdev->bd_disk) &
> + (bdev_zone_sectors(ns->bdev) - 1));
> +}
> +
> +/*
> + * ZNS related Admin and I/O command handlers.
> + */
> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
> +{
> + u8 zasl = req->sq->ctrl->subsys->zasl;
> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
> + struct nvme_id_ctrl_zns *id;
> + u16 status;
> +
> + id = kzalloc(sizeof(*id), GFP_KERNEL);
> + if (!id) {
> + status = NVME_SC_INTERNAL;
> + goto out;
> + }
> +
> + if (ctrl->ops->get_mdts)
> + id->zasl = min_t(u8, ctrl->ops->get_mdts(ctrl), zasl);
> + else
> + id->zasl = zasl;
> +
> + status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
> +
> + kfree(id);
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
> +{
> + struct nvme_id_ns_zns *id_zns;
> + u16 status = NVME_SC_SUCCESS;
> + u64 zsze;
> +
> + if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
> + req->error_loc = offsetof(struct nvme_identify, nsid);
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto out;
> + }
> +
> + id_zns = kzalloc(sizeof(*id_zns), GFP_KERNEL);
> + if (!id_zns) {
> + status = NVME_SC_INTERNAL;
> + goto out;
> + }
> +
> + req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
> + if (!req->ns) {
> + status = NVME_SC_INTERNAL;
> + goto done;
> + }
> +
> + if (!bdev_is_zoned(req->ns->bdev)) {
> + req->error_loc = offsetof(struct nvme_identify, nsid);
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto done;
> + }
> +
> + nvmet_ns_revalidate(req->ns);
> + zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
> + req->ns->blksize_shift;
> + id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
> + id_zns->mor = cpu_to_le32(bdev_max_open_zones(req->ns->bdev));
> + id_zns->mar = cpu_to_le32(bdev_max_active_zones(req->ns->bdev));
> +
> +done:
> + status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
> + kfree(id_zns);
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +struct nvmet_report_zone_data {
> + struct nvmet_ns *ns;
> + struct nvme_zone_report *rz;
> +};
> +
> +static int nvmet_bdev_report_zone_cb(struct blk_zone *z, unsigned int idx,
> + void *data)
> +{
> + struct nvmet_report_zone_data *report_zone_data = data;
> + struct nvme_zone_descriptor *entries = report_zone_data->rz->entries;
> + struct nvmet_ns *ns = report_zone_data->ns;
> +
> + entries[idx].zcap = nvmet_sect_to_lba(ns, z->capacity);
> + entries[idx].zslba = nvmet_sect_to_lba(ns, z->start);
> + entries[idx].wp = nvmet_sect_to_lba(ns, z->wp);
> + entries[idx].za = z->reset ? 1 << 2 : 0;
> + entries[idx].zt = z->type;
> + entries[idx].zs = z->cond << 4;
> +
> + return 0;
> +}
> +
> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
> +{
> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
> + struct nvmet_report_zone_data data = { .ns = req->ns };
> + unsigned int nr_zones;
> + int reported_zones;
> + u16 status;
> +
> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
> + sizeof(struct nvme_zone_descriptor);
You could move this right before the vmalloc call since it is first used there.
> +
> + status = nvmet_bdev_zns_checks(req);
> + if (status)
> + goto out;
> +
> + data.rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY | __GFP_ZERO);
> + if (!data.rz) {
> + status = NVME_SC_INTERNAL;
> + goto out;
> + }
> +
> + reported_zones = blkdev_report_zones(req->ns->bdev, sect, nr_zones,
> + nvmet_bdev_report_zone_cb,
> + &data);
> + if (reported_zones < 0) {
> + status = NVME_SC_INTERNAL;
> + goto out_free_report_zones;
> + }
> +
> + data.rz->nr_zones = cpu_to_le64(reported_zones);
> +
> + status = nvmet_copy_to_sgl(req, 0, data.rz, bufsize);
> +
> +out_free_report_zones:
> + kvfree(data.rz);
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
> +{
> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zms.slba);
> + sector_t nr_sect = bdev_zone_sectors(req->ns->bdev);
> + u16 status = NVME_SC_SUCCESS;
> + enum req_opf op;
> + int ret;
> +
> + if (req->cmd->zms.select_all)
> + nr_sect = get_capacity(req->ns->bdev->bd_disk);
> +
> + switch (req->cmd->zms.zsa) {
> + case NVME_ZONE_OPEN:
> + op = REQ_OP_ZONE_OPEN;
> + break;
> + case NVME_ZONE_CLOSE:
> + op = REQ_OP_ZONE_CLOSE;
> + break;
> + case NVME_ZONE_FINISH:
> + op = REQ_OP_ZONE_FINISH;
> + break;
> + case NVME_ZONE_RESET:
> + op = REQ_OP_ZONE_RESET;
> + break;
> + default:
> + status = NVME_SC_INVALID_FIELD;
> + goto out;
> + }
> +
> + ret = blkdev_zone_mgmt(req->ns->bdev, op, sect, nr_sect, GFP_KERNEL);
> + if (ret)
> + status = NVME_SC_INTERNAL;
> +out:
> + nvmet_req_complete(req, status);
> +}
> +
> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
> +{
> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
> + struct request_queue *q = req->ns->bdev->bd_disk->queue;
> + unsigned int max_sects = queue_max_zone_append_sectors(q);
> + u16 status = NVME_SC_SUCCESS;
> + unsigned int total_len = 0;
> + struct scatterlist *sg;
> + int ret = 0, sg_cnt;
> + struct bio *bio;
> +
> + if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
> + return;
> +
> + if (!req->sg_cnt) {
> + nvmet_req_complete(req, 0);
> + return;
> + }
> +
> + if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
> + bio = &req->b.inline_bio;
> + bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
> + } else {
> + bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
> + }
> +
> + bio_set_dev(bio, req->ns->bdev);
> + bio->bi_iter.bi_sector = sect;
> + bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
> + if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
> + bio->bi_opf |= REQ_FUA;
> +
> + for_each_sg(req->sg, sg, req->sg_cnt, sg_cnt) {
> + struct page *p = sg_page(sg);
> + unsigned int l = sg->length;
> + unsigned int o = sg->offset;
> + bool same_page = false;
> +
> + ret = bio_add_hw_page(q, bio, p, l, o, max_sects, &same_page);
> + if (ret != sg->length) {
> + status = NVME_SC_INTERNAL;
> + goto out_bio_put;
> + }
> + if (same_page)
> + put_page(p);
> +
> + total_len += sg->length;
> + }
> +
> + if (total_len != nvmet_rw_data_len(req)) {
> + status = NVME_SC_INTERNAL | NVME_SC_DNR;
> + goto out_bio_put;
> + }
> +
> + ret = submit_bio_wait(bio);
> + req->cqe->result.u64 = nvmet_sect_to_lba(req->ns,
> + bio->bi_iter.bi_sector);
> +
> +out_bio_put:
> + if (bio != &req->b.inline_bio)
> + bio_put(bio);
> + nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
> +}
Wasn't it the plan to integrate this function into the normal read-write
proccessing ? I remember Christoph commenting on that.
>
--
Damien Le Moal
Western Digital Research
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
2020-12-15 9:06 ` Damien Le Moal
@ 2020-12-15 23:06 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 23:06 UTC (permalink / raw)
To: Damien Le Moal, linux-block, linux-nvme; +Cc: sagi, hch
On 12/15/20 01:06, Damien Le Moal wrote:
> On 2020/12/15 15:03, Chaitanya Kulkarni wrote:
>> NVMe TP 4053 – Zoned Namespaces (ZNS) allows host software to
>> communicate with a non-volatile memory subsystem using zones for
>> NVMe protocol based controllers. NVMeOF already support the ZNS NVMe
>> Protocol compliant devices on the target in the passthru mode. There
>> are Generic zoned block devices like Shingled Magnetic Recording (SMR)
>> HDDs which are not based on the NVMe protocol.
>>
>> This patch adds ZNS backend to support the ZBDs for NVMeOF target.
>>
>> This support includes implementing the new command set NVME_CSI_ZNS,
>> adding different command handlers for ZNS command set such as
>> NVMe Identify Controller, NVMe Identify Namespace, NVMe Zone Append,
>> NVMe Zone Management Send and NVMe Zone Management Receive.
>>
>> With new command set identifier we also update the target command effects
>> logs to reflect the ZNS compliant commands.
>>
>> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
>> ---
>> drivers/nvme/target/Makefile | 1 +
>> drivers/nvme/target/admin-cmd.c | 26 +++
>> drivers/nvme/target/core.c | 1 +
>> drivers/nvme/target/io-cmd-bdev.c | 33 ++-
>> drivers/nvme/target/nvmet.h | 38 ++++
>> drivers/nvme/target/zns.c | 342 ++++++++++++++++++++++++++++++
>> 6 files changed, 433 insertions(+), 8 deletions(-)
>> create mode 100644 drivers/nvme/target/zns.c
>>
>> diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
>> index ebf91fc4c72e..9837e580fa7e 100644
>> --- a/drivers/nvme/target/Makefile
>> +++ b/drivers/nvme/target/Makefile
>> @@ -12,6 +12,7 @@ obj-$(CONFIG_NVME_TARGET_TCP) += nvmet-tcp.o
>> nvmet-y += core.o configfs.o admin-cmd.o fabrics-cmd.o \
>> discovery.o io-cmd-file.o io-cmd-bdev.o
>> nvmet-$(CONFIG_NVME_TARGET_PASSTHRU) += passthru.o
>> +nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o
>> nvme-loop-y += loop.o
>> nvmet-rdma-y += rdma.o
>> nvmet-fc-y += fc.o
>> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
>> index f4c0f3aca485..6f5279b15aa6 100644
>> --- a/drivers/nvme/target/admin-cmd.c
>> +++ b/drivers/nvme/target/admin-cmd.c
>> @@ -192,6 +192,15 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
>> log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
>> log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
>> break;
>> + case NVME_CSI_ZNS:
>> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
>> + u32 *iocs = log->iocs;
>> +
>> + iocs[nvme_cmd_zone_append] = cpu_to_le32(1 << 0);
>> + iocs[nvme_cmd_zone_mgmt_send] = cpu_to_le32(1 << 0);
>> + iocs[nvme_cmd_zone_mgmt_recv] = cpu_to_le32(1 << 0);
>> + }
>> + break;
>> default:
>> status = NVME_SC_INVALID_LOG_PAGE;
>> break;
>> @@ -614,6 +623,7 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
>>
>> static void nvmet_execute_identify_desclist(struct nvmet_req *req)
>> {
>> + u16 nvme_cis_zns = NVME_CSI_ZNS;
>> u16 status = 0;
>> off_t off = 0;
>>
>> @@ -638,6 +648,14 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
>> if (status)
>> goto out;
>> }
>> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
>> + if (req->ns->csi == NVME_CSI_ZNS)
>> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI,
>> + NVME_NIDT_CSI_LEN,
>> + &nvme_cis_zns, &off);
>> + if (status)
>> + goto out;
>> + }
>>
>> if (sg_zero_buffer(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE - off,
>> off) != NVME_IDENTIFY_DATA_SIZE - off)
>> @@ -655,8 +673,16 @@ static void nvmet_execute_identify(struct nvmet_req *req)
>> switch (req->cmd->identify.cns) {
>> case NVME_ID_CNS_NS:
>> return nvmet_execute_identify_ns(req);
>> + case NVME_ID_CNS_CS_NS:
>> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
>> + return nvmet_execute_identify_cns_cs_ns(req);
>> + break;
>> case NVME_ID_CNS_CTRL:
>> return nvmet_execute_identify_ctrl(req);
>> + case NVME_ID_CNS_CS_CTRL:
>> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
>> + return nvmet_execute_identify_cns_cs_ctrl(req);
>> + break;
>> case NVME_ID_CNS_NS_ACTIVE_LIST:
>> return nvmet_execute_identify_nslist(req);
>> case NVME_ID_CNS_NS_DESC_LIST:
>> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
>> index 672e4009f8d6..17a99c7134dc 100644
>> --- a/drivers/nvme/target/core.c
>> +++ b/drivers/nvme/target/core.c
>> @@ -1107,6 +1107,7 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
>> static inline bool nvmet_cc_css_check(u8 cc_css)
>> {
>> switch (cc_css <<= NVME_CC_CSS_SHIFT) {
>> + case NVME_CC_CSS_CSI:
>> case NVME_CC_CSS_NVM:
>> return true;
>> default:
>> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
>> index 23095bdfce06..6178ef643962 100644
>> --- a/drivers/nvme/target/io-cmd-bdev.c
>> +++ b/drivers/nvme/target/io-cmd-bdev.c
>> @@ -63,6 +63,14 @@ static void nvmet_bdev_ns_enable_integrity(struct nvmet_ns *ns)
>> }
>> }
>>
>> +void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
>> +{
>> + if (ns->bdev) {
>> + blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
>> + ns->bdev = NULL;
>> + }
>> +}
>> +
>> int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
>> {
>> int ret;
>> @@ -86,15 +94,15 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
>> if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY_T10))
>> nvmet_bdev_ns_enable_integrity(ns);
>>
>> - return 0;
>> -}
>> -
>> -void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
>> -{
>> - if (ns->bdev) {
>> - blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
>> - ns->bdev = NULL;
>> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && bdev_is_zoned(ns->bdev)) {
>> + if (!nvmet_bdev_zns_enable(ns)) {
>> + nvmet_bdev_ns_disable(ns);
>> + return -EINVAL;
>> + }
>> + ns->csi = NVME_CSI_ZNS;
>> }
>> +
>> + return 0;
>> }
>>
>> void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
>> @@ -448,6 +456,15 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
>> case nvme_cmd_write_zeroes:
>> req->execute = nvmet_bdev_execute_write_zeroes;
>> return 0;
>> + case nvme_cmd_zone_append:
>> + req->execute = nvmet_bdev_execute_zone_append;
>> + return 0;
>> + case nvme_cmd_zone_mgmt_recv:
>> + req->execute = nvmet_bdev_execute_zone_mgmt_recv;
>> + return 0;
>> + case nvme_cmd_zone_mgmt_send:
>> + req->execute = nvmet_bdev_execute_zone_mgmt_send;
>> + return 0;
>> default:
>> pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
>> req->sq->qid);
>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>> index 476b3cd91c65..7361665585a2 100644
>> --- a/drivers/nvme/target/nvmet.h
>> +++ b/drivers/nvme/target/nvmet.h
>> @@ -252,6 +252,10 @@ struct nvmet_subsys {
>> unsigned int admin_timeout;
>> unsigned int io_timeout;
>> #endif /* CONFIG_NVME_TARGET_PASSTHRU */
>> +
>> +#ifdef CONFIG_BLK_DEV_ZONED
>> + u8 zasl;
>> +#endif /* CONFIG_BLK_DEV_ZONED */
>> };
>>
>> static inline struct nvmet_subsys *to_subsys(struct config_item *item)
>> @@ -614,4 +618,38 @@ static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
>> return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
>> }
>>
>> +#ifdef CONFIG_BLK_DEV_ZONED
>> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
>> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req);
>> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req);
>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req);
>> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req);
>> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req);
>> +#else /* CONFIG_BLK_DEV_ZONED */
>> +static inline bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
>> +{
>> + return false;
>> +}
>> +static inline void
>> +nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_bdev_execute_zone_append(struct nvmet_req *req)
>> +{
>> +}
>> +#endif /* CONFIG_BLK_DEV_ZONED */
>> +
>> #endif /* _NVMET_H */
>> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
>> new file mode 100644
>> index 000000000000..3981baa647b2
>> --- /dev/null
>> +++ b/drivers/nvme/target/zns.c
>> @@ -0,0 +1,342 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * NVMe ZNS-ZBD command implementation.
>> + * Copyright (c) 2020-2021 HGST, a Western Digital Company.
>> + */
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +#include <linux/nvme.h>
>> +#include <linux/blkdev.h>
>> +#include "nvmet.h"
>> +
>> +/*
>> + * We set the Memory Page Size Minimum (MPSMIN) for target controller to 0
>> + * which gets added by 12 in the nvme_enable_ctrl() which results in 2^12 = 4k
>> + * as page_shift value. When calculating the ZASL use shift by 12.
>> + */
>> +#define NVMET_MPSMIN_SHIFT 12
>> +
>> +static u16 nvmet_bdev_zns_checks(struct nvmet_req *req)
>> +{
>> + u16 status = NVME_SC_SUCCESS;
>> +
>> + if (!bdev_is_zoned(req->ns->bdev)) {
>> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>> + goto out;
>> + }
>> +
>> + if (req->cmd->zmr.zra != NVME_ZRA_ZONE_REPORT) {
>> + status = NVME_SC_INVALID_FIELD;
>> + goto out;
>> + }
>> +
>> + if (req->cmd->zmr.zrasf != NVME_ZRASF_ZONE_REPORT_ALL) {
>> + status = NVME_SC_INVALID_FIELD;
>> + goto out;
>> + }
>> +
>> + if (req->cmd->zmr.pr != NVME_REPORT_ZONE_PARTIAL)
>> + status = NVME_SC_INVALID_FIELD;
>> +
>> +out:
>> + return status;
>> +}
>> +
>> +/*
>> + * ZNS related command implementation and helpers.
>> + */
>> +
>> +static inline u8 nvmet_zasl(unsigned int zone_append_sects)
>> +{
>> + /*
>> + * Zone Append Size Limit is the value experessed in the units
>> + * of minimum memory page size (i.e. 12) and is reported power of 2.
>> + */
>> + return ilog2((zone_append_sects << 9) >> NVMET_MPSMIN_SHIFT);
>> +}
>> +
>> +static inline bool nvmet_zns_update_zasl(struct nvmet_ns *ns)
>> +{
>> + struct request_queue *q = ns->bdev->bd_disk->queue;
>> + u8 zasl = nvmet_zasl(queue_max_zone_append_sectors(q));
>> +
>> + if (ns->subsys->zasl)
>> + return ns->subsys->zasl < zasl ? false : true;
>> +
>> + ns->subsys->zasl = zasl;
>> + return true;
>> +}
>> +
>> +
>> +static int nvmet_bdev_validate_zns_zones_cb(struct blk_zone *z,
>> + unsigned int idx, void *data)
>> +{
>> + if (z->type == BLK_ZONE_TYPE_CONVENTIONAL)
>> + return -EOPNOTSUPP;
>> + return 0;
>> +}
>> +
>> +static bool nvmet_bdev_has_conv_zones(struct block_device *bdev)
>> +{
>> + int ret;
>> +
>> + if (bdev->bd_disk->queue->conv_zones_bitmap)
>> + return true;
>> +
>> + ret = blkdev_report_zones(bdev, 0, blkdev_nr_zones(bdev->bd_disk),
>> + nvmet_bdev_validate_zns_zones_cb, NULL);
>> +
>> + return ret < 0 ? true : false;
>> +}
>> +
>> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
>> +{
>> + if (nvmet_bdev_has_conv_zones(ns->bdev))
>> + return false;
>> +
>> + /*
>> + * For ZBC and ZAC devices, writes into sequential zones must be aligned
>> + * to the device physical block size. So use this value as the logical
>> + * block size to avoid errors.
>> + */
>> + ns->blksize_shift = blksize_bits(bdev_physical_block_size(ns->bdev));
>> +
>> + if (!nvmet_zns_update_zasl(ns))
>> + return false;
>> +
>> + return !(get_capacity(ns->bdev->bd_disk) &
>> + (bdev_zone_sectors(ns->bdev) - 1));
>> +}
>> +
>> +/*
>> + * ZNS related Admin and I/O command handlers.
>> + */
>> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
>> +{
>> + u8 zasl = req->sq->ctrl->subsys->zasl;
>> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
>> + struct nvme_id_ctrl_zns *id;
>> + u16 status;
>> +
>> + id = kzalloc(sizeof(*id), GFP_KERNEL);
>> + if (!id) {
>> + status = NVME_SC_INTERNAL;
>> + goto out;
>> + }
>> +
>> + if (ctrl->ops->get_mdts)
>> + id->zasl = min_t(u8, ctrl->ops->get_mdts(ctrl), zasl);
>> + else
>> + id->zasl = zasl;
>> +
>> + status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
>> +
>> + kfree(id);
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
>> +{
>> + struct nvme_id_ns_zns *id_zns;
>> + u16 status = NVME_SC_SUCCESS;
>> + u64 zsze;
>> +
>> + if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
>> + req->error_loc = offsetof(struct nvme_identify, nsid);
>> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>> + goto out;
>> + }
>> +
>> + id_zns = kzalloc(sizeof(*id_zns), GFP_KERNEL);
>> + if (!id_zns) {
>> + status = NVME_SC_INTERNAL;
>> + goto out;
>> + }
>> +
>> + req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
>> + if (!req->ns) {
>> + status = NVME_SC_INTERNAL;
>> + goto done;
>> + }
>> +
>> + if (!bdev_is_zoned(req->ns->bdev)) {
>> + req->error_loc = offsetof(struct nvme_identify, nsid);
>> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>> + goto done;
>> + }
>> +
>> + nvmet_ns_revalidate(req->ns);
>> + zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
>> + req->ns->blksize_shift;
>> + id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
>> + id_zns->mor = cpu_to_le32(bdev_max_open_zones(req->ns->bdev));
>> + id_zns->mar = cpu_to_le32(bdev_max_active_zones(req->ns->bdev));
>> +
>> +done:
>> + status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
>> + kfree(id_zns);
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +struct nvmet_report_zone_data {
>> + struct nvmet_ns *ns;
>> + struct nvme_zone_report *rz;
>> +};
>> +
>> +static int nvmet_bdev_report_zone_cb(struct blk_zone *z, unsigned int idx,
>> + void *data)
>> +{
>> + struct nvmet_report_zone_data *report_zone_data = data;
>> + struct nvme_zone_descriptor *entries = report_zone_data->rz->entries;
>> + struct nvmet_ns *ns = report_zone_data->ns;
>> +
>> + entries[idx].zcap = nvmet_sect_to_lba(ns, z->capacity);
>> + entries[idx].zslba = nvmet_sect_to_lba(ns, z->start);
>> + entries[idx].wp = nvmet_sect_to_lba(ns, z->wp);
>> + entries[idx].za = z->reset ? 1 << 2 : 0;
>> + entries[idx].zt = z->type;
>> + entries[idx].zs = z->cond << 4;
>> +
>> + return 0;
>> +}
>> +
>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>> +{
>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>> + unsigned int nr_zones;
>> + int reported_zones;
>> + u16 status;
>> +
>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>> + sizeof(struct nvme_zone_descriptor);
> You could move this right before the vmalloc call since it is first used there.
There are only three lines between the this and the vmalloc, does that
really
going to make any difference ?
>> +
>> + status = nvmet_bdev_zns_checks(req);
>> + if (status)
>> + goto out;
>> +
>> + data.rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY | __GFP_ZERO);
>> + if (!data.rz) {
>> + status = NVME_SC_INTERNAL;
>> + goto out;
>> + }
>> +
>> + reported_zones = blkdev_report_zones(req->ns->bdev, sect, nr_zones,
>> + nvmet_bdev_report_zone_cb,
>> + &data);
>> + if (reported_zones < 0) {
>> + status = NVME_SC_INTERNAL;
>> + goto out_free_report_zones;
>> + }
>> +
>> + data.rz->nr_zones = cpu_to_le64(reported_zones);
>> +
>> + status = nvmet_copy_to_sgl(req, 0, data.rz, bufsize);
>> +
>> +out_free_report_zones:
>> + kvfree(data.rz);
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
>> +{
>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zms.slba);
>> + sector_t nr_sect = bdev_zone_sectors(req->ns->bdev);
>> + u16 status = NVME_SC_SUCCESS;
>> + enum req_opf op;
>> + int ret;
>> +
>> + if (req->cmd->zms.select_all)
>> + nr_sect = get_capacity(req->ns->bdev->bd_disk);
>> +
>> + switch (req->cmd->zms.zsa) {
>> + case NVME_ZONE_OPEN:
>> + op = REQ_OP_ZONE_OPEN;
>> + break;
>> + case NVME_ZONE_CLOSE:
>> + op = REQ_OP_ZONE_CLOSE;
>> + break;
>> + case NVME_ZONE_FINISH:
>> + op = REQ_OP_ZONE_FINISH;
>> + break;
>> + case NVME_ZONE_RESET:
>> + op = REQ_OP_ZONE_RESET;
>> + break;
>> + default:
>> + status = NVME_SC_INVALID_FIELD;
>> + goto out;
>> + }
>> +
>> + ret = blkdev_zone_mgmt(req->ns->bdev, op, sect, nr_sect, GFP_KERNEL);
>> + if (ret)
>> + status = NVME_SC_INTERNAL;
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
>> +{
>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
>> + struct request_queue *q = req->ns->bdev->bd_disk->queue;
>> + unsigned int max_sects = queue_max_zone_append_sectors(q);
>> + u16 status = NVME_SC_SUCCESS;
>> + unsigned int total_len = 0;
>> + struct scatterlist *sg;
>> + int ret = 0, sg_cnt;
>> + struct bio *bio;
>> +
>> + if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
>> + return;
>> +
>> + if (!req->sg_cnt) {
>> + nvmet_req_complete(req, 0);
>> + return;
>> + }
>> +
>> + if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
>> + bio = &req->b.inline_bio;
>> + bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
>> + } else {
>> + bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
>> + }
>> +
>> + bio_set_dev(bio, req->ns->bdev);
>> + bio->bi_iter.bi_sector = sect;
>> + bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
>> + if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
>> + bio->bi_opf |= REQ_FUA;
>> +
>> + for_each_sg(req->sg, sg, req->sg_cnt, sg_cnt) {
>> + struct page *p = sg_page(sg);
>> + unsigned int l = sg->length;
>> + unsigned int o = sg->offset;
>> + bool same_page = false;
>> +
>> + ret = bio_add_hw_page(q, bio, p, l, o, max_sects, &same_page);
>> + if (ret != sg->length) {
>> + status = NVME_SC_INTERNAL;
>> + goto out_bio_put;
>> + }
>> + if (same_page)
>> + put_page(p);
>> +
>> + total_len += sg->length;
>> + }
>> +
>> + if (total_len != nvmet_rw_data_len(req)) {
>> + status = NVME_SC_INTERNAL | NVME_SC_DNR;
>> + goto out_bio_put;
>> + }
>> +
>> + ret = submit_bio_wait(bio);
>> + req->cqe->result.u64 = nvmet_sect_to_lba(req->ns,
>> + bio->bi_iter.bi_sector);
>> +
>> +out_bio_put:
>> + if (bio != &req->b.inline_bio)
>> + bio_put(bio);
>> + nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
>> +}
> Wasn't it the plan to integrate this function into the normal read-write
> proccessing ? I remember Christoph commenting on that.
Yes and I did that, it looked ugly and there is not much common code
apart from following 12 lines :-
if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
return;
if (!req->sg_cnt) {
nvmet_req_complete(req, 0);
return;
}
bio = nvmet_req_bio_get(req, NULL);
bio_set_dev(bio, req->ns->bdev);
bio->bi_iter.bi_sector = sect;
bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
Instead of saving 12 lines of code I'd prefer to have clear modularize
zone-append handler and not touch rw fast path. I can add helpers to trim
downabove code which we need anyway in V8.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
@ 2020-12-15 23:06 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-15 23:06 UTC (permalink / raw)
To: Damien Le Moal, linux-block, linux-nvme; +Cc: sagi, hch
On 12/15/20 01:06, Damien Le Moal wrote:
> On 2020/12/15 15:03, Chaitanya Kulkarni wrote:
>> NVMe TP 4053 – Zoned Namespaces (ZNS) allows host software to
>> communicate with a non-volatile memory subsystem using zones for
>> NVMe protocol based controllers. NVMeOF already support the ZNS NVMe
>> Protocol compliant devices on the target in the passthru mode. There
>> are Generic zoned block devices like Shingled Magnetic Recording (SMR)
>> HDDs which are not based on the NVMe protocol.
>>
>> This patch adds ZNS backend to support the ZBDs for NVMeOF target.
>>
>> This support includes implementing the new command set NVME_CSI_ZNS,
>> adding different command handlers for ZNS command set such as
>> NVMe Identify Controller, NVMe Identify Namespace, NVMe Zone Append,
>> NVMe Zone Management Send and NVMe Zone Management Receive.
>>
>> With new command set identifier we also update the target command effects
>> logs to reflect the ZNS compliant commands.
>>
>> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
>> ---
>> drivers/nvme/target/Makefile | 1 +
>> drivers/nvme/target/admin-cmd.c | 26 +++
>> drivers/nvme/target/core.c | 1 +
>> drivers/nvme/target/io-cmd-bdev.c | 33 ++-
>> drivers/nvme/target/nvmet.h | 38 ++++
>> drivers/nvme/target/zns.c | 342 ++++++++++++++++++++++++++++++
>> 6 files changed, 433 insertions(+), 8 deletions(-)
>> create mode 100644 drivers/nvme/target/zns.c
>>
>> diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
>> index ebf91fc4c72e..9837e580fa7e 100644
>> --- a/drivers/nvme/target/Makefile
>> +++ b/drivers/nvme/target/Makefile
>> @@ -12,6 +12,7 @@ obj-$(CONFIG_NVME_TARGET_TCP) += nvmet-tcp.o
>> nvmet-y += core.o configfs.o admin-cmd.o fabrics-cmd.o \
>> discovery.o io-cmd-file.o io-cmd-bdev.o
>> nvmet-$(CONFIG_NVME_TARGET_PASSTHRU) += passthru.o
>> +nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o
>> nvme-loop-y += loop.o
>> nvmet-rdma-y += rdma.o
>> nvmet-fc-y += fc.o
>> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
>> index f4c0f3aca485..6f5279b15aa6 100644
>> --- a/drivers/nvme/target/admin-cmd.c
>> +++ b/drivers/nvme/target/admin-cmd.c
>> @@ -192,6 +192,15 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
>> log->iocs[nvme_cmd_dsm] = cpu_to_le32(1 << 0);
>> log->iocs[nvme_cmd_write_zeroes] = cpu_to_le32(1 << 0);
>> break;
>> + case NVME_CSI_ZNS:
>> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
>> + u32 *iocs = log->iocs;
>> +
>> + iocs[nvme_cmd_zone_append] = cpu_to_le32(1 << 0);
>> + iocs[nvme_cmd_zone_mgmt_send] = cpu_to_le32(1 << 0);
>> + iocs[nvme_cmd_zone_mgmt_recv] = cpu_to_le32(1 << 0);
>> + }
>> + break;
>> default:
>> status = NVME_SC_INVALID_LOG_PAGE;
>> break;
>> @@ -614,6 +623,7 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
>>
>> static void nvmet_execute_identify_desclist(struct nvmet_req *req)
>> {
>> + u16 nvme_cis_zns = NVME_CSI_ZNS;
>> u16 status = 0;
>> off_t off = 0;
>>
>> @@ -638,6 +648,14 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
>> if (status)
>> goto out;
>> }
>> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
>> + if (req->ns->csi == NVME_CSI_ZNS)
>> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI,
>> + NVME_NIDT_CSI_LEN,
>> + &nvme_cis_zns, &off);
>> + if (status)
>> + goto out;
>> + }
>>
>> if (sg_zero_buffer(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE - off,
>> off) != NVME_IDENTIFY_DATA_SIZE - off)
>> @@ -655,8 +673,16 @@ static void nvmet_execute_identify(struct nvmet_req *req)
>> switch (req->cmd->identify.cns) {
>> case NVME_ID_CNS_NS:
>> return nvmet_execute_identify_ns(req);
>> + case NVME_ID_CNS_CS_NS:
>> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
>> + return nvmet_execute_identify_cns_cs_ns(req);
>> + break;
>> case NVME_ID_CNS_CTRL:
>> return nvmet_execute_identify_ctrl(req);
>> + case NVME_ID_CNS_CS_CTRL:
>> + if (req->cmd->identify.csi == NVME_CSI_ZNS)
>> + return nvmet_execute_identify_cns_cs_ctrl(req);
>> + break;
>> case NVME_ID_CNS_NS_ACTIVE_LIST:
>> return nvmet_execute_identify_nslist(req);
>> case NVME_ID_CNS_NS_DESC_LIST:
>> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
>> index 672e4009f8d6..17a99c7134dc 100644
>> --- a/drivers/nvme/target/core.c
>> +++ b/drivers/nvme/target/core.c
>> @@ -1107,6 +1107,7 @@ static inline u8 nvmet_cc_iocqes(u32 cc)
>> static inline bool nvmet_cc_css_check(u8 cc_css)
>> {
>> switch (cc_css <<= NVME_CC_CSS_SHIFT) {
>> + case NVME_CC_CSS_CSI:
>> case NVME_CC_CSS_NVM:
>> return true;
>> default:
>> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
>> index 23095bdfce06..6178ef643962 100644
>> --- a/drivers/nvme/target/io-cmd-bdev.c
>> +++ b/drivers/nvme/target/io-cmd-bdev.c
>> @@ -63,6 +63,14 @@ static void nvmet_bdev_ns_enable_integrity(struct nvmet_ns *ns)
>> }
>> }
>>
>> +void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
>> +{
>> + if (ns->bdev) {
>> + blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
>> + ns->bdev = NULL;
>> + }
>> +}
>> +
>> int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
>> {
>> int ret;
>> @@ -86,15 +94,15 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
>> if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY_T10))
>> nvmet_bdev_ns_enable_integrity(ns);
>>
>> - return 0;
>> -}
>> -
>> -void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
>> -{
>> - if (ns->bdev) {
>> - blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
>> - ns->bdev = NULL;
>> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && bdev_is_zoned(ns->bdev)) {
>> + if (!nvmet_bdev_zns_enable(ns)) {
>> + nvmet_bdev_ns_disable(ns);
>> + return -EINVAL;
>> + }
>> + ns->csi = NVME_CSI_ZNS;
>> }
>> +
>> + return 0;
>> }
>>
>> void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
>> @@ -448,6 +456,15 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
>> case nvme_cmd_write_zeroes:
>> req->execute = nvmet_bdev_execute_write_zeroes;
>> return 0;
>> + case nvme_cmd_zone_append:
>> + req->execute = nvmet_bdev_execute_zone_append;
>> + return 0;
>> + case nvme_cmd_zone_mgmt_recv:
>> + req->execute = nvmet_bdev_execute_zone_mgmt_recv;
>> + return 0;
>> + case nvme_cmd_zone_mgmt_send:
>> + req->execute = nvmet_bdev_execute_zone_mgmt_send;
>> + return 0;
>> default:
>> pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
>> req->sq->qid);
>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>> index 476b3cd91c65..7361665585a2 100644
>> --- a/drivers/nvme/target/nvmet.h
>> +++ b/drivers/nvme/target/nvmet.h
>> @@ -252,6 +252,10 @@ struct nvmet_subsys {
>> unsigned int admin_timeout;
>> unsigned int io_timeout;
>> #endif /* CONFIG_NVME_TARGET_PASSTHRU */
>> +
>> +#ifdef CONFIG_BLK_DEV_ZONED
>> + u8 zasl;
>> +#endif /* CONFIG_BLK_DEV_ZONED */
>> };
>>
>> static inline struct nvmet_subsys *to_subsys(struct config_item *item)
>> @@ -614,4 +618,38 @@ static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
>> return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
>> }
>>
>> +#ifdef CONFIG_BLK_DEV_ZONED
>> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
>> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req);
>> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req);
>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req);
>> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req);
>> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req);
>> +#else /* CONFIG_BLK_DEV_ZONED */
>> +static inline bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
>> +{
>> + return false;
>> +}
>> +static inline void
>> +nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
>> +{
>> +}
>> +static inline void
>> +nvmet_bdev_execute_zone_append(struct nvmet_req *req)
>> +{
>> +}
>> +#endif /* CONFIG_BLK_DEV_ZONED */
>> +
>> #endif /* _NVMET_H */
>> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
>> new file mode 100644
>> index 000000000000..3981baa647b2
>> --- /dev/null
>> +++ b/drivers/nvme/target/zns.c
>> @@ -0,0 +1,342 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * NVMe ZNS-ZBD command implementation.
>> + * Copyright (c) 2020-2021 HGST, a Western Digital Company.
>> + */
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +#include <linux/nvme.h>
>> +#include <linux/blkdev.h>
>> +#include "nvmet.h"
>> +
>> +/*
>> + * We set the Memory Page Size Minimum (MPSMIN) for target controller to 0
>> + * which gets added by 12 in the nvme_enable_ctrl() which results in 2^12 = 4k
>> + * as page_shift value. When calculating the ZASL use shift by 12.
>> + */
>> +#define NVMET_MPSMIN_SHIFT 12
>> +
>> +static u16 nvmet_bdev_zns_checks(struct nvmet_req *req)
>> +{
>> + u16 status = NVME_SC_SUCCESS;
>> +
>> + if (!bdev_is_zoned(req->ns->bdev)) {
>> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>> + goto out;
>> + }
>> +
>> + if (req->cmd->zmr.zra != NVME_ZRA_ZONE_REPORT) {
>> + status = NVME_SC_INVALID_FIELD;
>> + goto out;
>> + }
>> +
>> + if (req->cmd->zmr.zrasf != NVME_ZRASF_ZONE_REPORT_ALL) {
>> + status = NVME_SC_INVALID_FIELD;
>> + goto out;
>> + }
>> +
>> + if (req->cmd->zmr.pr != NVME_REPORT_ZONE_PARTIAL)
>> + status = NVME_SC_INVALID_FIELD;
>> +
>> +out:
>> + return status;
>> +}
>> +
>> +/*
>> + * ZNS related command implementation and helpers.
>> + */
>> +
>> +static inline u8 nvmet_zasl(unsigned int zone_append_sects)
>> +{
>> + /*
>> + * Zone Append Size Limit is the value experessed in the units
>> + * of minimum memory page size (i.e. 12) and is reported power of 2.
>> + */
>> + return ilog2((zone_append_sects << 9) >> NVMET_MPSMIN_SHIFT);
>> +}
>> +
>> +static inline bool nvmet_zns_update_zasl(struct nvmet_ns *ns)
>> +{
>> + struct request_queue *q = ns->bdev->bd_disk->queue;
>> + u8 zasl = nvmet_zasl(queue_max_zone_append_sectors(q));
>> +
>> + if (ns->subsys->zasl)
>> + return ns->subsys->zasl < zasl ? false : true;
>> +
>> + ns->subsys->zasl = zasl;
>> + return true;
>> +}
>> +
>> +
>> +static int nvmet_bdev_validate_zns_zones_cb(struct blk_zone *z,
>> + unsigned int idx, void *data)
>> +{
>> + if (z->type == BLK_ZONE_TYPE_CONVENTIONAL)
>> + return -EOPNOTSUPP;
>> + return 0;
>> +}
>> +
>> +static bool nvmet_bdev_has_conv_zones(struct block_device *bdev)
>> +{
>> + int ret;
>> +
>> + if (bdev->bd_disk->queue->conv_zones_bitmap)
>> + return true;
>> +
>> + ret = blkdev_report_zones(bdev, 0, blkdev_nr_zones(bdev->bd_disk),
>> + nvmet_bdev_validate_zns_zones_cb, NULL);
>> +
>> + return ret < 0 ? true : false;
>> +}
>> +
>> +bool nvmet_bdev_zns_enable(struct nvmet_ns *ns)
>> +{
>> + if (nvmet_bdev_has_conv_zones(ns->bdev))
>> + return false;
>> +
>> + /*
>> + * For ZBC and ZAC devices, writes into sequential zones must be aligned
>> + * to the device physical block size. So use this value as the logical
>> + * block size to avoid errors.
>> + */
>> + ns->blksize_shift = blksize_bits(bdev_physical_block_size(ns->bdev));
>> +
>> + if (!nvmet_zns_update_zasl(ns))
>> + return false;
>> +
>> + return !(get_capacity(ns->bdev->bd_disk) &
>> + (bdev_zone_sectors(ns->bdev) - 1));
>> +}
>> +
>> +/*
>> + * ZNS related Admin and I/O command handlers.
>> + */
>> +void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
>> +{
>> + u8 zasl = req->sq->ctrl->subsys->zasl;
>> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
>> + struct nvme_id_ctrl_zns *id;
>> + u16 status;
>> +
>> + id = kzalloc(sizeof(*id), GFP_KERNEL);
>> + if (!id) {
>> + status = NVME_SC_INTERNAL;
>> + goto out;
>> + }
>> +
>> + if (ctrl->ops->get_mdts)
>> + id->zasl = min_t(u8, ctrl->ops->get_mdts(ctrl), zasl);
>> + else
>> + id->zasl = zasl;
>> +
>> + status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
>> +
>> + kfree(id);
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
>> +{
>> + struct nvme_id_ns_zns *id_zns;
>> + u16 status = NVME_SC_SUCCESS;
>> + u64 zsze;
>> +
>> + if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
>> + req->error_loc = offsetof(struct nvme_identify, nsid);
>> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>> + goto out;
>> + }
>> +
>> + id_zns = kzalloc(sizeof(*id_zns), GFP_KERNEL);
>> + if (!id_zns) {
>> + status = NVME_SC_INTERNAL;
>> + goto out;
>> + }
>> +
>> + req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
>> + if (!req->ns) {
>> + status = NVME_SC_INTERNAL;
>> + goto done;
>> + }
>> +
>> + if (!bdev_is_zoned(req->ns->bdev)) {
>> + req->error_loc = offsetof(struct nvme_identify, nsid);
>> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>> + goto done;
>> + }
>> +
>> + nvmet_ns_revalidate(req->ns);
>> + zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
>> + req->ns->blksize_shift;
>> + id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
>> + id_zns->mor = cpu_to_le32(bdev_max_open_zones(req->ns->bdev));
>> + id_zns->mar = cpu_to_le32(bdev_max_active_zones(req->ns->bdev));
>> +
>> +done:
>> + status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
>> + kfree(id_zns);
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +struct nvmet_report_zone_data {
>> + struct nvmet_ns *ns;
>> + struct nvme_zone_report *rz;
>> +};
>> +
>> +static int nvmet_bdev_report_zone_cb(struct blk_zone *z, unsigned int idx,
>> + void *data)
>> +{
>> + struct nvmet_report_zone_data *report_zone_data = data;
>> + struct nvme_zone_descriptor *entries = report_zone_data->rz->entries;
>> + struct nvmet_ns *ns = report_zone_data->ns;
>> +
>> + entries[idx].zcap = nvmet_sect_to_lba(ns, z->capacity);
>> + entries[idx].zslba = nvmet_sect_to_lba(ns, z->start);
>> + entries[idx].wp = nvmet_sect_to_lba(ns, z->wp);
>> + entries[idx].za = z->reset ? 1 << 2 : 0;
>> + entries[idx].zt = z->type;
>> + entries[idx].zs = z->cond << 4;
>> +
>> + return 0;
>> +}
>> +
>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>> +{
>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>> + unsigned int nr_zones;
>> + int reported_zones;
>> + u16 status;
>> +
>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>> + sizeof(struct nvme_zone_descriptor);
> You could move this right before the vmalloc call since it is first used there.
There are only three lines between the this and the vmalloc, does that
really
going to make any difference ?
>> +
>> + status = nvmet_bdev_zns_checks(req);
>> + if (status)
>> + goto out;
>> +
>> + data.rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY | __GFP_ZERO);
>> + if (!data.rz) {
>> + status = NVME_SC_INTERNAL;
>> + goto out;
>> + }
>> +
>> + reported_zones = blkdev_report_zones(req->ns->bdev, sect, nr_zones,
>> + nvmet_bdev_report_zone_cb,
>> + &data);
>> + if (reported_zones < 0) {
>> + status = NVME_SC_INTERNAL;
>> + goto out_free_report_zones;
>> + }
>> +
>> + data.rz->nr_zones = cpu_to_le64(reported_zones);
>> +
>> + status = nvmet_copy_to_sgl(req, 0, data.rz, bufsize);
>> +
>> +out_free_report_zones:
>> + kvfree(data.rz);
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req)
>> +{
>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zms.slba);
>> + sector_t nr_sect = bdev_zone_sectors(req->ns->bdev);
>> + u16 status = NVME_SC_SUCCESS;
>> + enum req_opf op;
>> + int ret;
>> +
>> + if (req->cmd->zms.select_all)
>> + nr_sect = get_capacity(req->ns->bdev->bd_disk);
>> +
>> + switch (req->cmd->zms.zsa) {
>> + case NVME_ZONE_OPEN:
>> + op = REQ_OP_ZONE_OPEN;
>> + break;
>> + case NVME_ZONE_CLOSE:
>> + op = REQ_OP_ZONE_CLOSE;
>> + break;
>> + case NVME_ZONE_FINISH:
>> + op = REQ_OP_ZONE_FINISH;
>> + break;
>> + case NVME_ZONE_RESET:
>> + op = REQ_OP_ZONE_RESET;
>> + break;
>> + default:
>> + status = NVME_SC_INVALID_FIELD;
>> + goto out;
>> + }
>> +
>> + ret = blkdev_zone_mgmt(req->ns->bdev, op, sect, nr_sect, GFP_KERNEL);
>> + if (ret)
>> + status = NVME_SC_INTERNAL;
>> +out:
>> + nvmet_req_complete(req, status);
>> +}
>> +
>> +void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
>> +{
>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
>> + struct request_queue *q = req->ns->bdev->bd_disk->queue;
>> + unsigned int max_sects = queue_max_zone_append_sectors(q);
>> + u16 status = NVME_SC_SUCCESS;
>> + unsigned int total_len = 0;
>> + struct scatterlist *sg;
>> + int ret = 0, sg_cnt;
>> + struct bio *bio;
>> +
>> + if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
>> + return;
>> +
>> + if (!req->sg_cnt) {
>> + nvmet_req_complete(req, 0);
>> + return;
>> + }
>> +
>> + if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
>> + bio = &req->b.inline_bio;
>> + bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
>> + } else {
>> + bio = bio_alloc(GFP_KERNEL, req->sg_cnt);
>> + }
>> +
>> + bio_set_dev(bio, req->ns->bdev);
>> + bio->bi_iter.bi_sector = sect;
>> + bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
>> + if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
>> + bio->bi_opf |= REQ_FUA;
>> +
>> + for_each_sg(req->sg, sg, req->sg_cnt, sg_cnt) {
>> + struct page *p = sg_page(sg);
>> + unsigned int l = sg->length;
>> + unsigned int o = sg->offset;
>> + bool same_page = false;
>> +
>> + ret = bio_add_hw_page(q, bio, p, l, o, max_sects, &same_page);
>> + if (ret != sg->length) {
>> + status = NVME_SC_INTERNAL;
>> + goto out_bio_put;
>> + }
>> + if (same_page)
>> + put_page(p);
>> +
>> + total_len += sg->length;
>> + }
>> +
>> + if (total_len != nvmet_rw_data_len(req)) {
>> + status = NVME_SC_INTERNAL | NVME_SC_DNR;
>> + goto out_bio_put;
>> + }
>> +
>> + ret = submit_bio_wait(bio);
>> + req->cqe->result.u64 = nvmet_sect_to_lba(req->ns,
>> + bio->bi_iter.bi_sector);
>> +
>> +out_bio_put:
>> + if (bio != &req->b.inline_bio)
>> + bio_put(bio);
>> + nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL : status);
>> +}
> Wasn't it the plan to integrate this function into the normal read-write
> proccessing ? I remember Christoph commenting on that.
Yes and I did that, it looked ugly and there is not much common code
apart from following 12 lines :-
if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
return;
if (!req->sg_cnt) {
nvmet_req_complete(req, 0);
return;
}
bio = nvmet_req_bio_get(req, NULL);
bio_set_dev(bio, req->ns->bdev);
bio->bi_iter.bi_sector = sect;
bio->bi_opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
Instead of saving 12 lines of code I'd prefer to have clear modularize
zone-append handler and not touch rw fast path. I can add helpers to trim
downabove code which we need anyway in V8.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
2020-12-15 23:06 ` Chaitanya Kulkarni
@ 2020-12-15 23:13 ` Damien Le Moal
-1 siblings, 0 replies; 26+ messages in thread
From: Damien Le Moal @ 2020-12-15 23:13 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block, linux-nvme; +Cc: sagi, hch
On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
[...]
>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>> +{
>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>> + unsigned int nr_zones;
>>> + int reported_zones;
>>> + u16 status;
>>> +
>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>> + sizeof(struct nvme_zone_descriptor);
>> You could move this right before the vmalloc call since it is first used there.
> There are only three lines between the this and the vmalloc, does that
> really
> going to make any difference ?
It makes the code far easier to read and understand at a quick glance without
having to go up and down reading to be reminded what nr_zones was. That also
would avoid changes to sneak in between these related statements, making things
even harder to read.
I personally like to think of code as a natural language text: if statements
related to each other are not grouped in a single paragraph, the text is really
hard to understand...
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
@ 2020-12-15 23:13 ` Damien Le Moal
0 siblings, 0 replies; 26+ messages in thread
From: Damien Le Moal @ 2020-12-15 23:13 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block, linux-nvme; +Cc: sagi, hch
On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
[...]
>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>> +{
>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>> + unsigned int nr_zones;
>>> + int reported_zones;
>>> + u16 status;
>>> +
>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>> + sizeof(struct nvme_zone_descriptor);
>> You could move this right before the vmalloc call since it is first used there.
> There are only three lines between the this and the vmalloc, does that
> really
> going to make any difference ?
It makes the code far easier to read and understand at a quick glance without
having to go up and down reading to be reminded what nr_zones was. That also
would avoid changes to sneak in between these related statements, making things
even harder to read.
I personally like to think of code as a natural language text: if statements
related to each other are not grouped in a single paragraph, the text is really
hard to understand...
--
Damien Le Moal
Western Digital Research
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
2020-12-15 23:13 ` Damien Le Moal
@ 2020-12-16 3:13 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-16 3:13 UTC (permalink / raw)
To: Damien Le Moal, linux-block, linux-nvme; +Cc: sagi, hch
On 12/15/20 15:13, Damien Le Moal wrote:
> On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
> [...]
>>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>>> +{
>>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>>> + unsigned int nr_zones;
>>>> + int reported_zones;
>>>> + u16 status;
>>>> +
>>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>>> + sizeof(struct nvme_zone_descriptor);
>>> You could move this right before the vmalloc call since it is first used there.
>> There are only three lines between the this and the vmalloc, does that
>> really
>> going to make any difference ?
> It makes the code far easier to read and understand at a quick glance without
> having to go up and down reading to be reminded what nr_zones was. That also
> would avoid changes to sneak in between these related statements, making things
> even harder to read.
>
> I personally like to think of code as a natural language text: if statements
> related to each other are not grouped in a single paragraph, the text is really
> hard to understand...
>
hmmm, why can't we use a macro and like everywhere else in zns.c
we can declare-init the nr_zones which will make nr_zones initialization
uniform withall the code with a meaningful name.
How about following (untested) ?
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 149bc8ce7010..6c497b60cd98 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -201,18 +201,19 @@ static int nvmet_bdev_report_zone_cb(struct
blk_zone *z, unsigned int idx,
return 0;
}
+#define NVMET_NR_ZONES_FROM_BUFSIZE(bufsize) \
+ ((bufsize - sizeof(struct nvme_zone_report)) / \
+ sizeof(struct nvme_zone_descriptor))
+
void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
{
sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
struct nvmet_report_zone_data data = { .ns = req->ns };
- unsigned int nr_zones;
+ unsigned int nr_zones = NVMET_NR_ZONES_FROM_BUFSIZE(bufsize);
int reported_zones;
u16 status;
- nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
- sizeof(struct nvme_zone_descriptor);
-
status = nvmet_bdev_zns_checks(req);
if (status)
goto out;
> -- Damien Le Moal Western Digital Research
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
@ 2020-12-16 3:13 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-16 3:13 UTC (permalink / raw)
To: Damien Le Moal, linux-block, linux-nvme; +Cc: sagi, hch
On 12/15/20 15:13, Damien Le Moal wrote:
> On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
> [...]
>>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>>> +{
>>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>>> + unsigned int nr_zones;
>>>> + int reported_zones;
>>>> + u16 status;
>>>> +
>>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>>> + sizeof(struct nvme_zone_descriptor);
>>> You could move this right before the vmalloc call since it is first used there.
>> There are only three lines between the this and the vmalloc, does that
>> really
>> going to make any difference ?
> It makes the code far easier to read and understand at a quick glance without
> having to go up and down reading to be reminded what nr_zones was. That also
> would avoid changes to sneak in between these related statements, making things
> even harder to read.
>
> I personally like to think of code as a natural language text: if statements
> related to each other are not grouped in a single paragraph, the text is really
> hard to understand...
>
hmmm, why can't we use a macro and like everywhere else in zns.c
we can declare-init the nr_zones which will make nr_zones initialization
uniform withall the code with a meaningful name.
How about following (untested) ?
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 149bc8ce7010..6c497b60cd98 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -201,18 +201,19 @@ static int nvmet_bdev_report_zone_cb(struct
blk_zone *z, unsigned int idx,
return 0;
}
+#define NVMET_NR_ZONES_FROM_BUFSIZE(bufsize) \
+ ((bufsize - sizeof(struct nvme_zone_report)) / \
+ sizeof(struct nvme_zone_descriptor))
+
void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
{
sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
struct nvmet_report_zone_data data = { .ns = req->ns };
- unsigned int nr_zones;
+ unsigned int nr_zones = NVMET_NR_ZONES_FROM_BUFSIZE(bufsize);
int reported_zones;
u16 status;
- nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
- sizeof(struct nvme_zone_descriptor);
-
status = nvmet_bdev_zns_checks(req);
if (status)
goto out;
> -- Damien Le Moal Western Digital Research
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
2020-12-16 3:13 ` Chaitanya Kulkarni
@ 2020-12-16 3:43 ` Damien Le Moal
-1 siblings, 0 replies; 26+ messages in thread
From: Damien Le Moal @ 2020-12-16 3:43 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block, linux-nvme; +Cc: sagi, hch
On 2020/12/16 12:13, Chaitanya Kulkarni wrote:
> On 12/15/20 15:13, Damien Le Moal wrote:
>> On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
>> [...]
>>>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>>>> +{
>>>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>>>> + unsigned int nr_zones;
>>>>> + int reported_zones;
>>>>> + u16 status;
>>>>> +
>>>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>>>> + sizeof(struct nvme_zone_descriptor);
>>>> You could move this right before the vmalloc call since it is first used there.
>>> There are only three lines between the this and the vmalloc, does that
>>> really
>>> going to make any difference ?
>> It makes the code far easier to read and understand at a quick glance without
>> having to go up and down reading to be reminded what nr_zones was. That also
>> would avoid changes to sneak in between these related statements, making things
>> even harder to read.
>>
>> I personally like to think of code as a natural language text: if statements
>> related to each other are not grouped in a single paragraph, the text is really
>> hard to understand...
>>
> hmmm, why can't we use a macro and like everywhere else in zns.c
> we can declare-init the nr_zones which will make nr_zones initialization
> uniform withall the code with a meaningful name.
>
> How about following (untested) ?
>
> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
> index 149bc8ce7010..6c497b60cd98 100644
> --- a/drivers/nvme/target/zns.c
> +++ b/drivers/nvme/target/zns.c
> @@ -201,18 +201,19 @@ static int nvmet_bdev_report_zone_cb(struct
> blk_zone *z, unsigned int idx,
> return 0;
> }
>
> +#define NVMET_NR_ZONES_FROM_BUFSIZE(bufsize) \
> + ((bufsize - sizeof(struct nvme_zone_report)) / \
> + sizeof(struct nvme_zone_descriptor))
> +
> void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
> {
> sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
> u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
> struct nvmet_report_zone_data data = { .ns = req->ns };
> - unsigned int nr_zones;
> + unsigned int nr_zones = NVMET_NR_ZONES_FROM_BUFSIZE(bufsize);
Hiding calculations in a macro does not help readability. And I do not see the
point since this is used in one place only. If you want to isolate the report
buffer allocation & nr zones calculation, then something like what scsi does in
sd_zbc_alloc_report_buffer() (in drivers/scsi/sd_zbc.c) is in my opinion much
cleaner.
> int reported_zones;
> u16 status;
>
> - nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
> - sizeof(struct nvme_zone_descriptor);
> -
> status = nvmet_bdev_zns_checks(req);
> if (status)
> goto out;
>
>> -- Damien Le Moal Western Digital Research
>
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
@ 2020-12-16 3:43 ` Damien Le Moal
0 siblings, 0 replies; 26+ messages in thread
From: Damien Le Moal @ 2020-12-16 3:43 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block, linux-nvme; +Cc: sagi, hch
On 2020/12/16 12:13, Chaitanya Kulkarni wrote:
> On 12/15/20 15:13, Damien Le Moal wrote:
>> On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
>> [...]
>>>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>>>> +{
>>>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>>>> + unsigned int nr_zones;
>>>>> + int reported_zones;
>>>>> + u16 status;
>>>>> +
>>>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>>>> + sizeof(struct nvme_zone_descriptor);
>>>> You could move this right before the vmalloc call since it is first used there.
>>> There are only three lines between the this and the vmalloc, does that
>>> really
>>> going to make any difference ?
>> It makes the code far easier to read and understand at a quick glance without
>> having to go up and down reading to be reminded what nr_zones was. That also
>> would avoid changes to sneak in between these related statements, making things
>> even harder to read.
>>
>> I personally like to think of code as a natural language text: if statements
>> related to each other are not grouped in a single paragraph, the text is really
>> hard to understand...
>>
> hmmm, why can't we use a macro and like everywhere else in zns.c
> we can declare-init the nr_zones which will make nr_zones initialization
> uniform withall the code with a meaningful name.
>
> How about following (untested) ?
>
> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
> index 149bc8ce7010..6c497b60cd98 100644
> --- a/drivers/nvme/target/zns.c
> +++ b/drivers/nvme/target/zns.c
> @@ -201,18 +201,19 @@ static int nvmet_bdev_report_zone_cb(struct
> blk_zone *z, unsigned int idx,
> return 0;
> }
>
> +#define NVMET_NR_ZONES_FROM_BUFSIZE(bufsize) \
> + ((bufsize - sizeof(struct nvme_zone_report)) / \
> + sizeof(struct nvme_zone_descriptor))
> +
> void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
> {
> sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
> u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
> struct nvmet_report_zone_data data = { .ns = req->ns };
> - unsigned int nr_zones;
> + unsigned int nr_zones = NVMET_NR_ZONES_FROM_BUFSIZE(bufsize);
Hiding calculations in a macro does not help readability. And I do not see the
point since this is used in one place only. If you want to isolate the report
buffer allocation & nr zones calculation, then something like what scsi does in
sd_zbc_alloc_report_buffer() (in drivers/scsi/sd_zbc.c) is in my opinion much
cleaner.
> int reported_zones;
> u16 status;
>
> - nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
> - sizeof(struct nvme_zone_descriptor);
> -
> status = nvmet_bdev_zns_checks(req);
> if (status)
> goto out;
>
>> -- Damien Le Moal Western Digital Research
>
>
--
Damien Le Moal
Western Digital Research
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
2020-12-16 3:43 ` Damien Le Moal
@ 2020-12-16 5:09 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-16 5:09 UTC (permalink / raw)
To: Damien Le Moal, linux-block, linux-nvme; +Cc: sagi, hch
On 12/15/20 7:43 PM, Damien Le Moal wrote:
> On 2020/12/16 12:13, Chaitanya Kulkarni wrote:
>> On 12/15/20 15:13, Damien Le Moal wrote:
>>> On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
>>> [...]
>>>>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>>>>> +{
>>>>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>>>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>>>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>>>>> + unsigned int nr_zones;
>>>>>> + int reported_zones;
>>>>>> + u16 status;
>>>>>> +
>>>>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>>>>> + sizeof(struct nvme_zone_descriptor);
>>>>> You could move this right before the vmalloc call since it is first used there.
>>>> There are only three lines between the this and the vmalloc, does that
>>>> really
>>>> going to make any difference ?
>>> It makes the code far easier to read and understand at a quick glance without
>>> having to go up and down reading to be reminded what nr_zones was. That also
>>> would avoid changes to sneak in between these related statements, making things
>>> even harder to read.
>>>
>>> I personally like to think of code as a natural language text: if statements
>>> related to each other are not grouped in a single paragraph, the text is really
>>> hard to understand...
>>>
>> hmmm, why can't we use a macro and like everywhere else in zns.c
>> we can declare-init the nr_zones which will make nr_zones initialization
>> uniform withall the code with a meaningful name.
>>
>> How about following (untested) ?
>>
>> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
>> index 149bc8ce7010..6c497b60cd98 100644
>> --- a/drivers/nvme/target/zns.c
>> +++ b/drivers/nvme/target/zns.c
>> @@ -201,18 +201,19 @@ static int nvmet_bdev_report_zone_cb(struct
>> blk_zone *z, unsigned int idx,
>> return 0;
>> }
>>
>> +#define NVMET_NR_ZONES_FROM_BUFSIZE(bufsize) \
>> + ((bufsize - sizeof(struct nvme_zone_report)) / \
>> + sizeof(struct nvme_zone_descriptor))
>> +
>> void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>> {
>> sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>> u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>> struct nvmet_report_zone_data data = { .ns = req->ns };
>> - unsigned int nr_zones;
>> + unsigned int nr_zones = NVMET_NR_ZONES_FROM_BUFSIZE(bufsize);
> Hiding calculations in a macro does not help readability. And I do not see the
> point since this is used in one place only. If you want to isolate the report
> buffer allocation & nr zones calculation, then something like what scsi does in
> sd_zbc_alloc_report_buffer() (in drivers/scsi/sd_zbc.c) is in my opinion much
> cleaner.
This is what I was thinking about it since we also do similar buffer
allocation calculation
on the host side. Let me see if I can add that to V8.
>> int reported_zones;
>> u16 status;
>>
>> - nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>> - sizeof(struct nvme_zone_descriptor);
>> -
>> status = nvmet_bdev_zns_checks(req);
>> if (status)
>> goto out;
>>
>>> -- Damien Le Moal Western Digital Research
>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support
@ 2020-12-16 5:09 ` Chaitanya Kulkarni
0 siblings, 0 replies; 26+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-16 5:09 UTC (permalink / raw)
To: Damien Le Moal, linux-block, linux-nvme; +Cc: sagi, hch
On 12/15/20 7:43 PM, Damien Le Moal wrote:
> On 2020/12/16 12:13, Chaitanya Kulkarni wrote:
>> On 12/15/20 15:13, Damien Le Moal wrote:
>>> On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
>>> [...]
>>>>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>>>>> +{
>>>>>> + sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>>>>> + u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>>>>> + struct nvmet_report_zone_data data = { .ns = req->ns };
>>>>>> + unsigned int nr_zones;
>>>>>> + int reported_zones;
>>>>>> + u16 status;
>>>>>> +
>>>>>> + nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>>>>> + sizeof(struct nvme_zone_descriptor);
>>>>> You could move this right before the vmalloc call since it is first used there.
>>>> There are only three lines between the this and the vmalloc, does that
>>>> really
>>>> going to make any difference ?
>>> It makes the code far easier to read and understand at a quick glance without
>>> having to go up and down reading to be reminded what nr_zones was. That also
>>> would avoid changes to sneak in between these related statements, making things
>>> even harder to read.
>>>
>>> I personally like to think of code as a natural language text: if statements
>>> related to each other are not grouped in a single paragraph, the text is really
>>> hard to understand...
>>>
>> hmmm, why can't we use a macro and like everywhere else in zns.c
>> we can declare-init the nr_zones which will make nr_zones initialization
>> uniform withall the code with a meaningful name.
>>
>> How about following (untested) ?
>>
>> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
>> index 149bc8ce7010..6c497b60cd98 100644
>> --- a/drivers/nvme/target/zns.c
>> +++ b/drivers/nvme/target/zns.c
>> @@ -201,18 +201,19 @@ static int nvmet_bdev_report_zone_cb(struct
>> blk_zone *z, unsigned int idx,
>> return 0;
>> }
>>
>> +#define NVMET_NR_ZONES_FROM_BUFSIZE(bufsize) \
>> + ((bufsize - sizeof(struct nvme_zone_report)) / \
>> + sizeof(struct nvme_zone_descriptor))
>> +
>> void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>> {
>> sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>> u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>> struct nvmet_report_zone_data data = { .ns = req->ns };
>> - unsigned int nr_zones;
>> + unsigned int nr_zones = NVMET_NR_ZONES_FROM_BUFSIZE(bufsize);
> Hiding calculations in a macro does not help readability. And I do not see the
> point since this is used in one place only. If you want to isolate the report
> buffer allocation & nr zones calculation, then something like what scsi does in
> sd_zbc_alloc_report_buffer() (in drivers/scsi/sd_zbc.c) is in my opinion much
> cleaner.
This is what I was thinking about it since we also do similar buffer
allocation calculation
on the host side. Let me see if I can add that to V8.
>> int reported_zones;
>> u16 status;
>>
>> - nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>> - sizeof(struct nvme_zone_descriptor);
>> -
>> status = nvmet_bdev_zns_checks(req);
>> if (status)
>> goto out;
>>
>>> -- Damien Le Moal Western Digital Research
>>
>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-12-16 5:11 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 6:02 [PATCH V7 0/6] nvmet: add ZBD backend support Chaitanya Kulkarni
2020-12-15 6:02 ` Chaitanya Kulkarni
2020-12-15 6:03 ` [PATCH V7 1/6] block: export bio_add_hw_pages() Chaitanya Kulkarni
2020-12-15 6:03 ` Chaitanya Kulkarni
2020-12-15 6:03 ` [PATCH V7 2/6] nvmet: add lba to sect conversion helpers Chaitanya Kulkarni
2020-12-15 6:03 ` Chaitanya Kulkarni
2020-12-15 6:03 ` [PATCH V7 3/6] nvmet: add NVM command set identifier support Chaitanya Kulkarni
2020-12-15 6:03 ` Chaitanya Kulkarni
2020-12-15 6:03 ` [PATCH V7 4/6] nvmet: add ZBD over ZNS backend support Chaitanya Kulkarni
2020-12-15 6:03 ` Chaitanya Kulkarni
2020-12-15 9:06 ` Damien Le Moal
2020-12-15 9:06 ` Damien Le Moal
2020-12-15 23:06 ` Chaitanya Kulkarni
2020-12-15 23:06 ` Chaitanya Kulkarni
2020-12-15 23:13 ` Damien Le Moal
2020-12-15 23:13 ` Damien Le Moal
2020-12-16 3:13 ` Chaitanya Kulkarni
2020-12-16 3:13 ` Chaitanya Kulkarni
2020-12-16 3:43 ` Damien Le Moal
2020-12-16 3:43 ` Damien Le Moal
2020-12-16 5:09 ` Chaitanya Kulkarni
2020-12-16 5:09 ` Chaitanya Kulkarni
2020-12-15 6:03 ` [PATCH V7 5/6] nvmet: add bio get helper for different backends Chaitanya Kulkarni
2020-12-15 6:03 ` Chaitanya Kulkarni
2020-12-15 6:03 ` [PATCH V7 6/6] nvmet: add bio put " Chaitanya Kulkarni
2020-12-15 6:03 ` Chaitanya Kulkarni
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.