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]

Message-ID: <20200730151405.GC6332@rowland.harvard.edu>
Date:   Thu, 30 Jul 2020 11:14:05 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     Martin Kepplinger <martin.kepplinger@...i.sm>
Cc:     Bart Van Assche <bvanassche@....org>, 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 Thu, Jul 30, 2020 at 10:05:50AM +0200, Martin Kepplinger wrote:
> On 29.06.20 18:15, Alan Stern wrote:
> > On Mon, Jun 29, 2020 at 11:42:59AM +0200, Martin Kepplinger wrote:
> >>
> >>
> >> On 26.06.20 17:44, Alan Stern wrote:
> >>> Martin's best approach would be to add some debugging code to find out why 
> >>> blk_queue_enter() isn't calling bkl_pm_request_resume(), or why that call 
> >>> doesn't lead to pm_request_resume().
> >>>
> >>> Alan Stern
> >>>
> >>
> >> Hi Alan,
> >>
> >> blk_queue_enter() always - especially when sd is runtime suspended and I
> >> try to mount as above - sets success to be true for me, so never
> >> continues down to bkl_pm_request_resume(). All I see is "PM: Removing
> >> info for No Bus:sda1".
> > 
> > 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.
> > 
> > Below is my attempt to fix this up.  I'm not sure that the patch is 
> > entirely correct, but it should fix this logic bug.  I would appreciate a 
> > critical review.
> > 
> > Martin, does this fix the problem?
> > 
> > Alan Stern
> 
> Hi Alan,
> 
> So in the block layer your change below indeed fixes the problem and if
> you want to submit that 1:1 feel free to add
> 
> Tested-by: Martin Kepplinger <martin.kepplinger@...i.sm>
> 
> thanks for your help in this!

Thank you for _your_ help!

The next merge window is coming up soon.  I think I'll wait until it is 
over before submitting the patch (maintainers tend to be too busy to 
consider new patches during a merge window).

But I am still open to comments or criticism of the patch in the 
meantime.  There hasn't been any feedback since Bart's initial set of 
questions.

Alan Stern