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@xxxxxxx>
thanks for your help in this!
martin
>
>
>
> Index: usb-devel/block/blk-core.c
> ===================================================================
> --- usb-devel.orig/block/blk-core.c
> +++ usb-devel/block/blk-core.c
> @@ -423,7 +423,8 @@ int blk_queue_enter(struct request_queue
> * responsible for ensuring that that counter is
> * globally visible before the queue is unfrozen.
> */
> - if (pm || !blk_queue_pm_only(q)) {
> + if ((pm && q->rpm_status != RPM_SUSPENDED) ||
> + !blk_queue_pm_only(q)) {
> success = true;
> } else {
> percpu_ref_put(&q->q_usage_counter);
> @@ -448,8 +449,7 @@ int blk_queue_enter(struct request_queue
>
> wait_event(q->mq_freeze_wq,
> (!q->mq_freeze_depth &&
> - (pm || (blk_pm_request_resume(q),
> - !blk_queue_pm_only(q)))) ||
> + blk_pm_resume_queue(pm, q)) ||
> blk_queue_dying(q));
> if (blk_queue_dying(q))
> return -ENODEV;
> Index: usb-devel/block/blk-pm.h
> ===================================================================
> --- usb-devel.orig/block/blk-pm.h
> +++ usb-devel/block/blk-pm.h
> @@ -6,11 +6,14 @@
> #include <linux/pm_runtime.h>
>
> #ifdef CONFIG_PM
> -static inline void blk_pm_request_resume(struct request_queue *q)
> +static inline int blk_pm_resume_queue(const bool pm, struct request_queue *q)
> {
> - if (q->dev && (q->rpm_status == RPM_SUSPENDED ||
> - q->rpm_status == RPM_SUSPENDING))
> - pm_request_resume(q->dev);
> + if (!q->dev || !blk_queue_pm_only(q))
> + return 1; /* Nothing to do */
> + if (pm && q->rpm_status != RPM_SUSPENDED)
> + return 1; /* Request allowed */
> + pm_request_resume(q->dev);
> + return 0;
> }
>
> static inline void blk_pm_mark_last_busy(struct request *rq)
> @@ -44,8 +47,9 @@ static inline void blk_pm_put_request(st
> --rq->q->nr_pending;
> }
> #else
> -static inline void blk_pm_request_resume(struct request_queue *q)
> +static inline int blk_pm_resume_queue(const bool pm, struct request_queue *q)
> {
> + return 1;
> }
>
> static inline void blk_pm_mark_last_busy(struct request *rq)
>
