Re: [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq

Ming Lei
2018-04-28T08:27:08Z

On Fri, Apr 27, 2018 at 04:16:48PM +0000, Bart Van Assche wrote:
> On Fri, 2018-04-20 at 14:57 +0800, Ming Lei wrote:
> > +struct scsi_host_mq_in_flight {
> > + int cnt;
> > +};
> > +
> > +static void scsi_host_check_in_flight(struct request *rq, void *data,
> > + bool reserved)
> > +{
> > + struct scsi_host_mq_in_flight *in_flight = data;
> > +
> > + if (blk_mq_request_started(rq))
> > + in_flight->cnt++;
> > +}
> > +
> > /**
> > * scsi_host_busy - Return the host busy counter
> > * @shost: Pointer to Scsi_Host to inc.
> > **/
> > int scsi_host_busy(struct Scsi_Host *shost)
> > {
> > - return atomic_read(&shost->host_busy);
> > + struct scsi_host_mq_in_flight in_flight = {
> > + .cnt = 0,
> > + };
> > +
> > + if (!shost->use_blk_mq)
> > + return atomic_read(&shost->host_busy);
> > +
> > + blk_mq_tagset_busy_iter(&shost->tag_set, scsi_host_check_in_flight,
> > + &in_flight);
> > + return in_flight.cnt;
> > }
> > EXPORT_SYMBOL(scsi_host_busy);
>
> This patch introduces a subtle behavior change that has not been explained
> in the commit message. If a SCSI request gets requeued that results in a
> decrease of the .host_busy counter by scsi_device_unbusy() before the request
> is requeued and an increase of the host_busy counter when scsi_queue_rq() is
> called again. During that time such requests have the state MQ_RQ_COMPLETE and
> hence blk_mq_request_started() will return true and scsi_host_check_in_flight()

No, __blk_mq_requeue_request() will change the rq state into MQ_RQ_IDLE,
so such issue you worried about, please look at scsi_mq_requeue_cmd(),
which calls blk_mq_requeue_request(), which puts driver tag and updates
rq's state to IDLE.

> will include these requests. In other words, this patch introduces a subtle
> behavior change that has not been explained in the commit message. Hence I'm
> doubt that this change is correct.

As I explained above, no such issue.

Thanks,
Ming