On (01/20/11 09:35), Vivek Goyal wrote: > Hi Sergey, > Hi Vivek, > Can we expand a little bit on comment that why do we need to have > q->queue_lock initialized here now. Basically in the past nobody tried > to take q->queue_lock in blk_cleanup_queue() path hence things just > worked. Now blk throttling code is new and it takes q->queue_lock hence we > run into issues. This could be true for some other future code too. > > Secondly currently blk throttle code seems to be the only user dependent > on this lock initialization. So it might make sense to move this code > closer to the actual call and blk_release_queue() might be even better > place to do it atleast for now. > I'm afraid it's not safe to move NULL-check-and-fix out from blk_cleanup_queue, since we're performing elevator_exit(q->elevator) call, which may depend on q->queue_lock. For example, (./cfq-iosched.c: .elevator_exit_fn = cfq_exit_queue) cfq_exit_queue uses q->queue_lock. Sergey