[PATCH] blk-cgroup: save IRQ state in blkg_tryget_closest()

[PATCH] blk-cgroup: save IRQ state in blkg_tryget_closest()

[Hao Zhang]

From: Hao Zhang <zhanghao1@kylinos.cn>

bio_set_dev() associates the bio with a blkg through bio_associate_blkg().
If the blkg lookup misses, blkg_tryget_closest() takes q->queue_lock with
spin_lock_irq() and releases it with spin_unlock_irq(), which
unconditionally enables local interrupts.

Callers may call bio_set_dev() with interrupts already disabled, e.g.
dm-thin's pool_map() does so while holding pool->lock taken with
spin_lock_irq().  The nested spin_unlock_irq() then enables interrupts
while pool->lock is still held, so an I/O completion softirq can run on
the same CPU, re-acquire pool->lock (thin_endio(), or overwrite_endio()
-> complete_mapping_preparation()) and deadlock.  lockdep reports this
as inconsistent SOFTIRQ-ON-W to IN-SOFTIRQ-W usage.

Commit 3a762de55b4e ("block: save irq state in blkg_lookup_create()")
fixed the same problem while the lock lived in blkg_lookup_create(), but
commit 9327a865e395 ("blk-cgroup: don't nest queue_lock under rcu in
blkg_lookup_create()") moved the locking into blkg_tryget_closest() and
reverted it to spin_lock_irq().

Save and restore the caller's IRQ state instead.

Fixes: 9327a865e395 ("blk-cgroup: don't nest queue_lock under rcu in blkg_lookup_create()")
Cc: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
 block/blk-cgroup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 2b5c29434e42..b56db1cc6778 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -2091,6 +2091,7 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,
 	struct request_queue *q = bio->bi_bdev->bd_queue;
 	struct blkcg *blkcg = css_to_blkcg(css);
 	struct blkcg_gq *blkg;
+	unsigned long flags;
 
 	rcu_read_lock();
 	blkg = blkg_lookup(blkcg, q);
@@ -2105,11 +2106,11 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,
 	 * Fast path failed, we're probably issuing IO in this cgroup the first
 	 * time, hold lock to create new blkg.
 	 */
-	spin_lock_irq(&q->queue_lock);
+	spin_lock_irqsave(&q->queue_lock, flags);
 	blkg = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk);
 	if (blkg)
 		blkg = blkg_lookup_tryget(blkg);
-	spin_unlock_irq(&q->queue_lock);
+	spin_unlock_irqrestore(&q->queue_lock, flags);
 
 	return blkg;
 }

base-commit: 08df884136f1c1197bab2a27814404fd329d9aac
-- 
2.15.0

Re: [PATCH] blk-cgroup: save IRQ state in blkg_tryget_closest()

[Tejun Heo]

On Sat, Sep 12, 2026 at 12:04:57AM +0800, Hao Zhang wrote:
> From: Hao Zhang <zhanghao1@kylinos.cn>
> 
> bio_set_dev() associates the bio with a blkg through bio_associate_blkg().
> If the blkg lookup misses, blkg_tryget_closest() takes q->queue_lock with
> spin_lock_irq() and releases it with spin_unlock_irq(), which
> unconditionally enables local interrupts.
> 
> Callers may call bio_set_dev() with interrupts already disabled, e.g.
> dm-thin's pool_map() does so while holding pool->lock taken with
> spin_lock_irq().  The nested spin_unlock_irq() then enables interrupts
> while pool->lock is still held, so an I/O completion softirq can run on
> the same CPU, re-acquire pool->lock (thin_endio(), or overwrite_endio()
> -> complete_mapping_preparation()) and deadlock.  lockdep reports this
> as inconsistent SOFTIRQ-ON-W to IN-SOFTIRQ-W usage.
> 
> Commit 3a762de55b4e ("block: save irq state in blkg_lookup_create()")
> fixed the same problem while the lock lived in blkg_lookup_create(), but
> commit 9327a865e395 ("blk-cgroup: don't nest queue_lock under rcu in
> blkg_lookup_create()") moved the locking into blkg_tryget_closest() and
> reverted it to spin_lock_irq().
> 
> Save and restore the caller's IRQ state instead.
> 
> Fixes: 9327a865e395 ("blk-cgroup: don't nest queue_lock under rcu in blkg_lookup_create()")
> Cc: Ming Lei <ming.lei@redhat.com>
> Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

Re: [PATCH] blk-cgroup: save IRQ state in blkg_tryget_closest()

[Jens Axboe]

On Sat, 12 Sep 2026 00:04:57 +0800, Hao Zhang wrote:
> bio_set_dev() associates the bio with a blkg through bio_associate_blkg().
> If the blkg lookup misses, blkg_tryget_closest() takes q->queue_lock with
> spin_lock_irq() and releases it with spin_unlock_irq(), which
> unconditionally enables local interrupts.
> 
> Callers may call bio_set_dev() with interrupts already disabled, e.g.
> dm-thin's pool_map() does so while holding pool->lock taken with
> spin_lock_irq().  The nested spin_unlock_irq() then enables interrupts
> while pool->lock is still held, so an I/O completion softirq can run on
> the same CPU, re-acquire pool->lock (thin_endio(), or overwrite_endio()
> -> complete_mapping_preparation()) and deadlock.  lockdep reports this
> as inconsistent SOFTIRQ-ON-W to IN-SOFTIRQ-W usage.
> 
> [...]

Applied, thanks!

[1/1] blk-cgroup: save IRQ state in blkg_tryget_closest()
      commit: 327c428ba79b13e4cc5253333a9d5c0aa5579bdf

Best regards,
-- 
Jens Axboe