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

[Lin Ming]

On Wed, Jun 6, 2012 at 2:27 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 22 May 2012, Lin Ming wrote:
>
>> > Next, you have to change the parts of the block layer responsible for
>> > taking a request from the queue and handing it to the lower-level
>> > driver (both peek and get).  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.
>
> I just realized this isn't quite right.  In fact, when the status is
> RPM_SUSPENDING or RPM_RESUMING you need to search through the queue for
> the first request with REQ_PM set.
>
> Otherwise you can end up in a deadlock.  For example, suppose the
> device is suspended and a normal I/O request arrives.  It gets put on
> the queue and a runtime resume is issued.  The sd resume method then
> submits a "spin-up drive" command to the queue with REQ_PM set, and it
> doesn't return until this command is finished.  But the command won't
> even start, because it isn't at the head of the queue -- the I/O
> request is, and it prevents the "spin-up" command from running.

At least for SCSI sd driver, the PM request is always queued at the head.

scsi_execute()
  blk_execute_rq(req->q, NULL, req, 1);

"1" means the request will be inserted at the head.

blk_execute_rq_nowait()
   spin_lock_irq(q->queue_lock);
   __elv_add_request(q, rq, where);
   __blk_run_queue(q)
   spin_unlock_irq(q->queue_lock);

So the request is inserted at the head and executed immediatly.

>
> There are two other issues you also need to address.  The first is
> simple: Make sure your changes don't cause any harm if CONFIG_PM or
> CONFIG_PM_RUNTIME is set to N.
>
> Second, near the end of scsi_pm.c:scsi_bus_resume_common(), you can see
> that during resume from a system sleep, the device automatically gets
> set back to RPM_ACTIVE.  You want the queue's runtime status to remain
> in sync with the device's status; therefore you need to call
> blk_pre_runtime_resume near the start of this function and
> blk_post_runtime_resume near the end.  (This is the drawback of having
> separate rpm_status fields for the queue and the device.)
>
> Alan Stern

I am travelling through June 10.
I'll investigate these when I'm back.

Thanks,
Lin Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/