On 2019/11/17 17:08, Ming Lei wrote: > Now the requeue queue is run in scsi_end_request() unconditionally if both > target queue and host queue is ready. We should have re-run request queue > only after this device queue becomes busy for restarting this LUN only. > > Recently Long Li reported that cost of run queue may be very heavy in > case of high queue depth. So improve this situation by only running > requesut queue when this LUN is busy. s/requesut/request Also, shouldn't this patch have the tag: Reported-by: Long Li <longli@xxxxxxxxxxxxx> ? Another remark is that Long's approach is generic to the block layer while your patch here is scsi specific. I wonder if the same problem cannot happen with other drivers too ? > > Cc: Jens Axboe <axboe@xxxxxxxxx> > Cc: Ewan D. Milne <emilne@xxxxxxxxxx> > Cc: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx> > Cc: Hannes Reinecke <hare@xxxxxxx> > Cc: Bart Van Assche <bvanassche@xxxxxxx> > Cc: Damien Le Moal <damien.lemoal@xxxxxxx> > Cc: Long Li <longli@xxxxxxxxxxxxx> > Cc: linux-block@xxxxxxxxxxxxxxx > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx> > --- > drivers/scsi/scsi_lib.c | 29 +++++++++++++++++++++++++++-- > include/scsi/scsi_device.h | 1 + > 2 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 379533ce8661..212903d5f43c 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -612,7 +612,7 @@ static bool scsi_end_request(struct request *req, blk_status_t error, > if (scsi_target(sdev)->single_lun || > !list_empty(&sdev->host->starved_list)) > kblockd_schedule_work(&sdev->requeue_work); > - else > + else if (READ_ONCE(sdev->restart)) > blk_mq_run_hw_queues(q, true); > > percpu_ref_put(&q->q_usage_counter); > @@ -1632,8 +1632,33 @@ static bool scsi_mq_get_budget(struct blk_mq_hw_ctx *hctx) > struct request_queue *q = hctx->queue; > struct scsi_device *sdev = q->queuedata; > > - if (scsi_dev_queue_ready(q, sdev)) > + if (scsi_dev_queue_ready(q, sdev)) { > + WRITE_ONCE(sdev->restart, 0); > return true; > + } > + > + /* > + * If all in-flight requests originated from this LUN are completed > + * before setting .restart, sdev->device_busy will be observed as > + * zero, then blk_mq_delay_run_hw_queue() will dispatch this request > + * soon. Otherwise, completion of one of these request will observe > + * the .restart flag, and the request queue will be run for handling > + * this request, see scsi_end_request(). > + * > + * However, the .restart flag may be cleared from other dispatch code > + * path after one inflight request is completed, then: > + * > + * 1) if this reqquest is dispatched from scheduler queue or sw queue one > + * by one, this request will be handled in that dispatch path too given > + * the request still stays at scheduler/sw queue when calling .get_budget() > + * callback. > + * > + * 2) if this request is dispatched from hctx->dispatch or > + * blk_mq_flush_busy_ctxs(), this request will be put into hctx->dispatch > + * list soon, and blk-mq will be responsible for covering it, see > + * blk_mq_dispatch_rq_list(). > + */ > + WRITE_ONCE(sdev->restart, 1); > > if (atomic_read(&sdev->device_busy) == 0 && !scsi_device_blocked(sdev)) > blk_mq_delay_run_hw_queue(hctx, SCSI_QUEUE_DELAY); > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h > index 202f4d6a4342..9d8ca662ae86 100644 > --- a/include/scsi/scsi_device.h > +++ b/include/scsi/scsi_device.h > @@ -109,6 +109,7 @@ struct scsi_device { > atomic_t device_busy; /* commands actually active on LLDD */ > atomic_t device_blocked; /* Device returned QUEUE_FULL. */ > > + unsigned int restart; > spinlock_t list_lock; > struct list_head cmd_list; /* queue of in use SCSI Command structures */ > struct list_head starved_entry; > -- Damien Le Moal Western Digital Research