Re: [PATCH 1/6] xen-blkfront: avoid to use start/stop queue

Ming Lei
2017-07-12T02:59:55Z

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