On Wed, Nov 28, 2012 at 01:45:56PM +0100, Bart Van Assche wrote:
> Running a queue must continue after it has been marked dying until
> it has been marked dead. So the function blk_run_queue_async() must
> not schedule delayed work after blk_cleanup_queue() has marked a queue
> dead. Hence add a test for that queue state in blk_run_queue_async()
> and make sure that queue_unplugged() invokes that function with the
> queue lock held. This avoids that the queue state can change after
> it has been tested and before mod_delayed_work() is invoked. Drop
> the queue dying test in queue_unplugged() since it is now
> superfluous: __blk_run_queue() already tests whether or not the
> queue is dead.
>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> Cc: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Mike Christie <michaelc@xxxxxxxxxxx>
> Cc: Jens Axboe <axboe@xxxxxxxxx>
Acked-by: Tejun Heo <tj@xxxxxxxxxx>
But, some nits.
> @@ -219,12 +219,13 @@ static void blk_delay_work(struct work_struct *work)
> * Description:
> * Sometimes queueing needs to be postponed for a little while, to allow
> * resources to come back. This function will make sure that queueing is
> - * restarted around the specified time.
> + * restarted around the specified time. Queue lock must be held.
> */
Comment is fine but lockdep_assert_held() is way more effective.
> void blk_delay_queue(struct request_queue *q, unsigned long msecs)
> {
> - queue_delayed_work(kblockd_workqueue, &q->delay_work,
> - msecs_to_jiffies(msecs));
> + if (likely(!blk_queue_dead(q)))
> + queue_delayed_work(kblockd_workqueue, &q->delay_work,
> + msecs_to_jiffies(msecs));
> }
> EXPORT_SYMBOL(blk_delay_queue);
>
> @@ -334,11 +335,11 @@ EXPORT_SYMBOL(__blk_run_queue);
> *
> * Description:
> * Tells kblockd to perform the equivalent of @blk_run_queue on behalf
> - * of us.
> + * of us. The caller must hold the queue lock.
> */
Ditto.
> void blk_run_queue_async(struct request_queue *q)
> {
> - if (likely(!blk_queue_stopped(q)))
> + if (likely(!blk_queue_stopped(q) && !blk_queue_dead(q)))
> mod_delayed_work(kblockd_workqueue, &q->delay_work, 0);
> }
> EXPORT_SYMBOL(blk_run_queue_async);
Thanks.
--
tejun
--
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
