Re: [PATCH v6 04/13] block: Avoid scheduling delayed work on a dead queue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/02/12 14:26, Tejun Heo wrote:
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.

I agree. There is a reason though I have not yet added a lockdep_assert_held() statement in these functions: there are still unprotected calls of blk_run_queue_async() in drivers/md/dm.c and drivers/scsi/scsi_transport_fc.c.

Bart.

--
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


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux