Re: 3.6-rc5 cgroups blkio throttle + md regression

[Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Thu, Sep 20, 2012 at 12:20:38PM -0700, Tejun Heo wrote:
> On Thu, Sep 20, 2012 at 03:17:16PM -0400, Vivek Goyal wrote:
> > I suspect we are looping in retry code because bio based queues never come
> > out of bypass mode.
> > 
> >                 /*
> >                  * If queue was bypassing, we should retry.  Do so after a
> >                  * short msleep().  It isn't strictly necessary but queue
> >                  * can be bypassing for some time and it's always nice to
> >                  * avoid busy looping.
> >                  */
> >                 if (ret == -EBUSY) {
> >                         msleep(10);
> >                         ret = restart_syscall();
> >                 }
> 
> Yeah, I incorrectly assumed that bio based drivers would call
> blk_init_allocated_queue().  I think we need to move the initial
> bypass_end call to blk_register_queue().  Like the following.  Not
> completely sure yet tho.

Hmm.., I am just trying to remember the details hence thinking loud.

So issue was that we did not want to call synchronize_rcu() in queue exit
path (ex. cfq_exit_queue()) because some driver could call create/destroy
queues very frequently during device/disk discovery.

So we started queue in bypass mode so that if it is destroyed early, it
will be destroyed in bypass mode and we shall never have to call
synchronize_rcu().

So assumption here is that blk_register_queue() will be called by
add_disk() and hopefully add_disk() will be called by driver only if
it is not planning to tear down the queue soon. If this assumption is
right, then this patch should work, I think.

Thanks
Vivek

> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 4b4dbdf..cbb019a 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -714,9 +714,6 @@ blk_init_allocated_queue(struct request_queue *q, request_fn_proc *rfn,
>  		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);
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 9628b29..f53802e 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -527,6 +527,8 @@ int blk_register_queue(struct gendisk *disk)
>  	if (WARN_ON(!q))
>  		return -ENXIO;
>  
> +	blk_queue_bypass_end(q);
> +
>  	ret = blk_trace_init_sysfs(dev);
>  	if (ret)
>  		return ret;