Messages in this thread | | | Date | Mon, 5 Oct 2015 16:44:03 -0700 | Subject | Re: [PATCH 1/2] block: generic request_queue reference counting | From | Dan Williams <> |
| |
On Sat, Oct 3, 2015 at 11:40 PM, Christoph Hellwig <hch@lst.de> wrote: > On Tue, Sep 29, 2015 at 08:41:31PM -0400, Dan Williams wrote: >> Allow pmem, and other synchronous/bio-based block drivers, to fallback >> on a per-cpu reference count managed by the core for tracking queue >> live/dead state. >> >> The existing per-cpu reference count for the blk_mq case is promoted to >> be used in all block i/o scenarios. This involves initializing it by >> default, waiting for it to drop to zero at exit, and holding a live >> reference over the invocation of q->make_request_fn() in >> generic_make_request(). The blk_mq code continues to take its own >> reference per blk_mq request and retains the ability to freeze the >> queue, but the check that the queue is frozen is moved to >> generic_make_request(). >> >> This fixes crash signatures like the following: >> >> BUG: unable to handle kernel paging request at ffff880140000000 >> [..] >> Call Trace: >> [<ffffffff8145e8bf>] ? copy_user_handle_tail+0x5f/0x70 >> [<ffffffffa004e1e0>] pmem_do_bvec.isra.11+0x70/0xf0 [nd_pmem] >> [<ffffffffa004e331>] pmem_make_request+0xd1/0x200 [nd_pmem] >> [<ffffffff811c3162>] ? mempool_alloc+0x72/0x1a0 >> [<ffffffff8141f8b6>] generic_make_request+0xd6/0x110 >> [<ffffffff8141f966>] submit_bio+0x76/0x170 >> [<ffffffff81286dff>] submit_bh_wbc+0x12f/0x160 >> [<ffffffff81286e62>] submit_bh+0x12/0x20 >> [<ffffffff813395bd>] jbd2_write_superblock+0x8d/0x170 >> [<ffffffff8133974d>] jbd2_mark_journal_empty+0x5d/0x90 >> [<ffffffff813399cb>] jbd2_journal_destroy+0x24b/0x270 >> [<ffffffff810bc4ca>] ? put_pwq_unlocked+0x2a/0x30 >> [<ffffffff810bc6f5>] ? destroy_workqueue+0x225/0x250 >> [<ffffffff81303494>] ext4_put_super+0x64/0x360 >> [<ffffffff8124ab1a>] generic_shutdown_super+0x6a/0xf0 >> >> Cc: Jens Axboe <axboe@kernel.dk> >> Cc: Keith Busch <keith.busch@intel.com> >> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> >> Suggested-by: Christoph Hellwig <hch@lst.de> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> --- >> block/blk-core.c | 71 +++++++++++++++++++++++++++++++++++++------ >> block/blk-mq-sysfs.c | 6 ---- >> block/blk-mq.c | 80 ++++++++++++++---------------------------------- >> block/blk-sysfs.c | 3 +- >> block/blk.h | 14 ++++++++ >> include/linux/blk-mq.h | 1 - >> include/linux/blkdev.h | 2 + >> 7 files changed, 102 insertions(+), 75 deletions(-) >> >> diff --git a/block/blk-core.c b/block/blk-core.c >> index 2eb722d48773..6062550baaef 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -554,19 +554,17 @@ void blk_cleanup_queue(struct request_queue *q) >> * Drain all requests queued before DYING marking. Set DEAD flag to >> * prevent that q->request_fn() gets invoked after draining finished. >> */ >> - if (q->mq_ops) { >> - blk_mq_freeze_queue(q); >> - spin_lock_irq(lock); >> - } else { >> - spin_lock_irq(lock); >> + blk_freeze_queue(q); >> + spin_lock_irq(lock); >> + if (!q->mq_ops) >> __blk_drain_queue(q, true); >> - } > > __blk_drain_queue really ought to be moved into blk_freeze_queue so it > has equivlaent functionality for mq vs !mq. But maybe that can be a > separate patch.
It's not clear why the cleanup path is gating __blk_drain_queue on ->mq_ops? __blk_drain_queue already gets called from blkcg_init_queue(). Also, it seems blk_get_flush_queue() is already handling the mq_ops vs !mp_ops case, but I'd have to take a closer look. Definitely seems like follow-on patch material...
| |