Re: [PATCH] scsi: core: Run queue first after running device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx wrote:
>>> From: Li Jinlin<lijinlin3@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> Signed-off-by: Qiu Laibin <qiulaibin@xxxxxxxxxx>
>>> Signed-off-by: Wu Bo <wubo40@xxxxxxxxxx>
>>
>> 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);
>>> -- 
>>
>> .
> 
> .



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux