On Wed, Sep 28, 2011 at 01:53:04PM -0400, Christoph Hellwig wrote: > On Wed, Sep 28, 2011 at 01:48:59PM -0400, Vivek Goyal wrote: > > I am wondering if we should retain blk_throtl_exit() in blk_cleanup_queue() > > before lock swap and just move elevator cleanup in blk_release_queue(). > > > > A note to myself, I should probably enhance blk_throtl_exit() to look for any > > queued throttled bio and single their completion with error (-ENODEV) or > > something like that. > > The root of this evil is how queue_lock is implemented and (ab)used. > Instead of letting the driver assign a pointer to make the core use > its locks we really need to make the queue_lock a lock embedded directly > into the queue, and drivers may or may not use that lock for their > internal data structures. For high performance drivers they preferable > should use their own locks as queue_lock is far too contended already > for any high IOPS device. The same applies to throtteling btw - > instead of overloading an already highly contended lock it really > should have its own. I had thought of implementing a separate lock for throttling. Then I noticed few operations like checking for queue flags where I would be required to hold queue locks. So I could do lock nesting. hold throttling_lock; hold queue_lock; IIRC, I also had noticed some operations where queue might want to call into throttling with queue lock held and that would have led to lock order problems. So I had given up on the idea and continued to use queue lock for throttling. Thought it could still probably be done if one could justify additional complexity. Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
