* [PATCHv13 00/11] block write streams with nvme fdp @ 2024-12-10 19:47 Keith Busch 2024-12-10 19:47 ` [PATCHv13 05/11] block: expose write streams for block device nodes Keith Busch ` (4 more replies) 0 siblings, 5 replies; 16+ messages in thread From: Keith Busch @ 2024-12-10 19:47 UTC (permalink / raw) To: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: sagi, asml.silence, anuj20.g, joshi.k, Keith Busch From: Keith Busch <[email protected]> Previous discussion threads: v12: https://lore.kernel.org/linux-nvme/[email protected]/T/#u v11: https://lore.kernel.org/linux-nvme/[email protected]/T/#u Changes from v12: - Removed statx. We need additional time to consider the best way to expose these attributes. Until then, applications can fallback to the sysfs block attribute to query write stream settings. - More verbose logging on error. - Added reviews. - Explicitly clear the return value to 0 for unsupported or unrecognized FDP configs; while it's not technically needed (it's already 0 in these paths), it makes it clear that a non-error return wasn't an accidental oversight. - Fixed the compiler warnings from unitialized variable; switched the do-while to a more clear for-loop. - Fixed long-line wrapping - Memory leak when cleaning up the namespace head. - Don't rescan FDP configuration if we successfully set it up before. The namespaces' FDP configuration is static. - Better function name querying the size granularity of the FDP reclaim unit. Christoph Hellwig (7): fs: add a write stream field to the kiocb block: add a bi_write_stream field block: introduce a write_stream_granularity queue limit block: expose write streams for block device nodes nvme: add a nvme_get_log_lsi helper nvme: pass a void pointer to nvme_get/set_features for the result nvme: add FDP definitions Keith Busch (4): block: introduce max_write_streams queue limit io_uring: enable per-io write streams nvme: register fdp parameters with the block layer nvme: use fdp streams if write stream is provided Documentation/ABI/stable/sysfs-block | 15 +++ block/bio.c | 2 + block/blk-crypto-fallback.c | 1 + block/blk-merge.c | 4 + block/blk-sysfs.c | 6 + block/bounce.c | 1 + block/fops.c | 23 ++++ drivers/nvme/host/core.c | 186 ++++++++++++++++++++++++++- drivers/nvme/host/nvme.h | 7 +- include/linux/blk_types.h | 1 + include/linux/blkdev.h | 16 +++ include/linux/fs.h | 1 + include/linux/nvme.h | 77 +++++++++++ include/uapi/linux/io_uring.h | 4 + io_uring/io_uring.c | 2 + io_uring/rw.c | 1 + 16 files changed, 341 insertions(+), 6 deletions(-) -- 2.43.5 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv13 05/11] block: expose write streams for block device nodes 2024-12-10 19:47 [PATCHv13 00/11] block write streams with nvme fdp Keith Busch @ 2024-12-10 19:47 ` Keith Busch [not found] ` <CGME20241211060149epcas5p4d16e78bd3d184e57465c3622a2c8e98d@epcas5p4.samsung.com> 2024-12-10 19:47 ` [PATCHv13 06/11] io_uring: enable per-io write streams Keith Busch ` (3 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: Keith Busch @ 2024-12-10 19:47 UTC (permalink / raw) To: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: sagi, asml.silence, anuj20.g, joshi.k, Hannes Reinecke, Keith Busch From: Christoph Hellwig <[email protected]> Use the per-kiocb write stream if provided, or map temperature hints to write streams (which is a bit questionable, but this shows how it is done). Reviewed-by: Hannes Reinecke <[email protected]> Signed-off-by: Christoph Hellwig <[email protected]> [kbusch: removed statx reporting] Signed-off-by: Keith Busch <[email protected]> --- block/fops.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/block/fops.c b/block/fops.c index 6d5c4fc5a2168..f16aa39bf5bad 100644 --- a/block/fops.c +++ b/block/fops.c @@ -73,6 +73,7 @@ static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb, } bio.bi_iter.bi_sector = pos >> SECTOR_SHIFT; bio.bi_write_hint = file_inode(iocb->ki_filp)->i_write_hint; + bio.bi_write_stream = iocb->ki_write_stream; bio.bi_ioprio = iocb->ki_ioprio; if (iocb->ki_flags & IOCB_ATOMIC) bio.bi_opf |= REQ_ATOMIC; @@ -206,6 +207,7 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, for (;;) { bio->bi_iter.bi_sector = pos >> SECTOR_SHIFT; bio->bi_write_hint = file_inode(iocb->ki_filp)->i_write_hint; + bio->bi_write_stream = iocb->ki_write_stream; bio->bi_private = dio; bio->bi_end_io = blkdev_bio_end_io; bio->bi_ioprio = iocb->ki_ioprio; @@ -333,6 +335,7 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb, dio->iocb = iocb; bio->bi_iter.bi_sector = pos >> SECTOR_SHIFT; bio->bi_write_hint = file_inode(iocb->ki_filp)->i_write_hint; + bio->bi_write_stream = iocb->ki_write_stream; bio->bi_end_io = blkdev_bio_end_io_async; bio->bi_ioprio = iocb->ki_ioprio; @@ -398,6 +401,26 @@ static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter) if (blkdev_dio_invalid(bdev, iocb, iter)) return -EINVAL; + if (iov_iter_rw(iter) == WRITE) { + u16 max_write_streams = bdev_max_write_streams(bdev); + + if (iocb->ki_write_stream) { + if (iocb->ki_write_stream > max_write_streams) + return -EINVAL; + } else if (max_write_streams) { + enum rw_hint write_hint = + file_inode(iocb->ki_filp)->i_write_hint; + + /* + * Just use the write hint as write stream for block + * device writes. This assumes no file system is + * mounted that would use the streams differently. + */ + if (write_hint <= max_write_streams) + iocb->ki_write_stream = write_hint; + } + } + nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS + 1); if (likely(nr_pages <= BIO_MAX_VECS)) { if (is_sync_kiocb(iocb)) -- 2.43.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
[parent not found: <CGME20241211060149epcas5p4d16e78bd3d184e57465c3622a2c8e98d@epcas5p4.samsung.com>]
* Re: [PATCHv13 05/11] block: expose write streams for block device nodes [not found] ` <CGME20241211060149epcas5p4d16e78bd3d184e57465c3622a2c8e98d@epcas5p4.samsung.com> @ 2024-12-11 5:53 ` Nitesh Shetty 0 siblings, 0 replies; 16+ messages in thread From: Nitesh Shetty @ 2024-12-11 5:53 UTC (permalink / raw) To: Keith Busch Cc: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring, sagi, asml.silence, anuj20.g, joshi.k, Hannes Reinecke, Keith Busch [-- Attachment #1: Type: text/plain, Size: 465 bytes --] On 10/12/24 11:47AM, Keith Busch wrote: >From: Christoph Hellwig <[email protected]> > >Use the per-kiocb write stream if provided, or map temperature hints to >write streams (which is a bit questionable, but this shows how it is >done). > >Reviewed-by: Hannes Reinecke <[email protected]> >Signed-off-by: Christoph Hellwig <[email protected]> >[kbusch: removed statx reporting] >Signed-off-by: Keith Busch <[email protected]> >--- Reviewed-by: Nitesh Shetty <[email protected]> [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv13 06/11] io_uring: enable per-io write streams 2024-12-10 19:47 [PATCHv13 00/11] block write streams with nvme fdp Keith Busch 2024-12-10 19:47 ` [PATCHv13 05/11] block: expose write streams for block device nodes Keith Busch @ 2024-12-10 19:47 ` Keith Busch [not found] ` <CGME20241211065235epcas5p2a233ac902da67bf2d755d21d98e6eb21@epcas5p2.samsung.com> 2024-12-10 19:47 ` [PATCHv13 10/11] nvme: register fdp parameters with the block layer Keith Busch ` (2 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: Keith Busch @ 2024-12-10 19:47 UTC (permalink / raw) To: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: sagi, asml.silence, anuj20.g, joshi.k, Keith Busch, Hannes Reinecke From: Keith Busch <[email protected]> Allow userspace to pass a per-I/O write stream in the SQE: __u8 write_stream; The __u8 type matches the size the filesystems and block layer support. Application can query the supported values from the statx max_write_streams field. Unsupported values are ignored by file operations that do not support write streams or rejected with an error by those that support them. Reviewed-by: Hannes Reinecke <[email protected]> Signed-off-by: Keith Busch <[email protected]> --- include/uapi/linux/io_uring.h | 4 ++++ io_uring/io_uring.c | 2 ++ io_uring/rw.c | 1 + 3 files changed, 7 insertions(+) diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index 38f0d6b10eaf7..986a480e3b9c2 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -92,6 +92,10 @@ struct io_uring_sqe { __u16 addr_len; __u16 __pad3[1]; }; + struct { + __u8 write_stream; + __u8 __pad4[3]; + }; }; union { struct { diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index ae36aa702f463..b561c5e8879ac 100644 --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -3869,6 +3869,8 @@ static int __init io_uring_init(void) BUILD_BUG_SQE_ELEM(44, __s32, splice_fd_in); BUILD_BUG_SQE_ELEM(44, __u32, file_index); BUILD_BUG_SQE_ELEM(44, __u16, addr_len); + BUILD_BUG_SQE_ELEM(44, __u8, write_stream); + BUILD_BUG_SQE_ELEM(45, __u8, __pad4[0]); BUILD_BUG_SQE_ELEM(46, __u16, __pad3[0]); BUILD_BUG_SQE_ELEM(48, __u64, addr3); BUILD_BUG_SQE_ELEM_SIZE(48, 0, cmd); diff --git a/io_uring/rw.c b/io_uring/rw.c index 5b24fd8b69f62..416ccd89a77ed 100644 --- a/io_uring/rw.c +++ b/io_uring/rw.c @@ -316,6 +316,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe, } rw->kiocb.dio_complete = NULL; rw->kiocb.ki_flags = 0; + rw->kiocb.ki_write_stream = READ_ONCE(sqe->write_stream); rw->addr = READ_ONCE(sqe->addr); rw->len = READ_ONCE(sqe->len); -- 2.43.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
[parent not found: <CGME20241211065235epcas5p2a233ac902da67bf2d755d21d98e6eb21@epcas5p2.samsung.com>]
* Re: [PATCHv13 06/11] io_uring: enable per-io write streams [not found] ` <CGME20241211065235epcas5p2a233ac902da67bf2d755d21d98e6eb21@epcas5p2.samsung.com> @ 2024-12-11 6:44 ` Nitesh Shetty 0 siblings, 0 replies; 16+ messages in thread From: Nitesh Shetty @ 2024-12-11 6:44 UTC (permalink / raw) To: Keith Busch Cc: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring, sagi, asml.silence, anuj20.g, joshi.k, Keith Busch, Hannes Reinecke [-- Attachment #1: Type: text/plain, Size: 371 bytes --] On 10/12/24 11:47AM, Keith Busch wrote: >From: Keith Busch <[email protected]> > >Allow userspace to pass a per-I/O write stream in the SQE: > > __u8 write_stream; > >The __u8 type matches the size the filesystems and block layer support. > >Application can query the supported values from the statx s/statx/sysfs Reviewed-by: Nitesh Shetty <[email protected]> [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv13 10/11] nvme: register fdp parameters with the block layer 2024-12-10 19:47 [PATCHv13 00/11] block write streams with nvme fdp Keith Busch 2024-12-10 19:47 ` [PATCHv13 05/11] block: expose write streams for block device nodes Keith Busch 2024-12-10 19:47 ` [PATCHv13 06/11] io_uring: enable per-io write streams Keith Busch @ 2024-12-10 19:47 ` Keith Busch [not found] ` <CGME20241211064906epcas5p3828a664815dd92ac462aa22c3e64d469@epcas5p3.samsung.com> ` (4 more replies) 2024-12-11 7:13 ` [PATCHv13 00/11] block write streams with nvme fdp Christoph Hellwig [not found] ` <[email protected]> 4 siblings, 5 replies; 16+ messages in thread From: Keith Busch @ 2024-12-10 19:47 UTC (permalink / raw) To: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: sagi, asml.silence, anuj20.g, joshi.k, Keith Busch From: Keith Busch <[email protected]> Register the device data placement limits if supported. This is just registering the limits with the block layer. Nothing beyond reporting these attributes is happening in this patch. Merges parts from a patch by Christoph Hellwig <[email protected]> Link: https://lore.kernel.org/linux-nvme/[email protected]/ Signed-off-by: Keith Busch <[email protected]> --- drivers/nvme/host/core.c | 139 +++++++++++++++++++++++++++++++++++++++ drivers/nvme/host/nvme.h | 2 + 2 files changed, 141 insertions(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index c2a3585a3fa59..f7aeda601fcd6 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -38,6 +38,8 @@ struct nvme_ns_info { u32 nsid; __le32 anagrpid; u8 pi_offset; + u16 endgid; + u64 runs; bool is_shared; bool is_readonly; bool is_ready; @@ -1613,6 +1615,7 @@ static int nvme_ns_info_from_identify(struct nvme_ctrl *ctrl, info->is_shared = id->nmic & NVME_NS_NMIC_SHARED; info->is_readonly = id->nsattr & NVME_NS_ATTR_RO; info->is_ready = true; + info->endgid = le16_to_cpu(id->endgid); if (ctrl->quirks & NVME_QUIRK_BOGUS_NID) { dev_info(ctrl->device, "Ignoring bogus Namespace Identifiers\n"); @@ -1653,6 +1656,7 @@ static int nvme_ns_info_from_id_cs_indep(struct nvme_ctrl *ctrl, info->is_ready = id->nstat & NVME_NSTAT_NRDY; info->is_rotational = id->nsfeat & NVME_NS_ROTATIONAL; info->no_vwc = id->nsfeat & NVME_NS_VWC_NOT_PRESENT; + info->endgid = le16_to_cpu(id->endgid); } kfree(id); return ret; @@ -2147,6 +2151,127 @@ static int nvme_update_ns_info_generic(struct nvme_ns *ns, return ret; } +static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl, + struct nvme_ns_info *info, u8 fdp_idx) +{ + struct nvme_fdp_config_log hdr, *h; + struct nvme_fdp_config_desc *desc; + size_t size = sizeof(hdr); + int i, n, ret; + void *log; + + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, + NVME_CSI_NVM, &hdr, size, 0, info->endgid); + if (ret) { + dev_warn(ctrl->device, + "FDP configs log header status:0x%x endgid:%x\n", ret, + info->endgid); + return ret; + } + + size = le32_to_cpu(hdr.sze); + h = kzalloc(size, GFP_KERNEL); + if (!h) { + dev_warn(ctrl->device, + "failed to allocate %lu bytes for FDP config log\n", + size); + return -ENOMEM; + } + + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, + NVME_CSI_NVM, h, size, 0, info->endgid); + if (ret) { + dev_warn(ctrl->device, + "FDP configs log status:0x%x endgid:%x\n", ret, + info->endgid); + goto out; + } + + n = le16_to_cpu(h->numfdpc) + 1; + if (fdp_idx > n) { + dev_warn(ctrl->device, "FDP index:%d out of range:%d\n", + fdp_idx, n); + /* Proceed without registering FDP streams */ + ret = 0; + goto out; + } + + log = h + 1; + desc = log; + for (i = 0; i < fdp_idx; i++) { + log += le16_to_cpu(desc->dsze); + desc = log; + } + + if (le32_to_cpu(desc->nrg) > 1) { + dev_warn(ctrl->device, "FDP NRG > 1 not supported\n"); + ret = 0; + goto out; + } + + info->runs = le64_to_cpu(desc->runs); +out: + kfree(h); + return ret; +} + +static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info) +{ + struct nvme_ns_head *head = ns->head; + struct nvme_ctrl *ctrl = ns->ctrl; + struct nvme_fdp_ruh_status *ruhs; + struct nvme_fdp_config fdp; + struct nvme_command c = {}; + size_t size; + int ret; + + /* + * The FDP configuration is static for the lifetime of the namespace, + * so return immediately if we've already registered this namespaces's + * streams. + */ + if (head->nr_plids) + return 0; + + ret = nvme_get_features(ctrl, NVME_FEAT_FDP, info->endgid, NULL, 0, + &fdp); + if (ret) { + dev_warn(ctrl->device, "FDP get feature status:0x%x\n", ret); + return ret; + } + + if (!(fdp.flags & FDPCFG_FDPE)) + return 0; + + ret = nvme_query_fdp_granularity(ctrl, info, fdp.fdpcidx); + if (!info->runs) + return ret; + + size = struct_size(ruhs, ruhsd, S8_MAX - 1); + ruhs = kzalloc(size, GFP_KERNEL); + if (!ruhs) { + dev_warn(ctrl->device, + "failed to allocate %lu bytes for FDP io-mgmt\n", + size); + return -ENOMEM; + } + + c.imr.opcode = nvme_cmd_io_mgmt_recv; + c.imr.nsid = cpu_to_le32(head->ns_id); + c.imr.mo = NVME_IO_MGMT_RECV_MO_RUHS; + c.imr.numd = cpu_to_le32(nvme_bytes_to_numd(size)); + ret = nvme_submit_sync_cmd(ns->queue, &c, ruhs, size); + if (ret) { + dev_warn(ctrl->device, "FDP io-mgmt status:0x%x\n", ret); + goto free; + } + + head->nr_plids = le16_to_cpu(ruhs->nruhsd); +free: + kfree(ruhs); + return ret; +} + static int nvme_update_ns_info_block(struct nvme_ns *ns, struct nvme_ns_info *info) { @@ -2183,6 +2308,12 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns, goto out; } + if (ns->ctrl->ctratt & NVME_CTRL_ATTR_FDPS) { + ret = nvme_query_fdp_info(ns, info); + if (ret < 0) + goto out; + } + blk_mq_freeze_queue(ns->disk->queue); ns->head->lba_shift = id->lbaf[lbaf].ds; ns->head->nuse = le64_to_cpu(id->nuse); @@ -2216,6 +2347,12 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns, if (!nvme_init_integrity(ns->head, &lim, info)) capacity = 0; + lim.max_write_streams = ns->head->nr_plids; + if (lim.max_write_streams) + lim.write_stream_granularity = max(info->runs, U32_MAX); + else + lim.write_stream_granularity = 0; + ret = queue_limits_commit_update(ns->disk->queue, &lim); if (ret) { blk_mq_unfreeze_queue(ns->disk->queue); @@ -2318,6 +2455,8 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_ns_info *info) ns->head->disk->flags |= GENHD_FL_HIDDEN; else nvme_init_integrity(ns->head, &lim, info); + lim.max_write_streams = ns_lim->max_write_streams; + lim.write_stream_granularity = ns_lim->write_stream_granularity; ret = queue_limits_commit_update(ns->head->disk->queue, &lim); set_capacity_and_notify(ns->head->disk, get_capacity(ns->disk)); diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index c1995d89ffdb8..4b412cd8001f1 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -491,6 +491,8 @@ struct nvme_ns_head { struct device cdev_device; struct gendisk *disk; + + u16 nr_plids; #ifdef CONFIG_NVME_MULTIPATH struct bio_list requeue_list; spinlock_t requeue_lock; -- 2.43.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
[parent not found: <CGME20241211064906epcas5p3828a664815dd92ac462aa22c3e64d469@epcas5p3.samsung.com>]
* Re: [PATCHv13 10/11] nvme: register fdp parameters with the block layer [not found] ` <CGME20241211064906epcas5p3828a664815dd92ac462aa22c3e64d469@epcas5p3.samsung.com> @ 2024-12-11 6:41 ` Nitesh Shetty 0 siblings, 0 replies; 16+ messages in thread From: Nitesh Shetty @ 2024-12-11 6:41 UTC (permalink / raw) To: Keith Busch Cc: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring, sagi, asml.silence, anuj20.g, joshi.k, Keith Busch [-- Attachment #1: Type: text/plain, Size: 1008 bytes --] On 10/12/24 11:47AM, Keith Busch wrote: >From: Keith Busch <[email protected]> > >Register the device data placement limits if supported. This is just >registering the limits with the block layer. Nothing beyond reporting >these attributes is happening in this patch. > >Merges parts from a patch by Christoph Hellwig <[email protected]> >Link: https://lore.kernel.org/linux-nvme/[email protected]/ > >Signed-off-by: Keith Busch <[email protected]> >--- > drivers/nvme/host/core.c | 139 +++++++++++++++++++++++++++++++++++++++ > drivers/nvme/host/nvme.h | 2 + > 2 files changed, 141 insertions(+) > >diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c >index c2a3585a3fa59..f7aeda601fcd6 100644 >--- a/drivers/nvme/host/core.c >+++ b/drivers/nvme/host/core.c >+ /* >+ * The FDP configuration is static for the lifetime of the namespace, >+ * so return immediately if we've already registered this namespaces's Nit: namespace's Reviewed-by: Nitesh Shetty <[email protected]> [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 10/11] nvme: register fdp parameters with the block layer 2024-12-10 19:47 ` [PATCHv13 10/11] nvme: register fdp parameters with the block layer Keith Busch [not found] ` <CGME20241211064906epcas5p3828a664815dd92ac462aa22c3e64d469@epcas5p3.samsung.com> @ 2024-12-11 9:30 ` John Garry 2024-12-11 15:55 ` Keith Busch 2024-12-11 11:23 ` Hannes Reinecke ` (2 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: John Garry @ 2024-12-11 9:30 UTC (permalink / raw) To: Keith Busch, axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: sagi, asml.silence, anuj20.g, joshi.k, Keith Busch On 10/12/2024 19:47, Keith Busch wrote: > > +static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl, > + struct nvme_ns_info *info, u8 fdp_idx) > +{ > + struct nvme_fdp_config_log hdr, *h; > + struct nvme_fdp_config_desc *desc; > + size_t size = sizeof(hdr); > + int i, n, ret; > + void *log; > + > + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, > + NVME_CSI_NVM, &hdr, size, 0, info->endgid); > + if (ret) { > + dev_warn(ctrl->device, > + "FDP configs log header status:0x%x endgid:%x\n", ret, > + info->endgid); About endgid, I guess that there is a good reason but sometimes "0x" is prefixed for hex prints and sometimes not. Maybe no prefix is used when we know that the variable is to hold a value from a HW register / memory structure - I don't know. further nitpicking: And ret holds a kernel error code - the driver seems inconsistent for printing this. Sometimes it's %d and sometimes 0x%x. > + return ret; > + } > + > + size = le32_to_cpu(hdr.sze); > + h = kzalloc(size, GFP_KERNEL); > + if (!h) { > + dev_warn(ctrl->device, > + "failed to allocate %lu bytes for FDP config log\n", > + size); do we normally print ENOMEM messages? I see that the bytes is printed, but I assume that this is a sane value (of little note). > + return -ENOMEM; > + } > + > + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, > + NVME_CSI_NVM, h, size, 0, info->endgid); > + if (ret) { > + dev_warn(ctrl->device, > + "FDP configs log status:0x%x endgid:%x\n", ret, > + info->endgid); > + goto out; > + } > + > + n = le16_to_cpu(h->numfdpc) + 1; > + if (fdp_idx > n) { > + dev_warn(ctrl->device, "FDP index:%d out of range:%d\n", > + fdp_idx, n); > + /* Proceed without registering FDP streams */> + ret = 0; nit: maybe you want to be explicit, but logically ret is already 0 > + goto out; > + } > + > + log = h + 1; > + desc = log; > + for (i = 0; i < fdp_idx; i++) { > + log += le16_to_cpu(desc->dsze); > + desc = log; > + } > + > + if (le32_to_cpu(desc->nrg) > 1) { > + dev_warn(ctrl->device, "FDP NRG > 1 not supported\n"); > + ret = 0; Same here > + goto out; > + } > + > + info->runs = le64_to_cpu(desc->runs); > +out: > + kfree(h); > + return ret; > +} ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 10/11] nvme: register fdp parameters with the block layer 2024-12-11 9:30 ` John Garry @ 2024-12-11 15:55 ` Keith Busch 0 siblings, 0 replies; 16+ messages in thread From: Keith Busch @ 2024-12-11 15:55 UTC (permalink / raw) To: John Garry Cc: Keith Busch, axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring, sagi, asml.silence, anuj20.g, joshi.k On Wed, Dec 11, 2024 at 09:30:37AM +0000, John Garry wrote: > On 10/12/2024 19:47, Keith Busch wrote: > > +static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl, > > + struct nvme_ns_info *info, u8 fdp_idx) > > +{ > > + struct nvme_fdp_config_log hdr, *h; > > + struct nvme_fdp_config_desc *desc; > > + size_t size = sizeof(hdr); > > + int i, n, ret; > > + void *log; > > + > > + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, > > + NVME_CSI_NVM, &hdr, size, 0, info->endgid); > > + if (ret) { > > + dev_warn(ctrl->device, > > + "FDP configs log header status:0x%x endgid:%x\n", ret, > > + info->endgid); > > About endgid, I guess that there is a good reason but sometimes "0x" is > prefixed for hex prints and sometimes not. Maybe no prefix is used when we > know that the variable is to hold a value from a HW register / memory > structure - I don't know. %d for endgid is probably a better choice. > further nitpicking: And ret holds a kernel error code - the driver seems > inconsistent for printing this. Sometimes it's %d and sometimes 0x%x. It's either an -errno or an nvme status. "%x" is easier to decode if it's an nvme status, which is probably the more interesting case to debug. > > + return ret; > > + } > > + > > + size = le32_to_cpu(hdr.sze); > > + h = kzalloc(size, GFP_KERNEL); > > + if (!h) { > > + dev_warn(ctrl->device, > > + "failed to allocate %lu bytes for FDP config log\n", > > + size); > > do we normally print ENOMEM messages? I see that the bytes is printed, but I > assume that this is a sane value (of little note). I suppose not. > > + return -ENOMEM; > > + } > > + > > + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, > > + NVME_CSI_NVM, h, size, 0, info->endgid); > > + if (ret) { > > + dev_warn(ctrl->device, > > + "FDP configs log status:0x%x endgid:%x\n", ret, > > + info->endgid); > > + goto out; > > + } > > + > > + n = le16_to_cpu(h->numfdpc) + 1; > > + if (fdp_idx > n) { > > + dev_warn(ctrl->device, "FDP index:%d out of range:%d\n", > > + fdp_idx, n); > > + /* Proceed without registering FDP streams */> + ret = 0; > > nit: maybe you want to be explicit, but logically ret is already 0 Yeah, we know its zero already, but there are static analyisis tools that think returning without setting an error return code was a mistake, and that we really meant to return something else like -EINVAL. We definitely want to return 0 here, so this setting exists only to prevent future "help". ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 10/11] nvme: register fdp parameters with the block layer 2024-12-10 19:47 ` [PATCHv13 10/11] nvme: register fdp parameters with the block layer Keith Busch [not found] ` <CGME20241211064906epcas5p3828a664815dd92ac462aa22c3e64d469@epcas5p3.samsung.com> 2024-12-11 9:30 ` John Garry @ 2024-12-11 11:23 ` Hannes Reinecke 2024-12-11 14:40 ` kernel test robot 2024-12-11 17:18 ` kernel test robot 4 siblings, 0 replies; 16+ messages in thread From: Hannes Reinecke @ 2024-12-11 11:23 UTC (permalink / raw) To: Keith Busch, axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: sagi, asml.silence, anuj20.g, joshi.k, Keith Busch On 12/10/24 20:47, Keith Busch wrote: > From: Keith Busch <[email protected]> > > Register the device data placement limits if supported. This is just > registering the limits with the block layer. Nothing beyond reporting > these attributes is happening in this patch. > > Merges parts from a patch by Christoph Hellwig <[email protected]> > Link: https://lore.kernel.org/linux-nvme/[email protected]/ > > Signed-off-by: Keith Busch <[email protected]> > --- > drivers/nvme/host/core.c | 139 +++++++++++++++++++++++++++++++++++++++ > drivers/nvme/host/nvme.h | 2 + > 2 files changed, 141 insertions(+) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index c2a3585a3fa59..f7aeda601fcd6 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -38,6 +38,8 @@ struct nvme_ns_info { > u32 nsid; > __le32 anagrpid; > u8 pi_offset; > + u16 endgid; > + u64 runs; > bool is_shared; > bool is_readonly; > bool is_ready; > @@ -1613,6 +1615,7 @@ static int nvme_ns_info_from_identify(struct nvme_ctrl *ctrl, > info->is_shared = id->nmic & NVME_NS_NMIC_SHARED; > info->is_readonly = id->nsattr & NVME_NS_ATTR_RO; > info->is_ready = true; > + info->endgid = le16_to_cpu(id->endgid); > if (ctrl->quirks & NVME_QUIRK_BOGUS_NID) { > dev_info(ctrl->device, > "Ignoring bogus Namespace Identifiers\n"); > @@ -1653,6 +1656,7 @@ static int nvme_ns_info_from_id_cs_indep(struct nvme_ctrl *ctrl, > info->is_ready = id->nstat & NVME_NSTAT_NRDY; > info->is_rotational = id->nsfeat & NVME_NS_ROTATIONAL; > info->no_vwc = id->nsfeat & NVME_NS_VWC_NOT_PRESENT; > + info->endgid = le16_to_cpu(id->endgid); > } > kfree(id); > return ret; > @@ -2147,6 +2151,127 @@ static int nvme_update_ns_info_generic(struct nvme_ns *ns, > return ret; > } > > +static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl, > + struct nvme_ns_info *info, u8 fdp_idx) > +{ > + struct nvme_fdp_config_log hdr, *h; > + struct nvme_fdp_config_desc *desc; > + size_t size = sizeof(hdr); > + int i, n, ret; > + void *log; > + > + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, > + NVME_CSI_NVM, &hdr, size, 0, info->endgid); > + if (ret) { > + dev_warn(ctrl->device, > + "FDP configs log header status:0x%x endgid:%x\n", ret, > + info->endgid); > + return ret; > + } > + > + size = le32_to_cpu(hdr.sze); size should be bounded to avoid overly large memory allocations when the header is garbled. > + h = kzalloc(size, GFP_KERNEL); > + if (!h) { > + dev_warn(ctrl->device, > + "failed to allocate %lu bytes for FDP config log\n", > + size); > + return -ENOMEM; > + } > + > + ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, > + NVME_CSI_NVM, h, size, 0, info->endgid); > + if (ret) { > + dev_warn(ctrl->device, > + "FDP configs log status:0x%x endgid:%x\n", ret, > + info->endgid); > + goto out; > + } > + > + n = le16_to_cpu(h->numfdpc) + 1; > + if (fdp_idx > n) { > + dev_warn(ctrl->device, "FDP index:%d out of range:%d\n", > + fdp_idx, n); > + /* Proceed without registering FDP streams */ > + ret = 0; > + goto out; > + } > + > + log = h + 1; > + desc = log; > + for (i = 0; i < fdp_idx; i++) { > + log += le16_to_cpu(desc->dsze); > + desc = log; Check for the size of 'h' to ensure that you are not falling over the end ... Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 10/11] nvme: register fdp parameters with the block layer 2024-12-10 19:47 ` [PATCHv13 10/11] nvme: register fdp parameters with the block layer Keith Busch ` (2 preceding siblings ...) 2024-12-11 11:23 ` Hannes Reinecke @ 2024-12-11 14:40 ` kernel test robot 2024-12-11 17:18 ` kernel test robot 4 siblings, 0 replies; 16+ messages in thread From: kernel test robot @ 2024-12-11 14:40 UTC (permalink / raw) To: Keith Busch, axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: oe-kbuild-all, sagi, asml.silence, anuj20.g, joshi.k, Keith Busch Hi Keith, kernel test robot noticed the following build warnings: [auto build test WARNING on axboe-block/for-next] [also build test WARNING on next-20241211] [cannot apply to brauner-vfs/vfs.all hch-configfs/for-next linus/master v6.13-rc2] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Keith-Busch/fs-add-a-write-stream-field-to-the-kiocb/20241211-080803 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next patch link: https://lore.kernel.org/r/20241210194722.1905732-11-kbusch%40meta.com patch subject: [PATCHv13 10/11] nvme: register fdp parameters with the block layer config: i386-buildonly-randconfig-002-20241211 (https://download.01.org/0day-ci/archive/20241211/[email protected]/config) compiler: gcc-11 (Debian 11.3.0-12) 11.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241211/[email protected]/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <[email protected]> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ All warnings (new ones prefixed by >>): In file included from include/linux/device.h:15, from include/linux/async.h:14, from drivers/nvme/host/core.c:7: drivers/nvme/host/core.c: In function 'nvme_query_fdp_granularity': >> drivers/nvme/host/core.c:2176:26: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=] 2176 | "failed to allocate %lu bytes for FDP config log\n", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt' 156 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~ drivers/nvme/host/core.c:2175:17: note: in expansion of macro 'dev_warn' 2175 | dev_warn(ctrl->device, | ^~~~~~~~ drivers/nvme/host/core.c:2176:48: note: format string is defined here 2176 | "failed to allocate %lu bytes for FDP config log\n", | ~~^ | | | long unsigned int | %u In file included from include/linux/device.h:15, from include/linux/async.h:14, from drivers/nvme/host/core.c:7: drivers/nvme/host/core.c: In function 'nvme_query_fdp_info': drivers/nvme/host/core.c:2254:26: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=] 2254 | "failed to allocate %lu bytes for FDP io-mgmt\n", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt' 156 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~ drivers/nvme/host/core.c:2253:17: note: in expansion of macro 'dev_warn' 2253 | dev_warn(ctrl->device, | ^~~~~~~~ drivers/nvme/host/core.c:2254:48: note: format string is defined here 2254 | "failed to allocate %lu bytes for FDP io-mgmt\n", | ~~^ | | | long unsigned int | %u vim +2176 drivers/nvme/host/core.c 2153 2154 static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl, 2155 struct nvme_ns_info *info, u8 fdp_idx) 2156 { 2157 struct nvme_fdp_config_log hdr, *h; 2158 struct nvme_fdp_config_desc *desc; 2159 size_t size = sizeof(hdr); 2160 int i, n, ret; 2161 void *log; 2162 2163 ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, 2164 NVME_CSI_NVM, &hdr, size, 0, info->endgid); 2165 if (ret) { 2166 dev_warn(ctrl->device, 2167 "FDP configs log header status:0x%x endgid:%x\n", ret, 2168 info->endgid); 2169 return ret; 2170 } 2171 2172 size = le32_to_cpu(hdr.sze); 2173 h = kzalloc(size, GFP_KERNEL); 2174 if (!h) { 2175 dev_warn(ctrl->device, > 2176 "failed to allocate %lu bytes for FDP config log\n", 2177 size); 2178 return -ENOMEM; 2179 } 2180 2181 ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, 2182 NVME_CSI_NVM, h, size, 0, info->endgid); 2183 if (ret) { 2184 dev_warn(ctrl->device, 2185 "FDP configs log status:0x%x endgid:%x\n", ret, 2186 info->endgid); 2187 goto out; 2188 } 2189 2190 n = le16_to_cpu(h->numfdpc) + 1; 2191 if (fdp_idx > n) { 2192 dev_warn(ctrl->device, "FDP index:%d out of range:%d\n", 2193 fdp_idx, n); 2194 /* Proceed without registering FDP streams */ 2195 ret = 0; 2196 goto out; 2197 } 2198 2199 log = h + 1; 2200 desc = log; 2201 for (i = 0; i < fdp_idx; i++) { 2202 log += le16_to_cpu(desc->dsze); 2203 desc = log; 2204 } 2205 2206 if (le32_to_cpu(desc->nrg) > 1) { 2207 dev_warn(ctrl->device, "FDP NRG > 1 not supported\n"); 2208 ret = 0; 2209 goto out; 2210 } 2211 2212 info->runs = le64_to_cpu(desc->runs); 2213 out: 2214 kfree(h); 2215 return ret; 2216 } 2217 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 10/11] nvme: register fdp parameters with the block layer 2024-12-10 19:47 ` [PATCHv13 10/11] nvme: register fdp parameters with the block layer Keith Busch ` (3 preceding siblings ...) 2024-12-11 14:40 ` kernel test robot @ 2024-12-11 17:18 ` kernel test robot 4 siblings, 0 replies; 16+ messages in thread From: kernel test robot @ 2024-12-11 17:18 UTC (permalink / raw) To: Keith Busch, axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: llvm, oe-kbuild-all, sagi, asml.silence, anuj20.g, joshi.k, Keith Busch Hi Keith, kernel test robot noticed the following build warnings: [auto build test WARNING on axboe-block/for-next] [also build test WARNING on next-20241211] [cannot apply to brauner-vfs/vfs.all hch-configfs/for-next linus/master v6.13-rc2] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Keith-Busch/fs-add-a-write-stream-field-to-the-kiocb/20241211-080803 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next patch link: https://lore.kernel.org/r/20241210194722.1905732-11-kbusch%40meta.com patch subject: [PATCHv13 10/11] nvme: register fdp parameters with the block layer config: arm-randconfig-003-20241211 (https://download.01.org/0day-ci/archive/20241212/[email protected]/config) compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 2dc22615fd46ab2566d0f26d5ba234ab12dc4bf8) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241212/[email protected]/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <[email protected]> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ All warnings (new ones prefixed by >>): In file included from drivers/nvme/host/core.c:8: In file included from include/linux/blkdev.h:9: In file included from include/linux/blk_types.h:10: In file included from include/linux/bvec.h:10: In file included from include/linux/highmem.h:8: In file included from include/linux/cacheflush.h:5: In file included from arch/arm/include/asm/cacheflush.h:10: In file included from include/linux/mm.h:2223: include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion] 518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_" | ~~~~~~~~~~~ ^ ~~~ >> drivers/nvme/host/core.c:2177:5: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat] 2176 | "failed to allocate %lu bytes for FDP config log\n", | ~~~ | %zu 2177 | size); | ^~~~ include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn' 156 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__) | ~~~ ^~~~~~~~~~~ include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ~~~ ^~~~~~~~~~~ drivers/nvme/host/core.c:2255:5: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat] 2254 | "failed to allocate %lu bytes for FDP io-mgmt\n", | ~~~ | %zu 2255 | size); | ^~~~ include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn' 156 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__) | ~~~ ^~~~~~~~~~~ include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ~~~ ^~~~~~~~~~~ 3 warnings generated. vim +2177 drivers/nvme/host/core.c 2153 2154 static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl, 2155 struct nvme_ns_info *info, u8 fdp_idx) 2156 { 2157 struct nvme_fdp_config_log hdr, *h; 2158 struct nvme_fdp_config_desc *desc; 2159 size_t size = sizeof(hdr); 2160 int i, n, ret; 2161 void *log; 2162 2163 ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, 2164 NVME_CSI_NVM, &hdr, size, 0, info->endgid); 2165 if (ret) { 2166 dev_warn(ctrl->device, 2167 "FDP configs log header status:0x%x endgid:%x\n", ret, 2168 info->endgid); 2169 return ret; 2170 } 2171 2172 size = le32_to_cpu(hdr.sze); 2173 h = kzalloc(size, GFP_KERNEL); 2174 if (!h) { 2175 dev_warn(ctrl->device, 2176 "failed to allocate %lu bytes for FDP config log\n", > 2177 size); 2178 return -ENOMEM; 2179 } 2180 2181 ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, 2182 NVME_CSI_NVM, h, size, 0, info->endgid); 2183 if (ret) { 2184 dev_warn(ctrl->device, 2185 "FDP configs log status:0x%x endgid:%x\n", ret, 2186 info->endgid); 2187 goto out; 2188 } 2189 2190 n = le16_to_cpu(h->numfdpc) + 1; 2191 if (fdp_idx > n) { 2192 dev_warn(ctrl->device, "FDP index:%d out of range:%d\n", 2193 fdp_idx, n); 2194 /* Proceed without registering FDP streams */ 2195 ret = 0; 2196 goto out; 2197 } 2198 2199 log = h + 1; 2200 desc = log; 2201 for (i = 0; i < fdp_idx; i++) { 2202 log += le16_to_cpu(desc->dsze); 2203 desc = log; 2204 } 2205 2206 if (le32_to_cpu(desc->nrg) > 1) { 2207 dev_warn(ctrl->device, "FDP NRG > 1 not supported\n"); 2208 ret = 0; 2209 goto out; 2210 } 2211 2212 info->runs = le64_to_cpu(desc->runs); 2213 out: 2214 kfree(h); 2215 return ret; 2216 } 2217 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 00/11] block write streams with nvme fdp 2024-12-10 19:47 [PATCHv13 00/11] block write streams with nvme fdp Keith Busch ` (2 preceding siblings ...) 2024-12-10 19:47 ` [PATCHv13 10/11] nvme: register fdp parameters with the block layer Keith Busch @ 2024-12-11 7:13 ` Christoph Hellwig 2024-12-12 5:51 ` Kanchan Joshi [not found] ` <[email protected]> 4 siblings, 1 reply; 16+ messages in thread From: Christoph Hellwig @ 2024-12-11 7:13 UTC (permalink / raw) To: Keith Busch Cc: axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring, sagi, asml.silence, anuj20.g, joshi.k, Keith Busch On Tue, Dec 10, 2024 at 11:47:11AM -0800, Keith Busch wrote: > Changes from v12: The changes looks good to me. I'll ty to send out an API for querying the paramters in the next so that we don't merge the granularity as dead code, but I'll need a bit more time to also write proper tests and stuff. For that it probably makes sense to expose these streams using null_blk so that we can actually have a block test. If someone feels like speeding things up I'd also be happy for someone else to take that work. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 00/11] block write streams with nvme fdp 2024-12-11 7:13 ` [PATCHv13 00/11] block write streams with nvme fdp Christoph Hellwig @ 2024-12-12 5:51 ` Kanchan Joshi 2024-12-12 6:05 ` Christoph Hellwig 0 siblings, 1 reply; 16+ messages in thread From: Kanchan Joshi @ 2024-12-12 5:51 UTC (permalink / raw) To: Christoph Hellwig, Keith Busch Cc: axboe, linux-block, linux-nvme, linux-fsdevel, io-uring, sagi, asml.silence, anuj20.g, Keith Busch On 12/11/2024 12:43 PM, Christoph Hellwig wrote: > The changes looks good to me. I'll ty to send out an API for querying > the paramters in the next so that we don't merge the granularity as dead > code What do you have in mind for this API. New fcntl or ioctl? With ability limited to querying only or.... ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv13 00/11] block write streams with nvme fdp 2024-12-12 5:51 ` Kanchan Joshi @ 2024-12-12 6:05 ` Christoph Hellwig 0 siblings, 0 replies; 16+ messages in thread From: Christoph Hellwig @ 2024-12-12 6:05 UTC (permalink / raw) To: Kanchan Joshi Cc: Christoph Hellwig, Keith Busch, axboe, linux-block, linux-nvme, linux-fsdevel, io-uring, sagi, asml.silence, anuj20.g, Keith Busch On Thu, Dec 12, 2024 at 11:21:07AM +0530, Kanchan Joshi wrote: > On 12/11/2024 12:43 PM, Christoph Hellwig wrote: > > The changes looks good to me. I'll ty to send out an API for querying > > the paramters in the next so that we don't merge the granularity as dead > > code > > What do you have in mind for this API. New fcntl or ioctl? > With ability limited to querying only or.... Yes, new fcntl or ioctl, query the number of streams and (nominal) stream granularity as a start. There is a few other things that might be useful: - check if the size is just nominal or not, aka if the device can shorten it. FDP currently allows for that, but given that notifications for that are out of bounds it makes a complete mess of software actually trying to use it for more than simple hot/cold separation like cachelib, so we find a way to query that in the spec. - reporting of the remaining capacity per stream, although I'm not sure if that should be in the same ioctl/fcntl, or better done using a separate interface that has the stream number as input and the capacity as out, which would seem a lot simpler ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <[email protected]>]
* Re: [PATCHv13 04/11] block: introduce a write_stream_granularity queue limit [not found] ` <[email protected]> @ 2024-12-11 8:46 ` John Garry 0 siblings, 0 replies; 16+ messages in thread From: John Garry @ 2024-12-11 8:46 UTC (permalink / raw) To: Keith Busch, axboe, hch, linux-block, linux-nvme, linux-fsdevel, io-uring Cc: sagi, asml.silence, anuj20.g, joshi.k, Hannes Reinecke, Nitesh Shetty, Keith Busch > > unsigned int max_open_zones; > unsigned int max_active_zones; > @@ -1249,6 +1250,12 @@ static inline unsigned short bdev_max_write_streams(struct block_device *bdev) > return bdev_limits(bdev)->max_write_streams; > } > > +static inline unsigned int > +bdev_write_stream_granularity(struct block_device *bdev) is this referenced anywhere? I see that Nitesh mentioned something similar also previously > +{ > + return bdev_limits(bdev)->write_stream_granularity; > +} > + > static inline unsigned queue_logical_block_size(const struct request_queue *q) > { > return q->limits.logical_block_size; ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-12-12 6:06 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-10 19:47 [PATCHv13 00/11] block write streams with nvme fdp Keith Busch
2024-12-10 19:47 ` [PATCHv13 05/11] block: expose write streams for block device nodes Keith Busch
[not found] ` <CGME20241211060149epcas5p4d16e78bd3d184e57465c3622a2c8e98d@epcas5p4.samsung.com>
2024-12-11 5:53 ` Nitesh Shetty
2024-12-10 19:47 ` [PATCHv13 06/11] io_uring: enable per-io write streams Keith Busch
[not found] ` <CGME20241211065235epcas5p2a233ac902da67bf2d755d21d98e6eb21@epcas5p2.samsung.com>
2024-12-11 6:44 ` Nitesh Shetty
2024-12-10 19:47 ` [PATCHv13 10/11] nvme: register fdp parameters with the block layer Keith Busch
[not found] ` <CGME20241211064906epcas5p3828a664815dd92ac462aa22c3e64d469@epcas5p3.samsung.com>
2024-12-11 6:41 ` Nitesh Shetty
2024-12-11 9:30 ` John Garry
2024-12-11 15:55 ` Keith Busch
2024-12-11 11:23 ` Hannes Reinecke
2024-12-11 14:40 ` kernel test robot
2024-12-11 17:18 ` kernel test robot
2024-12-11 7:13 ` [PATCHv13 00/11] block write streams with nvme fdp Christoph Hellwig
2024-12-12 5:51 ` Kanchan Joshi
2024-12-12 6:05 ` Christoph Hellwig
[not found] ` <[email protected]>
2024-12-11 8:46 ` [PATCHv13 04/11] block: introduce a write_stream_granularity queue limit John Garry
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox