Messages in this thread | | | Date | Wed, 18 Jun 2014 09:19:26 -0700 | From | Jens Axboe <> | Subject | Re: [PATCHSET block/for-next] blk-mq: update freezing |
| |
On 2014-06-18 08:21, Tejun Heo wrote: > Hello, Jens. > > While reading through blk-mq, I spotted several issues and this > patchset addresses them. The biggest one is how freezing is > implemented. Coupling it with bypassing doesn't seem to work well and > there's a subtle bug in the perpcu switch implementation. > > I don't think open-coding this level of percpu logic is a good idea. > It tends to be very error-prone and difficult to follow. The barrier > problem is the only thing I spotted but it's very difficult to say > that it's correct. percpu_ref already implements most of what's > necessary to implement this sort of percpu switch and I added the > missing bits in a recent patchset and converted blk-mq freezing > mechanism to use it in this patch. > > It's far simpler and easier to wrap one's head around, and, as it's > shared with other mechanisms including aio and cgroups, we should have > better testing coverage and more eyes scrutinizing it. > > This patchset contains the following six patches. > > 0001-blk-mq-make-blk_mq_stop_hw_queue-reliably-block-queu.patch > 0002-blk-mq-fix-a-memory-ordering-bug-in-blk_mq_queue_ent.patch > 0003-block-blk-mq-draining-can-t-be-skipped-even-if-bypas.patch > 0004-blk-mq-decouble-blk-mq-freezing-from-generic-bypassi.patch > 0005-blk-mq-collapse-__blk_mq_drain_queue-into-blk_mq_fre.patch > 0006-blk-mq-use-percpu_ref-for-mq-usage-count.patch > > 0001-0003 are fix patches that can be applied separately. > > 0004 decouples blk-mq freezing from queue bypassing. > > 0005-0006 replace the custom percpu switch with percpu_ref. > > This patchset is on top of > > percpu/for-3.17 6fbc07bbe2b5 ("percpu: invoke __verify_pcpu_ptr() from the generic part of accessors and operations") > +[1] [PATCHSET percpu/for-3.17] percpu: implement percpu_ref_reinit() > > and available in the following git branch. > > git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git review-mq-percpu_ref > > block/blk-core.c | 13 ++++--- > block/blk-mq.c | 90 ++++++++++++++++--------------------------------- > block/blk-mq.h | 2 - > block/blk-sysfs.c | 2 - > include/linux/blkdev.h | 4 +- > 5 files changed, 44 insertions(+), 67 deletions(-)
Thanks Tejun, this looks pretty good. I was worried the tryget_live() would be too expensive, but that looks not to be the case. I like the cleanups to using a general mechanism. I'll run this through some functional and performance testing.
-- Jens Axboe
| |