| Message ID | 20171013180532.29304-7-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 2017-10-14 at 02:05 +0800, Ming Lei wrote: > In the following patch, we will implement scsi_get_budget() > which need to call scsi_prep_state_check() when rq isn't > dequeued yet. My understanding is that this change is only needed because scsi_mq_get_budget() calls scsi_prep_state_check() with the req argument set to NULL. Since scsi_prep_state_check() tests for sdev states other than SDEV_RUNNING and since the purpose of this series is to optimize performance for the case SDEV_RUNNING I'm not sure it's a good idea to make scsi_mq_get_budget() call scsi_prep_state_check(). And if scsi_mq_get_budget() wouldn't call scsi_prep_state_check() then this patch is not necessary. Bart.
On Fri, Oct 13, 2017 at 11:16:12PM +0000, Bart Van Assche wrote: > On Sat, 2017-10-14 at 02:05 +0800, Ming Lei wrote: > > In the following patch, we will implement scsi_get_budget() > > which need to call scsi_prep_state_check() when rq isn't > > dequeued yet. > > My understanding is that this change is only needed because scsi_mq_get_budget() > calls scsi_prep_state_check() with the req argument set to NULL. Since > scsi_prep_state_check() tests for sdev states other than SDEV_RUNNING and since > the purpose of this series is to optimize performance for the case SDEV_RUNNING > I'm not sure it's a good idea to make scsi_mq_get_budget() call > scsi_prep_state_check(). And if scsi_mq_get_budget() wouldn't call > scsi_prep_state_check() then this patch is not necessary. The purpose is that I don't want to change the current SCSI logic of queuing request. For example, if sdev state becomes OFFLINE or DEL, the current logic is to fail the request immediately. If the state isn't checked in scsi_mq_get_budget(), we have to try to obtain budget first, if it isn't available in the state of OFFLINE of DEL, there might be risk of I/O hang. So I suggest to check the state in scsi_mq_get_budget(), just like what the current scsi_queue_rq() does.
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 9cf6a80fe297..d159bb085714 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1301,7 +1301,7 @@ scsi_prep_state_check(struct scsi_device *sdev, struct request *req) /* * If the devices is blocked we defer normal commands. */ - if (!(req->rq_flags & RQF_PREEMPT)) + if (req && !(req->rq_flags & RQF_PREEMPT)) ret = BLKPREP_DEFER; break; default: @@ -1310,7 +1310,7 @@ scsi_prep_state_check(struct scsi_device *sdev, struct request *req) * special commands. In particular any user initiated * command is not allowed. */ - if (!(req->rq_flags & RQF_PREEMPT)) + if (req && !(req->rq_flags & RQF_PREEMPT)) ret = BLKPREP_KILL; break; }
In the following patch, we will implement scsi_get_budget() which need to call scsi_prep_state_check() when rq isn't dequeued yet. Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/scsi_lib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)