linux-kernel - Re: [PATCHSET block/for-next] blk-mq: update freezing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53A1BC0E.8040704@kernel.dk>
Date:	Wed, 18 Jun 2014 09:19:26 -0700
From:	Jens Axboe <axboe@...nel.dk>
To:	Tejun Heo <tj@...nel.org>
CC:	linux-kernel@...r.kernel.org, kmo@...erainc.com,
	nab@...ux-iscsi.org
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/