lkml.org 
[lkml]   [2011]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 10/10] block: fix request_queue lifetime handling by making blk_queue_cleanup() proper shutdown
On Tue, Oct 18, 2011 at 09:26:24PM -0700, Tejun Heo wrote:

[..]
> void blk_cleanup_queue(struct request_queue *q)
> {
> - /*
> - * We know we have process context here, so we can be a little
> - * cautious and ensure that pending block actions on this device
> - * are done before moving on. Going into this function, we should
> - * not have processes doing IO to this device.
> - */
> - blk_sync_queue(q);
> -
> - del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
> + /* mark @q DEAD, no new request or merges will be allowed afterwards */
> mutex_lock(&q->sysfs_lock);
> - queue_flag_set_unlocked(QUEUE_FLAG_DEAD, q);
> + spin_lock_irq(q->queue_lock);
> + queue_flag_set(QUEUE_FLAG_NOMERGES, q);
> + queue_flag_set(QUEUE_FLAG_NOXMERGES, q);
> + queue_flag_set(QUEUE_FLAG_DEAD, q);
> + spin_unlock_irq(q->queue_lock);
> mutex_unlock(&q->sysfs_lock);
>
> + /* drain all requests queued before DEAD marking */
> + blk_drain_queue(q, true);
> +
> + /* @q won't process any more request, flush async actions */
> + del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
> + blk_sync_queue(q);
> +
> + /* @q is and will stay empty, shutdown and put */
> if (q->elevator)
> elevator_exit(q->elevator);
> -
> blk_throtl_exit(q);

This is based on for-next tree? I am cloning it now. In Linus tree,
I see that blk_throtl_exit() has been moved to blk_release_queue() again.
So now we probably need to bring it back in blk_cleanup_queue(). That


[..]
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 900a0c9..8edb949 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -309,6 +309,10 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td)
> struct blkio_cgroup *blkcg;
> struct request_queue *q = td->queue;
>
> + /* no throttling for dead queue */
> + if (unlikely(test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)))
> + return NULL;
> +

May be blk_throtl_bio() is probably a better place to do this check, just
before callig throtl_get_tg().

Thanks
Vivek


\
 
 \ /
  Last update: 2011-10-19 18:21    [from the cache]
©2003-2011 Jasper Spaans