On Tue, Jul 11, 2017 at 06:41:29PM +0000, Bart Van Assche wrote:

On Wed, 2017-07-12 at 02:20 +0800, Ming Lei wrote:

quoted

This interfaces will be removed soon, so use quiesce and
unquiesce instead, which should be more safe.

The only one usage will be removed in the following
congestion control patches.

Hello Ming,

The title of this patch is misleading since this patch does not touch the calls
related to queue congestion (blk_mq_stop_hw_queue() and
blk_mq_start_stopped_hw_queues()). I assume that you meant that this patch avoids
that the xen-blkfront driver uses blk_mq_(start|stop)_hw_queues() (with queues in
plural form)? Can you please reflect that in the subject of this and related
patches?

OK, will do it in V2.

Additionally, it's probably a good idea that this is not just an interface change
but that this kind of patches fix a (hard to trigger?) race condition.

quoted

 static inline void kick_pending_request_queues_locked(struct blkfront_ring_info *rinfo)
 {
-	if (!RING_FULL(&rinfo->ring))
+	if (!RING_FULL(&rinfo->ring)) {
 		blk_mq_start_stopped_hw_queues(rinfo->dev_info->rq, true);
+		blk_mq_kick_requeue_list(rinfo->dev_info->rq);
+	}
 }
 
 static void kick_pending_request_queues(struct blkfront_ring_info *rinfo)

@@ -1225,7 +1227,8 @@ static void kick_pending_request_queues(struct blkfront_ring_info *rinfo)
 	unsigned long flags;
 
 	spin_lock_irqsave(&rinfo->ring_lock, flags);
-	kick_pending_request_queues_locked(rinfo);
+	if (!RING_FULL(&rinfo->ring))
+		blk_mq_run_hw_queues(rinfo->dev_info->rq, true);
 	spin_unlock_irqrestore(&rinfo->ring_lock, flags);
 }

Why do some kick_pending_request_queues_locked() kick the requeue list and why
has the above kick_pending_request_queues_locked() call been converted into a
blk_mq_run_hw_queues() call and thereby ignores the requeue list?

kick_pending_request_queues_locked() is used in req completion path,
which belongs to congestion control, so this patch doesn't touch
kick_pending_request_queues_locked(), which will be switched to
generic congestion control in patch 5.

For kick_pending_request_queues(), this patch replaces
blk_mq_start_stopped_hw_queues() with blk_mq_run_hw_queues()
because run queue is often the real purpose of this function,
especially in non-congestion control path.


Thanks,
Ming