Messages in this thread | | | Date | Thu, 17 Feb 2011 11:35:36 +1100 | From | NeilBrown <> | Subject | Re: blk_throtl_exit taking q->queue_lock is problematic |
| |
On Wed, 16 Feb 2011 10:53:05 -0500 Vivek Goyal <vgoyal@redhat.com> wrote:
> On Wed, Feb 16, 2011 at 06:31:14PM +1100, NeilBrown wrote: > > > > > > Hi, > > > > I recently discovered that blk_throtl_exit takes ->queue_lock when a blockdev > > is finally released. > > > > This is a problem for because by that time the queue_lock doesn't exist any > > more. It is in a separate data structure controlled by the RAID personality > > and by the time that the block device is being destroyed the raid personality > > has shutdown and the data structure containing the lock has been freed. > > > > This has not been a problem before. Nothing else takes queue_lock after > > blk_cleanup_queue. > > I agree that this is a problem. blk_throtl_exit() needs queue lock to > avoid other races with cgroup code and for avoiding races for its > lists etc. > > > > > I could of course set queue_lock to point to __queue_lock and initialise that, > > but it seems untidy and probably violates some locking requirements. > > > > Is there some way you could use some other lock - maybe a global lock, or > > maybe used __queue_lock directly ??? > > Initially I had put blk_throtl_exit() in blk_cleanup_queue() where it is > known that ->queue_lock is still around. Due to a bug, Jens moved it > to blk_release_queue(). I still think that blk_cleanup_queue() is a better > place to call blk_throtl_exit().
Why do you say that it is known that ->queue_lock is still around in blk_cleanup_queue? In md it isn't. :-( Is there some (other) reason that it needs to be?
Thanks, NeilBrown
| |