Re: [PATCH] scsi: sd: add runtime pm to open / release

From: Alan Stern
Date: Tue Jun 30 2020 - 14:02:58 EST


On Tue, Jun 30, 2020 at 08:59:00AM -0700, Bart Van Assche wrote:
> On 2020-06-29 09:15, Alan Stern wrote:
> > Aha. Looking at this more closely, it's apparent that the code in
> > blk-core.c contains a logic bug: It assumes that if the BLK_MQ_REQ_PREEMPT
> > flag is set then the request can be issued regardless of the queue's
> > runtime status. That is not correct when the queue is suspended.
>
> Are you sure of this? In the past (legacy block layer) no requests were
> processed for queues in state RPM_SUSPENDED. However, that function and
> its successor blk_pm_allow_request() are gone. The following code was
> removed by commit 7cedffec8e75 ("block: Make blk_get_request() block for
> non-PM requests while suspended").
>
> static struct request *blk_pm_peek_request(struct request_queue *q,
> struct request *rq)
> {
> if (q->dev && (q->rpm_status == RPM_SUSPENDED ||
> (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM))))
> return NULL;
> else
> return rq;
> }

No, it wasn't. Another routine, blk_pm_allow_request(), was removed by
that commit, but almost no other code was deleted. Maybe you're thinking
of a different commit?

In any case, I don't understand the point you're trying to make.

Here's what I _do_ understand: While the queue is in the RPM_SUSPENDED
state, it can't carry out any requests at all. If a request is added to
the queue, the queue must be resumed before the request can be issued to
the lower-layer drivers -- no matter what flags are set in the request.

Right now there doesn't seem to be any mechanism for resuming the queue
if an REQ_PREEMPT request is added while the queue is suspended.

Alan Stern