On Fri, Mar 22, 2019 at 10:18:54AM +0800, zhengbin (A) wrote:
> So blk_mq_run_hw_queues should still be in mutex_lock(&sdev->state_mutex)??

I think so, this way should be safe given the pattern is used in
__scsi_remove_device() already.

BTW, please do not reply before the quoted text, and I'd suggest you
to read some linux kernel development FAQ:

http://vger.kernel.org/lkml/#s3-9

Thanks,
Ming

> 
> On 2019/3/22 10:11, Ming Lei wrote:
> > On Fri, Mar 22, 2019 at 09:45:54AM +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 <[email protected]>
> >> ---
> >>  drivers/scsi/scsi_sysfs.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> >> index 6a9040f..266e4c7 100644
> >> --- a/drivers/scsi/scsi_sysfs.c
> >> +++ b/drivers/scsi/scsi_sysfs.c
> >> @@ -772,6 +772,11 @@ store_state_field(struct device *dev, struct 
> >> device_attribute *attr,
> >>    mutex_lock(&sdev->state_mutex);
> >>    ret = scsi_device_set_state(sdev, state);
> >>    mutex_unlock(&sdev->state_mutex);
> >> +  /* If device state changes to SDEV_RUNNING, we need to run hw queue
> >> +   * to avoid io hung.
> >> +   */
> >> +  if (ret == 0 && state == SDEV_RUNNING)
> >> +          blk_mq_run_hw_queues(sdev->request_queue, true);
> >>
> > 
> > Thinking of further, this way isn't safe, given the caller should make
> > sure that the queue is alive before calling blk_mq_run_hw_queues().
> > 
> > Now it is in sysfs write path, and the scsi device can become gone
> > just before calling blk_mq_run_hw_queues().
> > 
> > 
> > Thanks,
> > Ming
> > 
> > .
> > 
> 

Reply via email to