- 04 May, 2019 6 commits
-
-
Ming Lei authored
hctx is always released after requeue is freed. With holding queue's kobject refcount, it is safe for driver to run queue, so one run queue might be scheduled after blk_sync_queue() is done. So moving the cancel of hctx->run_work into blk_mq_hw_sysfs_release() for avoiding run released queue. Cc: Dongli Zhang <dongli.zhang@oracle.com> Cc: James Smart <james.smart@broadcom.com> Cc: Bart Van Assche <bart.vanassche@wdc.com> Cc: linux-scsi@vger.kernel.org, Cc: Martin K . Petersen <martin.petersen@oracle.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>, Reviewed-by:
Bart Van Assche <bvanassche@acm.org> Reviewed-by:
Hannes Reinecke <hare@suse.com> Tested-by:
James Smart <james.smart@broadcom.com> Signed-off-by:
Ming Lei <ming.lei@redhat.com> Signed-off-by:
Jens Axboe <axboe@kernel.dk>
-
Ming Lei authored
In normal queue cleanup path, hctx is released after request queue is freed, see blk_mq_release(). However, in __blk_mq_update_nr_hw_queues(), hctx may be freed because of hw queues shrinking. This way is easy to cause use-after-free, because: one implicit rule is that it is safe to call almost all block layer APIs if the request queue is alive; and one hctx may be retrieved by one API, then the hctx can be freed by blk_mq_update_nr_hw_queues(); finally use-after-free is triggered. Fixes this issue by always freeing hctx after releasing request queue. If some hctxs are removed in blk_mq_update_nr_hw_queues(), introduce a per-queue list to hold them, then try to resuse these hctxs if numa node is matched. Cc: Dongli Zhang <dongli.zhang@oracle.com> Cc: James Smart <james.smart@broadcom.com> Cc: Bart Van Assche <bart.vanassche@wdc.com> Cc: linux-scsi@vger.kernel.org, Cc: Martin K . Petersen <martin.petersen@oracle.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>, Reviewed-by:
-
Ming Lei authored
Split blk_mq_alloc_and_init_hctx into two parts, and one is blk_mq_alloc_hctx() for allocating all hctx resources, another is blk_mq_init_hctx() for initializing hctx, which serves as counter-part of blk_mq_exit_hctx(). Cc: Dongli Zhang <dongli.zhang@oracle.com> Cc: James Smart <james.smart@broadcom.com> Cc: Bart Van Assche <bart.vanassche@wdc.com> Cc: linux-scsi@vger.kernel.org Cc: Martin K . Petersen <martin.petersen@oracle.com> Cc: Christoph Hellwig <hch@lst.de> Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com> Reviewed-by:
Christoph Hellwig <hch@lst.de> Tested-by:
-
Ming Lei authored
Once blk_cleanup_queue() returns, tags shouldn't be used any more, because blk_mq_free_tag_set() may be called. Commit 45a9c9d9 ("blk-mq: Fix a use-after-free") fixes this issue exactly. However, that commit introduces another issue. Before 45a9c9d9, we are allowed to run queue during cleaning up queue if the queue's kobj refcount is held. After that commit, queue can't be run during queue cleaning up, otherwise oops can be triggered easily because some fields of hctx are freed by blk_mq_free_queue() in blk_cleanup_queue(). We have invented ways for addressing this kind of issue before, such as: 8dc765d4 ("SCSI: fix queue cleanup race before queue initialization is done") c2856ae2 ("blk-mq: quiesce queue before freeing queue") But still can't cover all cases, recently James reports another such kind of issue: https://marc.info/?l=linux-scsi&m=155389088124782&w=2 This issue can be quite hard to address by previous way, given scsi_run_queue() may run requeues for other LUNs. Fixes the above issue by freeing hctx's resources in its release handler, and this way is safe becasue tags isn't needed for freeing such hctx resource. This approach follows typical design pattern wrt. kobject's release handler. Cc: Dongli Zhang <dongli.zhang@oracle.com> Cc: James Smart <james.smart@broadcom.com> Cc: Bart Van Assche <bart.vanassche@wdc.com> Cc: linux-scsi@vger.kernel.org, Cc: Martin K . Petersen <martin.petersen@oracle.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>, Reported-by:
-
Ming Lei authored
With holding queue's kobject refcount, it is safe for driver to schedule requeue. However, blk_mq_kick_requeue_list() may be called after blk_sync_queue() is done because of concurrent requeue activities, then requeue work may not be completed when freeing queue, and kernel oops is triggered. So moving the cancel of requeue_work into blk_mq_release() for avoiding race between requeue and freeing queue. Cc: Dongli Zhang <dongli.zhang@oracle.com> Cc: James Smart <james.smart@broadcom.com> Cc: Bart Van Assche <bart.vanassche@wdc.com> Cc: linux-scsi@vger.kernel.org, Cc: Martin K . Petersen <martin.petersen@oracle.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>, Reviewed-by:
Johannes Thumshirn <jthumshirn@suse.de> Reviewed-by:
-
Ming Lei authored
Just like aio/io_uring, we need to grab 2 refcount for queuing one request, one is for submission, another is for completion. If the request isn't queued from plug code path, the refcount grabbed in generic_make_request() serves for submission. In theroy, this refcount should have been released after the sumission(async run queue) is done. blk_freeze_queue() works with blk_sync_queue() together for avoiding race between cleanup queue and IO submission, given async run queue activities are canceled because hctx->run_work is scheduled with the refcount held, so it is fine to not hold the refcount when running the run queue work function for dispatch IO. However, if request is staggered into plug list, and finally queued from plug code path, the refcount in submission side is actually missed. And we may start to run queue after queue is removed because the queue's kobject refcount isn't guaranteed to be grabbed in flushing plug list context, then kernel oops is triggered, see the following race: blk_mq_flush_plug_list(): blk_mq_sched_insert_requests() insert requests to sw queue or scheduler queue blk_mq_run_hw_queue Because of concurrent run queue, all requests inserted above may be completed before calling the above blk_mq_run_hw_queue. Then queue can be freed during the above blk_mq_run_hw_queue(). Fixes the issue by grab .q_usage_counter before calling blk_mq_sched_insert_requests() in blk_mq_flush_plug_list(). This way is safe because the queue is absolutely alive before inserting request. Cc: Dongli Zhang <dongli.zhang@oracle.com> Cc: James Smart <james.smart@broadcom.com> Cc: linux-scsi@vger.kernel.org, Cc: Martin K . Petersen <martin.petersen@oracle.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>, Reviewed-by:
-
- 02 May, 2019 2 commits
-
-
git://git.infradead.org/nvmeJens Axboe authored
Pull NVMe updates from Christoph. * 'nvme-5.2' of git://git.infradead.org/nvme: nvmet: protect discovery change log event list iteration nvme: mark nvme_core_init and nvme_core_exit static nvme: move command size checks to the core nvme-fabrics: check more command sizes nvme-pci: check more command sizes nvme-pci: remove an unneeded variable initialization nvme-pci: unquiesce admin queue on shutdown nvme-pci: shutdown on timeout during deletion nvme-pci: fix psdt field for single segment sgls nvme-multipath: don't print ANA group state by default nvme-multipath: split bios with the ns_head bio_set before submitting nvme-tcp: fix possible null deref on a timed out io queue connect
-
Raul E Rangel authored
The comment was out of date. Reviewed-by:
Raul E Rangel <rrangel@chromium.org> Signed-off-by:
-
- 01 May, 2019 13 commits
-
-
Sagi Grimberg authored
When we iterate on the discovery subsystem controllers we need to protect against concurrent mutations to it. Signed-off-by:
Sagi Grimberg <sagi@grimberg.me> Reviewed-by:
Minwoo Im <minwoo.im@samsung.com> Signed-off-by:
-
Christoph Hellwig authored
Signed-off-by:
Keith Busch <keith.busch@intel.com> Reviewed-by:
Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
-
Christoph Hellwig authored
Most command aren't PCIe specific, so move the size checking for them to core.c Signed-off-by:
-
Minwoo Im authored
struct common_command provides a common structure for NVMe-oF command format. It also needs to be checked for unintended size growth. Signed-off-by:
Minwoo Im <minwoo.im.dev@gmail.com> Reviewed-by:
-
Minwoo Im authored
All the NVMe command has 64bytes fixed size so that it has been assured with BUILD_BUG_ON(). The remaining command structures in linux/nvme.h also need to be checked here. Signed-off-by:
-
Minwoo Im authored
Variable "n" will be assigned once kstrtoint() succeeds, otherwise it will not be referred because kstrtoint() will return an error which means go out from this function. Signed-off-by:
-
Keith Busch authored
Just like IO queues, the admin queue also will not be restarted after a controller shutdown. Unquiesce this queue so that we do not block request dispatch on a permanently disabled controller. Reported-by:
Yufen Yu <yuyufen@huawei.com> Signed-off-by:
-
Keith Busch authored
We do not restart a controller in a deleting state for timeout errors. When in this state, unblock potential request dispatchers with failed completions by shutting down the controller on timeout detection. Reported-by:
-
Klaus Birkelund Jensen authored
The shortcut for single segment SGL requests did not set the PSDT field to mark the request as using SGLs. Fixes: 29791057 ("nvme-pci: optimize mapping single segment requests using SGLs") Signed-off-by:
Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com> Signed-off-by:
-
Hannes Reinecke authored
Signed-off-by:
-
Hannes Reinecke authored
If the bio is moved to a different queue via blk_steal_bios() and the original queue is destroyed in nvme_remove_ns() we'll be ending with a crash in bio_endio() as the mempool for the split bio bvecs had already been destroyed. So split the bio using the original queue (which will remain during the lifetime of the bio) before sending it down to the underlying device. Signed-off-by:
-
Sagi Grimberg authored
If I/O queue connect times out, we might have freed the queue socket already, so check for that on the error path in nvme_tcp_start_queue. Signed-off-by:
-
Jens Axboe authored
It's not used outside this file. Fixes: 63120731 ("bcache: fix failure in journal relplay") Signed-off-by:
-
- 30 Apr, 2019 12 commits
-
-
Christoph Hellwig authored
Reviewed-by:
-
Christoph Hellwig authored
Various block layer files do not have any licensing information at all. Add SPDX tags for the default kernel GPLv2 license to those. Reviewed-by:
-
Christoph Hellwig authored
This file has no copyright notice, but was added as part of a commit adding another file using the default kernel GPLv2 license. Add a matching SPDX tag. Reviewed-by:
-
Christoph Hellwig authored
The file already has the correct SPDX header. Reviewed-by:
-
Christoph Hellwig authored
All these files have some form of the usual GPLv2 or later boilerplate. Switch them to use SPDX tags instead. Reviewed-by:
-
Christoph Hellwig authored
All these files have some form of the usual GPLv2 boilerplate. Switch them to use SPDX tags instead. Reviewed-by:
-
Christoph Hellwig authored
Share the bi_size update by moving the done label up, and duplicate the bv_len update in the two callers to get rid of the bvec_merge label. Signed-off-by:
-
Christoph Hellwig authored
We are never called with file system pages by defintions for the passthrough interface, and we also never undo any addition later these days. Signed-off-by:
-
Christoph Hellwig authored
The same page optimization is a rather odd corner case, which is not used outside bio.c and which really should not be used outside of bio.c either - we have better highlevel helpers like the rq/bio mapping helpers. Signed-off-by:
-
Christoph Hellwig authored
We only have two callers that need the integer loop iterator, and they can easily maintain it themselves. Suggested-by:
Matthew Wilcox <willy@infradead.org> Reviewed-by:
David Sterba <dsterba@suse.com> Reviewed-by:
Coly Li <colyli@suse.de> Reviewed-by:
-
Christoph Hellwig authored
Use a variable containing the buffer address instead of the to be removed integer iterator from bio_for_each_segment_all. Suggested-by:
-
Coly Li authored
Commit 95f18c9d ("bcache: avoid potential memleak of list of journal_replay(s) in the CACHE_SYNC branch of run_cache_set") forgets to remove the original define of LIST_HEAD(journal), which makes the change no take effect. This patch removes redundant variable LIST_HEAD(journal) from run_cache_set(), to make Shenghui's fix working. Fixes: 95f18c9d ("bcache: avoid potential memleak of list of journal_replay(s) in the CACHE_SYNC branch of run_cache_set") Reported-by:
Juha Aatrokoski <juha.aatrokoski@aalto.fi> Cc: Shenghui Wang <shhuiw@foxmail.com> Signed-off-by:
-
- 26 Apr, 2019 1 commit
-
-
git://git.infradead.org/nvmeJens Axboe authored
Pull NVMe changes from Christoph. * 'nvme-5.2' of git://git.infradead.org/nvme: nvme: set 0 capacity if namespace block size exceeds PAGE_SIZE nvme-rdma: fix typo in struct comment nvme-loop: kill timeout handler nvme-tcp: rename function to have nvme_tcp prefix nvme-rdma: fix a NULL deref when an admin connect times out nvme-tcp: fix a NULL deref when an admin connect times out nvmet-tcp: don't fail maxr2t greater than 1 nvmet-file: clamp-down file namespace lba_shift nvmet: include <linux/scatterlist.h> nvmet: return a specified error it subsys_alloc fails nvmet: rename nvme_completion instances from rsp to cqe nvmet-rdma: remove p2p_client initialization from fast-path
-
- 25 Apr, 2019 6 commits
-
-
Christoph Hellwig authored
The trim support in mtip32xx has been "temporarily" disabled for 6 years, which is 3/4 of the time the driver even exists in the tree. Remove it as it obviously is dead code now. Signed-off-by:
-
Sagi Grimberg authored
If our target exposed a namespace with a block size that is greater than PAGE_SIZE, set 0 capacity on the namespace as we do not support it. This issue encountered when the nvmet namespace was backed by a tempfile. Signed-off-by:
-
Minwoo Im authored
struct nvme_rdma_cm_rej has two different attributes: recfmt and sts. And sts will have value what this comment wanted to show. Signed-off-by:
-
Ming Lei authored
Firstly it doesn't make sense to handle timeout for loop: 1) for admin queue, the request is always completed in code path of queuing IO. 2) for normal IO request, the timeout on these IOs have been handled by underlying queue already. Secondly nvme-loop's timeout handler is simply broken, and easy to cause issue: 1) no any sync/protection between timeout and normal completion, and now it is driver's responsibility to deal with that; 2) bad reset implementation, blk_mq_update_nr_hw_queues() is called after all NSs's queue is stopped(quiesced), and easy to trigger deadlock. So kill the timeout handler. Signed-off-by:
-
Sagi Grimberg authored
usually nvme_ prefix is for core functions. While we're cleaning up, remove redundant empty lines Signed-off-by:
-
Sagi Grimberg authored
If we timeout the admin startup sequence we might not yet have an I/O tagset allocated which causes the teardown sequence to crash. Make nvme_tcp_teardown_io_queues safe by not iterating inflight tags if the tagset wasn't allocated. Fixes: 4c174e63 ("nvme-rdma: fix timeout handler") Signed-off-by:
-
