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

From: Martin Kepplinger <martin.kepplinger@puri.sm>
To: Alan Stern <stern@rowland.harvard.edu>
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 19:42:43 +0200	[thread overview]
Message-ID: <4c636f2d-af7f-bbde-a864-dbeb67c590ec@puri.sm> (raw)
In-Reply-To: <5152a510-bebf-bf33-f6b3-4549e50386ab@puri.sm>

On 26.08.20 09:48, Martin Kepplinger wrote:
> On 24.08.20 22:13, Alan Stern wrote:
>> On Mon, Aug 24, 2020 at 10:48:32AM -0700, Bart Van Assche wrote:
>>> On 2020-08-23 07:57, Alan Stern wrote:
>>>> The problem is fixed by checking that the queue's runtime-PM status
>>>> isn't RPM_SUSPENDED before allowing a request to be issued, [ ... ]
>>>
>>> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
>>>
>>> Can, can you help with testing this patch?
>>>
>>> Thanks,
>>>
>>> Bart.
>>
>> 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.

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?

> 
>>
>> As I recall, the request that failed was a read-ahead.  It's not clear 
>> to me why such a request would need to have BLK_MQ_REQ_PREEMPT set.  
>> Although there may be other reasons for having that flag, at this point 
>> we're concerned with requests that are part of the runtime-resume 
>> procedure.  A read-ahead does not fall into that category.
>>
>> Do you think you can find the answer?  Perhaps we should add a separate 
>> BLK_MQ_REQ_PM flag.