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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 Jun 2020 14:02:55 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     Bart Van Assche <bvanassche@....org>
Cc:     Martin Kepplinger <martin.kepplinger@...i.sm>, jejb@...ux.ibm.com,
        Can Guo <cang@...eaurora.org>, martin.petersen@...cle.com,
        linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...i.sm
Subject: Re: [PATCH] scsi: sd: add runtime pm to open / release

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