| Message ID | 1553146989-114842-1-git-send-email-zhengbin13@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] blk-mq: fix a hung issue when set device state to blocked and restore running | expand |
On Thu, Mar 21, 2019 at 01:43:09PM +0800, zhengbin wrote: > When I use dd test a SCSI device which use blk-mq in the following steps: > 1.echo "blocked" >/sys/block/sda/device/state > 2.dd if=/dev/sda of=/mnt/t.log bs=1M count=10 > 3.echo "running" >/sys/block/sda/device/state > dd should finish this work after step 3, unfortunately, still hung. > > After step2, the key code process is like this: > blk_mq_dispatch_rq_list-->scsi_queue_rq-->prep_to_mq > > prep_to_mq will return BLK_STS_RESOURCE, and scsi_queue_rq will transter > it to BLK_STS_DEV_RESOURCE, which means that driver can guarantee that > IO dispatch will be triggered in future when the resource is available. > Need to follow the rule if we set the device state to running. > > Signed-off-by: zhengbin <zhengbin13@huawei.com> > --- > drivers/scsi/scsi_sysfs.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index 6a9040f..342e513 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -771,9 +771,20 @@ store_state_field(struct device *dev, struct device_attribute *attr, > > mutex_lock(&sdev->state_mutex); > ret = scsi_device_set_state(sdev, state); > + /* If device state changes to SDEV_RUNNING, we need to run hw queue > + * to avoid io hung. > + */ > + if ((ret == 0) && (state == SDEV_RUNNING)) > + goto out_run_hw_queue; > mutex_unlock(&sdev->state_mutex); > > return ret == 0 ? count : -EINVAL; > + > +out_run_hw_queue: > + mutex_unlock(&sdev->state_mutex); > + blk_mq_run_hw_queues(sdev->request_queue, true); > + > + return count; > } > > static ssize_t > -- > 2.7.4 > Looks fine, Reviewed-by: Ming Lei <ming.lei@redhat.com> Thanks, Ming
On Thu, 2019-03-21 at 13:43 +0800, zhengbin wrote: > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index 6a9040f..342e513 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -771,9 +771,20 @@ store_state_field(struct device *dev, struct device_attribute *attr, > > mutex_lock(&sdev->state_mutex); > ret = scsi_device_set_state(sdev, state); > + /* If device state changes to SDEV_RUNNING, we need to run hw queue > + * to avoid io hung. > + */ > + if ((ret == 0) && (state == SDEV_RUNNING)) > + goto out_run_hw_queue; > mutex_unlock(&sdev->state_mutex); > > return ret == 0 ? count : -EINVAL; > + > +out_run_hw_queue: > + mutex_unlock(&sdev->state_mutex); > + blk_mq_run_hw_queues(sdev->request_queue, true); > + > + return count; > } Having two different code paths that call mutex_unlock() is unfortunate because I don't think that is necessary to have two such paths, whether or not blk_mq_run_hw_queues() is called with the state mutex held. If you repost this patch, please remove the superfluous parentheses from the if-condition. Thanks, Bart.
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 6a9040f..342e513 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -771,9 +771,20 @@ store_state_field(struct device *dev, struct device_attribute *attr, mutex_lock(&sdev->state_mutex); ret = scsi_device_set_state(sdev, state); + /* If device state changes to SDEV_RUNNING, we need to run hw queue + * to avoid io hung. + */ + if ((ret == 0) && (state == SDEV_RUNNING)) + goto out_run_hw_queue; mutex_unlock(&sdev->state_mutex); return ret == 0 ? count : -EINVAL; + +out_run_hw_queue: + mutex_unlock(&sdev->state_mutex); + blk_mq_run_hw_queues(sdev->request_queue, true); + + return count; } static ssize_t
When I use dd test a SCSI device which use blk-mq in the following steps: 1.echo "blocked" >/sys/block/sda/device/state 2.dd if=/dev/sda of=/mnt/t.log bs=1M count=10 3.echo "running" >/sys/block/sda/device/state dd should finish this work after step 3, unfortunately, still hung. After step2, the key code process is like this: blk_mq_dispatch_rq_list-->scsi_queue_rq-->prep_to_mq prep_to_mq will return BLK_STS_RESOURCE, and scsi_queue_rq will transter it to BLK_STS_DEV_RESOURCE, which means that driver can guarantee that IO dispatch will be triggered in future when the resource is available. Need to follow the rule if we set the device state to running. Signed-off-by: zhengbin <zhengbin13@huawei.com> --- drivers/scsi/scsi_sysfs.c | 11 +++++++++++ 1 file changed, 11 insertions(+) -- 2.7.4