Skip to content
  • Vivek Goyal's avatar
    block: Move blk_throtl_exit() call to blk_cleanup_queue() · da527770
    Vivek Goyal authored
    Move blk_throtl_exit() in blk_cleanup_queue() as blk_throtl_exit() is
    written in such a way that it needs queue lock. In blk_release_queue()
    there is no gurantee that ->queue_lock is still around.
    
    Initially blk_throtl_exit() was in blk_cleanup_queue() but Ingo reported
    one problem.
    
      https://lkml.org/lkml/2010/10/23/86
    
      And a quick fix moved blk_throtl_exit() to blk_release_queue().
    
            commit 7ad58c02
    
    
            Author: Jens Axboe <jaxboe@fusionio.com>
            Date:   Sat Oct 23 20:40:26 2010 +0200
    
            block: fix use-after-free bug in blk throttle code
    
    This patch reverts above change and does not try to shutdown the
    throtl work in blk_sync_queue(). By avoiding call to
    throtl_shutdown_timer_wq() from blk_sync_queue(), we should also avoid
    the problem reported by Ingo.
    
    blk_sync_queue() seems to be used only by md driver and it seems to be
    using it to make sure q->unplug_fn is not called as md registers its
    own unplug functions and it is about to free up the data structures
    used by unplug_fn(). Block throttle does not call back into unplug_fn()
    or into md. So there is no need to cancel blk throttle work.
    
    In fact I think cancelling block throttle work is bad because it might
    happen that some bios are throttled and scheduled to be dispatched later
    with the help of pending work and if work is cancelled, these bios might
    never be dispatched.
    
    Block layer also uses blk_sync_queue() during blk_cleanup_queue() and
    blk_release_queue() time. That should be safe as we are also calling
    blk_throtl_exit() which should make sure all the throttling related
    data structures are cleaned up.
    
    Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
    Signed-off-by: default avatarJens Axboe <jaxboe@fusionio.com>
    da527770