Messages in this thread | | | Subject | Re: [PATCH] scsi: core: Run queue first after running device | From | lijinlin <> | Date | Fri, 6 Aug 2021 20:21:42 +0800 |
| |
On 2021/8/6 17:00, John Garry wrote: > On 06/08/2021 09:58, John Garry wrote: > > And the patch subject is ambiguous >
Thanks. I will modify the patch subject and message in patch v2.
>> On 05/08/2021 15:32, lijinlin3@huawei.com wrote: >>> From: Li Jinlin<lijinlin3@huawei.com> >>> >>> We found a hang issue, the test steps are as follows: >>> 1. echo "blocked" >/sys/block/sda/device/state >>> 2. dd if=/dev/sda of=/mnt/t.log bs=1M count=10 >>> 3. echo none > /sys/block/sda/queue/scheduler >>> 4. echo "running" >/sys/block/sda/device/state >>> >>> Step3 and Step4 should finish this work after Step4, but them hangs. >>> >>> CPU#0 CPU#1 CPU#2 >>> --------------- ---------------- ---------------- >>> Step1: blocking device >>> >>> Step2: dd xxxx >>> ^^^^^^ get request >>> q_usage_counter++ >>> >>> Step3: switching scheculer >>> elv_iosched_store >>> elevator_switch >>> blk_mq_freeze_queue >>> blk_freeze_queue >>> > blk_freeze_queue_start >>> ^^^^^^ mq_freeze_depth++ >>> >>> > blk_mq_run_hw_queues >>> ^^^^^^ can't run queue when dev blocked >>> >>> > blk_mq_freeze_queue_wait >>> ^^^^^^ Hang here!!! >>> wait q_usage_counter==0 >>> >>> Step4: running device >>> store_state_field >>> scsi_rescan_device >>> scsi_attach_vpd >>> scsi_vpd_inquiry >>> __scsi_execute >>> blk_get_request >>> blk_mq_alloc_request >>> blk_queue_enter >>> ^^^^^^ Hang here!!! >>> wait mq_freeze_depth==0 >>> >>> blk_mq_run_hw_queues >>> ^^^^^^ dispatch IO, q_usage_counter will reduce to zero >>> >>> blk_mq_unfreeze_queue >>> ^^^^^ mq_freeze_depth-- >>> >>> Step3 and Step4 wait for each other, caused hangs. >>> >>> This requires run queue frist to fix this issue when the device state >> >> frist>> >>> changes to SDEV_RUNNING. >>> >>> Fixes: f0f82e2476f6 ("scsi: core: Fix capacity set to zero after offlinining device") >>> Signed-off-by: Li Jinlin <lijinlin3@huawei.com> >>> Signed-off-by: Qiu Laibin <qiulaibin@huawei.com> >>> Signed-off-by: Wu Bo <wubo40@huawei.com> >> >> what kind of SoB is this? >> >>> --- >>> drivers/scsi/scsi_sysfs.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c >>> index c3a710bceba0..aa701582c950 100644 >>> --- a/drivers/scsi/scsi_sysfs.c >>> +++ b/drivers/scsi/scsi_sysfs.c >>> @@ -809,12 +809,12 @@ store_state_field(struct device *dev, struct device_attribute *attr, >>> ret = scsi_device_set_state(sdev, state); >>> /* >>> * If the device state changes to SDEV_RUNNING, we need to >>> - * rescan the device to revalidate it, and run the queue to >>> - * avoid I/O hang. >>> + * run the queue to avoid I/O hang, and rescan the device >>> + * to revalidate it. >> >> A bit more description of the IO hang would be useful >> >>> */ >>> if (ret == 0 && state == SDEV_RUNNING) { >>> - scsi_rescan_device(dev); >>> blk_mq_run_hw_queues(sdev->request_queue, true); >>> + scsi_rescan_device(dev); >> >> This would not have happened if scsi_rescan_device() was ran outside the mutex lock region, like I suggested originally. >> >> Indeed, I doubt blk_mq_run_hw_queues() needs to be run with the sdev state_mutex held either.
I think blk_mq_run_hw_queues() and scsi_rescan_device() needs to be run with the sdev state_mutex held. Otherwise, Step4 will hang in the following situations.
CPU#0 CPU#1 CPU#2 CPU#3 --------------- ---------------- ----------------- ---------------- Step1: blocking device
Step2: dd xxxx ^^^^^^ get request q_usage_counter++
Step3: switching scheculer elv_iosched_store elevator_switch blk_mq_freeze_queue blk_freeze_queue > blk_freeze_queue_start ^^^^^^ mq_freeze_depth++
> blk_mq_run_hw_queues ^^^^^^ can't run queue when dev blocked
> blk_mq_freeze_queue_wait ^^^^^^ Hang here!!! wait q_usage_counter==0
Step4: running device store_state_field mutex_lock(&sdev->state_mutex); scsi_device_set_state(sdev, state); mutex_unlock(&sdev->state_mutex); Step5: blocking device store_state_field mutex_lock(&sdev->state_mutex); scsi_device_set_state(sdev, state); blk_mq_run_hw_queues ^^^^^^ can't run queue when dev blocked mutex_unlock(&sdev->state_mutex); scsi_rescan_device scsi_attach_vpd scsi_vpd_inquiry __scsi_execute blk_get_request blk_mq_alloc_request blk_queue_enter ^^^^^^ Hang here!!! wait mq_freeze_depth==0
>> >>> } >>> mutex_unlock(&sdev->state_mutex); >>> -- >> >> . > > .
| |