From 80d43cbd46155744ee450d2476ee4fcf2917ae9b Mon Sep 17 00:00:00 2001 From: Md Haris Iqbal Date: Wed, 28 Apr 2021 08:13:56 +0200 Subject: block/rnbd-clt: Change queue_depth type in rnbd_clt_session to size_t The member queue_depth in the structure rnbd_clt_session is read from the rtrs client side using the function rtrs_clt_query, which in turn is read from the rtrs_clt structure. It should really be of type size_t. Fixes: 90426e89f54db ("block/rnbd: client: private header with client structs and functions") Signed-off-by: Md Haris Iqbal Reviewed-by: Guoqing Jiang Signed-off-by: Gioh Kim Link: https://lore.kernel.org/r/20210428061359.206794-2-gi-oh.kim@ionos.com Signed-off-by: Jens Axboe --- drivers/block/rnbd/rnbd-clt.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h index 451e7383738f..b5322c5aaac0 100644 --- a/drivers/block/rnbd/rnbd-clt.h +++ b/drivers/block/rnbd/rnbd-clt.h @@ -87,7 +87,7 @@ struct rnbd_clt_session { DECLARE_BITMAP(cpu_queues_bm, NR_CPUS); int __percpu *cpu_rr; /* per-cpu var for CPU round-robin */ atomic_t busy; - int queue_depth; + size_t queue_depth; u32 max_io_size; struct blk_mq_tag_set tag_set; u32 nr_poll_queues; -- cgit v1.2.3 From 292660fa35e8917a78235d39722edf9bbc04cab7 Mon Sep 17 00:00:00 2001 From: Dima Stepanov Date: Wed, 28 Apr 2021 08:13:57 +0200 Subject: block/rnbd: Fix style issues This patch fixes some style issues detected by scripts/checkpatch.pl * Resolve spacing and tab issues * Remove extra braces in rnbd_get_iu * Use num_possible_cpus() instead of NR_CPUS in alloc_sess * Fix the comments styling in rnbd_queue_rq Signed-off-by: Dima Stepanov Signed-off-by: Gioh Kim Signed-off-by: Md Haris Iqbal Signed-off-by: Jack Wang Link: https://lore.kernel.org/r/20210428061359.206794-3-gi-oh.kim@ionos.com Signed-off-by: Jens Axboe --- drivers/block/rnbd/rnbd-clt.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c index c01786afe1b1..15f831159c31 100644 --- a/drivers/block/rnbd/rnbd-clt.c +++ b/drivers/block/rnbd/rnbd-clt.c @@ -88,7 +88,7 @@ static int rnbd_clt_set_dev_attr(struct rnbd_clt_dev *dev, dev->discard_alignment = le32_to_cpu(rsp->discard_alignment); dev->secure_discard = le16_to_cpu(rsp->secure_discard); dev->rotational = rsp->rotational; - dev->wc = !!(rsp->cache_policy & RNBD_WRITEBACK); + dev->wc = !!(rsp->cache_policy & RNBD_WRITEBACK); dev->fua = !!(rsp->cache_policy & RNBD_FUA); dev->max_hw_sectors = sess->max_io_size / SECTOR_SIZE; @@ -351,9 +351,8 @@ static struct rnbd_iu *rnbd_get_iu(struct rnbd_clt_session *sess, struct rtrs_permit *permit; iu = kzalloc(sizeof(*iu), GFP_KERNEL); - if (!iu) { + if (!iu) return NULL; - } permit = rnbd_get_permit(sess, con_type, wait); if (unlikely(!permit)) { @@ -805,7 +804,7 @@ static struct rnbd_clt_session *alloc_sess(const char *sessname) mutex_init(&sess->lock); INIT_LIST_HEAD(&sess->devs_list); INIT_LIST_HEAD(&sess->list); - bitmap_zero(sess->cpu_queues_bm, NR_CPUS); + bitmap_zero(sess->cpu_queues_bm, num_possible_cpus()); init_waitqueue_head(&sess->rtrs_waitq); refcount_set(&sess->refcount, 1); @@ -1148,7 +1147,8 @@ static blk_status_t rnbd_queue_rq(struct blk_mq_hw_ctx *hctx, iu->sgt.sgl = iu->first_sgl; err = sg_alloc_table_chained(&iu->sgt, /* Even-if the request has no segment, - * sglist must have one entry at least */ + * sglist must have one entry at least. + */ blk_rq_nr_phys_segments(rq) ? : 1, iu->sgt.sgl, RNBD_INLINE_SG_CNT); -- cgit v1.2.3 From 1056ad829ec43f9b705b507c2093b05e2088b0b7 Mon Sep 17 00:00:00 2001 From: Md Haris Iqbal Date: Wed, 28 Apr 2021 08:13:58 +0200 Subject: block/rnbd-clt: Check the return value of the function rtrs_clt_query In case none of the paths are in connected state, the function rtrs_clt_query returns an error. In such a case, error out since the values in the rtrs_attrs structure would be garbage. Fixes: f7a7a5c228d45 ("block/rnbd: client: main functionality") Signed-off-by: Md Haris Iqbal Reviewed-by: Guoqing Jiang Signed-off-by: Jack Wang Signed-off-by: Gioh Kim Link: https://lore.kernel.org/r/20210428061359.206794-4-gi-oh.kim@ionos.com Signed-off-by: Jens Axboe --- drivers/block/rnbd/rnbd-clt.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c index 15f831159c31..f855bf1fa8d5 100644 --- a/drivers/block/rnbd/rnbd-clt.c +++ b/drivers/block/rnbd/rnbd-clt.c @@ -691,7 +691,11 @@ static void remap_devs(struct rnbd_clt_session *sess) return; } - rtrs_clt_query(sess->rtrs, &attrs); + err = rtrs_clt_query(sess->rtrs, &attrs); + if (err) { + pr_err("rtrs_clt_query(\"%s\"): %d\n", sess->sessname, err); + return; + } mutex_lock(&sess->lock); sess->max_io_size = attrs.max_io_size; @@ -1294,7 +1298,11 @@ find_and_get_or_create_sess(const char *sessname, err = PTR_ERR(sess->rtrs); goto wake_up_and_put; } - rtrs_clt_query(sess->rtrs, &attrs); + + err = rtrs_clt_query(sess->rtrs, &attrs); + if (err) + goto close_rtrs; + sess->max_io_size = attrs.max_io_size; sess->queue_depth = attrs.queue_depth; sess->nr_poll_queues = nr_poll_queues; -- cgit v1.2.3 From 1e31016b6926c996e9113619c2ce1f42ad74ddd1 Mon Sep 17 00:00:00 2001 From: Gioh Kim Date: Wed, 28 Apr 2021 08:13:59 +0200 Subject: block/rnbd: Remove all likely and unlikely The IO performance test with fio after removing the likely and unlikely macros in all if-statement shows no performance drop. They do not help for the performance of rnbd. The fio test did random read on 32 rnbd devices and 64 processes. Test environment: - AMD Opteron(tm) Processor 6386 SE - 125G memory - kernel version: 5.4.86 - gcc version: gcc (Debian 8.3.0-6) 8.3.0 - Infiniband controller: InfiniBand: Mellanox Technologies MT26428 [ConnectX VPI PCIe 2.0 5GT/s - IB QDR / 10GigE] (rev b0) before read: IOPS=549k, BW=2146MiB/s read: IOPS=544k, BW=2125MiB/s read: IOPS=553k, BW=2158MiB/s read: IOPS=535k, BW=2089MiB/s read: IOPS=543k, BW=2122MiB/s read: IOPS=552k, BW=2154MiB/s average: IOPS=546k, BW=2132MiB/s after read: IOPS=556k, BW=2172MiB/s read: IOPS=561k, BW=2191MiB/s read: IOPS=552k, BW=2156MiB/s read: IOPS=551k, BW=2154MiB/s read: IOPS=562k, BW=2194MiB/s ----------- average: IOPS=556k, BW=2173MiB/s The IOPS and bandwidth got better slightly after removing likely/unlikely. (IOPS= +1.8% BW= +1.9%) But we cannot make sure that removing the likely/unlikely help the performance because it depends on various situations. We only make sure that removing the likely/unlikely does not drop the performance. Signed-off-by: Gioh Kim Reviewed-by: Md Haris Iqbal Link: https://lore.kernel.org/r/20210428061359.206794-5-gi-oh.kim@ionos.com Signed-off-by: Jens Axboe --- drivers/block/rnbd/rnbd-clt.c | 24 ++++++++++++------------ drivers/block/rnbd/rnbd-srv.c | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c index f855bf1fa8d5..c604a402cd5c 100644 --- a/drivers/block/rnbd/rnbd-clt.c +++ b/drivers/block/rnbd/rnbd-clt.c @@ -241,7 +241,7 @@ static bool rnbd_rerun_if_needed(struct rnbd_clt_session *sess) cpu_q = rnbd_get_cpu_qlist(sess, nxt_cpu(cpu_q->cpu))) { if (!spin_trylock_irqsave(&cpu_q->requeue_lock, flags)) continue; - if (unlikely(!test_bit(cpu_q->cpu, sess->cpu_queues_bm))) + if (!test_bit(cpu_q->cpu, sess->cpu_queues_bm)) goto unlock; q = list_first_entry_or_null(&cpu_q->requeue_list, typeof(*q), requeue_list); @@ -320,7 +320,7 @@ static struct rtrs_permit *rnbd_get_permit(struct rnbd_clt_session *sess, struct rtrs_permit *permit; permit = rtrs_clt_get_permit(sess->rtrs, con_type, wait); - if (likely(permit)) + if (permit) /* We have a subtle rare case here, when all permits can be * consumed before busy counter increased. This is safe, * because loser will get NULL as a permit, observe 0 busy @@ -355,7 +355,7 @@ static struct rnbd_iu *rnbd_get_iu(struct rnbd_clt_session *sess, return NULL; permit = rnbd_get_permit(sess, con_type, wait); - if (unlikely(!permit)) { + if (!permit) { kfree(iu); return NULL; } @@ -1050,7 +1050,7 @@ static int rnbd_client_xfer_request(struct rnbd_clt_dev *dev, }; err = rtrs_clt_request(rq_data_dir(rq), &req_ops, rtrs, permit, &vec, 1, size, iu->sgt.sgl, sg_cnt); - if (unlikely(err)) { + if (err) { rnbd_clt_err_rl(dev, "RTRS failed to transfer IO, err: %d\n", err); return err; @@ -1081,7 +1081,7 @@ static bool rnbd_clt_dev_add_to_requeue(struct rnbd_clt_dev *dev, cpu_q = get_cpu_ptr(sess->cpu_queues); spin_lock_irqsave(&cpu_q->requeue_lock, flags); - if (likely(!test_and_set_bit_lock(0, &q->in_list))) { + if (!test_and_set_bit_lock(0, &q->in_list)) { if (WARN_ON(!list_empty(&q->requeue_list))) goto unlock; @@ -1093,7 +1093,7 @@ static bool rnbd_clt_dev_add_to_requeue(struct rnbd_clt_dev *dev, */ smp_mb__before_atomic(); } - if (likely(atomic_read(&sess->busy))) { + if (atomic_read(&sess->busy)) { list_add_tail(&q->requeue_list, &cpu_q->requeue_list); } else { /* Very unlikely, but possible: busy counter was @@ -1121,7 +1121,7 @@ static void rnbd_clt_dev_kick_mq_queue(struct rnbd_clt_dev *dev, if (delay != RNBD_DELAY_IFBUSY) blk_mq_delay_run_hw_queue(hctx, delay); - else if (unlikely(!rnbd_clt_dev_add_to_requeue(dev, q))) + else if (!rnbd_clt_dev_add_to_requeue(dev, q)) /* * If session is not busy we have to restart * the queue ourselves. @@ -1138,12 +1138,12 @@ static blk_status_t rnbd_queue_rq(struct blk_mq_hw_ctx *hctx, int err; blk_status_t ret = BLK_STS_IOERR; - if (unlikely(dev->dev_state != DEV_STATE_MAPPED)) + if (dev->dev_state != DEV_STATE_MAPPED) return BLK_STS_IOERR; iu->permit = rnbd_get_permit(dev->sess, RTRS_IO_CON, RTRS_PERMIT_NOWAIT); - if (unlikely(!iu->permit)) { + if (!iu->permit) { rnbd_clt_dev_kick_mq_queue(dev, hctx, RNBD_DELAY_IFBUSY); return BLK_STS_RESOURCE; } @@ -1165,9 +1165,9 @@ static blk_status_t rnbd_queue_rq(struct blk_mq_hw_ctx *hctx, blk_mq_start_request(rq); err = rnbd_client_xfer_request(dev, rq, iu); - if (likely(err == 0)) + if (err == 0) return BLK_STS_OK; - if (unlikely(err == -EAGAIN || err == -ENOMEM)) { + if (err == -EAGAIN || err == -ENOMEM) { rnbd_clt_dev_kick_mq_queue(dev, hctx, 10/*ms*/); ret = BLK_STS_RESOURCE; } @@ -1584,7 +1584,7 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname, struct rnbd_clt_dev *dev; int ret; - if (unlikely(exists_devpath(pathname, sessname))) + if (exists_devpath(pathname, sessname)) return ERR_PTR(-EEXIST); sess = find_and_get_or_create_sess(sessname, paths, path_cnt, port_nr, nr_poll_queues); diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c index 899dd9d7c10b..aafecfe97055 100644 --- a/drivers/block/rnbd/rnbd-srv.c +++ b/drivers/block/rnbd/rnbd-srv.c @@ -104,7 +104,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session *srv_sess) rcu_read_lock(); sess_dev = xa_load(&srv_sess->index_idr, dev_id); - if (likely(sess_dev)) + if (sess_dev) ret = kref_get_unless_zero(&sess_dev->kref); rcu_read_unlock(); -- cgit v1.2.3 From be4f361d69f4487ab56eb67b0cd0559fb1895af2 Mon Sep 17 00:00:00 2001 From: Bhaskar Chowdhury Date: Wed, 28 Apr 2021 17:35:21 +0200 Subject: s390: dasd: Mundane spelling fixes s/Subssystem/Subsystem/ ......two different places s/reportet/reported/ s/managemnet/management/ Signed-off-by: Bhaskar Chowdhury Acked-by: Randy Dunlap Signed-off-by: Stefan Haberland Link: https://lore.kernel.org/r/20210428153521.2050899-2-sth@linux.ibm.com Signed-off-by: Jens Axboe --- drivers/s390/block/dasd_eckd.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/s390/block/dasd_eckd.h b/drivers/s390/block/dasd_eckd.h index ca24a78a256e..73651211789f 100644 --- a/drivers/s390/block/dasd_eckd.h +++ b/drivers/s390/block/dasd_eckd.h @@ -52,7 +52,7 @@ #define DASD_ECKD_CCW_RCD 0xFA #define DASD_ECKD_CCW_DSO 0xF7 -/* Define Subssystem Function / Orders */ +/* Define Subsystem Function / Orders */ #define DSO_ORDER_RAS 0x81 /* @@ -110,7 +110,7 @@ #define DASD_ECKD_PG_GROUPED 0x10 /* - * Size that is reportet for large volumes in the old 16-bit no_cyl field + * Size that is reported for large volumes in the old 16-bit no_cyl field */ #define LV_COMPAT_CYL 0xFFFE @@ -555,7 +555,7 @@ struct dasd_dso_ras_ext_range { } __packed; /* - * Define Subsytem Operation - Release Allocated Space + * Define Subsystem Operation - Release Allocated Space */ struct dasd_dso_ras_data { __u8 order; @@ -676,7 +676,7 @@ struct dasd_eckd_private { struct dasd_ext_pool_sum eps; u32 real_cyl; - /* alias managemnet */ + /* alias management */ struct dasd_uid uid; struct alias_pav_group *pavgroup; struct alias_lcu *lcu; -- cgit v1.2.3 From c646790a1fcae7738972accc41ccaa4983e5c234 Mon Sep 17 00:00:00 2001 From: Gioh Kim Date: Thu, 29 Apr 2021 11:27:41 +0200 Subject: RDMA/rtrs: fix uninitialized symbol 'cnt' rtrs_clt_rdma_cq_direct returns an ninitialized value in cnt if there is no session. This patch makes rtrs_clt_rdma_cq_direct returns a negative value for block layer not to try again. Fixes: 2958a995edc94 ("block/rnbd-clt: Support polling mode for IO latency optimization") Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Gioh Kim Signed-off-by: Jack Wang Link: https://lore.kernel.org/r/20210429092741.266533-1-gi-oh.kim@ionos.com Signed-off-by: Jens Axboe --- drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c index b74a872387c4..934a2ff18e7f 100644 --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c @@ -2896,7 +2896,8 @@ EXPORT_SYMBOL(rtrs_clt_request); int rtrs_clt_rdma_cq_direct(struct rtrs_clt *clt, unsigned int index) { - int cnt; + /* If no path, return -1 for block layer not to try again */ + int cnt = -1; struct rtrs_con *con; struct rtrs_clt_sess *sess; struct path_it it; -- cgit v1.2.3 From cd2c7545ae1beac3b6aae033c7f31193b3255946 Mon Sep 17 00:00:00 2001 From: Changheun Lee Date: Mon, 3 May 2021 18:52:03 +0900 Subject: bio: limit bio max size bio size can grow up to 4GB when muli-page bvec is enabled. but sometimes it would lead to inefficient behaviors. in case of large chunk direct I/O, - 32MB chunk read in user space - all pages for 32MB would be merged to a bio structure if the pages physical addresses are contiguous. it makes some delay to submit until merge complete. bio max size should be limited to a proper size. When 32MB chunk read with direct I/O option is coming from userspace, kernel behavior is below now in do_direct_IO() loop. it's timeline. | bio merge for 32MB. total 8,192 pages are merged. | total elapsed time is over 2ms. |------------------ ... ----------------------->| | 8,192 pages merged a bio. | at this time, first bio submit is done. | 1 bio is split to 32 read request and issue. |---------------> |---------------> |---------------> ...... |---------------> |--------------->| total 19ms elapsed to complete 32MB read done from device. | If bio max size is limited with 1MB, behavior is changed below. | bio merge for 1MB. 256 pages are merged for each bio. | total 32 bio will be made. | total elapsed time is over 2ms. it's same. | but, first bio submit timing is fast. about 100us. |--->|--->|--->|---> ... -->|--->|--->|--->|--->| | 256 pages merged a bio. | at this time, first bio submit is done. | and 1 read request is issued for 1 bio. |---------------> |---------------> |---------------> ...... |---------------> |--------------->| total 17ms elapsed to complete 32MB read done from device. | As a result, read request issue timing is faster if bio max size is limited. Current kernel behavior with multipage bvec, super large bio can be created. And it lead to delay first I/O request issue. Signed-off-by: Changheun Lee Reviewed-by: Bart Van Assche Link: https://lore.kernel.org/r/20210503095203.29076-1-nanich.lee@samsung.com Signed-off-by: Jens Axboe --- block/bio.c | 13 +++++++++++-- block/blk-settings.c | 5 +++++ include/linux/bio.h | 4 +++- include/linux/blkdev.h | 2 ++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/block/bio.c b/block/bio.c index 44205dfb6b60..221dc56ba22f 100644 --- a/block/bio.c +++ b/block/bio.c @@ -255,6 +255,13 @@ void bio_init(struct bio *bio, struct bio_vec *table, } EXPORT_SYMBOL(bio_init); +unsigned int bio_max_size(struct bio *bio) +{ + struct block_device *bdev = bio->bi_bdev; + + return bdev ? bdev->bd_disk->queue->limits.bio_max_bytes : UINT_MAX; +} + /** * bio_reset - reinitialize a bio * @bio: bio to reset @@ -866,7 +873,7 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page, struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1]; if (page_is_mergeable(bv, page, len, off, same_page)) { - if (bio->bi_iter.bi_size > UINT_MAX - len) { + if (bio->bi_iter.bi_size > bio_max_size(bio) - len) { *same_page = false; return false; } @@ -995,6 +1002,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter) { unsigned short nr_pages = bio->bi_max_vecs - bio->bi_vcnt; unsigned short entries_left = bio->bi_max_vecs - bio->bi_vcnt; + unsigned int bytes_left = bio_max_size(bio) - bio->bi_iter.bi_size; struct bio_vec *bv = bio->bi_io_vec + bio->bi_vcnt; struct page **pages = (struct page **)bv; bool same_page = false; @@ -1010,7 +1018,8 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter) BUILD_BUG_ON(PAGE_PTRS_PER_BVEC < 2); pages += entries_left * (PAGE_PTRS_PER_BVEC - 1); - size = iov_iter_get_pages(iter, pages, LONG_MAX, nr_pages, &offset); + size = iov_iter_get_pages(iter, pages, bytes_left, nr_pages, + &offset); if (unlikely(size <= 0)) return size ? size : -EFAULT; diff --git a/block/blk-settings.c b/block/blk-settings.c index 9c009090c4b5..c646503e55d2 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -31,6 +31,7 @@ EXPORT_SYMBOL_GPL(blk_queue_rq_timeout); */ void blk_set_default_limits(struct queue_limits *lim) { + lim->bio_max_bytes = UINT_MAX; lim->max_segments = BLK_MAX_SEGMENTS; lim->max_discard_segments = 1; lim->max_integrity_segments = 0; @@ -139,6 +140,10 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto limits->logical_block_size >> SECTOR_SHIFT); limits->max_sectors = max_sectors; + if (check_shl_overflow(max_sectors, SECTOR_SHIFT, + &limits->bio_max_bytes)) + limits->bio_max_bytes = UINT_MAX; + q->backing_dev_info->io_pages = max_sectors >> (PAGE_SHIFT - 9); } EXPORT_SYMBOL(blk_queue_max_hw_sectors); diff --git a/include/linux/bio.h b/include/linux/bio.h index a0b4cfdf62a4..f1a99f0a240c 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -106,6 +106,8 @@ static inline void *bio_data(struct bio *bio) return NULL; } +extern unsigned int bio_max_size(struct bio *bio); + /** * bio_full - check if the bio is full * @bio: bio to check @@ -119,7 +121,7 @@ static inline bool bio_full(struct bio *bio, unsigned len) if (bio->bi_vcnt >= bio->bi_max_vecs) return true; - if (bio->bi_iter.bi_size > UINT_MAX - len) + if (bio->bi_iter.bi_size > bio_max_size(bio) - len) return true; return false; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index b91ba6207365..40c7c4d87aa1 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -327,6 +327,8 @@ enum blk_bounce { }; struct queue_limits { + unsigned int bio_max_bytes; + enum blk_bounce bounce; unsigned long seg_boundary_mask; unsigned long virt_boundary_mask; -- cgit v1.2.3 From 48145b62563a9ae1ad631d6b576c6b9a798fcbec Mon Sep 17 00:00:00 2001 From: Minwoo Im Date: Thu, 22 Apr 2021 17:04:07 +0900 Subject: nvme: fix controller ioctl through ns_head In multipath case, we should consider namespace attachment with controllers in a subsystem when we find out the live controller for the namespace. This patch manually reverted the commit 3557a4409701 ("nvme: don't bother to look up a namespace for controller ioctls") with few more updates to nvme_ns_head_chr_ioctl which has been newly updated. Fixes: 3557a4409701 ("nvme: don't bother to look up a namespace for controller ioctls") Cc: Christoph Hellwig Signed-off-by: Minwoo Im Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 22 ---------------- drivers/nvme/host/ioctl.c | 65 ++++++++++++++++++++++++++++++----------------- drivers/nvme/host/nvme.h | 1 - 3 files changed, 41 insertions(+), 47 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index b6f7815fa239..c1c196459d79 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1999,28 +1999,6 @@ static const struct block_device_operations nvme_bdev_ops = { .pr_ops = &nvme_pr_ops, }; -#ifdef CONFIG_NVME_MULTIPATH -struct nvme_ctrl *nvme_find_get_live_ctrl(struct nvme_subsystem *subsys) -{ - struct nvme_ctrl *ctrl; - int ret; - - ret = mutex_lock_killable(&nvme_subsystems_lock); - if (ret) - return ERR_PTR(ret); - list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) { - if (ctrl->state == NVME_CTRL_LIVE) - goto found; - } - mutex_unlock(&nvme_subsystems_lock); - return ERR_PTR(-EWOULDBLOCK); -found: - nvme_get_ctrl(ctrl); - mutex_unlock(&nvme_subsystems_lock); - return ctrl; -} -#endif /* CONFIG_NVME_MULTIPATH */ - static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled) { unsigned long timeout = diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 502f8e4a2a1f..9557ead02de1 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -370,41 +370,45 @@ long nvme_ns_chr_ioctl(struct file *file, unsigned int cmd, unsigned long arg) } #ifdef CONFIG_NVME_MULTIPATH -static int nvme_ns_head_ctrl_ioctl(struct nvme_ns_head *head, - unsigned int cmd, void __user *argp) +static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd, + void __user *argp, struct nvme_ns_head *head, int srcu_idx) { - struct nvme_ctrl *ctrl = nvme_find_get_live_ctrl(head->subsys); + struct nvme_ctrl *ctrl = ns->ctrl; int ret; - if (IS_ERR(ctrl)) - return PTR_ERR(ctrl); - ret = nvme_ctrl_ioctl(ctrl, cmd, argp); - nvme_put_ctrl(ctrl); - return ret; -} + nvme_get_ctrl(ns->ctrl); + nvme_put_ns_from_disk(head, srcu_idx); + ret = nvme_ctrl_ioctl(ns->ctrl, cmd, argp); -static int nvme_ns_head_ns_ioctl(struct nvme_ns_head *head, - unsigned int cmd, void __user *argp) -{ - int srcu_idx = srcu_read_lock(&head->srcu); - struct nvme_ns *ns = nvme_find_path(head); - int ret = -EWOULDBLOCK; - - if (ns) - ret = nvme_ns_ioctl(ns, cmd, argp); - srcu_read_unlock(&head->srcu, srcu_idx); + nvme_put_ctrl(ctrl); return ret; } int nvme_ns_head_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg) { - struct nvme_ns_head *head = bdev->bd_disk->private_data; + struct nvme_ns_head *head = NULL; void __user *argp = (void __user *)arg; + struct nvme_ns *ns; + int srcu_idx, ret; + + ns = nvme_get_ns_from_disk(bdev->bd_disk, &head, &srcu_idx); + if (unlikely(!ns)) + return -EWOULDBLOCK; + /* + * Handle ioctls that apply to the controller instead of the namespace + * seperately and drop the ns SRCU reference early. This avoids a + * deadlock when deleting namespaces using the passthrough interface. + */ if (is_ctrl_ioctl(cmd)) - return nvme_ns_head_ctrl_ioctl(head, cmd, argp); - return nvme_ns_head_ns_ioctl(head, cmd, argp); + ret = nvme_ns_head_ctrl_ioctl(ns, cmd, argp, head, srcu_idx); + else { + ret = nvme_ns_ioctl(ns, cmd, argp); + nvme_put_ns_from_disk(head, srcu_idx); + } + + return ret; } long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd, @@ -414,10 +418,23 @@ long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd, struct nvme_ns_head *head = container_of(cdev, struct nvme_ns_head, cdev); void __user *argp = (void __user *)arg; + struct nvme_ns *ns; + int srcu_idx, ret; + + srcu_idx = srcu_read_lock(&head->srcu); + ns = nvme_find_path(head); + if (!ns) { + srcu_read_unlock(&head->srcu, srcu_idx); + return -EWOULDBLOCK; + } if (is_ctrl_ioctl(cmd)) - return nvme_ns_head_ctrl_ioctl(head, cmd, argp); - return nvme_ns_head_ns_ioctl(head, cmd, argp); + return nvme_ns_head_ctrl_ioctl(ns, cmd, argp, head, srcu_idx); + + ret = nvme_ns_ioctl(ns, cmd, argp); + nvme_put_ns_from_disk(head, srcu_idx); + + return ret; } #endif /* CONFIG_NVME_MULTIPATH */ diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 773dde5b231d..c1e086a0bc3f 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -664,7 +664,6 @@ struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk, void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx); bool nvme_tryget_ns_head(struct nvme_ns_head *head); void nvme_put_ns_head(struct nvme_ns_head *head); -struct nvme_ctrl *nvme_find_get_live_ctrl(struct nvme_subsystem *subsys); int nvme_cdev_add(struct cdev *cdev, struct device *cdev_device, const struct file_operations *fops, struct module *owner); void nvme_cdev_del(struct cdev *cdev, struct device *cdev_device); -- cgit v1.2.3 From 4c74d1f80381996027bacc4f6c554948ef9bf374 Mon Sep 17 00:00:00 2001 From: Kanchan Joshi Date: Tue, 27 Apr 2021 12:17:46 +0530 Subject: nvme: add nvme_get_ns helper Add a helper to avoid opencoding ns->kref increment. Decrement is already done via nvme_put_ns helper. Signed-off-by: Kanchan Joshi Reviewed-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index c1c196459d79..f2d1f2d699b8 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -576,6 +576,11 @@ static void nvme_free_ns(struct kref *kref) kfree(ns); } +static inline bool nvme_get_ns(struct nvme_ns *ns) +{ + return kref_get_unless_zero(&ns->kref); +} + void nvme_put_ns(struct nvme_ns *ns) { kref_put(&ns->kref, nvme_free_ns); @@ -1494,7 +1499,7 @@ static int nvme_ns_open(struct nvme_ns *ns) /* should never be called due to GENHD_FL_HIDDEN */ if (WARN_ON_ONCE(nvme_ns_head_multipath(ns->head))) goto fail; - if (!kref_get_unless_zero(&ns->kref)) + if (!nvme_get_ns(ns)) goto fail; if (!try_module_get(ns->ctrl->ops->module)) goto fail_put_ns; @@ -3582,7 +3587,7 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid) down_read(&ctrl->namespaces_rwsem); list_for_each_entry(ns, &ctrl->namespaces, list) { if (ns->head->ns_id == nsid) { - if (!kref_get_unless_zero(&ns->kref)) + if (!nvme_get_ns(ns)) continue; ret = ns; break; -- cgit v1.2.3 From 51ad06cd698cb9ff280a769ed8d57210a1d2266d Mon Sep 17 00:00:00 2001 From: Kanchan Joshi Date: Tue, 27 Apr 2021 12:17:47 +0530 Subject: nvme: avoid memset for passthrough requests nvme_clear_nvme_request() clears the nvme_command, which is unncessary for passthrough requests as nvme_command is overwritten immediately. Move clearing part from this helper to the caller, so that double memset for passthrough requests is avoided. Signed-off-by: Kanchan Joshi Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index f2d1f2d699b8..61e122cecc2a 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -589,9 +589,6 @@ EXPORT_SYMBOL_NS_GPL(nvme_put_ns, NVME_TARGET_PASSTHRU); static inline void nvme_clear_nvme_request(struct request *req) { - struct nvme_command *cmd = nvme_req(req)->cmd; - - memset(cmd, 0, sizeof(*cmd)); nvme_req(req)->retries = 0; nvme_req(req)->flags = 0; req->rq_flags |= RQF_DONTPREP; @@ -903,8 +900,10 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req) struct nvme_command *cmd = nvme_req(req)->cmd; blk_status_t ret = BLK_STS_OK; - if (!(req->rq_flags & RQF_DONTPREP)) + if (!(req->rq_flags & RQF_DONTPREP)) { nvme_clear_nvme_request(req); + memset(cmd, 0, sizeof(*cmd)); + } switch (req_op(req)) { case REQ_OP_DRV_IN: -- cgit v1.2.3 From a97157440e1e69c35d7804d3b72da0c626ef28e6 Mon Sep 17 00:00:00 2001 From: Tao Chiu Date: Mon, 26 Apr 2021 10:53:10 +0800 Subject: nvme: move the fabrics queue ready check routines to core queue_rq() in pci only checks if the dispatched queue (nvmeq) is ready, e.g. not being suspended. Since nvme_alloc_admin_tags() in reset flow restarts the admin queue, users are able to submit admin commands to a controller before reset_work() completes. Commands submitted under this condition may interfere with commands that performs identify, IO queue setup in reset_work(), and may result in a hang described in the following patch. As seen in the fabrics, user commands are prevented from being executed under inproper controller states. We may reuse this logic to maintain a clear admin queue during reset_work(). Signed-off-by: Tao Chiu Signed-off-by: Cody Wong Reviewed-by: Leon Chien Reviewed-by: Keith Busch Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 60 +++++++++++++++++++++++++++++++++++++++++++++ drivers/nvme/host/fabrics.c | 57 ------------------------------------------ drivers/nvme/host/fabrics.h | 13 ---------- drivers/nvme/host/fc.c | 4 +-- drivers/nvme/host/nvme.h | 15 ++++++++++++ drivers/nvme/host/rdma.c | 4 +-- drivers/nvme/host/tcp.c | 4 +-- drivers/nvme/target/loop.c | 4 +-- 8 files changed, 83 insertions(+), 78 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 61e122cecc2a..522c9b229f80 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -639,6 +639,66 @@ static struct request *nvme_alloc_request_qid(struct request_queue *q, return req; } +/* + * For something we're not in a state to send to the device the default action + * is to busy it and retry it after the controller state is recovered. However, + * if the controller is deleting or if anything is marked for failfast or + * nvme multipath it is immediately failed. + * + * Note: commands used to initialize the controller will be marked for failfast. + * Note: nvme cli/ioctl commands are marked for failfast. + */ +blk_status_t nvme_fail_nonready_command(struct nvme_ctrl *ctrl, + struct request *rq) +{ + if (ctrl->state != NVME_CTRL_DELETING_NOIO && + ctrl->state != NVME_CTRL_DEAD && + !test_bit(NVME_CTRL_FAILFAST_EXPIRED, &ctrl->flags) && + !blk_noretry_request(rq) && !(rq->cmd_flags & REQ_NVME_MPATH)) + return BLK_STS_RESOURCE; + return nvme_host_path_error(rq); +} +EXPORT_SYMBOL_GPL(nvme_fail_nonready_command); + +bool __nvme_check_ready(struct nvme_ctrl *ctrl, struct request *rq, + bool queue_live) +{ + struct nvme_request *req = nvme_req(rq); + + /* + * currently we have a problem sending passthru commands + * on the admin_q if the controller is not LIVE because we can't + * make sure that they are going out after the admin connect, + * controller enable and/or other commands in the initialization + * sequence. until the controller will be LIVE, fail with + * BLK_STS_RESOURCE so that they will be rescheduled. + */ + if (rq->q == ctrl->admin_q && (req->flags & NVME_REQ_USERCMD)) + return false; + + if (ctrl->ops->flags & NVME_F_FABRICS) { + /* + * Only allow commands on a live queue, except for the connect + * command, which is require to set the queue live in the + * appropinquate states. + */ + switch (ctrl->state) { + case NVME_CTRL_CONNECTING: + if (blk_rq_is_passthrough(rq) && nvme_is_fabrics(req->cmd) && + req->cmd->fabrics.fctype == nvme_fabrics_type_connect) + return true; + break; + default: + break; + case NVME_CTRL_DEAD: + return false; + } + } + + return queue_live; +} +EXPORT_SYMBOL_GPL(__nvme_check_ready); + static int nvme_toggle_streams(struct nvme_ctrl *ctrl, bool enable) { struct nvme_command c; diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 13c2747e3d00..a2bb7fc63a73 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -533,63 +533,6 @@ static struct nvmf_transport_ops *nvmf_lookup_transport( return NULL; } -/* - * For something we're not in a state to send to the device the default action - * is to busy it and retry it after the controller state is recovered. However, - * if the controller is deleting or if anything is marked for failfast or - * nvme multipath it is immediately failed. - * - * Note: commands used to initialize the controller will be marked for failfast. - * Note: nvme cli/ioctl commands are marked for failfast. - */ -blk_status_t nvmf_fail_nonready_command(struct nvme_ctrl *ctrl, - struct request *rq) -{ - if (ctrl->state != NVME_CTRL_DELETING_NOIO && - ctrl->state != NVME_CTRL_DEAD && - !test_bit(NVME_CTRL_FAILFAST_EXPIRED, &ctrl->flags) && - !blk_noretry_request(rq) && !(rq->cmd_flags & REQ_NVME_MPATH)) - return BLK_STS_RESOURCE; - return nvme_host_path_error(rq); -} -EXPORT_SYMBOL_GPL(nvmf_fail_nonready_command); - -bool __nvmf_check_ready(struct nvme_ctrl *ctrl, struct request *rq, - bool queue_live) -{ - struct nvme_request *req = nvme_req(rq); - - /* - * currently we have a problem sending passthru commands - * on the admin_q if the controller is not LIVE because we can't - * make sure that they are going out after the admin connect, - * controller enable and/or other commands in the initialization - * sequence. until the controller will be LIVE, fail with - * BLK_STS_RESOURCE so that they will be rescheduled. - */ - if (rq->q == ctrl->admin_q && (req->flags & NVME_REQ_USERCMD)) - return false; - - /* - * Only allow commands on a live queue, except for the connect command, - * which is require to set the queue live in the appropinquate states. - */ - switch (ctrl->state) { - case NVME_CTRL_CONNECTING: - if (blk_rq_is_passthrough(rq) && nvme_is_fabrics(req->cmd) && - req->cmd->fabrics.fctype == nvme_fabrics_type_connect) - return true; - break; - default: - break; - case NVME_CTRL_DEAD: - return false; - } - - return queue_live; -} -EXPORT_SYMBOL_GPL(__nvmf_check_ready); - static const match_table_t opt_tokens = { { NVMF_OPT_TRANSPORT, "transport=%s" }, { NVMF_OPT_TRADDR, "traddr=%s" }, diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h index 888b108d87a4..d7f7974dc208 100644 --- a/drivers/nvme/host/fabrics.h +++ b/drivers/nvme/host/fabrics.h @@ -184,20 +184,7 @@ void nvmf_unregister_transport(struct nvmf_transport_ops *ops); void nvmf_free_options(struct nvmf_ctrl_options *opts); int nvmf_get_address(struct nvme_ctrl *ctrl, char *buf, int size); bool nvmf_should_reconnect(struct nvme_ctrl *ctrl); -blk_status_t nvmf_fail_nonready_command(struct nvme_ctrl *ctrl, - struct request *rq); -bool __nvmf_check_ready(struct nvme_ctrl *ctrl, struct request *rq, - bool queue_live); bool nvmf_ip_options_match(struct nvme_ctrl *ctrl, struct nvmf_ctrl_options *opts); -static inline bool nvmf_check_ready(struct nvme_ctrl *ctrl, struct request *rq, - bool queue_live) -{ - if (likely(ctrl->state == NVME_CTRL_LIVE || - ctrl->state == NVME_CTRL_DELETING)) - return true; - return __nvmf_check_ready(ctrl, rq, queue_live); -} - #endif /* _NVME_FABRICS_H */ diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c index 9b9b7be0f412..d9ab9e7871d0 100644 --- a/drivers/nvme/host/fc.c +++ b/drivers/nvme/host/fc.c @@ -2766,8 +2766,8 @@ nvme_fc_queue_rq(struct blk_mq_hw_ctx *hctx, blk_status_t ret; if (ctrl->rport->remoteport.port_state != FC_OBJSTATE_ONLINE || - !nvmf_check_ready(&queue->ctrl->ctrl, rq, queue_ready)) - return nvmf_fail_nonready_command(&queue->ctrl->ctrl, rq); + !nvme_check_ready(&queue->ctrl->ctrl, rq, queue_ready)) + return nvme_fail_nonready_command(&queue->ctrl->ctrl, rq); ret = nvme_setup_cmd(ns, rq); if (ret) diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index c1e086a0bc3f..05f31a2c64bb 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -638,6 +638,21 @@ struct request *nvme_alloc_request(struct request_queue *q, struct nvme_command *cmd, blk_mq_req_flags_t flags); void nvme_cleanup_cmd(struct request *req); blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req); +blk_status_t nvme_fail_nonready_command(struct nvme_ctrl *ctrl, + struct request *req); +bool __nvme_check_ready(struct nvme_ctrl *ctrl, struct request *rq, + bool queue_live); + +static inline bool nvme_check_ready(struct nvme_ctrl *ctrl, struct request *rq, + bool queue_live) +{ + if (likely(ctrl->state == NVME_CTRL_LIVE)) + return true; + if (ctrl->ops->flags & NVME_F_FABRICS && + ctrl->state == NVME_CTRL_DELETING) + return true; + return __nvme_check_ready(ctrl, rq, queue_live); +} int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd, void *buf, unsigned bufflen); int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd, diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index 660c774fa9e1..37943dc4c2c1 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -2050,8 +2050,8 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx, WARN_ON_ONCE(rq->tag < 0); - if (!nvmf_check_ready(&queue->ctrl->ctrl, rq, queue_ready)) - return nvmf_fail_nonready_command(&queue->ctrl->ctrl, rq); + if (!nvme_check_ready(&queue->ctrl->ctrl, rq, queue_ready)) + return nvme_fail_nonready_command(&queue->ctrl->ctrl, rq); dev = queue->device->dev; diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 75435cdb156c..0222e23f5936 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -2338,8 +2338,8 @@ static blk_status_t nvme_tcp_queue_rq(struct blk_mq_hw_ctx *hctx, bool queue_ready = test_bit(NVME_TCP_Q_LIVE, &queue->flags); blk_status_t ret; - if (!nvmf_check_ready(&queue->ctrl->ctrl, rq, queue_ready)) - return nvmf_fail_nonready_command(&queue->ctrl->ctrl, rq); + if (!nvme_check_ready(&queue->ctrl->ctrl, rq, queue_ready)) + return nvme_fail_nonready_command(&queue->ctrl->ctrl, rq); ret = nvme_tcp_setup_cmd_pdu(ns, rq); if (unlikely(ret)) diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c index 6665da3b634f..74b3b150e1a5 100644 --- a/drivers/nvme/target/loop.c +++ b/drivers/nvme/target/loop.c @@ -138,8 +138,8 @@ static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, bool queue_ready = test_bit(NVME_LOOP_Q_LIVE, &queue->flags); blk_status_t ret; - if (!nvmf_check_ready(&queue->ctrl->ctrl, req, queue_ready)) - return nvmf_fail_nonready_command(&queue->ctrl->ctrl, req); + if (!nvme_check_ready(&queue->ctrl->ctrl, req, queue_ready)) + return nvme_fail_nonready_command(&queue->ctrl->ctrl, req); ret = nvme_setup_cmd(ns, req); if (ret) -- cgit v1.2.3 From d4060d2be1132596154f31f4d57976bd103e969d Mon Sep 17 00:00:00 2001 From: Tao Chiu Date: Mon, 26 Apr 2021 10:53:55 +0800 Subject: nvme-pci: fix controller reset hang when racing with nvme_timeout reset_work() in nvme-pci may hang forever in the following scenario: 1) A reset caused by a command timeout occurs due to a controller being temporarily irresponsive. 2) nvme_reset_work() restarts admin queue at nvme_alloc_admin_tags(). At the same time, a user-submitted admin command is queued and waiting for completion. Then, reset_work() changes its state to CONNECTING, and submits an identify command. 3) However, the controller does still not respond to any command, causing a timeout being fired at the user-submitted command. Unfortunately, nvme_timeout() does not see the completion on cq, and any timeout that takes place under CONNECTING state causes a controller shutdown. 4) Normally, the identify command in reset_work() would be canceled with SC_HOST_ABORTED by nvme_dev_disable(), then reset_work can tear down the controller accordingly. But the controller happens to return online and respond the identify command before nvme_dev_disable() should have been reaped it off. 5) reset_work() continues to setup_io_queues() as it observes no error in init_identify(). However, the admin queue has already been quiesced in dev_disable(). Thus, any following commands would be blocked forever in blk_execute_rq(). This can be fixed by restricting usercmd commands when controller is not in a LIVE state in nvme_queue_rq(), as what has been done previously in fabrics. ``` nvme_reset_work(): | nvme_alloc_admin_tags() | | nvme_submit_user_cmd(): nvme_init_identify(): | ... __nvme_submit_sync_cmd(): | ... | ... ---------------------------------------> nvme_timeout(): (Controller starts reponding commands) | nvme_dev_disable(, true): nvme_setup_io_queues(): | __nvme_submit_sync_cmd(): | (hung in blk_execute_rq | since run_hw_queue sees | queue quiesced) | ``` Signed-off-by: Tao Chiu Signed-off-by: Cody Wong Reviewed-by: Leon Chien Reviewed-by: Keith Busch Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 09d4c5f99fc3..a29b170701fc 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -933,6 +933,9 @@ static blk_status_t nvme_queue_rq(struct blk_mq_hw_ctx *hctx, if (unlikely(!test_bit(NVMEQ_ENABLED, &nvmeq->flags))) return BLK_STS_IOERR; + if (!nvme_check_ready(&dev->ctrl, req, true)) + return nvme_fail_nonready_command(&dev->ctrl, req); + ret = nvme_setup_cmd(ns, req); if (ret) return ret; -- cgit v1.2.3 From ce86dad222e9074d3ec174ec81cb463a770331b5 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 3 May 2021 19:03:03 +0200 Subject: nvme-multipath: reset bdev to ns head when failover When a request finally completes in end_io() after it has failed over, the bdev pointer can be stale and thus the system can crash. Set the bdev back to ns head, so the request is map to an active path when resubmitted. Signed-off-by: Daniel Wagner Reviewed-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- drivers/nvme/host/multipath.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 0d0de3433f37..0551796517e6 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -70,6 +70,7 @@ void nvme_failover_req(struct request *req) struct nvme_ns *ns = req->q->queuedata; u16 status = nvme_req(req)->status & 0x7ff; unsigned long flags; + struct bio *bio; nvme_mpath_clear_current_path(ns); @@ -84,6 +85,8 @@ void nvme_failover_req(struct request *req) } spin_lock_irqsave(&ns->head->requeue_lock, flags); + for (bio = req->bio; bio; bio = bio->bi_next) + bio_set_dev(bio, ns->head->disk->part0); blk_steal_bios(&ns->head->requeue_list, req); spin_unlock_irqrestore(&ns->head->requeue_lock, flags); -- cgit v1.2.3 From 4a20342572f66c5b20a1ee680f5ac0a13703748f Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Wed, 28 Apr 2021 21:25:58 -0700 Subject: nvmet: remove unsupported command noise Nothing can stop a host from submitting invalid commands. The target just needs to respond with an appropriate status, but that's not a target error. Demote invalid command messages to the debug level so these events don't spam the kernel logs. Reported-by: Yi Zhang Signed-off-by: Keith Busch Reviewed-by: Klaus Jensen Reviewed-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/admin-cmd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index d2a26ff3f7b3..e7a367cf6d36 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -307,7 +307,7 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req) case NVME_LOG_ANA: return nvmet_execute_get_log_page_ana(req); } - pr_err("unhandled lid %d on qid %d\n", + pr_debug("unhandled lid %d on qid %d\n", req->cmd->get_log_page.lid, req->sq->qid); req->error_loc = offsetof(struct nvme_get_log_page_command, lid); nvmet_req_complete(req, NVME_SC_INVALID_FIELD | NVME_SC_DNR); @@ -659,7 +659,7 @@ static void nvmet_execute_identify(struct nvmet_req *req) return nvmet_execute_identify_desclist(req); } - pr_err("unhandled identify cns %d on qid %d\n", + pr_debug("unhandled identify cns %d on qid %d\n", req->cmd->identify.cns, req->sq->qid); req->error_loc = offsetof(struct nvme_identify, cns); nvmet_req_complete(req, NVME_SC_INVALID_FIELD | NVME_SC_DNR); @@ -977,7 +977,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req) return 0; } - pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode, + pr_debug("unhandled cmd %d on qid %d\n", cmd->common.opcode, req->sq->qid); req->error_loc = offsetof(struct nvme_common_command, opcode); return NVME_SC_INVALID_OPCODE | NVME_SC_DNR; -- cgit v1.2.3 From cf7b39a0cbf6bf57aa07a008d46cf695add05b4c Mon Sep 17 00:00:00 2001 From: yangerkun Date: Thu, 1 Apr 2021 15:18:07 +0800 Subject: block: reexpand iov_iter after read/write We get a bug: BUG: KASAN: slab-out-of-bounds in iov_iter_revert+0x11c/0x404 lib/iov_iter.c:1139 Read of size 8 at addr ffff0000d3fb11f8 by task CPU: 0 PID: 12582 Comm: syz-executor.2 Not tainted 5.10.0-00843-g352c8610ccd2 #2 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x0/0x2d0 arch/arm64/kernel/stacktrace.c:132 show_stack+0x28/0x34 arch/arm64/kernel/stacktrace.c:196 __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x110/0x164 lib/dump_stack.c:118 print_address_description+0x78/0x5c8 mm/kasan/report.c:385 __kasan_report mm/kasan/report.c:545 [inline] kasan_report+0x148/0x1e4 mm/kasan/report.c:562 check_memory_region_inline mm/kasan/generic.c:183 [inline] __asan_load8+0xb4/0xbc mm/kasan/generic.c:252 iov_iter_revert+0x11c/0x404 lib/iov_iter.c:1139 io_read fs/io_uring.c:3421 [inline] io_issue_sqe+0x2344/0x2d64 fs/io_uring.c:5943 __io_queue_sqe+0x19c/0x520 fs/io_uring.c:6260 io_queue_sqe+0x2a4/0x590 fs/io_uring.c:6326 io_submit_sqe fs/io_uring.c:6395 [inline] io_submit_sqes+0x4c0/0xa04 fs/io_uring.c:6624 __do_sys_io_uring_enter fs/io_uring.c:9013 [inline] __se_sys_io_uring_enter fs/io_uring.c:8960 [inline] __arm64_sys_io_uring_enter+0x190/0x708 fs/io_uring.c:8960 __invoke_syscall arch/arm64/kernel/syscall.c:36 [inline] invoke_syscall arch/arm64/kernel/syscall.c:48 [inline] el0_svc_common arch/arm64/kernel/syscall.c:158 [inline] do_el0_svc+0x120/0x290 arch/arm64/kernel/syscall.c:227 el0_svc+0x1c/0x28 arch/arm64/kernel/entry-common.c:367 el0_sync_handler+0x98/0x170 arch/arm64/kernel/entry-common.c:383 el0_sync+0x140/0x180 arch/arm64/kernel/entry.S:670 Allocated by task 12570: stack_trace_save+0x80/0xb8 kernel/stacktrace.c:121 kasan_save_stack mm/kasan/common.c:48 [inline] kasan_set_track mm/kasan/common.c:56 [inline] __kasan_kmalloc+0xdc/0x120 mm/kasan/common.c:461 kasan_kmalloc+0xc/0x14 mm/kasan/common.c:475 __kmalloc+0x23c/0x334 mm/slub.c:3970 kmalloc include/linux/slab.h:557 [inline] __io_alloc_async_data+0x68/0x9c fs/io_uring.c:3210 io_setup_async_rw fs/io_uring.c:3229 [inline] io_read fs/io_uring.c:3436 [inline] io_issue_sqe+0x2954/0x2d64 fs/io_uring.c:5943 __io_queue_sqe+0x19c/0x520 fs/io_uring.c:6260 io_queue_sqe+0x2a4/0x590 fs/io_uring.c:6326 io_submit_sqe fs/io_uring.c:6395 [inline] io_submit_sqes+0x4c0/0xa04 fs/io_uring.c:6624 __do_sys_io_uring_enter fs/io_uring.c:9013 [inline] __se_sys_io_uring_enter fs/io_uring.c:8960 [inline] __arm64_sys_io_uring_enter+0x190/0x708 fs/io_uring.c:8960 __invoke_syscall arch/arm64/kernel/syscall.c:36 [inline] invoke_syscall arch/arm64/kernel/syscall.c:48 [inline] el0_svc_common arch/arm64/kernel/syscall.c:158 [inline] do_el0_svc+0x120/0x290 arch/arm64/kernel/syscall.c:227 el0_svc+0x1c/0x28 arch/arm64/kernel/entry-common.c:367 el0_sync_handler+0x98/0x170 arch/arm64/kernel/entry-common.c:383 el0_sync+0x140/0x180 arch/arm64/kernel/entry.S:670 Freed by task 12570: stack_trace_save+0x80/0xb8 kernel/stacktrace.c:121 kasan_save_stack mm/kasan/common.c:48 [inline] kasan_set_track+0x38/0x6c mm/kasan/common.c:56 kasan_set_free_info+0x20/0x40 mm/kasan/generic.c:355 __kasan_slab_free+0x124/0x150 mm/kasan/common.c:422 kasan_slab_free+0x10/0x1c mm/kasan/common.c:431 slab_free_hook mm/slub.c:1544 [inline] slab_free_freelist_hook mm/slub.c:1577 [inline] slab_free mm/slub.c:3142 [inline] kfree+0x104/0x38c mm/slub.c:4124 io_dismantle_req fs/io_uring.c:1855 [inline] __io_free_req+0x70/0x254 fs/io_uring.c:1867 io_put_req_find_next fs/io_uring.c:2173 [inline] __io_queue_sqe+0x1fc/0x520 fs/io_uring.c:6279 __io_req_task_submit+0x154/0x21c fs/io_uring.c:2051 io_req_task_submit+0x2c/0x44 fs/io_uring.c:2063 task_work_run+0xdc/0x128 kernel/task_work.c:151 get_signal+0x6f8/0x980 kernel/signal.c:2562 do_signal+0x108/0x3a4 arch/arm64/kernel/signal.c:658 do_notify_resume+0xbc/0x25c arch/arm64/kernel/signal.c:722 work_pending+0xc/0x180 blkdev_read_iter can truncate iov_iter's count since the count + pos may exceed the size of the blkdev. This will confuse io_read that we have consume the iovec. And once we do the iov_iter_revert in io_read, we will trigger the slab-out-of-bounds. Fix it by reexpand the count with size has been truncated. blkdev_write_iter can trigger the problem too. Signed-off-by: yangerkun Acked-by: Pavel Begunkov Link: https://lore.kernel.org/r/20210401071807.3328235-1-yangerkun@huawei.com Signed-off-by: Jens Axboe --- fs/block_dev.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/fs/block_dev.c b/fs/block_dev.c index a5244e08b6c8..eb265d72fce8 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1677,6 +1677,7 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) struct inode *bd_inode = bdev_file_inode(file); loff_t size = i_size_read(bd_inode); struct blk_plug plug; + size_t shorted = 0; ssize_t ret; if (bdev_read_only(I_BDEV(bd_inode))) @@ -1694,12 +1695,17 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) if ((iocb->ki_flags & (IOCB_NOWAIT | IOCB_DIRECT)) == IOCB_NOWAIT) return -EOPNOTSUPP; - iov_iter_truncate(from, size - iocb->ki_pos); + size -= iocb->ki_pos; + if (iov_iter_count(from) > size) { + shorted = iov_iter_count(from) - size; + iov_iter_truncate(from, size); + } blk_start_plug(&plug); ret = __generic_file_write_iter(iocb, from); if (ret > 0) ret = generic_write_sync(iocb, ret); + iov_iter_reexpand(from, iov_iter_count(from) + shorted); blk_finish_plug(&plug); return ret; } @@ -1711,13 +1717,21 @@ ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to) struct inode *bd_inode = bdev_file_inode(file); loff_t size = i_size_read(bd_inode); loff_t pos = iocb->ki_pos; + size_t shorted = 0; + ssize_t ret; if (pos >= size) return 0; size -= pos; - iov_iter_truncate(to, size); - return generic_file_read_iter(iocb, to); + if (iov_iter_count(to) > size) { + shorted = iov_iter_count(to) - size; + iov_iter_truncate(to, size); + } + + ret = generic_file_read_iter(iocb, to); + iov_iter_reexpand(to, iov_iter_count(to) + shorted); + return ret; } EXPORT_SYMBOL_GPL(blkdev_read_iter); -- cgit v1.2.3