Hi Bart
On 07/27/2018 11:09 PM, Bart Van Assche wrote:
> On Fri, 2018-07-27 at 09:57 +0800, jianchao.wang wrote:
>> If q_usage_counter is not zero here, we will leave the request_queue in preempt only mode.
>
> That's on purpose. If q_usage_counter is not zero then blk_pre_runtime_suspend()
> will return -EBUSY. That error code will be passed to blk_post_runtime_suspend()
> and that will cause that function to clear QUEUE_FLAG_PREEMPT_ONLY.
>
static int sdev_runtime_suspend(struct device *dev)
{
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
struct scsi_device *sdev = to_scsi_device(dev);
int err = 0;
err = blk_pre_runtime_suspend(sdev->request_queue);
if (err)
return err;
if (pm && pm->runtime_suspend)
err = pm->runtime_suspend(dev);
blk_post_runtime_suspend(sdev->request_queue, err);
return err;
}
If blk_pre_runtime_suspend returns -EBUSY, blk_post_runtime_suspend will not be invoked.
>> The request_queue should be set to preempt only mode only when we confirm we could set
>> rpm_status to RPM_SUSPENDING or RPM_RESUMING.
>
> Why do you think this?
https://marc.info/?l=linux-scsi&m=133727953625963&w=2
"
If q->rpm_status is RPM_SUSPENDED, they shouldn't do anything -- act as though the queue is
empty. If q->rpm_status is RPM_SUSPENDING or RPM_RESUMING, they should hand over the request
only if it has the REQ_PM flag set.
"
In additon, if we set the preempt only here unconditionally, the normal IO will be blocked
during the blk_pre_runtime_suspend. In your patch, q_usage_counter will be switched to atomic mode,
this could cost some time. Is it really OK ?
Thanks
Jianchao
>
> Bart.
>
>
