From 5a1e59533380a3fd04593e4ab2d4633ebf7745c1 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 22 Feb 2018 07:24:08 -0800 Subject: nvme-fabrics: don't check for non-NULL module in nvmf_register_transport THIS_MODULE evaluates to NULL when used from code built into the kernel, thus breaking built-in transport modules. Remove the bogus check. Fixes: 0de5cd36 ("nvme-fabrics: protect against module unload during create_ctrl") Signed-off-by: Christoph Hellwig Reviewed-by: Sagi Grimberg Reviewed-by: Johannes Thumshirn Signed-off-by: Keith Busch --- drivers/nvme/host/fabrics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 5dd4ceefed8f..a1c58e35075e 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -493,7 +493,7 @@ EXPORT_SYMBOL_GPL(nvmf_should_reconnect); */ int nvmf_register_transport(struct nvmf_transport_ops *ops) { - if (!ops->create_ctrl || !ops->module) + if (!ops->create_ctrl) return -EINVAL; down_write(&nvmf_transports_rwsem); -- cgit v1.2.3-59-g8ed1b From 0d30992395b1ed0e006960de1651b44cd51be791 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 22 Feb 2018 07:24:09 -0800 Subject: nvme-rdma: use blk_rq_payload_bytes instead of blk_rq_bytes blk_rq_bytes does the wrong thing for special payloads like discards and might cause the driver to not set up a SGL. Signed-off-by: Christoph Hellwig Reviewed-by: Sagi Grimberg Reviewed-by: Johannes Thumshirn Signed-off-by: Keith Busch --- drivers/nvme/host/rdma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index 2bc059f7d73c..acc9eb21c242 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -1051,7 +1051,7 @@ static void nvme_rdma_unmap_data(struct nvme_rdma_queue *queue, struct nvme_rdma_device *dev = queue->device; struct ib_device *ibdev = dev->dev; - if (!blk_rq_bytes(rq)) + if (!blk_rq_payload_bytes(rq)) return; if (req->mr) { @@ -1166,7 +1166,7 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue, c->common.flags |= NVME_CMD_SGL_METABUF; - if (!blk_rq_bytes(rq)) + if (!blk_rq_payload_bytes(rq)) return nvme_rdma_set_sg_null(c); req->sg_table.sgl = req->first_sgl; -- cgit v1.2.3-59-g8ed1b From 796b0b8d8dea191d9f64e0be8ab58d8f3586bcde Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 22 Feb 2018 07:24:10 -0800 Subject: nvmet-loop: use blk_rq_payload_bytes for sgl selection blk_rq_bytes does the wrong thing for special payloads like discards and might cause the driver to not set up a SGL. Signed-off-by: Christoph Hellwig Reviewed-by: Sagi Grimberg Reviewed-by: Johannes Thumshirn Signed-off-by: Keith Busch --- drivers/nvme/target/loop.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c index 7991ec3a17db..861d1509b22b 100644 --- a/drivers/nvme/target/loop.c +++ b/drivers/nvme/target/loop.c @@ -184,7 +184,7 @@ static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, return BLK_STS_OK; } - if (blk_rq_bytes(req)) { + if (blk_rq_payload_bytes(req)) { iod->sg_table.sgl = iod->first_sgl; if (sg_alloc_table_chained(&iod->sg_table, blk_rq_nr_phys_segments(req), @@ -193,7 +193,7 @@ static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, iod->req.sg = iod->sg_table.sgl; iod->req.sg_cnt = blk_rq_map_sg(req->q, req, iod->sg_table.sgl); - iod->req.transfer_len = blk_rq_bytes(req); + iod->req.transfer_len = blk_rq_payload_bytes(req); } blk_mq_start_request(req); -- cgit v1.2.3-59-g8ed1b From 0bd1ed4860d0f5f836aa8371797689a3779d1bf5 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Sat, 10 Feb 2018 08:46:17 +0800 Subject: block: pass inclusive 'lend' parameter to truncate_inode_pages_range The 'lend' parameter of truncate_inode_pages_range is required to be inclusive, so follow the rule. This patch fixes one memory corruption triggered by discard. Cc: Cc: Dmitry Monakhov Fixes: 351499a172c0 ("block: Invalidate cache on discard v2") Reviewed-by: Bart Van Assche Signed-off-by: Ming Lei Signed-off-by: Jens Axboe --- block/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/ioctl.c b/block/ioctl.c index 1668506d8ed8..3884d810efd2 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -225,7 +225,7 @@ static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode, if (start + len > i_size_read(bdev->bd_inode)) return -EINVAL; - truncate_inode_pages_range(mapping, start, start + len); + truncate_inode_pages_range(mapping, start, start + len - 1); return blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, flags); } -- cgit v1.2.3-59-g8ed1b From 105976f517791aed3b11f8f53b308a2069d42055 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Fri, 23 Feb 2018 23:36:56 +0800 Subject: blk-mq: don't call io sched's .requeue_request when requeueing rq to ->dispatch __blk_mq_requeue_request() covers two cases: - one is that the requeued request is added to hctx->dispatch, such as blk_mq_dispatch_rq_list() - another case is that the request is requeued to io scheduler, such as blk_mq_requeue_request(). We should call io sched's .requeue_request callback only for the 2nd case. Cc: Paolo Valente Cc: Omar Sandoval Fixes: bd166ef183c2 ("blk-mq-sched: add framework for MQ capable IO schedulers") Cc: stable@vger.kernel.org Reviewed-by: Bart Van Assche Acked-by: Paolo Valente Signed-off-by: Ming Lei Signed-off-by: Jens Axboe --- block/blk-mq.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 357492712b0e..16e83e6df404 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -712,7 +712,6 @@ static void __blk_mq_requeue_request(struct request *rq) trace_block_rq_requeue(q, rq); wbt_requeue(q->rq_wb, &rq->issue_stat); - blk_mq_sched_requeue_request(rq); if (blk_mq_rq_state(rq) != MQ_RQ_IDLE) { blk_mq_rq_update_state(rq, MQ_RQ_IDLE); @@ -725,6 +724,9 @@ void blk_mq_requeue_request(struct request *rq, bool kick_requeue_list) { __blk_mq_requeue_request(rq); + /* this request will be re-inserted to io scheduler queue */ + blk_mq_sched_requeue_request(rq); + BUG_ON(blk_queued_rq(rq)); blk_mq_add_to_requeue_list(rq, true, kick_requeue_list); } -- cgit v1.2.3-59-g8ed1b From ba989a01469d027861e55c8f1121edadef757797 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Fri, 23 Feb 2018 23:36:57 +0800 Subject: block: kyber: fix domain token leak during requeue When requeuing request, the domain token should have been freed before re-inserting the request to io scheduler. Otherwise, the assigned domain token will be leaked, and IO hang can be caused. Cc: Paolo Valente Cc: Omar Sandoval Cc: stable@vger.kernel.org Reviewed-by: Bart Van Assche Signed-off-by: Ming Lei Signed-off-by: Jens Axboe --- block/kyber-iosched.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c index f95c60774ce8..0d6d25e32e1f 100644 --- a/block/kyber-iosched.c +++ b/block/kyber-iosched.c @@ -833,6 +833,7 @@ static struct elevator_type kyber_sched = { .limit_depth = kyber_limit_depth, .prepare_request = kyber_prepare_request, .finish_request = kyber_finish_request, + .requeue_request = kyber_finish_request, .completed_request = kyber_completed_request, .dispatch_request = kyber_dispatch_request, .has_work = kyber_has_work, -- cgit v1.2.3-59-g8ed1b From f25a2dfc20e3a3ed8fe6618c331799dd7bd01190 Mon Sep 17 00:00:00 2001 From: Jianchao Wang Date: Thu, 15 Feb 2018 19:13:41 +0800 Subject: nvme-pci: Fix nvme queue cleanup if IRQ setup fails This patch fixes nvme queue cleanup if requesting an IRQ handler for the queue's vector fails. It does this by resetting the cq_vector to the uninitialized value of -1 so it is ignored for a controller reset. Signed-off-by: Jianchao Wang [changelog updates, removed misc whitespace changes] Signed-off-by: Keith Busch --- drivers/nvme/host/pci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 6fe7af00a1f4..022b070e60b7 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -1452,7 +1452,7 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid) nvmeq->cq_vector = qid - 1; result = adapter_alloc_cq(dev, qid, nvmeq); if (result < 0) - return result; + goto release_vector; result = adapter_alloc_sq(dev, qid, nvmeq); if (result < 0) @@ -1466,9 +1466,12 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid) return result; release_sq: + dev->online_queues--; adapter_delete_sq(dev, qid); release_cq: adapter_delete_cq(dev, qid); + release_vector: + nvmeq->cq_vector = -1; return result; } -- cgit v1.2.3-59-g8ed1b From d9c10e5b8863cfb6886d1640386455075c6e979d Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 26 Feb 2018 12:51:43 +0100 Subject: direct-io: Fix sleep in atomic due to sync AIO Commit e864f39569f4 "fs: add RWF_DSYNC aand RWF_SYNC" added additional way for direct IO to become synchronous and thus trigger fsync from the IO completion handler. Then commit 9830f4be159b "fs: Use RWF_* flags for AIO operations" allowed these flags to be set for AIO as well. However that commit forgot to update the condition checking whether the IO completion handling should be defered to a workqueue and thus AIO DIO with RWF_[D]SYNC set will call fsync() from IRQ context resulting in sleep in atomic. Fix the problem by checking directly iocb flags (the same way as it is done in dio_complete()) instead of checking all conditions that could lead to IO being synchronous. CC: Christoph Hellwig CC: Goldwyn Rodrigues CC: stable@vger.kernel.org Reported-by: Mark Rutland Tested-by: Mark Rutland Fixes: 9830f4be159b29399d107bffb99e0132bc5aedd4 Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- fs/direct-io.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index a0ca9e48e993..1357ef563893 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -1274,8 +1274,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode, */ if (dio->is_async && iov_iter_rw(iter) == WRITE) { retval = 0; - if ((iocb->ki_filp->f_flags & O_DSYNC) || - IS_SYNC(iocb->ki_filp->f_mapping->host)) + if (iocb->ki_flags & IOCB_DSYNC) retval = dio_set_defer_completion(dio); else if (!dio->inode->i_sb->s_dio_done_wq) { /* -- cgit v1.2.3-59-g8ed1b From d52987b524ccd2e2165ddad9bcc087a6c3f5332c Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 26 Feb 2018 13:01:37 +0100 Subject: genhd: Fix leaked module reference for NVME devices Commit 8ddcd653257c "block: introduce GENHD_FL_HIDDEN" added handling of hidden devices to get_gendisk() but forgot to drop module reference which is also acquired by get_disk(). Drop the reference as necessary. Arguably the function naming here is misleading as put_disk() is *not* the counterpart of get_disk() but let's fix that in the follow up commit since that will be more intrusive. Fixes: 8ddcd653257c18a669fcb75ee42c37054908e0d6 CC: Christoph Hellwig Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- block/genhd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/genhd.c b/block/genhd.c index 88a53c188cb7..5098bffe6ba6 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -817,7 +817,10 @@ struct gendisk *get_gendisk(dev_t devt, int *partno) } if (disk && unlikely(disk->flags & GENHD_FL_HIDDEN)) { + struct module *owner = disk->fops->owner; + put_disk(disk); + module_put(owner); disk = NULL; } return disk; -- cgit v1.2.3-59-g8ed1b From 3079c22ea815775837a4f389ce2f7e1e7b202e09 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 26 Feb 2018 13:01:38 +0100 Subject: genhd: Rename get_disk() to get_disk_and_module() Rename get_disk() to get_disk_and_module() to make sure what the function does. It's not a great name but at least it is now clear that put_disk() is not it's counterpart. Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- block/genhd.c | 10 ++++------ drivers/block/amiflop.c | 2 +- drivers/block/ataflop.c | 2 +- drivers/block/brd.c | 2 +- drivers/block/floppy.c | 2 +- drivers/block/loop.c | 2 +- drivers/block/swim.c | 2 +- drivers/block/z2ram.c | 2 +- drivers/ide/ide-probe.c | 2 +- include/linux/genhd.h | 2 +- 10 files changed, 13 insertions(+), 15 deletions(-) diff --git a/block/genhd.c b/block/genhd.c index 5098bffe6ba6..21b2843b27d0 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -547,7 +547,7 @@ static int exact_lock(dev_t devt, void *data) { struct gendisk *p = data; - if (!get_disk(p)) + if (!get_disk_and_module(p)) return -1; return 0; } @@ -809,7 +809,7 @@ struct gendisk *get_gendisk(dev_t devt, int *partno) spin_lock_bh(&ext_devt_lock); part = idr_find(&ext_devt_idr, blk_mangle_minor(MINOR(devt))); - if (part && get_disk(part_to_disk(part))) { + if (part && get_disk_and_module(part_to_disk(part))) { *partno = part->partno; disk = part_to_disk(part); } @@ -1456,7 +1456,7 @@ struct gendisk *__alloc_disk_node(int minors, int node_id) } EXPORT_SYMBOL(__alloc_disk_node); -struct kobject *get_disk(struct gendisk *disk) +struct kobject *get_disk_and_module(struct gendisk *disk) { struct module *owner; struct kobject *kobj; @@ -1474,15 +1474,13 @@ struct kobject *get_disk(struct gendisk *disk) return kobj; } - -EXPORT_SYMBOL(get_disk); +EXPORT_SYMBOL(get_disk_and_module); void put_disk(struct gendisk *disk) { if (disk) kobject_put(&disk_to_dev(disk)->kobj); } - EXPORT_SYMBOL(put_disk); static void set_disk_ro_uevent(struct gendisk *gd, int ro) diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c index e5aa62fcf5a8..3aaf6af3ec23 100644 --- a/drivers/block/amiflop.c +++ b/drivers/block/amiflop.c @@ -1758,7 +1758,7 @@ static struct kobject *floppy_find(dev_t dev, int *part, void *data) if (unit[drive].type->code == FD_NODRIVE) return NULL; *part = 0; - return get_disk(unit[drive].gendisk); + return get_disk_and_module(unit[drive].gendisk); } static int __init amiga_floppy_probe(struct platform_device *pdev) diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c index 8bc3b9fd8dd2..dfb2c2622e5a 100644 --- a/drivers/block/ataflop.c +++ b/drivers/block/ataflop.c @@ -1917,7 +1917,7 @@ static struct kobject *floppy_find(dev_t dev, int *part, void *data) if (drive >= FD_MAX_UNITS || type > NUM_DISK_MINORS) return NULL; *part = 0; - return get_disk(unit[drive].disk); + return get_disk_and_module(unit[drive].disk); } static int __init atari_floppy_init (void) diff --git a/drivers/block/brd.c b/drivers/block/brd.c index 8028a3a7e7fd..deea78e485da 100644 --- a/drivers/block/brd.c +++ b/drivers/block/brd.c @@ -456,7 +456,7 @@ static struct kobject *brd_probe(dev_t dev, int *part, void *data) mutex_lock(&brd_devices_mutex); brd = brd_init_one(MINOR(dev) / max_part, &new); - kobj = brd ? get_disk(brd->brd_disk) : NULL; + kobj = brd ? get_disk_and_module(brd->brd_disk) : NULL; mutex_unlock(&brd_devices_mutex); if (new) diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c index eae484acfbbc..8ec7235fc93b 100644 --- a/drivers/block/floppy.c +++ b/drivers/block/floppy.c @@ -4505,7 +4505,7 @@ static struct kobject *floppy_find(dev_t dev, int *part, void *data) if (((*part >> 2) & 0x1f) >= ARRAY_SIZE(floppy_type)) return NULL; *part = 0; - return get_disk(disks[drive]); + return get_disk_and_module(disks[drive]); } static int __init do_floppy_init(void) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index d5fe720cf149..87855b5123a6 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -1922,7 +1922,7 @@ static struct kobject *loop_probe(dev_t dev, int *part, void *data) if (err < 0) kobj = NULL; else - kobj = get_disk(lo->lo_disk); + kobj = get_disk_and_module(lo->lo_disk); mutex_unlock(&loop_index_mutex); *part = 0; diff --git a/drivers/block/swim.c b/drivers/block/swim.c index 84434d3ea19b..64e066eba72e 100644 --- a/drivers/block/swim.c +++ b/drivers/block/swim.c @@ -799,7 +799,7 @@ static struct kobject *floppy_find(dev_t dev, int *part, void *data) return NULL; *part = 0; - return get_disk(swd->unit[drive].disk); + return get_disk_and_module(swd->unit[drive].disk); } static int swim_add_floppy(struct swim_priv *swd, enum drive_location location) diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c index 41c95c9b2ab4..8f9130ab5887 100644 --- a/drivers/block/z2ram.c +++ b/drivers/block/z2ram.c @@ -332,7 +332,7 @@ static const struct block_device_operations z2_fops = static struct kobject *z2_find(dev_t dev, int *part, void *data) { *part = 0; - return get_disk(z2ram_gendisk); + return get_disk_and_module(z2ram_gendisk); } static struct request_queue *z2_queue; diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c index 17fd55af4d92..caa20eb5f26b 100644 --- a/drivers/ide/ide-probe.c +++ b/drivers/ide/ide-probe.c @@ -928,7 +928,7 @@ static int exact_lock(dev_t dev, void *data) { struct gendisk *p = data; - if (!get_disk(p)) + if (!get_disk_and_module(p)) return -1; return 0; } diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 5e3531027b51..8e11b9321e55 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -600,7 +600,7 @@ extern void delete_partition(struct gendisk *, int); extern void printk_all_partitions(void); extern struct gendisk *__alloc_disk_node(int minors, int node_id); -extern struct kobject *get_disk(struct gendisk *disk); +extern struct kobject *get_disk_and_module(struct gendisk *disk); extern void put_disk(struct gendisk *disk); extern void blk_register_region(dev_t devt, unsigned long range, struct module *module, -- cgit v1.2.3-59-g8ed1b From 9df6c29912315186fef1c79cc15b758ace84175b Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 26 Feb 2018 13:01:39 +0100 Subject: genhd: Add helper put_disk_and_module() Add a proper counterpart to get_disk_and_module() - put_disk_and_module(). Currently it is opencoded in several places. Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 11 ++--------- block/genhd.c | 20 ++++++++++++++++---- fs/block_dev.c | 19 +++++-------------- include/linux/genhd.h | 1 + 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 4117524ca45b..c2033a232a44 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -812,7 +812,6 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, struct gendisk *disk; struct request_queue *q; struct blkcg_gq *blkg; - struct module *owner; unsigned int major, minor; int key_len, part, ret; char *body; @@ -904,9 +903,7 @@ fail_unlock: spin_unlock_irq(q->queue_lock); rcu_read_unlock(); fail: - owner = disk->fops->owner; - put_disk(disk); - module_put(owner); + put_disk_and_module(disk); /* * If queue was bypassing, we should retry. Do so after a * short msleep(). It isn't strictly necessary but queue @@ -931,13 +928,9 @@ EXPORT_SYMBOL_GPL(blkg_conf_prep); void blkg_conf_finish(struct blkg_conf_ctx *ctx) __releases(ctx->disk->queue->queue_lock) __releases(rcu) { - struct module *owner; - spin_unlock_irq(ctx->disk->queue->queue_lock); rcu_read_unlock(); - owner = ctx->disk->fops->owner; - put_disk(ctx->disk); - module_put(owner); + put_disk_and_module(ctx->disk); } EXPORT_SYMBOL_GPL(blkg_conf_finish); diff --git a/block/genhd.c b/block/genhd.c index 21b2843b27d0..4c0590434591 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -817,10 +817,7 @@ struct gendisk *get_gendisk(dev_t devt, int *partno) } if (disk && unlikely(disk->flags & GENHD_FL_HIDDEN)) { - struct module *owner = disk->fops->owner; - - put_disk(disk); - module_put(owner); + put_disk_and_module(disk); disk = NULL; } return disk; @@ -1483,6 +1480,21 @@ void put_disk(struct gendisk *disk) } EXPORT_SYMBOL(put_disk); +/* + * This is a counterpart of get_disk_and_module() and thus also of + * get_gendisk(). + */ +void put_disk_and_module(struct gendisk *disk) +{ + if (disk) { + struct module *owner = disk->fops->owner; + + put_disk(disk); + module_put(owner); + } +} +EXPORT_SYMBOL(put_disk_and_module); + static void set_disk_ro_uevent(struct gendisk *gd, int ro) { char event[] = "DISK_RO=1"; diff --git a/fs/block_dev.c b/fs/block_dev.c index 4a181fcb5175..1dbbf847911a 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1111,8 +1111,7 @@ static struct block_device *bd_start_claiming(struct block_device *bdev, else whole = bdgrab(bdev); - module_put(disk->fops->owner); - put_disk(disk); + put_disk_and_module(disk); if (!whole) return ERR_PTR(-ENOMEM); @@ -1407,7 +1406,6 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part); static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) { struct gendisk *disk; - struct module *owner; int ret; int partno; int perm = 0; @@ -1433,7 +1431,6 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) disk = get_gendisk(bdev->bd_dev, &partno); if (!disk) goto out; - owner = disk->fops->owner; disk_block_events(disk); mutex_lock_nested(&bdev->bd_mutex, for_part); @@ -1463,8 +1460,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) bdev->bd_queue = NULL; mutex_unlock(&bdev->bd_mutex); disk_unblock_events(disk); - put_disk(disk); - module_put(owner); + put_disk_and_module(disk); goto restart; } } @@ -1525,8 +1521,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) goto out_unlock_bdev; } /* only one opener holds refs to the module and disk */ - put_disk(disk); - module_put(owner); + put_disk_and_module(disk); } bdev->bd_openers++; if (for_part) @@ -1546,8 +1541,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) out_unlock_bdev: mutex_unlock(&bdev->bd_mutex); disk_unblock_events(disk); - put_disk(disk); - module_put(owner); + put_disk_and_module(disk); out: bdput(bdev); @@ -1770,8 +1764,6 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part) disk->fops->release(disk, mode); } if (!bdev->bd_openers) { - struct module *owner = disk->fops->owner; - disk_put_part(bdev->bd_part); bdev->bd_part = NULL; bdev->bd_disk = NULL; @@ -1779,8 +1771,7 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part) victim = bdev->bd_contains; bdev->bd_contains = NULL; - put_disk(disk); - module_put(owner); + put_disk_and_module(disk); } mutex_unlock(&bdev->bd_mutex); bdput(bdev); diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 8e11b9321e55..7f5906fe1b70 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -602,6 +602,7 @@ extern void printk_all_partitions(void); extern struct gendisk *__alloc_disk_node(int minors, int node_id); extern struct kobject *get_disk_and_module(struct gendisk *disk); extern void put_disk(struct gendisk *disk); +extern void put_disk_and_module(struct gendisk *disk); extern void blk_register_region(dev_t devt, unsigned long range, struct module *module, struct kobject *(*probe)(dev_t, int *, void *), -- cgit v1.2.3-59-g8ed1b From 897366537fb65e87755b822360c230354c3fc73b Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 26 Feb 2018 13:01:40 +0100 Subject: genhd: Fix use after free in __blkdev_get() When two blkdev_open() calls race with device removal and recreation, __blkdev_get() can use looked up gendisk after it is freed: CPU0 CPU1 CPU2 del_gendisk(disk); bdev_unhash_inode(inode); blkdev_open() blkdev_open() bdev = bd_acquire(inode); - creates and returns new inode bdev = bd_acquire(inode); - returns the same inode __blkdev_get(devt) __blkdev_get(devt) disk = get_gendisk(devt); - got structure of device going away disk = get_gendisk(devt); - got new device structure if (!bdev->bd_openers) { does the first open } if (!bdev->bd_openers) - false } else { put_disk_and_module(disk) - remember this was old device - this was last ref and disk is now freed } disk_unblock_events(disk); -> oops Fix the problem by making sure we drop reference to disk in __blkdev_get() only after we are really done with it. Reported-by: Hou Tao Tested-by: Hou Tao Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- fs/block_dev.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fs/block_dev.c b/fs/block_dev.c index 1dbbf847911a..fe41a76769fa 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1409,6 +1409,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) int ret; int partno; int perm = 0; + bool first_open = false; if (mode & FMODE_READ) perm |= MAY_READ; @@ -1435,6 +1436,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) disk_block_events(disk); mutex_lock_nested(&bdev->bd_mutex, for_part); if (!bdev->bd_openers) { + first_open = true; bdev->bd_disk = disk; bdev->bd_queue = disk->queue; bdev->bd_contains = bdev; @@ -1520,14 +1522,15 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) if (ret) goto out_unlock_bdev; } - /* only one opener holds refs to the module and disk */ - put_disk_and_module(disk); } bdev->bd_openers++; if (for_part) bdev->bd_part_count++; mutex_unlock(&bdev->bd_mutex); disk_unblock_events(disk); + /* only one opener holds refs to the module and disk */ + if (!first_open) + put_disk_and_module(disk); return 0; out_clear: -- cgit v1.2.3-59-g8ed1b From 56c0908c855afbb2bdda17c15d2879949a091ad3 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 26 Feb 2018 13:01:41 +0100 Subject: genhd: Fix BUG in blkdev_open() When two blkdev_open() calls for a partition race with device removal and recreation, we can hit BUG_ON(!bd_may_claim(bdev, whole, holder)) in blkdev_open(). The race can happen as follows: CPU0 CPU1 CPU2 del_gendisk() bdev_unhash_inode(part1); blkdev_open(part1, O_EXCL) blkdev_open(part1, O_EXCL) bdev = bd_acquire() bdev = bd_acquire() blkdev_get(bdev) bd_start_claiming(bdev) - finds old inode 'whole' bd_prepare_to_claim() -> 0 bdev_unhash_inode(whole); blkdev_get(bdev); bd_start_claiming(bdev) - finds new inode 'whole' bd_prepare_to_claim() - this also succeeds as we have different 'whole' here... - bad things happen now as we have two exclusive openers of the same bdev The problem here is that block device opens can see various intermediate states while gendisk is shutting down and then being recreated. We fix the problem by introducing new lookup_sem in gendisk that synchronizes gendisk deletion with get_gendisk() and furthermore by making sure that get_gendisk() does not return gendisk that is being (or has been) deleted. This makes sure that once we ever manage to look up newly created bdev inode, we are also guaranteed that following get_gendisk() will either return failure (and we fail open) or it returns gendisk for the new device and following bdget_disk() will return new bdev inode (i.e., blkdev_open() follows the path as if it is completely run after new device is created). Reported-and-analyzed-by: Hou Tao Tested-by: Hou Tao Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- block/genhd.c | 21 ++++++++++++++++++++- include/linux/genhd.h | 1 + 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/block/genhd.c b/block/genhd.c index 4c0590434591..9656f9e9f99e 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -717,6 +717,11 @@ void del_gendisk(struct gendisk *disk) blk_integrity_del(disk); disk_del_events(disk); + /* + * Block lookups of the disk until all bdevs are unhashed and the + * disk is marked as dead (GENHD_FL_UP cleared). + */ + down_write(&disk->lookup_sem); /* invalidate stuff */ disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE); @@ -731,6 +736,7 @@ void del_gendisk(struct gendisk *disk) bdev_unhash_inode(disk_devt(disk)); set_capacity(disk, 0); disk->flags &= ~GENHD_FL_UP; + up_write(&disk->lookup_sem); if (!(disk->flags & GENHD_FL_HIDDEN)) sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi"); @@ -816,9 +822,21 @@ struct gendisk *get_gendisk(dev_t devt, int *partno) spin_unlock_bh(&ext_devt_lock); } - if (disk && unlikely(disk->flags & GENHD_FL_HIDDEN)) { + if (!disk) + return NULL; + + /* + * Synchronize with del_gendisk() to not return disk that is being + * destroyed. + */ + down_read(&disk->lookup_sem); + if (unlikely((disk->flags & GENHD_FL_HIDDEN) || + !(disk->flags & GENHD_FL_UP))) { + up_read(&disk->lookup_sem); put_disk_and_module(disk); disk = NULL; + } else { + up_read(&disk->lookup_sem); } return disk; } @@ -1418,6 +1436,7 @@ struct gendisk *__alloc_disk_node(int minors, int node_id) kfree(disk); return NULL; } + init_rwsem(&disk->lookup_sem); disk->node_id = node_id; if (disk_expand_part_tbl(disk, 0)) { free_part_stats(&disk->part0); diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 7f5906fe1b70..c826b0b5232a 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -198,6 +198,7 @@ struct gendisk { void *private_data; int flags; + struct rw_semaphore lookup_sem; struct kobject *slave_dir; struct timer_rand_state *random; -- cgit v1.2.3-59-g8ed1b From 560e7cb2f3c7f09bbfb36cd0b900e24fddd20282 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 26 Feb 2018 13:01:42 +0100 Subject: blockdev: Avoid two active bdev inodes for one device When blkdev_open() races with device removal and creation it can happen that unhashed bdev inode gets associated with newly created gendisk like: CPU0 CPU1 blkdev_open() bdev = bd_acquire() del_gendisk() bdev_unhash_inode(bdev); remove device create new device with the same number __blkdev_get() disk = get_gendisk() - gets reference to gendisk of the new device Now another blkdev_open() will not find original 'bdev' as it got unhashed, create a new one and associate it with the same 'disk' at which point problems start as we have two independent page caches for one device. Fix the problem by verifying that the bdev inode didn't get unhashed before we acquired gendisk reference. That way we make sure gendisk can get associated only with visible bdev inodes. Tested-by: Hou Tao Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- fs/block_dev.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/fs/block_dev.c b/fs/block_dev.c index fe41a76769fa..fe09ef9c21f3 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1058,6 +1058,27 @@ retry: return 0; } +static struct gendisk *bdev_get_gendisk(struct block_device *bdev, int *partno) +{ + struct gendisk *disk = get_gendisk(bdev->bd_dev, partno); + + if (!disk) + return NULL; + /* + * Now that we hold gendisk reference we make sure bdev we looked up is + * not stale. If it is, it means device got removed and created before + * we looked up gendisk and we fail open in such case. Associating + * unhashed bdev with newly created gendisk could lead to two bdevs + * (and thus two independent caches) being associated with one device + * which is bad. + */ + if (inode_unhashed(bdev->bd_inode)) { + put_disk_and_module(disk); + return NULL; + } + return disk; +} + /** * bd_start_claiming - start claiming a block device * @bdev: block device of interest @@ -1094,7 +1115,7 @@ static struct block_device *bd_start_claiming(struct block_device *bdev, * @bdev might not have been initialized properly yet, look up * and grab the outer block device the hard way. */ - disk = get_gendisk(bdev->bd_dev, &partno); + disk = bdev_get_gendisk(bdev, &partno); if (!disk) return ERR_PTR(-ENXIO); @@ -1429,7 +1450,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) restart: ret = -ENXIO; - disk = get_gendisk(bdev->bd_dev, &partno); + disk = bdev_get_gendisk(bdev, &partno); if (!disk) goto out; -- cgit v1.2.3-59-g8ed1b From 9c72258870a95671aa301e21ea6639d1d3ec4111 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 26 Jan 2018 16:58:06 -0800 Subject: blktrace_api.h: fix comment for struct blk_user_trace_setup 'struct blk_user_trace_setup' is passed to BLKTRACESETUP, not BLKTRACESTART. Signed-off-by: Eric Biggers Signed-off-by: Jens Axboe --- include/uapi/linux/blktrace_api.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/blktrace_api.h b/include/uapi/linux/blktrace_api.h index 20d1490d6377..3c50e07ee833 100644 --- a/include/uapi/linux/blktrace_api.h +++ b/include/uapi/linux/blktrace_api.h @@ -131,7 +131,7 @@ enum { #define BLKTRACE_BDEV_SIZE 32 /* - * User setup structure passed with BLKTRACESTART + * User setup structure passed with BLKTRACESETUP */ struct blk_user_trace_setup { char name[BLKTRACE_BDEV_SIZE]; /* output */ -- cgit v1.2.3-59-g8ed1b From 02aa8a8b2b84531fa78b9a486d9b2a0700f7bc08 Mon Sep 17 00:00:00 2001 From: Coly Li Date: Tue, 27 Feb 2018 09:49:29 -0800 Subject: bcache: correct flash only vols (check all uuids) Commit 2831231d4c3f ("bcache: reduce cache_set devices iteration by devices_max_used") adds c->devices_max_used to reduce iteration of c->uuids elements, this value is updated in bcache_device_attach(). But for flash only volume, when calling flash_devs_run(), the function bcache_device_attach() is not called yet and c->devices_max_used is not updated. The unexpected result is, the flash only volume won't be run by flash_devs_run(). This patch fixes the issue by iterate all c->uuids elements in flash_devs_run(). c->devices_max_used will be updated properly when bcache_device_attach() gets called. [mlyle: commit subject edited for character limit] Fixes: 2831231d4c3f ("bcache: reduce cache_set devices iteration by devices_max_used") Reported-by: Tang Junhui Signed-off-by: Coly Li Reviewed-by: Michael Lyle Signed-off-by: Jens Axboe --- drivers/md/bcache/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index 312895788036..4d1d8dfb2d2a 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -1274,7 +1274,7 @@ static int flash_devs_run(struct cache_set *c) struct uuid_entry *u; for (u = c->uuids; - u < c->uuids + c->devices_max_used && !ret; + u < c->uuids + c->nr_uuids && !ret; u++) if (UUID_FLASH_ONLY(u)) ret = flash_dev_run(c, u); -- cgit v1.2.3-59-g8ed1b From 60eb34ec5526e264c2bbaea4f7512d714d791caf Mon Sep 17 00:00:00 2001 From: Tang Junhui Date: Tue, 27 Feb 2018 09:49:30 -0800 Subject: bcache: fix kcrashes with fio in RAID5 backend dev Kernel crashed when run fio in a RAID5 backend bcache device, the call trace is bellow: [ 440.012034] kernel BUG at block/blk-ioc.c:146! [ 440.012696] invalid opcode: 0000 [#1] SMP NOPTI [ 440.026537] CPU: 2 PID: 2205 Comm: md127_raid5 Not tainted 4.15.0 #8 [ 440.027441] Hardware name: HP ProLiant MicroServer Gen8, BIOS J06 07/16 /2015 [ 440.028615] RIP: 0010:put_io_context+0x8b/0x90 [ 440.029246] RSP: 0018:ffffa8c882b43af8 EFLAGS: 00010246 [ 440.029990] RAX: 0000000000000000 RBX: ffffa8c88294fca0 RCX: 0000000000 0f4240 [ 440.031006] RDX: 0000000000000004 RSI: 0000000000000286 RDI: ffffa8c882 94fca0 [ 440.032030] RBP: ffffa8c882b43b10 R08: 0000000000000003 R09: ffff949cb8 0c1700 [ 440.033206] R10: 0000000000000104 R11: 000000000000b71c R12: 00000000000 01000 [ 440.034222] R13: 0000000000000000 R14: ffff949cad84db70 R15: ffff949cb11 bd1e0 [ 440.035239] FS: 0000000000000000(0000) GS:ffff949cba280000(0000) knlGS: 0000000000000000 [ 440.060190] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 440.084967] CR2: 00007ff0493ef000 CR3: 00000002f1e0a002 CR4: 00000000001 606e0 [ 440.110498] Call Trace: [ 440.135443] bio_disassociate_task+0x1b/0x60 [ 440.160355] bio_free+0x1b/0x60 [ 440.184666] bio_put+0x23/0x30 [ 440.208272] search_free+0x23/0x40 [bcache] [ 440.231448] cached_dev_write_complete+0x31/0x70 [bcache] [ 440.254468] closure_put+0xb6/0xd0 [bcache] [ 440.277087] request_endio+0x30/0x40 [bcache] [ 440.298703] bio_endio+0xa1/0x120 [ 440.319644] handle_stripe+0x418/0x2270 [raid456] [ 440.340614] ? load_balance+0x17b/0x9c0 [ 440.360506] handle_active_stripes.isra.58+0x387/0x5a0 [raid456] [ 440.380675] ? __release_stripe+0x15/0x20 [raid456] [ 440.400132] raid5d+0x3ed/0x5d0 [raid456] [ 440.419193] ? schedule+0x36/0x80 [ 440.437932] ? schedule_timeout+0x1d2/0x2f0 [ 440.456136] md_thread+0x122/0x150 [ 440.473687] ? wait_woken+0x80/0x80 [ 440.491411] kthread+0x102/0x140 [ 440.508636] ? find_pers+0x70/0x70 [ 440.524927] ? kthread_associate_blkcg+0xa0/0xa0 [ 440.541791] ret_from_fork+0x35/0x40 [ 440.558020] Code: c2 48 00 5b 41 5c 41 5d 5d c3 48 89 c6 4c 89 e7 e8 bb c2 48 00 48 8b 3d bc 36 4b 01 48 89 de e8 7c f7 e0 ff 5b 41 5c 41 5d 5d c3 <0f> 0b 0f 1f 00 0f 1f 44 00 00 55 48 8d 47 b8 48 89 e5 41 57 41 [ 440.610020] RIP: put_io_context+0x8b/0x90 RSP: ffffa8c882b43af8 [ 440.628575] ---[ end trace a1fd79d85643a73e ]-- All the crash issue happened when a bypass IO coming, in such scenario s->iop.bio is pointed to the s->orig_bio. In search_free(), it finishes the s->orig_bio by calling bio_complete(), and after that, s->iop.bio became invalid, then kernel would crash when calling bio_put(). Maybe its upper layer's faulty, since bio should not be freed before we calling bio_put(), but we'd better calling bio_put() first before calling bio_complete() to notify upper layer ending this bio. This patch moves bio_complete() under bio_put() to avoid kernel crash. [mlyle: fixed commit subject for character limits] Reported-by: Matthias Ferdinand Tested-by: Matthias Ferdinand Signed-off-by: Tang Junhui Reviewed-by: Michael Lyle Signed-off-by: Jens Axboe --- drivers/md/bcache/request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c index 1a46b41dac70..6422846b546e 100644 --- a/drivers/md/bcache/request.c +++ b/drivers/md/bcache/request.c @@ -659,11 +659,11 @@ static void do_bio_hook(struct search *s, struct bio *orig_bio) static void search_free(struct closure *cl) { struct search *s = container_of(cl, struct search, cl); - bio_complete(s); if (s->iop.bio) bio_put(s->iop.bio); + bio_complete(s); closure_debug_destroy(cl); mempool_free(s, s->d->c->search); } -- cgit v1.2.3-59-g8ed1b From 0979962f5490abe75b3e2befb07a564fa0cf631b Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Mon, 12 Feb 2018 11:14:55 -0600 Subject: nbd: fix return value in error handling path It seems that the proper value to return in this particular case is the one contained into variable new_index instead of ret. Addresses-Coverity-ID: 1465148 ("Copy-paste error") Fixes: e46c7287b1c2 ("nbd: add a basic netlink interface") Reviewed-by: Omar Sandoval Signed-off-by: Gustavo A. R. Silva Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 5f2a4240a204..86258b00a1d4 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1591,7 +1591,7 @@ again: if (new_index < 0) { mutex_unlock(&nbd_index_mutex); printk(KERN_ERR "nbd: failed to add new device\n"); - return ret; + return new_index; } nbd = idr_find(&nbd_index_idr, new_index); } -- cgit v1.2.3-59-g8ed1b From 9bd82b1a4418d9b7db000bf557ed608f2872b7c9 Mon Sep 17 00:00:00 2001 From: Baegjae Sung Date: Wed, 28 Feb 2018 16:06:04 +0900 Subject: nvme-multipath: fix sysfs dangerously created links If multipathing is enabled, each NVMe subsystem creates a head namespace (e.g., nvme0n1) and multiple private namespaces (e.g., nvme0c0n1 and nvme0c1n1) in sysfs. When creating links for private namespaces, links of head namespace are used, so the namespace creation order must be followed (e.g., nvme0n1 -> nvme0c1n1). If the order is not followed, links of sysfs will be incomplete or kernel panic will occur. The kernel panic was: kernel BUG at fs/sysfs/symlink.c:27! Call Trace: nvme_mpath_add_disk_links+0x5d/0x80 [nvme_core] nvme_validate_ns+0x5c2/0x850 [nvme_core] nvme_scan_work+0x1af/0x2d0 [nvme_core] Correct order Context A Context B nvme0n1 nvme0c0n1 nvme0c1n1 Incorrect order Context A Context B nvme0c1n1 nvme0n1 nvme0c0n1 The nvme_mpath_add_disk (for creating head namespace) is called just before the nvme_mpath_add_disk_links (for creating private namespaces). In nvme_mpath_add_disk, the first context acquires the lock of subsystem and creates a head namespace, and other contexts do nothing by checking GENHD_FL_UP of a head namespace after waiting to acquire the lock. We verified the code with or without multipathing using three vendors of dual-port NVMe SSDs. Signed-off-by: Baegjae Sung Reviewed-by: Christoph Hellwig Signed-off-by: Keith Busch --- drivers/nvme/host/core.c | 12 +++--------- drivers/nvme/host/multipath.c | 15 ++++++++++----- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index f431c32774f3..6088ea13a6bf 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2835,7 +2835,7 @@ out: } static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid, - struct nvme_id_ns *id, bool *new) + struct nvme_id_ns *id) { struct nvme_ctrl *ctrl = ns->ctrl; bool is_shared = id->nmic & (1 << 0); @@ -2851,8 +2851,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid, ret = PTR_ERR(head); goto out_unlock; } - - *new = true; } else { struct nvme_ns_ids ids; @@ -2864,8 +2862,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid, ret = -EINVAL; goto out_unlock; } - - *new = false; } list_add_tail(&ns->siblings, &head->list); @@ -2936,7 +2932,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) struct nvme_id_ns *id; char disk_name[DISK_NAME_LEN]; int node = dev_to_node(ctrl->dev), flags = GENHD_FL_EXT_DEVT; - bool new = true; ns = kzalloc_node(sizeof(*ns), GFP_KERNEL, node); if (!ns) @@ -2962,7 +2957,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) if (id->ncap == 0) goto out_free_id; - if (nvme_init_ns_head(ns, nsid, id, &new)) + if (nvme_init_ns_head(ns, nsid, id)) goto out_free_id; nvme_setup_streams_ns(ctrl, ns); @@ -3028,8 +3023,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) pr_warn("%s: failed to register lightnvm sysfs group for identification\n", ns->disk->disk_name); - if (new) - nvme_mpath_add_disk(ns->head); + nvme_mpath_add_disk(ns->head); nvme_mpath_add_disk_links(ns); return; out_unlink_ns: diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 3b211d9e58b8..b7e5c6db4d92 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -198,11 +198,16 @@ void nvme_mpath_add_disk(struct nvme_ns_head *head) { if (!head->disk) return; - device_add_disk(&head->subsys->dev, head->disk); - if (sysfs_create_group(&disk_to_dev(head->disk)->kobj, - &nvme_ns_id_attr_group)) - pr_warn("%s: failed to create sysfs group for identification\n", - head->disk->disk_name); + + mutex_lock(&head->subsys->lock); + if (!(head->disk->flags & GENHD_FL_UP)) { + device_add_disk(&head->subsys->dev, head->disk); + if (sysfs_create_group(&disk_to_dev(head->disk)->kobj, + &nvme_ns_id_attr_group)) + pr_warn("%s: failed to create sysfs group for identification\n", + head->disk->disk_name); + } + mutex_unlock(&head->subsys->lock); } void nvme_mpath_add_disk_links(struct nvme_ns *ns) -- cgit v1.2.3-59-g8ed1b From bffd2b61670feef18d2535e9b53364d270a1c991 Mon Sep 17 00:00:00 2001 From: Max Gurtovoy Date: Wed, 24 Jan 2018 17:31:45 +0200 Subject: nvmet: fix PSDT field check in command format PSDT field section according to NVM_Express-1.3: "This field specifies whether PRPs or SGLs are used for any data transfer associated with the command. PRPs shall be used for all Admin commands for NVMe over PCIe. SGLs shall be used for all Admin and I/O commands for NVMe over Fabrics. This field shall be set to 01b for NVMe over Fabrics 1.0 implementations. Suggested-by: Idan Burstein Signed-off-by: Max Gurtovoy Reviewed-by: Christoph Hellwig Signed-off-by: Keith Busch --- drivers/nvme/target/core.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 0bd737117a80..a78029e4e5f4 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -520,9 +520,12 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq, goto fail; } - /* either variant of SGLs is fine, as we don't support metadata */ - if (unlikely((flags & NVME_CMD_SGL_ALL) != NVME_CMD_SGL_METABUF && - (flags & NVME_CMD_SGL_ALL) != NVME_CMD_SGL_METASEG)) { + /* + * For fabrics, PSDT field shall describe metadata pointer (MPTR) that + * contains an address of a single contiguous physical buffer that is + * byte aligned. + */ + if (unlikely((flags & NVME_CMD_SGL_ALL) != NVME_CMD_SGL_METABUF)) { status = NVME_SC_INVALID_FIELD | NVME_SC_DNR; goto fail; } -- cgit v1.2.3-59-g8ed1b From f3bc78d2d4b489590540ab2788d5376583e28173 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Wed, 28 Feb 2018 09:35:29 -0800 Subject: mq-deadline: Make sure to always unlock zones In case of a failed write request (all retries failed) and when using libata, the SCSI error handler calls scsi_finish_command(). In the case of blk-mq this means that scsi_mq_done() does not get called, that blk_mq_complete_request() does not get called and also that the mq-deadline .completed_request() method is not called. This results in the target zone of the failed write request being left in a locked state, preventing that any new write requests are issued to the same zone. Fix this by replacing the .completed_request() method with the .finish_request() method as this method is always called whether or not a request completes successfully. Since the .finish_request() method is only called by the blk-mq core if a .prepare_request() method exists, add a dummy .prepare_request() method. Fixes: 5700f69178e9 ("mq-deadline: Introduce zone locking support") Cc: Hannes Reinecke Reviewed-by: Ming Lei Signed-off-by: Damien Le Moal [ bvanassche: edited patch description ] Signed-off-by: Bart Van Assche Signed-off-by: Jens Axboe --- block/mq-deadline.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/block/mq-deadline.c b/block/mq-deadline.c index c56f211c8440..8ec0ba9f5386 100644 --- a/block/mq-deadline.c +++ b/block/mq-deadline.c @@ -535,13 +535,22 @@ static void dd_insert_requests(struct blk_mq_hw_ctx *hctx, spin_unlock(&dd->lock); } +/* + * Nothing to do here. This is defined only to ensure that .finish_request + * method is called upon request completion. + */ +static void dd_prepare_request(struct request *rq, struct bio *bio) +{ +} + /* * For zoned block devices, write unlock the target zone of * completed write requests. Do this while holding the zone lock * spinlock so that the zone is never unlocked while deadline_fifo_request() - * while deadline_next_request() are executing. + * or deadline_next_request() are executing. This function is called for + * all requests, whether or not these requests complete successfully. */ -static void dd_completed_request(struct request *rq) +static void dd_finish_request(struct request *rq) { struct request_queue *q = rq->q; @@ -756,7 +765,8 @@ static struct elevator_type mq_deadline = { .ops.mq = { .insert_requests = dd_insert_requests, .dispatch_request = dd_dispatch_request, - .completed_request = dd_completed_request, + .prepare_request = dd_prepare_request, + .finish_request = dd_finish_request, .next_request = elv_rb_latter_request, .former_request = elv_rb_former_request, .bio_merge = dd_bio_merge, -- cgit v1.2.3-59-g8ed1b From 7c5a0dcf557c6511a61e092ba887de28882fe857 Mon Sep 17 00:00:00 2001 From: Jiufei Xue Date: Tue, 27 Feb 2018 20:10:03 +0800 Subject: block: fix the count of PGPGOUT for WRITE_SAME The vm counters is counted in sectors, so we should do the conversation in submit_bio. Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index") Cc: stable@vger.kernel.org Reviewed-by: Omar Sandoval Reviewed-by: Christoph Hellwig Signed-off-by: Jiufei Xue Signed-off-by: Jens Axboe --- block/blk-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/blk-core.c b/block/blk-core.c index 2d1a7bbe0634..6d82c4f7fadd 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -2434,7 +2434,7 @@ blk_qc_t submit_bio(struct bio *bio) unsigned int count; if (unlikely(bio_op(bio) == REQ_OP_WRITE_SAME)) - count = queue_logical_block_size(bio->bi_disk->queue); + count = queue_logical_block_size(bio->bi_disk->queue) >> 9; else count = bio_sectors(bio); -- cgit v1.2.3-59-g8ed1b From 9c0fb1e313aaf4e8edec22433c8b22dd308e466c Mon Sep 17 00:00:00 2001 From: Jiufei Xue Date: Tue, 27 Feb 2018 20:10:18 +0800 Subject: block: display the correct diskname for bio bio_devname use __bdevname to display the device name, and can only show the major and minor of the part0, Fix this by using disk_name to display the correct name. Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index") Reviewed-by: Omar Sandoval Reviewed-by: Christoph Hellwig Signed-off-by: Jiufei Xue Signed-off-by: Jens Axboe --- block/partition-generic.c | 6 ++++++ include/linux/bio.h | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/block/partition-generic.c b/block/partition-generic.c index 91622db9aedf..08dabcd8b6ae 100644 --- a/block/partition-generic.c +++ b/block/partition-generic.c @@ -51,6 +51,12 @@ const char *bdevname(struct block_device *bdev, char *buf) EXPORT_SYMBOL(bdevname); +const char *bio_devname(struct bio *bio, char *buf) +{ + return disk_name(bio->bi_disk, bio->bi_partno, buf); +} +EXPORT_SYMBOL(bio_devname); + /* * There's very little reason to use this, you should really * have a struct block_device just about everywhere and use diff --git a/include/linux/bio.h b/include/linux/bio.h index d0eb659fa733..ce547a25e8ae 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -511,6 +511,7 @@ void zero_fill_bio(struct bio *bio); extern struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *); extern void bvec_free(mempool_t *, struct bio_vec *, unsigned int); extern unsigned int bvec_nr_vecs(unsigned short idx); +extern const char *bio_devname(struct bio *bio, char *buffer); #define bio_set_dev(bio, bdev) \ do { \ @@ -529,9 +530,6 @@ do { \ #define bio_dev(bio) \ disk_devt((bio)->bi_disk) -#define bio_devname(bio, buf) \ - __bdevname(bio_dev(bio), (buf)) - #ifdef CONFIG_BLK_CGROUP int bio_associate_blkcg(struct bio *bio, struct cgroup_subsys_state *blkcg_css); void bio_disassociate_task(struct bio *bio); -- cgit v1.2.3-59-g8ed1b From 158e61865a31ef7abf39629c37285810504d60b5 Mon Sep 17 00:00:00 2001 From: Jiufei Xue Date: Tue, 27 Feb 2018 20:10:22 +0800 Subject: block: fix a typo Fix a typo in pkt_start_recovery. Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index") Reviewed-by: Christoph Hellwig Signed-off-by: Jiufei Xue Signed-off-by: Jens Axboe --- drivers/block/pktcdvd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index 531a0915066b..c61d20c9f3f8 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -1122,7 +1122,7 @@ static int pkt_start_recovery(struct packet_data *pkt) pkt->sector = new_sector; bio_reset(pkt->bio); - bio_set_set(pkt->bio, pd->bdev); + bio_set_dev(pkt->bio, pd->bdev); bio_set_op_attrs(pkt->bio, REQ_OP_WRITE, 0); pkt->bio->bi_iter.bi_sector = new_sector; pkt->bio->bi_iter.bi_size = pkt->frames * CD_FRAMESIZE; -- cgit v1.2.3-59-g8ed1b