On Wed, May 02, 2018 at 10:23:35AM +0800, jianchao.wang wrote:

Hi ming

On 04/29/2018 11:41 PM, Ming Lei wrote:

quoted

 
+static void __blk_unquiesce_timeout(struct request_queue *q)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	q->timeout_off = false;
+	spin_unlock_irqrestore(q->queue_lock, flags);
+}
+
+void blk_unquiesce_timeout(struct request_queue *q)
+{
+	__blk_unquiesce_timeout(q);
+	mod_timer(&q->timeout, jiffies + q->rq_timeout);
+}
+EXPORT_SYMBOL(blk_unquiesce_timeout);
+
+void blk_quiesce_timeout(struct request_queue *q)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	q->timeout_off = true;
+	spin_unlock_irqrestore(q->queue_lock, flags);
+
+	del_timer_sync(&q->timeout);
+	cancel_work_sync(&q->timeout_work);
+}
+EXPORT_SYMBOL(blk_quiesce_timeout);
+
 /**
  * blk_sync_queue - cancel any pending callbacks on a queue
  * @q: the queue

@@ -408,8 +437,7 @@ EXPORT_SYMBOL(blk_stop_queue);
  */
 void blk_sync_queue(struct request_queue *q)
 {
-	del_timer_sync(&q->timeout);
-	cancel_work_sync(&q->timeout_work);
+	blk_quiesce_timeout(q);
 
 	if (q->mq_ops) {
 		struct blk_mq_hw_ctx *hctx;

@@ -421,6 +449,8 @@ void blk_sync_queue(struct request_queue *q)
 	} else {
 		cancel_delayed_work_sync(&q->delay_work);
 	}
+
+	__blk_unquiesce_timeout(q);
 }
 EXPORT_SYMBOL(blk_sync_queue);
 

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 0dc9e341c2a7..890dd3b3e3e1 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c

@@ -901,6 +901,15 @@ static void blk_mq_timeout_work(struct work_struct *work)
 	};
 	struct blk_mq_hw_ctx *hctx;
 	int i;
+	bool timeout_off;
+	unsigned long flags;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	timeout_off = q->timeout_off;
+	spin_unlock_irqrestore(q->queue_lock, flags);
+
+	if (timeout_off)
+		return;

Looks like there is still race as following:

blk_quiesce_timeout                          blk_mq_timeout_work
                                               -> timeout_off = q->timeout_off; //still a false
  -> q->timeout_off = true;
  -> del_timer_sync(&q->timeout);
                                               -> mod_timer
  -> cancel_work_sync(&q->timeout_work);

No, there isn't such race, the 'mod_timer' doesn't make a difference
because 'q->timeout_off' will be visible in new work func after
cancel_work_sync() returns. So even the timer is expired, work func
still returns immediately.

That means del_timer_sync() isn't absolutely necessary too.

Thanks,
Ming