Re: [PATCH] block: Fix bug in runtime-resume handling

From: Alan Stern <stern@rowland.harvard.edu>
To: Martin Kepplinger <martin.kepplinger@puri.sm>
Cc: Bart Van Assche <bvanassche@acm.org>,
	Can Guo <cang@codeaurora.org>,
	linux-scsi@vger.kernel.org, linux-block@vger.kernel.org,
	kernel@puri.sm
Subject: Re: [PATCH] block: Fix bug in runtime-resume handling
Date: Thu, 27 Aug 2020 16:29:52 -0400	[thread overview]
Message-ID: <20200827202952.GA449067@rowland.harvard.edu> (raw)
In-Reply-To: <4c636f2d-af7f-bbde-a864-dbeb67c590ec@puri.sm>

On Thu, Aug 27, 2020 at 07:42:43PM +0200, Martin Kepplinger wrote:
> On 26.08.20 09:48, Martin Kepplinger wrote:
> > On 24.08.20 22:13, Alan Stern wrote:

> >> Martin:
> >>
> >> (I forgot to ask this question several weeks ago, while you were running 
> >> your tests.  Better ask it now before I forget again...)
> >>
> >> I suspect the old runtime-PM code in the block layer would have worked 
> >> okay in your SD cardreader test if the BLK_MQ_REQ_PREEMPT flag had not 
> >> been set.  Do you know why the flag was set, or what line of code caused 
> >> it to be set?
> > 
> > Correct. if not set, I could handle all I need in the scsi error path.
> 
> this thread becomes a bit confusing. I thought about REQ_FAILFAST_DEV
> but you're talking about something different.
> 
> the only place I see BLK_MQ_REQ_PREEMPT getting passed on is in
> __scsi_execute() which is the case when mounting/unmounting. At least
> that about the only place I can find.

Ah yes, I see what you mean.

> I remember *only* your block pm fix would let me mount/unmount, but not
> use files yet (REQ_FAILFAST_DEV and so on).
> 
> When I revert your fix and remove BLK_MQ_REQ_PREEMPT from being passed
> on to blk_get_request() in __scsi_execute(), that line gets executed
> exactly once during startup and I'm missing the /dev/sda device from the
> cardreader then.
> 
> Is this what you're asking?

Not quite sure, but it doesn't matter.  Removing BLK_MQ_REQ_PREEMPT in 
__scsi_execute() is probably not a safe thing to do.

Instead, look at sd_resume().  That routine calls __scsi_execute() 
indirectly through sd_start_stop_device(), and the only reason it does 
this is because the sdkp->device->manage_start_stop flag is set.  You 
ought to be able to clear this flag in sysfs, by writing to 
/sys/block/sda/device/scsi_disk/*/manage_start_stop.  If you do this 
before allowing the card reader to go into runtime suspend, does it then 
resume okay?

(Yes, I know you still won't be able to read it because of the FAILFAST 
flag.  I just want to know if the runtime resume actually takes place.)

Alan Stern