linux-kernel - Re: [PATCH 0/4] blk-rq-qos: fix possible deadlock

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aO78EFfuT_o5Gcng@fedora>
Date: Wed, 15 Oct 2025 09:42:40 +0800
From: Ming Lei <ming.lei@...hat.com>
To: Yu Kuai <hailan@...uai.org.cn>
Cc: Nilay Shroff <nilay@...ux.ibm.com>, Yu Kuai <yukuai3@...wei.com>,
	tj@...nel.org, josef@...icpanda.com, axboe@...nel.dk,
	cgroups@...r.kernel.org, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org, yukuai1@...weicloud.com,
	yi.zhang@...wei.com, yangerkun@...wei.com, johnny.chenyi@...wei.com
Subject: Re: [PATCH 0/4] blk-rq-qos: fix possible deadlock

On Tue, Oct 14, 2025 at 07:14:16PM +0800, Yu Kuai wrote:
> Hi,
> 
> 在 2025/10/14 18:58, Nilay Shroff 写道:
> > 
> > On 10/14/25 7:51 AM, Yu Kuai wrote:
> > > Currently rq-qos debugfs entries is created from rq_qos_add(), while
> > > rq_qos_add() requires queue to be freezed. This can deadlock because
> > > 
> > > creating new entries can trigger fs reclaim.
> > > 
> > > Fix this problem by delaying creating rq-qos debugfs entries until
> > > it's initialization is complete.
> > > 
> > > - For wbt, it can be initialized by default of by blk-sysfs, fix it by
> > >    delaying after wbt_init();
> > > - For other policies, they can only be initialized by blkg configuration,
> > >    fix it by delaying to blkg_conf_end();
> > > 
> > > Noted this set is cooked on the top of my other thread:
> > > https://lore.kernel.org/all/20251010091446.3048529-1-yukuai@kernel.org/
> > > 
> > > And the deadlock can be reporduced with above thead, by running blktests
> > > throtl/001 with wbt enabled by default. While the deadlock is really a
> > > long term problem.
> > > 
> > While freezing the queue we also mark GFP_NOIO scope, so doesn't that
> > help avoid fs-reclaim? Or maybe if you can share the lockdep splat
> > encountered running throtl/001?
> 
> Yes, we can avoid fs-reclaim if queue is freezing, however,
> because debugfs is a generic file system, and we can't avoid fs reclaim from
> all context. There is still
> 
> Following is the log with above set and wbt enabled by default, the set acquire
> lock order by:
> 
> freeze queue -> elevator lock -> rq_qos_mutex -> blkcg_mutex
> 
> However, fs-reclaim from other context cause the deadlock report.
> 
> 
> [   45.632372][  T531] ======================================================
> [   45.633734][  T531] WARNING: possible circular locking dependency detected
> [   45.635062][  T531] 6.17.0-gfd4a560a0864-dirty #30 Not tainted
> [   45.636220][  T531] ------------------------------------------------------
> [   45.637587][  T531] check/531 is trying to acquire lock:
> [   45.638626][  T531] ffff9473884382b0 (&q->rq_qos_mutex){+.+.}-{4:4}, at: blkg_
> conf_start+0x116/0x190
> [   45.640416][  T531]
> [   45.640416][  T531] but task is already holding lock:
> [   45.641828][  T531] ffff9473884385d8 (&q->elevator_lock){+.+.}-{4:4}, at: blkg
> _conf_start+0x108/0x190
> [   45.643322][  T531]
> [   45.643322][  T531] which lock already depends on the new lock.
> [   45.643322][  T531]
> [   45.644862][  T531]
> [   45.644862][  T531] the existing dependency chain (in reverse order) is:
> [   45.646046][  T531]
> [   45.646046][  T531] -> #5 (&q->elevator_lock){+.+.}-{4:4}:
> [   45.647052][  T531]        __mutex_lock+0xd3/0x8d0
> [   45.647716][  T531]        blkg_conf_start+0x108/0x190
> [   45.648395][  T531]        tg_set_limit+0x74/0x300
> [   45.649046][  T531]        kernfs_fop_write_iter+0x14a/0x210
> [   45.649813][  T531]        vfs_write+0x29e/0x550
> [   45.650413][  T531]        ksys_write+0x74/0xf0
> [   45.651032][  T531]        do_syscall_64+0xbb/0x380
> [   45.651730][  T531] entry_SYSCALL_64_after_hwframe+0x77/0x7f

Not sure why elevator lock is grabbed in throttle code, which looks a elevator lock
misuse, what does the elevator try to protect here?

The comment log doesn't explain the usage too:

```
  /*
   * Similar to blkg_conf_open_bdev, but additionally freezes the queue,
   * acquires q->elevator_lock, and ensures the correct locking order
   * between q->elevator_lock and q->rq_qos_mutex.
   *
   * This function returns negative error on failure. On success it returns
   * memflags which must be saved and later passed to blkg_conf_exit_frozen
   * for restoring the memalloc scope.
   */
```

I think it is still order issue between queue freeze and q->rq_qos_mutex
first, which need to be solved first.


Thanks, 
Ming