On Mon, 2018-07-30 at 09:56 +0800, jianchao.wang wrote:

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)�

Ds-

	int err = 0;
=20
	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, er=

r);

=20
	return err;
}
=20
If blk_pre_runtime_suspend returns -EBUSY, blk_post�

F8-runtime_suspend will not be invoked.

Right, I will fix this.

quoted

quoted

The request_queue should be set to preempt only mode =

only when we confirm we could set

quoted

quoted

rpm_status to RPM_SUSPENDING or RPM_RESUMING.

=20
Why do you think this?

=20
https://marc.info/?l=linux-scsi&m=133727953625963&w�

D0-2

"
If q->rpm_status is RPM_SUSPENDED, they shouldn't do anyt=

hing -- act as though the queue is

empty.  If q->rpm_status is RPM_SUSPENDING or RPM_RES=

UMING, they should hand over the request

only if it has the REQ_PM flag set.
"

I think the blk_pre_runtime_suspend() callers guarantee that q-=

rpm_status == RPM_ACTIVE

before blk_pre_runtime_suspend() is called. I will add a WARN�
F8-ON_ONCE() statement that
verifies that.

In additon, if we set the preempt only here unconditionally, the norm=

al 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 ?

I will see what I can do about this.

Bart.