| 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
| ||
|
Message-ID: <4697314.LvFx2qVVIh@rafael.j.wysocki>
Date: Wed, 26 Nov 2025 21:17:51 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Bart Van Assche <bvanassche@....org>
Cc: Yang Yang <yang.yang@...o.com>, Jens Axboe <axboe@...nel.dk>,
Pavel Machek <pavel@...nel.org>, Len Brown <lenb@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Danilo Krummrich <dakr@...nel.org>, linux-block@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject:
Re: [PATCH 1/2] PM: runtime: Fix I/O hang due to race between resume and
runtime disable
On Wednesday, November 26, 2025 8:34:54 PM CET Rafael J. Wysocki wrote:
> On Wed, Nov 26, 2025 at 8:16 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
> >
> > On Wed, Nov 26, 2025 at 7:06 PM Bart Van Assche <bvanassche@....org> wrote:
> > >
> > > On 11/26/25 3:30 AM, Rafael J. Wysocki wrote:
> > > > On Wed, Nov 26, 2025 at 11:17 AM Yang Yang <yang.yang@...o.com> wrote:
> > > >> T1: T2:
> > > >> blk_queue_enter
> > > >> blk_pm_resume_queue
> > > >> pm_request_resume
> > > >
> > > > Shouldn't this be pm_runtime_resume() rather?
> > >
> > > I tried to make that change on an Android device. As a result, the
> > > kernel complaint shown below appeared. My understanding is that sleeping
> > > in atomic context can trigger a deadlock and hence is not allowed.
> > >
> > > [ 13.728890][ T1] WARNING: CPU: 6 PID: 1 at
> > > kernel/sched/core.c:9714 __might_sleep+0x78/0x84
> > > [ 13.758800][ T1] Call trace:
> > > [ 13.759027][ T1] __might_sleep+0x78/0x84
> > > [ 13.759340][ T1] __pm_runtime_resume+0x40/0xb8
> > > [ 13.759781][ T1] __bio_queue_enter+0xc0/0x1cc
> > > [ 13.760153][ T1] blk_mq_submit_bio+0x884/0xadc
> > > [ 13.760548][ T1] __submit_bio+0x2c8/0x49c
> > > [ 13.760879][ T1] __submit_bio_noacct_mq+0x38/0x88
> > > [ 13.761242][ T1] submit_bio_noacct_nocheck+0x4fc/0x7b8
> > > [ 13.761631][ T1] submit_bio+0x214/0x4c0
> > > [ 13.761941][ T1] mpage_readahead+0x1b8/0x1fc
> > > [ 13.762284][ T1] blkdev_readahead+0x18/0x28
> > > [ 13.762660][ T1] page_cache_ra_unbounded+0x310/0x4d8
> > > [ 13.763072][ T1] page_cache_ra_order+0xc0/0x5b0
> > > [ 13.763434][ T1] page_cache_sync_ra+0x17c/0x268
> > > [ 13.763782][ T1] filemap_read+0x4c4/0x12f4
> > > [ 13.764125][ T1] blkdev_read_iter+0x100/0x164
> > > [ 13.764475][ T1] vfs_read+0x188/0x348
> > > [ 13.764789][ T1] __se_sys_pread64+0x84/0xc8
> > > [ 13.765180][ T1] __arm64_sys_pread64+0x1c/0x2c
> > > [ 13.765556][ T1] invoke_syscall+0x58/0xf0
> > > [ 13.765876][ T1] do_el0_svc+0x8c/0xe0
> > > [ 13.766172][ T1] el0_svc+0x50/0xd4
> > > [ 13.766583][ T1] el0t_64_sync_handler+0x20/0xf4
> > > [ 13.766932][ T1] el0t_64_sync+0x1bc/0x1c0
> > > [ 13.767294][ T1] irq event stamp: 2589614
> > > [ 13.767592][ T1] hardirqs last enabled at (2589613):
> > > [<ffffffc0800eaf24>] finish_lock_switch+0x70/0x108
> > > [ 13.768283][ T1] hardirqs last disabled at (2589614):
> > > [<ffffffc0814b66f4>] el1_dbg+0x24/0x80
> > > [ 13.768875][ T1] softirqs last enabled at (2589370):
> > > [<ffffffc080082a7c>] ____do_softirq+0x10/0x20
> > > [ 13.769529][ T1] softirqs last disabled at (2589349):
> > > [<ffffffc080082a7c>] ____do_softirq+0x10/0x20
> > >
> > > I think that the filemap_invalidate_lock_shared() call in
> > > page_cache_ra_unbounded() forbids sleeping in submit_bio().
> >
> > The wait_event() macro in __bio_queue_enter() calls might_sleep() at
> > the very beginning, so why would it not complain?
> >
> > IIUC, this is the WARN_ONCE() in __might_sleep() about the task state
> > being different from TASK_RUNNING, which triggers because
> > prepare_to_wait_event() changes the task state to
> > TASK_UNINTERRUPTIBLE.
> >
> > This means that calling pm_runtime_resume() cannot be part of the
> > wait_event() condition, so blk_pm_resume_queue() and the wait_event()
> > macros involving it would need some rewriting.
>
> Interestingly enough, the pm_request_resume() call in
> blk_pm_resume_queue() is not even necessary in the __bio_queue_enter()
> case because pm is false there and it doesn't even check
> q->rpm_status.
>
> So in fact the resume is only necessary in blk_queue_enter() if pm is nonzero.
If I'm not completely in the weeds, something like the patch below should be
doable.
Also, I'd consider using pm_runtime_get_noresume() and pm_runtime_put_noidle()
in blk_queue_enter() and blk_queue_exit(), respectively, in the "pm != 0" case
to prevent the device from suspending while the .q_usage_counter ref is held.
---
block/blk-core.c | 6 +++---
block/blk-pm.h | 7 ++++---
2 files changed, 7 insertions(+), 6 deletions(-)
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -309,6 +309,8 @@ int blk_queue_enter(struct request_queue
if (flags & BLK_MQ_REQ_NOWAIT)
return -EAGAIN;
+ /* if necessary, resume .dev (assume success). */
+ blk_pm_resume_queue(pm, q);
/*
* read pair of barrier in blk_freeze_queue_start(), we need to
* order reading __PERCPU_REF_DEAD flag of .q_usage_counter and
@@ -318,9 +320,7 @@ int blk_queue_enter(struct request_queue
*/
smp_rmb();
wait_event(q->mq_freeze_wq,
- (!q->mq_freeze_depth &&
- blk_pm_resume_queue(pm, q)) ||
- blk_queue_dying(q));
+ !q->mq_freeze_depth || blk_queue_dying(q));
if (blk_queue_dying(q))
return -ENODEV;
}
--- a/block/blk-pm.h
+++ b/block/blk-pm.h
@@ -10,9 +10,10 @@ static inline int blk_pm_resume_queue(co
{
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);
+
+ if (pm)
+ pm_runtime_resume(q->dev);
+
return 0;
}
Powered by blists - more mailing lists