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

[Ming Lei <ming.lei@redhat.com>]

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