From: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Joseph Glanville
<joseph.glanville-2MxvZkOi9dvvnOemgxGiVw@public.gmane.org>
Cc: cgroups <cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: Re: 3.6-rc5 cgroups blkio throttle + md regression
Date: Wed, 19 Sep 2012 15:42:31 -0400 [thread overview]
Message-ID: <20120919194231.GF31860@redhat.com> (raw)
In-Reply-To: <CAOzFzEhWk_7oQFOyW6Ri_9Cvsshj2s3pa=Oo-p8uL6r340MoTw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Sep 20, 2012 at 04:20:42AM +1000, Joseph Glanville wrote:
> Hi,
>
> I booted the machine under bare metal to continue bisecting.
> Thankfully this allowed me to locate the commit that causes the
> problem.
>
I tested it and I am also noticing the hang. I can see this hang on
dm devices also.
I suspect this issue is related to bio based drivers. We exit the
bypass mode in blk_init_allocated_queue() and that will be called
only for request based drivers. So for bio based drivers may be
we never exit the bypass mode and this issue is somehow side
affect of that.
Thanks
Vivek
> git bisect reports this as the offending commit:
> commit b82d4b197c782ced82a8b7b76664125d2d3c156c
> Author: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Date: Fri Apr 13 13:11:31 2012 -0700
>
> blkcg: make request_queue bypassing on allocation
>
> With the previous change to guarantee bypass visiblity for RCU read
> lock regions, entering bypass mode involves non-trivial overhead and
> future changes are scheduled to make use of bypass mode during init
> path. Combined it may end up adding noticeable delay during boot.
>
> This patch makes request_queue start its life in bypass mode, which is
> ended on queue init completion at the end of
> blk_init_allocated_queue(), and updates blk_queue_bypass_start() such
> that draining and RCU synchronization are performed only when the
> queue actually enters bypass mode.
>
> This avoids unnecessarily switching in and out of bypass mode during
> init avoiding the overhead and any nasty surprises which may step from
> leaving bypass mode on half-initialized queues.
>
> The boot time overhead was pointed out by Vivek.
>
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index f2db628..3b02ba3 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -421,14 +421,18 @@ void blk_drain_queue(struct request_queue *q,
> bool drain_all)
> */
> void blk_queue_bypass_start(struct request_queue *q)
> {
> + bool drain;
> +
> spin_lock_irq(q->queue_lock);
> - q->bypass_depth++;
> + drain = !q->bypass_depth++;
> queue_flag_set(QUEUE_FLAG_BYPASS, q);
> spin_unlock_irq(q->queue_lock);
>
> - blk_drain_queue(q, false);
> - /* ensure blk_queue_bypass() is %true inside RCU read lock */
> - synchronize_rcu();
> + if (drain) {
> + blk_drain_queue(q, false);
> + /* ensure blk_queue_bypass() is %true inside RCU read lock */
> + synchronize_rcu();
> + }
> }
> EXPORT_SYMBOL_GPL(blk_queue_bypass_start);
>
> @@ -577,6 +581,15 @@ struct request_queue *blk_alloc_queue_node(gfp_t
> gfp_mask, int node_id)
> */
> q->queue_lock = &q->__queue_lock;
>
> + /*
> + * A queue starts its life with bypass turned on to avoid
> + * unnecessary bypass on/off overhead and nasty surprises during
> + * init. The initial bypass will be finished at the end of
> + * blk_init_allocated_queue().
> + */
> + q->bypass_depth = 1;
> + __set_bit(QUEUE_FLAG_BYPASS, &q->queue_flags);
> +
> if (blkcg_init_queue(q))
> goto fail_id;
>
> @@ -672,15 +685,15 @@ blk_init_allocated_queue(struct request_queue
> *q, request_fn_proc *rfn,
>
> q->sg_reserved_size = INT_MAX;
>
> - /*
> - * all done
> - */
> - if (!elevator_init(q, NULL)) {
> - blk_queue_congestion_threshold(q);
> - return q;
> - }
> + /* init elevator */
> + if (elevator_init(q, NULL))
> + return NULL;
>
> - return NULL;
> + blk_queue_congestion_threshold(q);
> +
> + /* all done, end the initial bypass */
> + blk_queue_bypass_end(q);
> + return q;
> }
> EXPORT_SYMBOL(blk_init_allocated_queue);
>
>
> Reverting this commit fixes the regression. :)
>
> Joseph.
>
> --
> CTO | Orion Virtualisation Solutions | www.orionvm.com.au
> Phone: 1300 56 99 52 | Mobile: 0428 754 846
next prev parent reply other threads:[~2012-09-19 19:42 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-16 17:31 3.6-rc5 cgroups blkio throttle + md regression Joseph Glanville
[not found] ` <CAOzFzEhf2LfT0BCNPAgPgxZ3=pj2KvJ4Z6kP7XF8nnxag1dfvg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-09-19 16:28 ` Joseph Glanville
[not found] ` <CAOzFzEiC4313K4H9393ffzNyBo398BPYSxTk7ZEmuH4GfW5qtw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-09-19 18:20 ` Joseph Glanville
[not found] ` <CAOzFzEhWk_7oQFOyW6Ri_9Cvsshj2s3pa=Oo-p8uL6r340MoTw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-09-19 19:42 ` Vivek Goyal [this message]
[not found] ` <20120919194231.GF31860-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-09-20 18:31 ` Tejun Heo
[not found] ` <20120920183153.GI28934-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-09-20 18:42 ` Vivek Goyal
[not found] ` <20120920184219.GH4681-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-09-20 19:17 ` Vivek Goyal
[not found] ` <20120920191716.GI4681-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-09-20 19:20 ` Tejun Heo
[not found] ` <20120920192038.GJ28934-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-09-20 19:32 ` Vivek Goyal
[not found] ` <20120920193227.GJ4681-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-09-20 20:17 ` Tejun Heo
2012-09-20 19:57 ` Vivek Goyal
[not found] ` <20120920195759.GK4681-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-09-20 20:18 ` Tejun Heo
[not found] ` <20120920201815.GB7264-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-09-20 21:08 ` [PATCH 1/2] block: lift the initial queue bypass mode on blk_register_queue() instead of blk_init_allocated_queue() Tejun Heo
[not found] ` <20120920210852.GC7264-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-09-20 21:09 ` [PATCH 2/2] block: fix request_queue->flags initialization Tejun Heo
[not found] ` <20120920210930.GD7264-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-09-21 13:25 ` Vivek Goyal
2012-09-21 13:25 ` [PATCH 1/2] block: lift the initial queue bypass mode on blk_register_queue() instead of blk_init_allocated_queue() Vivek Goyal
2012-09-21 13:25 ` Jens Axboe
[not found] ` <505C6AD4.6030206-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2012-10-16 10:00 ` Joseph Glanville
[not found] ` <CAOzFzEiCWazLEfjo=w8c+7qCce98Q6faW1uwGm-tRmCNPJUztw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-10-16 19:11 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120919194231.GF31860@redhat.com \
--to=vgoyal-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=joseph.glanville-2MxvZkOi9dvvnOemgxGiVw@public.gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).