2020-09-14 07:25:35

by YangYang

Subject: [PATCH] blk-mq: fix hang issue in blk_queue_enter()

There is a race between blk_queue_enter() and block layer's runtime
suspend.

CPU0 CPU1
--------------------------------- -------------------------------
blk_pre_runtime_suspend(q) { blk_queue_enter() {
/* q->rpm_status=RPM_ACTIVE */
blk_set_pm_only(q)
/* q->pm_only=1 */
blk_freeze_queue_start(q)
blk_mq_unfreeze_queue(q)
if (percpu_ref_tryget_live()) {
/* pm=0 && q->pm_only=1 */
if (pm || !blk_queue_pm_only(q)) {
} else {
percpu_ref_put()
}
}
wait_event(q->mq_freeze_wq,
(!q->mq_freeze_depth &&
/* q->rpm_status=RPM_ACTIVE
q->pm_only=1 */
(pm || (blk_pm_request_resume(q),
!blk_queue_pm_only(q)))) ||
blk_queue_dying(q))
}
spin_lock_irq(&q->queue_lock)
q->rpm_status = RPM_SUSPENDING
spin_unlock_irq(&q->queue_lock)
}

At this point blk_pm_request_resume() missed the chance to resume the
queue, so blk_queue_enter() may wait here forever.
The solution is to wake up the mq_freeze_wq after runtime suspend
completed, make blk_pm_request_resume() reexamine the q->rpm_status flag.

Signed-off-by: Yang Yang <[email protected]>
---
block/blk-pm.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/block/blk-pm.c b/block/blk-pm.c
index b85234d758f7..dec7d0aef606 100644
--- a/block/blk-pm.c
+++ b/block/blk-pm.c
@@ -132,6 +132,8 @@ void blk_post_runtime_suspend(struct request_queue *q, int err)

if (err)
blk_clear_pm_only(q);
+ else
+ wake_up_all(&q->mq_freeze_wq);
}
EXPORT_SYMBOL(blk_post_runtime_suspend);

--
2.17.1

2020-09-14 16:57:32

by Bart Van Assche

Subject: Re: [PATCH] blk-mq: fix hang issue in blk_queue_enter()

On 2020-09-14 00:19, Yang Yang wrote:
> There is a race between blk_queue_enter() and block layer's runtime
> suspend.
>
> CPU0 CPU1
> --------------------------------- -------------------------------
> blk_pre_runtime_suspend(q) { blk_queue_enter() {
> /* q->rpm_status=RPM_ACTIVE */
> blk_set_pm_only(q)
> /* q->pm_only=1 */
> blk_freeze_queue_start(q)
> blk_mq_unfreeze_queue(q)
> if (percpu_ref_tryget_live()) {
> /* pm=0 && q->pm_only=1 */
> if (pm || !blk_queue_pm_only(q)) {
> } else {
> percpu_ref_put()
> }
> }
> wait_event(q->mq_freeze_wq,
> (!q->mq_freeze_depth &&
> /* q->rpm_status=RPM_ACTIVE
> q->pm_only=1 */
> (pm || (blk_pm_request_resume(q),
> !blk_queue_pm_only(q)))) ||
> blk_queue_dying(q))
> }
> spin_lock_irq(&q->queue_lock)
> q->rpm_status = RPM_SUSPENDING
> spin_unlock_irq(&q->queue_lock)
> }
>
> At this point blk_pm_request_resume() missed the chance to resume the
> queue, so blk_queue_enter() may wait here forever.
> The solution is to wake up the mq_freeze_wq after runtime suspend
> completed, make blk_pm_request_resume() reexamine the q->rpm_status flag.
>
> Signed-off-by: Yang Yang <[email protected]>
> ---
> block/blk-pm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/block/blk-pm.c b/block/blk-pm.c
> index b85234d758f7..dec7d0aef606 100644
> --- a/block/blk-pm.c
> +++ b/block/blk-pm.c
> @@ -132,6 +132,8 @@ void blk_post_runtime_suspend(struct request_queue *q, int err)
>
> if (err)
> blk_clear_pm_only(q);
> + else
> + wake_up_all(&q->mq_freeze_wq);
> }
> EXPORT_SYMBOL(blk_post_runtime_suspend);

Please verify whether the following patch series also fixes the reported
hang: https://lore.kernel.org/linux-block/[email protected]/T/#t

Thanks,

Bart.

2020-09-15 08:38:03

by YangYang

Subject: Re: [PATCH] blk-mq: fix hang issue in blk_queue_enter()

On 2020/9/14 21:35, Markus Elfring wrote:
> …
>> The solution is to wake up the mq_freeze_wq after runtime suspend
>> completed, make blk_pm_request_resume() reexamine the q->rpm_status flag.
>
> * Would an imperative wording become helpful for the change description?
>
> * How do you think about to add the tag “Fixes” to the commit message?
>

Thank you for your suggestions.

--
Yang Yang