Re: [RFC v2 PATCH 2/4] block: add queue runtime pm callbacks

Alan Stern
2012-05-22T15:56:24Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 2003 bytes --]

On Tue, 22 May 2012, Lin Ming wrote:

> > (Or maybe it would be easier to make q->rpm_status be a pointer to
> > q->dev->power.rpm_status.  That way, if CONFIG_PM_RUNTIME isn't enabled
> > or block_runtime_pm_init() hasn't been called, you can have
> > q->rpm_status simply point to a static value that is permanently set to
> > RPM_ACTIVE.)
>
> I think we need q->rpm_status.
> Block layer check ->rpm_status and client driver set this status.

No, the client driver should not have to set any status values. The
client driver should do as little as possible.

> And the status is synchronized with queue's spin lock.

Right, and the client driver should not need to acquire the queue's
lock.

> If we use q->dev->power.rpm_status then how to sync it between block
> layer and client driver?
> Do you mean block layer will need to acquire q->dev->power.lock?

That's not what I mean.

What synchronization are you concerned about? The most important race
seems to be when a new request is added to the queue at the same time
as a runtime suspend begins.

If q->dev->power.rpm_status has already been set to RPM_SUSPENDING or
RPM_SUSPENDED when the request is submitted, the block layer should
call pm_runtime_request_resume(). Thus if the suspend succeeds, the
device will be resumed immediately afterward. And if the suspend
fails, the new request will be handled as usual (note that the
block_*_runtime_* routines might need to kick-start the queue to get it
going again).

Alternatively, if q->dev->power.rpm_status is still equal to RPM_ACTIVE
when the request is submitted, the block layer will simply accept the
request. If the request is still on the queue when
block_pre_runtime_suspend is called, it will return -EBUSY and the
suspend will fail.

The only synchronization needed to make this work is that the
block_{pre,post}_runtime_suspend routines need to hold the queue's lock
while checking to see if any requests are in the queue. You'd expect
that anyway.

Alan Stern