* [RFC PATCH v2 1/8] block: associate blkg in submit_bio instead of bio_set_dev
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 2/8] blk-throttle: protect throttle state with td lock Yu Kuai
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
bio_set_dev(), bio_init() and bio_reset() associate a bio with a blkg for
its target queue. That association may have to create a new blkg, and
currently there are lots of callers that are under atomic context.
Move the association out of those helpers and into the submit path, which
is always sleepable (submit_bio_noacct() already does might_sleep()):
- submit_bio() associates new I/O before bio_set_ioprio(), whose
blkcg_set_ioprio() reads the policy from bio->bi_blkg.
- submit_bio_noacct() (re)associates when a bio has no blkg yet or was
remapped to a different queue. blk_throtl_bio() and the rq_qos
throttlers (iocost, iolatency) pair bio->bi_blkg with the queue of
bio->bi_bdev, so a remapped bio must be reassociated to the new queue.
- bio_set_dev() no longer associates; instead it drops the existing blkg
when the device changes, since that blkg is tied to the old queue.
bio_init()/bio_reset() leave bi_blkg NULL.
Introduce bio_disassociate_blkg() to drop a bio's blkg reference and use it
from bio_set_dev(), bio_uninit() and the bio freeing path, replacing their
open-coded blkg_put().
This is the first step to convert protecting blkg with blkcg_mutex.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bio.c | 18 ++----------------
block/blk-cgroup.c | 18 ++++++++++++++++++
block/blk-core.c | 21 +++++++++++++++++++++
include/linux/bio.h | 7 +++++--
4 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 6a2f6fc3413e..ea030c5e39a4 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -179,12 +179,7 @@ static inline gfp_t try_alloc_gfp(gfp_t gfp)
void bio_uninit(struct bio *bio)
{
-#ifdef CONFIG_BLK_CGROUP
- if (bio->bi_blkg) {
- blkg_put(bio->bi_blkg);
- bio->bi_blkg = NULL;
- }
-#endif
+ bio_disassociate_blkg(bio);
if (bio_integrity(bio))
bio_integrity_free(bio);
@@ -235,8 +230,6 @@ void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table,
#ifdef CONFIG_BLK_CGROUP
bio->bi_blkg = NULL;
bio->issue_time_ns = 0;
- if (bdev)
- bio_associate_blkg(bio);
#ifdef CONFIG_BLK_CGROUP_IOCOST
bio->bi_iocost_cost = 0;
#endif
@@ -280,8 +273,6 @@ void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf)
atomic_set(&bio->__bi_remaining, 1);
bio->bi_io_vec = bv;
bio->bi_bdev = bdev;
- if (bio->bi_bdev)
- bio_associate_blkg(bio);
bio->bi_opf = opf;
}
EXPORT_SYMBOL(bio_reset);
@@ -1803,17 +1794,12 @@ void bio_endio(struct bio *bio)
goto again;
}
-#ifdef CONFIG_BLK_CGROUP
/*
* Release cgroup info. We shouldn't have to do this here, but quite
* a few callers of bio_init fail to call bio_uninit, so we cover up
* for that here at least for now.
*/
- if (bio->bi_blkg) {
- blkg_put(bio->bi_blkg);
- bio->bi_blkg = NULL;
- }
-#endif
+ bio_disassociate_blkg(bio);
if (bio->bi_end_io)
bio->bi_end_io(bio);
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index d9676126c5b5..618e5566fa52 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -2170,6 +2170,24 @@ void bio_clone_blkg_association(struct bio *dst, struct bio *src)
}
EXPORT_SYMBOL_GPL(bio_clone_blkg_association);
+/**
+ * bio_disassociate_blkg - disassociate a bio from its blkg
+ * @bio: target bio
+ *
+ * Drop the blkg reference held by @bio and clear the association. This is
+ * used when a bio's target device changes (e.g. via bio_set_dev()): the old
+ * blkg is tied to the previous request_queue, so it is dropped here and the
+ * bio is reassociated to the new queue from the submit path.
+ */
+void bio_disassociate_blkg(struct bio *bio)
+{
+ if (bio->bi_blkg) {
+ blkg_put(bio->bi_blkg);
+ bio->bi_blkg = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(bio_disassociate_blkg);
+
static int blk_cgroup_io_type(struct bio *bio)
{
if (op_is_discard(bio->bi_opf))
diff --git a/block/blk-core.c b/block/blk-core.c
index 365641266c9e..8103643b39fc 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -822,6 +822,20 @@ void submit_bio_noacct(struct bio *bio)
might_sleep();
+ /*
+ * Associate the bio with a blkg for its target queue here, where it is
+ * safe to sleep, instead of in bio_set_dev()/bio_init() which may run
+ * under a spinlock. bio_set_dev() no longer associates, so a freshly
+ * allocated or remapped bio has no blkg until it reaches the submit path.
+ * Reassociate only when the bio was remapped to a different queue since
+ * it was last associated; blk_throtl_bio() and the rq_qos throttlers
+ * rely on bio->bi_blkg matching the queue of bio->bi_bdev.
+ */
+#ifdef CONFIG_BLK_CGROUP
+ if (!bio->bi_blkg || bio->bi_blkg->q != q)
+ bio_associate_blkg(bio);
+#endif
+
/*
* For a REQ_NOWAIT based request, return -EOPNOTSUPP
* if queue does not support NOWAIT.
@@ -958,6 +972,13 @@ void submit_bio(struct bio *bio)
count_vm_events(PGPGOUT, bio_sectors(bio));
}
+ /*
+ * bio_set_ioprio() -> blkcg_set_ioprio() reads the policy from
+ * bio->bi_blkg, so associate the blkg (for new I/O, the first time) before
+ * it runs. This is the sleepable entry point for new I/O; remapped bios
+ * that reach submit_bio_noacct() directly are reassociated there.
+ */
+ bio_associate_blkg(bio);
bio_set_ioprio(bio);
submit_bio_noacct(bio);
}
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 8f33f717b14f..7a7509c8a59a 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -507,6 +507,7 @@ void bio_associate_blkg(struct bio *bio);
void bio_associate_blkg_from_css(struct bio *bio,
struct cgroup_subsys_state *css);
void bio_clone_blkg_association(struct bio *dst, struct bio *src);
+void bio_disassociate_blkg(struct bio *bio);
void blkcg_punt_bio_submit(struct bio *bio);
#else /* CONFIG_BLK_CGROUP */
static inline void bio_associate_blkg(struct bio *bio) { }
@@ -515,6 +516,7 @@ static inline void bio_associate_blkg_from_css(struct bio *bio,
{ }
static inline void bio_clone_blkg_association(struct bio *dst,
struct bio *src) { }
+static inline void bio_disassociate_blkg(struct bio *bio) { }
static inline void blkcg_punt_bio_submit(struct bio *bio)
{
submit_bio(bio);
@@ -524,10 +526,11 @@ static inline void blkcg_punt_bio_submit(struct bio *bio)
static inline void bio_set_dev(struct bio *bio, struct block_device *bdev)
{
bio_clear_flag(bio, BIO_REMAPPED);
- if (bio->bi_bdev != bdev)
+ if (bio->bi_bdev != bdev) {
bio_clear_flag(bio, BIO_BPS_THROTTLED);
+ bio_disassociate_blkg(bio);
+ }
bio->bi_bdev = bdev;
- bio_associate_blkg(bio);
}
/*
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 2/8] blk-throttle: protect throttle state with td lock
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 1/8] block: associate blkg in submit_bio instead of bio_set_dev Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 3/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
Throttle currently uses queue_lock for both blkcg topology and its own
runtime state. This blocks moving blkg topology protection to blkcg_mutex
cleanly.
Add a throttle-private spinlock and use it for throttle service queues,
pending timers, runtime counters and config updates. Keep queue_lock only
where the current intermediate code still walks blkcg topology.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-throttle.c | 87 ++++++++++++++++++++++++++++++++++----------
1 file changed, 67 insertions(+), 20 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index ffc3b70065d4..7bca2805404f 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -30,6 +30,9 @@ static struct workqueue_struct *kthrotld_workqueue;
struct throtl_data
{
+ /* protects throttle service queues and group runtime state */
+ spinlock_t lock;
+
/* service tree for active throtl groups */
struct throtl_service_queue service_queue;
@@ -346,11 +349,16 @@ static void tg_update_has_rules(struct throtl_grp *tg)
static void throtl_pd_online(struct blkg_policy_data *pd)
{
struct throtl_grp *tg = pd_to_tg(pd);
+ struct throtl_data *td = tg->td;
+ unsigned long flags;
+
+ spin_lock_irqsave(&td->lock, flags);
/*
* We don't want new groups to escape the limits of its ancestors.
* Update has_rules[] after a new group is brought online.
*/
tg_update_has_rules(tg);
+ spin_unlock_irqrestore(&td->lock, flags);
}
static void tg_release(struct rcu_head *rcu)
@@ -368,7 +376,7 @@ static void throtl_pd_free(struct blkg_policy_data *pd)
{
struct throtl_grp *tg = pd_to_tg(pd);
- timer_delete_sync(&tg->service_queue.pending_timer);
+ timer_shutdown_sync(&tg->service_queue.pending_timer);
call_rcu(&pd->rcu_head, tg_release);
}
@@ -1142,9 +1150,9 @@ static void throtl_pending_timer_fn(struct timer_list *t)
else
q = td->queue;
- spin_lock_irq(&q->queue_lock);
+ spin_lock_irq(&td->lock);
- if (!q->root_blkg)
+ if (!READ_ONCE(q->root_blkg))
goto out_unlock;
again:
@@ -1168,9 +1176,9 @@ static void throtl_pending_timer_fn(struct timer_list *t)
break;
/* this dispatch windows is still open, relax and repeat */
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&td->lock);
cpu_relax();
- spin_lock_irq(&q->queue_lock);
+ spin_lock_irq(&td->lock);
}
if (!dispatched)
@@ -1193,7 +1201,7 @@ static void throtl_pending_timer_fn(struct timer_list *t)
queue_work(kthrotld_workqueue, &td->dispatch_work);
}
out_unlock:
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&td->lock);
}
/**
@@ -1209,7 +1217,6 @@ static void blk_throtl_dispatch_work_fn(struct work_struct *work)
struct throtl_data *td = container_of(work, struct throtl_data,
dispatch_work);
struct throtl_service_queue *td_sq = &td->service_queue;
- struct request_queue *q = td->queue;
struct bio_list bio_list_on_stack;
struct bio *bio;
struct blk_plug plug;
@@ -1217,11 +1224,11 @@ static void blk_throtl_dispatch_work_fn(struct work_struct *work)
bio_list_init(&bio_list_on_stack);
- spin_lock_irq(&q->queue_lock);
+ spin_lock_irq(&td->lock);
for (rw = READ; rw <= WRITE; rw++)
while ((bio = throtl_pop_queued(td_sq, NULL, rw)))
bio_list_add(&bio_list_on_stack, bio);
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&td->lock);
if (!bio_list_empty(&bio_list_on_stack)) {
blk_start_plug(&plug);
@@ -1299,7 +1306,7 @@ static void tg_conf_updated(struct throtl_grp *tg, bool global)
rcu_read_unlock();
/*
- * We're already holding queue_lock and know @tg is valid. Let's
+ * We're already holding td->lock and know @tg is valid. Let's
* apply the new config directly.
*
* Restart the slices for both READ and WRITES. It might happen
@@ -1327,6 +1334,7 @@ static int blk_throtl_init(struct gendisk *disk)
return -ENOMEM;
INIT_WORK(&td->dispatch_work, blk_throtl_dispatch_work_fn);
+ spin_lock_init(&td->lock);
throtl_service_queue_init(&td->service_queue);
memflags = blk_mq_freeze_queue(disk->queue);
@@ -1381,6 +1389,7 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of,
v = U64_MAX;
tg = blkg_to_tg(ctx.blkg);
+ spin_lock_irq(&tg->td->lock);
tg_update_carryover(tg);
if (is_u64)
@@ -1389,6 +1398,7 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of,
*(unsigned int *)((void *)tg + of_cft(of)->private) = v;
tg_conf_updated(tg, false);
+ spin_unlock_irq(&tg->td->lock);
ret = 0;
unprep:
@@ -1563,6 +1573,7 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of,
goto close_bdev;
tg = blkg_to_tg(ctx.blkg);
+ spin_lock_irq(&tg->td->lock);
tg_update_carryover(tg);
v[0] = tg->bps[READ];
@@ -1586,11 +1597,11 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of,
p = tok;
strsep(&p, "=");
if (!p || (sscanf(p, "%llu", &val) != 1 && strcmp(p, "max")))
- goto unprep;
+ goto unlock;
ret = -ERANGE;
if (!val)
- goto unprep;
+ goto unlock;
ret = -EINVAL;
if (!strcmp(tok, "rbps"))
@@ -1602,7 +1613,7 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of,
else if (!strcmp(tok, "wiops"))
v[3] = min_t(u64, val, UINT_MAX);
else
- goto unprep;
+ goto unlock;
}
tg->bps[READ] = v[0];
@@ -1611,7 +1622,11 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of,
tg->iops[WRITE] = v[3];
tg_conf_updated(tg, false);
+ spin_unlock_irq(&tg->td->lock);
ret = 0;
+ goto unprep;
+unlock:
+ spin_unlock_irq(&tg->td->lock);
unprep:
blkg_conf_unprep(&ctx);
close_bdev:
@@ -1636,6 +1651,28 @@ static void throtl_shutdown_wq(struct request_queue *q)
cancel_work_sync(&td->dispatch_work);
}
+static void throtl_shutdown_timers(struct request_queue *q)
+{
+ struct throtl_data *td = q->td;
+ struct blkcg_gq *blkg;
+
+ /*
+ * blkg_destroy_all() has already offlined the policy, but blkg policy
+ * data is freed asynchronously. Shut down per-group timers before
+ * freeing td, as their callbacks still dereference tg->td.
+ */
+ mutex_lock(&q->blkcg_mutex);
+ list_for_each_entry(blkg, &q->blkg_list, q_node) {
+ struct throtl_grp *tg = blkg_to_tg(blkg);
+
+ if (tg)
+ timer_shutdown_sync(&tg->service_queue.pending_timer);
+ }
+ mutex_unlock(&q->blkcg_mutex);
+
+ timer_shutdown_sync(&td->service_queue.pending_timer);
+}
+
static void tg_flush_bios(struct throtl_grp *tg)
{
struct throtl_service_queue *sq = &tg->service_queue;
@@ -1669,7 +1706,13 @@ static void tg_flush_bios(struct throtl_grp *tg)
static void throtl_pd_offline(struct blkg_policy_data *pd)
{
- tg_flush_bios(pd_to_tg(pd));
+ struct throtl_grp *tg = pd_to_tg(pd);
+ struct throtl_data *td = tg->td;
+ unsigned long flags;
+
+ spin_lock_irqsave(&td->lock, flags);
+ tg_flush_bios(tg);
+ spin_unlock_irqrestore(&td->lock, flags);
}
struct blkcg_policy blkcg_policy_throtl = {
@@ -1725,6 +1768,7 @@ static void tg_cancel_writeback_bios(struct throtl_grp *tg,
void blk_throtl_cancel_bios(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
+ struct throtl_data *td = q->td;
struct cgroup_subsys_state *pos_css;
struct blkcg_gq *blkg;
struct bio_list cancel_bios[2] = { };
@@ -1734,6 +1778,7 @@ void blk_throtl_cancel_bios(struct gendisk *disk)
return;
spin_lock_irq(&q->queue_lock);
+ spin_lock(&td->lock);
/*
* queue_lock is held, rcu lock is not needed here technically.
* However, rcu lock is still held to emphasize that following
@@ -1752,6 +1797,7 @@ void blk_throtl_cancel_bios(struct gendisk *disk)
tg_cancel_writeback_bios(blkg_to_tg(blkg), cancel_bios);
}
rcu_read_unlock();
+ spin_unlock(&td->lock);
spin_unlock_irq(&q->queue_lock);
for (rw = READ; rw <= WRITE; rw++) {
@@ -1791,7 +1837,6 @@ static bool tg_within_limit(struct throtl_grp *tg, struct bio *bio, bool rw)
bool __blk_throtl_bio(struct bio *bio)
{
- struct request_queue *q = bdev_get_queue(bio->bi_bdev);
struct blkcg_gq *blkg = bio->bi_blkg;
struct throtl_qnode *qn = NULL;
struct throtl_grp *tg = blkg_to_tg(blkg);
@@ -1801,7 +1846,7 @@ bool __blk_throtl_bio(struct bio *bio)
struct throtl_data *td = tg->td;
rcu_read_lock();
- spin_lock_irq(&q->queue_lock);
+ spin_lock_irq(&td->lock);
sq = &tg->service_queue;
while (true) {
@@ -1877,7 +1922,7 @@ bool __blk_throtl_bio(struct bio *bio)
}
out_unlock:
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&td->lock);
rcu_read_unlock();
return throttled;
@@ -1886,17 +1931,19 @@ bool __blk_throtl_bio(struct bio *bio)
void blk_throtl_exit(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
+ struct throtl_data *td = q->td;
/*
* blkg_destroy_all() already deactivate throtl policy, just check and
* free throtl data.
*/
- if (!q->td)
+ if (!td)
return;
- timer_delete_sync(&q->td->service_queue.pending_timer);
+ throtl_shutdown_timers(q);
throtl_shutdown_wq(q);
- kfree(q->td);
+ q->td = NULL;
+ kfree(td);
}
static int __init throtl_init(void)
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 3/8] blk-cgroup: protect blkgs with blkcg_mutex
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 1/8] block: associate blkg in submit_bio instead of bio_set_dev Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 2/8] blk-throttle: protect throttle state with td lock Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 4/8] blk-cgroup: remove blkg radix tree preloading Yu Kuai
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
queue_lock is still needed by block core users, but blkcg no longer needs
it for blkg topology now that throttle runtime state has a private lock.
Move queue-local blkg synchronization to q->blkcg_mutex. Hold it while
looking up, creating and destroying blkgs, while preparing and undoing
configuration, and while activating or deactivating policies.
Update the BFQ, iocost, iolatency and throttle paths which walk
q->blkg_list or access per-blkg policy state to use the same lock.
blkcg->lock still protects blkcg-local radix tree and list updates. Some
lookups under blkcg_mutex can race with blkcg updates done for other
queues, so keep those lookups in RCU read-side critical sections. In
particular, protect the parent lookup in blkg_create() and the parent walk
in blkg_lookup_create().
Now that blkg association happens from the sleepable submit path, the
queue_lock -> blkcg_mutex conversion lets blkg creation sleep properly.
Nowait bio handling (failing submit instead of sleeping) is added in a
follow-up patch.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-cgroup.c | 10 +--
block/blk-cgroup.c | 182 ++++++++++++++++++++++--------------------
block/blk-cgroup.h | 16 ++--
block/blk-core.c | 2 +
block/blk-iocost.c | 5 +-
block/blk-iolatency.c | 7 +-
block/blk-throttle.c | 10 +--
7 files changed, 121 insertions(+), 111 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index e82ff03bda02..42614aa78cd4 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -426,7 +426,7 @@ static void bfqg_stats_xfer_dead(struct bfq_group *bfqg)
parent = bfqg_parent(bfqg);
- lockdep_assert_held(&bfqg_to_blkg(bfqg)->q->queue_lock);
+ lockdep_assert_held(&bfqg_to_blkg(bfqg)->q->blkcg_mutex);
if (unlikely(!parent))
return;
@@ -874,7 +874,7 @@ static void bfq_reparent_active_queues(struct bfq_data *bfqd,
* and reparent its children entities.
* @pd: descriptor of the policy going offline.
*
- * blkio already grabs the queue_lock for us, so no need to use
+ * blkio already grabs the blkcg_mutex for us, so no need to use
* RCU-based magic
*/
static void bfq_pd_offline(struct blkg_policy_data *pd)
@@ -947,8 +947,7 @@ void bfq_end_wr_async(struct bfq_data *bfqd)
struct blkcg_gq *blkg;
mutex_lock(&q->blkcg_mutex);
- spin_lock_irq(&q->queue_lock);
- spin_lock(&bfqd->lock);
+ spin_lock_irq(&bfqd->lock);
list_for_each_entry(blkg, &q->blkg_list, q_node) {
struct bfq_group *bfqg = blkg_to_bfqg(blkg);
@@ -957,8 +956,7 @@ void bfq_end_wr_async(struct bfq_data *bfqd)
}
bfq_end_wr_async_queues(bfqd, bfqd->root_group);
- spin_unlock(&bfqd->lock);
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&bfqd->lock);
mutex_unlock(&q->blkcg_mutex);
}
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 618e5566fa52..03e1d4d34921 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -131,9 +131,7 @@ static void blkg_free_workfn(struct work_struct *work)
blkcg_policy[i]->pd_free_fn(blkg->pd[i]);
if (blkg->parent)
blkg_put(blkg->parent);
- spin_lock_irq(&q->queue_lock);
list_del_init(&blkg->q_node);
- spin_unlock_irq(&q->queue_lock);
mutex_unlock(&q->blkcg_mutex);
/*
@@ -382,7 +380,7 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
struct blkcg_gq *blkg;
int i, ret;
- lockdep_assert_held(&disk->queue->queue_lock);
+ lockdep_assert_held(&disk->queue->blkcg_mutex);
/* request_queue is dying, do not create/recreate a blkg */
if (blk_queue_dying(disk->queue)) {
@@ -402,12 +400,15 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
/* link parent */
if (blkcg_parent(blkcg)) {
+ rcu_read_lock();
blkg->parent = blkg_lookup(blkcg_parent(blkcg), disk->queue);
if (WARN_ON_ONCE(!blkg->parent)) {
+ rcu_read_unlock();
ret = -ENODEV;
goto err_free_blkg;
}
blkg_get(blkg->parent);
+ rcu_read_unlock();
}
/* invoke per-policy init */
@@ -419,7 +420,7 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
}
/* insert */
- spin_lock(&blkcg->lock);
+ spin_lock_irq(&blkcg->lock);
ret = radix_tree_insert(&blkcg->blkg_tree, disk->queue->id, blkg);
if (likely(!ret)) {
hlist_add_head_rcu(&blkg->blkcg_node, &blkcg->blkg_list);
@@ -436,7 +437,7 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
}
blkg->online = true;
}
- spin_unlock(&blkcg->lock);
+ spin_unlock_irq(&blkcg->lock);
if (!ret)
return blkg;
@@ -459,7 +460,7 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
* Lookup blkg for the @blkcg - @disk pair. If it doesn't exist, try to
* create one. blkg creation is performed recursively from blkcg_root such
* that all non-root blkg's have access to the parent blkg. This function
- * should be called under RCU read lock and takes @disk->queue->queue_lock.
+ * must be called with @disk->queue->blkcg_mutex held.
*
* Returns the blkg or the closest blkg if blkg_create() fails as it walks
* down from root.
@@ -491,6 +492,7 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
struct blkcg *parent = blkcg_parent(blkcg);
struct blkcg_gq *ret_blkg = q->root_blkg;
+ rcu_read_lock();
while (parent) {
blkg = blkg_lookup(parent, q);
if (blkg) {
@@ -501,6 +503,7 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
pos = parent;
parent = blkcg_parent(parent);
}
+ rcu_read_unlock();
blkg = blkg_create(pos, disk, NULL);
if (IS_ERR(blkg)) {
@@ -519,7 +522,7 @@ static void blkg_destroy(struct blkcg_gq *blkg)
struct blkcg *blkcg = blkg->blkcg;
int i;
- lockdep_assert_held(&blkg->q->queue_lock);
+ lockdep_assert_held(&blkg->q->blkcg_mutex);
lockdep_assert_held(&blkcg->lock);
/*
@@ -547,8 +550,8 @@ static void blkg_destroy(struct blkcg_gq *blkg)
hlist_del_init_rcu(&blkg->blkcg_node);
/*
- * Both setting lookup hint to and clearing it from @blkg are done
- * under queue_lock. If it's not pointing to @blkg now, it never
+ * Both setting lookup hint to and clearing it from @blkg are done under
+ * blkcg_mutex. If it's not pointing to @blkg now, it never
* will. Hint assignment itself can race safely.
*/
if (rcu_access_pointer(blkcg->blkg_hint) == blkg)
@@ -569,24 +572,21 @@ static void blkg_destroy_all(struct gendisk *disk)
int i;
restart:
- spin_lock_irq(&q->queue_lock);
+ mutex_lock(&q->blkcg_mutex);
list_for_each_entry(blkg, &q->blkg_list, q_node) {
struct blkcg *blkcg = blkg->blkcg;
if (hlist_unhashed(&blkg->blkcg_node))
continue;
- spin_lock(&blkcg->lock);
+ spin_lock_irq(&blkcg->lock);
blkg_destroy(blkg);
- spin_unlock(&blkcg->lock);
+ spin_unlock_irq(&blkcg->lock);
- /*
- * in order to avoid holding the spin lock for too long, release
- * it when a batch of blkgs are destroyed.
- */
+ /* Avoid holding blkcg_mutex for too long. */
if (!(--count)) {
count = BLKG_DESTROY_BATCH_SIZE;
- spin_unlock_irq(&q->queue_lock);
+ mutex_unlock(&q->blkcg_mutex);
cond_resched();
goto restart;
}
@@ -605,7 +605,7 @@ static void blkg_destroy_all(struct gendisk *disk)
}
q->root_blkg = NULL;
- spin_unlock_irq(&q->queue_lock);
+ mutex_unlock(&q->blkcg_mutex);
wake_up_var(&q->root_blkg);
}
@@ -822,8 +822,8 @@ EXPORT_SYMBOL_GPL(blkg_conf_open_bdev);
* @ctx->blkg to the blkg being configured.
*
* blkg_conf_open_bdev() must be called on @ctx beforehand. On success, this
- * function returns with queue lock held and must be followed by
- * blkg_conf_close_bdev().
+ * function returns with blkcg_mutex held and must be followed by
+ * blkg_conf_unprep().
*/
int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
struct blkg_conf_ctx *ctx)
@@ -841,7 +841,6 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
/* Prevent concurrent with blkcg_deactivate_policy() */
mutex_lock(&q->blkcg_mutex);
- spin_lock_irq(&q->queue_lock);
if (!blkcg_policy_enabled(q, pol)) {
ret = -EOPNOTSUPP;
@@ -862,35 +861,34 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
struct blkcg_gq *new_blkg;
parent = blkcg_parent(blkcg);
+ rcu_read_lock();
while (parent && !blkg_lookup(parent, q)) {
pos = parent;
parent = blkcg_parent(parent);
}
-
- /* Drop locks to do new blkg allocation with GFP_KERNEL. */
- spin_unlock_irq(&q->queue_lock);
+ rcu_read_unlock();
new_blkg = blkg_alloc(pos, disk, GFP_NOIO);
if (unlikely(!new_blkg)) {
ret = -ENOMEM;
- goto fail_exit;
+ goto fail_unlock;
}
if (radix_tree_preload(GFP_KERNEL)) {
blkg_free(new_blkg);
ret = -ENOMEM;
- goto fail_exit;
+ goto fail_unlock;
}
- spin_lock_irq(&q->queue_lock);
-
if (!blkcg_policy_enabled(q, pol)) {
blkg_free(new_blkg);
ret = -EOPNOTSUPP;
goto fail_preloaded;
}
+ rcu_read_lock();
blkg = blkg_lookup(pos, q);
+ rcu_read_unlock();
if (blkg) {
blkg_free(new_blkg);
} else {
@@ -907,15 +905,12 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
goto success;
}
success:
- mutex_unlock(&q->blkcg_mutex);
ctx->blkg = blkg;
return 0;
fail_preloaded:
radix_tree_preload_end();
fail_unlock:
- spin_unlock_irq(&q->queue_lock);
-fail_exit:
mutex_unlock(&q->blkcg_mutex);
/*
* If queue was bypassing, we should retry. Do so after a
@@ -938,7 +933,7 @@ EXPORT_SYMBOL_GPL(blkg_conf_prep);
void blkg_conf_unprep(struct blkg_conf_ctx *ctx)
{
WARN_ON_ONCE(!ctx->blkg);
- spin_unlock_irq(&ctx->bdev->bd_disk->queue->queue_lock);
+ mutex_unlock(&ctx->bdev->bd_disk->queue->blkcg_mutex);
ctx->blkg = NULL;
}
EXPORT_SYMBOL_GPL(blkg_conf_unprep);
@@ -1258,8 +1253,9 @@ static struct blkcg_gq *blkcg_get_first_blkg(struct blkcg *blkcg)
* blkcg_destroy_blkgs - responsible for shooting down blkgs
* @blkcg: blkcg of interest
*
- * blkgs should be removed while holding both q and blkcg locks. As blkcg lock
- * is nested inside q lock, this function performs reverse double lock dancing.
+ * blkgs should be removed while holding both q->blkcg_mutex and blkcg->lock.
+ * As blkcg->lock is nested inside q->blkcg_mutex, this function performs
+ * reverse double lock dancing.
* Destroying the blkgs releases the reference held on the blkcg's css allowing
* blkcg_css_free to eventually be called.
*
@@ -1274,13 +1270,13 @@ static void blkcg_destroy_blkgs(struct blkcg *blkcg)
while ((blkg = blkcg_get_first_blkg(blkcg))) {
struct request_queue *q = blkg->q;
- spin_lock_irq(&q->queue_lock);
- spin_lock(&blkcg->lock);
+ mutex_lock(&q->blkcg_mutex);
+ spin_lock_irq(&blkcg->lock);
blkg_destroy(blkg);
- spin_unlock(&blkcg->lock);
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&blkcg->lock);
+ mutex_unlock(&q->blkcg_mutex);
blkg_put(blkg);
cond_resched();
@@ -1472,21 +1468,20 @@ int blkcg_init_disk(struct gendisk *disk)
preloaded = !radix_tree_preload(GFP_KERNEL);
/* Make sure the root blkg exists. */
- /* spin_lock_irq can serve as RCU read-side critical section. */
- spin_lock_irq(&q->queue_lock);
+ mutex_lock(&q->blkcg_mutex);
blkg = blkg_create(&blkcg_root, disk, new_blkg);
if (IS_ERR(blkg))
goto err_unlock;
q->root_blkg = blkg;
- spin_unlock_irq(&q->queue_lock);
if (preloaded)
radix_tree_preload_end();
+ mutex_unlock(&q->blkcg_mutex);
return 0;
err_unlock:
- spin_unlock_irq(&q->queue_lock);
+ mutex_unlock(&q->blkcg_mutex);
if (preloaded)
radix_tree_preload_end();
return PTR_ERR(blkg);
@@ -1526,6 +1521,42 @@ struct cgroup_subsys io_cgrp_subsys = {
};
EXPORT_SYMBOL_GPL(io_cgrp_subsys);
+static void blkg_free_policy_data(struct blkcg_gq *blkg,
+ const struct blkcg_policy *pol)
+{
+ struct blkcg *blkcg = blkg->blkcg;
+ struct blkg_policy_data *pd;
+ bool online = false;
+
+ lockdep_assert_held(&blkg->q->blkcg_mutex);
+
+ /*
+ * ->pd_offline_fn() may need blkg->pd[] to stay installed, while
+ * ->pd_free_fn() can sleep. Mark offline under blkcg->lock, run
+ * the offline callback, detach under blkcg->lock, then free.
+ */
+ spin_lock_irq(&blkcg->lock);
+ pd = blkg->pd[pol->plid];
+ if (pd) {
+ online = pd->online;
+ pd->online = false;
+ }
+ spin_unlock_irq(&blkcg->lock);
+
+ if (!pd)
+ return;
+
+ if (online && pol->pd_offline_fn)
+ pol->pd_offline_fn(pd);
+
+ spin_lock_irq(&blkcg->lock);
+ WARN_ON_ONCE(blkg->pd[pol->plid] != pd);
+ WRITE_ONCE(blkg->pd[pol->plid], NULL);
+ spin_unlock_irq(&blkcg->lock);
+
+ pol->pd_free_fn(pd);
+}
+
/**
* blkcg_activate_policy - activate a blkcg policy on a gendisk
* @disk: gendisk of interest
@@ -1535,9 +1566,9 @@ EXPORT_SYMBOL_GPL(io_cgrp_subsys);
* bypass mode to populate its blkgs with policy_data for @pol.
*
* Activation happens with @disk bypassed, so nobody would be accessing blkgs
- * from IO path. Update of each blkg is protected by both queue and blkcg
- * locks so that holding either lock and testing blkcg_policy_enabled() is
- * always enough for dereferencing policy data.
+ * from IO path. Update of each blkg is protected by q->blkcg_mutex and
+ * blkcg->lock so that holding either lock and testing blkcg_policy_enabled()
+ * is always enough for dereferencing policy data.
*
* The caller is responsible for synchronizing [de]activations and policy
* [un]registerations. Returns 0 on success, -errno on failure.
@@ -1563,8 +1594,9 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
if (queue_is_mq(q))
memflags = blk_mq_freeze_queue(q);
+
retry:
- spin_lock_irq(&q->queue_lock);
+ mutex_lock(&q->blkcg_mutex);
/* blkg_list is pushed at the head, reverse walk to initialize parents first */
list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) {
@@ -1572,14 +1604,15 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
if (blkg->pd[pol->plid])
continue;
+ if (hlist_unhashed(&blkg->blkcg_node))
+ continue;
- /* If prealloc matches, use it; otherwise try GFP_NOWAIT */
+ /* If prealloc matches, use it; otherwise try GFP_NOWAIT. */
if (blkg == pinned_blkg) {
pd = pd_prealloc;
pd_prealloc = NULL;
} else {
- pd = pol->pd_alloc_fn(disk, blkg->blkcg,
- GFP_NOWAIT);
+ pd = pol->pd_alloc_fn(disk, blkg->blkcg, GFP_NOWAIT);
}
if (!pd) {
@@ -1592,7 +1625,7 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
blkg_get(blkg);
pinned_blkg = blkg;
- spin_unlock_irq(&q->queue_lock);
+ mutex_unlock(&q->blkcg_mutex);
if (pd_prealloc)
pol->pd_free_fn(pd_prealloc);
@@ -1600,11 +1633,10 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
GFP_KERNEL);
if (pd_prealloc)
goto retry;
- else
- goto enomem;
+ goto enomem;
}
- spin_lock(&blkg->blkcg->lock);
+ spin_lock_irq(&blkg->blkcg->lock);
pd->blkg = blkg;
pd->plid = pol->plid;
@@ -1617,14 +1649,14 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
pol->pd_online_fn(pd);
pd->online = true;
- spin_unlock(&blkg->blkcg->lock);
+ spin_unlock_irq(&blkg->blkcg->lock);
}
__set_bit(pol->plid, q->blkcg_pols);
ret = 0;
- spin_unlock_irq(&q->queue_lock);
out:
+ mutex_unlock(&q->blkcg_mutex);
if (queue_is_mq(q))
blk_mq_unfreeze_queue(q, memflags);
if (pinned_blkg)
@@ -1635,23 +1667,9 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
enomem:
/* alloc failed, take down everything */
- spin_lock_irq(&q->queue_lock);
- list_for_each_entry(blkg, &q->blkg_list, q_node) {
- struct blkcg *blkcg = blkg->blkcg;
- struct blkg_policy_data *pd;
-
- spin_lock(&blkcg->lock);
- pd = blkg->pd[pol->plid];
- if (pd) {
- if (pd->online && pol->pd_offline_fn)
- pol->pd_offline_fn(pd);
- pd->online = false;
- pol->pd_free_fn(pd);
- WRITE_ONCE(blkg->pd[pol->plid], NULL);
- }
- spin_unlock(&blkcg->lock);
- }
- spin_unlock_irq(&q->queue_lock);
+ mutex_lock(&q->blkcg_mutex);
+ list_for_each_entry(blkg, &q->blkg_list, q_node)
+ blkg_free_policy_data(blkg, pol);
ret = -ENOMEM;
goto out;
}
@@ -1679,24 +1697,12 @@ void blkcg_deactivate_policy(struct gendisk *disk,
memflags = blk_mq_freeze_queue(q);
mutex_lock(&q->blkcg_mutex);
- spin_lock_irq(&q->queue_lock);
__clear_bit(pol->plid, q->blkcg_pols);
- list_for_each_entry(blkg, &q->blkg_list, q_node) {
- struct blkcg *blkcg = blkg->blkcg;
+ list_for_each_entry(blkg, &q->blkg_list, q_node)
+ blkg_free_policy_data(blkg, pol);
- spin_lock(&blkcg->lock);
- if (blkg->pd[pol->plid]) {
- if (blkg->pd[pol->plid]->online && pol->pd_offline_fn)
- pol->pd_offline_fn(blkg->pd[pol->plid]);
- pol->pd_free_fn(blkg->pd[pol->plid]);
- blkg->pd[pol->plid] = NULL;
- }
- spin_unlock(&blkcg->lock);
- }
-
- spin_unlock_irq(&q->queue_lock);
mutex_unlock(&q->blkcg_mutex);
if (queue_is_mq(q))
@@ -2087,11 +2093,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);
+ mutex_lock(&q->blkcg_mutex);
blkg = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk);
if (blkg)
blkg = blkg_lookup_tryget(blkg);
- spin_unlock_irq(&q->queue_lock);
+ mutex_unlock(&q->blkcg_mutex);
return blkg;
}
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index 615390f751aa..5aaf2d54d17e 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -66,7 +66,7 @@ struct blkcg_gq {
/* reference count */
struct percpu_ref refcnt;
- /* is this blkg online? protected by both blkcg and q locks */
+ /* is this blkg online? protected by blkcg->lock and q->blkcg_mutex */
bool online;
struct blkg_iostat_set __percpu *iostat_cpu;
@@ -224,9 +224,9 @@ int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx)
__cond_acquires(0, &ctx->bdev->bd_queue->rq_qos_mutex);
int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
struct blkg_conf_ctx *ctx)
- __cond_acquires(0, &ctx->bdev->bd_disk->queue->queue_lock);
+ __cond_acquires(0, &ctx->bdev->bd_disk->queue->blkcg_mutex);
void blkg_conf_unprep(struct blkg_conf_ctx *ctx)
- __releases(ctx->bdev->bd_disk->queue->queue_lock);
+ __releases(ctx->bdev->bd_disk->queue->blkcg_mutex);
void blkg_conf_close_bdev(struct blkg_conf_ctx *ctx)
__releases(&ctx->bdev->bd_queue->rq_qos_mutex);
@@ -255,7 +255,7 @@ static inline bool bio_issue_as_root_blkg(struct bio *bio)
*
* Lookup blkg for the @blkcg - @q pair.
*
- * Must be called in a RCU critical section.
+ * Must be called in a RCU critical section or with q->blkcg_mutex held.
*/
static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg,
struct request_queue *q)
@@ -266,7 +266,7 @@ static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg,
return q->root_blkg;
blkg = rcu_dereference_check(blkcg->blkg_hint,
- lockdep_is_held(&q->queue_lock));
+ lockdep_is_held(&q->blkcg_mutex));
if (blkg && blkg->q == q)
return blkg;
@@ -350,9 +350,9 @@ static inline void blkg_put(struct blkcg_gq *blkg)
* @p_blkg: target blkg to walk descendants of
*
* Walk @c_blkg through the descendants of @p_blkg. Must be used with RCU
- * read locked. If called under either blkcg or queue lock, the iteration
- * is guaranteed to include all and only online blkgs. The caller may
- * update @pos_css by calling css_rightmost_descendant() to skip subtree.
+ * read locked. If called under either blkcg->lock or q->blkcg_mutex, the
+ * iteration is guaranteed to include all and only online blkgs. The caller
+ * may update @pos_css by calling css_rightmost_descendant() to skip subtree.
* @p_blkg is included in the iteration and the first node to be visited.
*/
#define blkg_for_each_descendant_pre(d_blkg, pos_css, p_blkg) \
diff --git a/block/blk-core.c b/block/blk-core.c
index 8103643b39fc..0a1d2b09a5d9 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -965,6 +965,8 @@ static void bio_set_ioprio(struct bio *bio)
*/
void submit_bio(struct bio *bio)
{
+ might_sleep();
+
if (bio_op(bio) == REQ_OP_READ) {
task_io_account_read(bio->bi_iter.bi_size);
count_vm_events(PGPGIN, bio_sectors(bio));
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 8b2aeba2e1e3..ae50d143e4fc 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3143,6 +3143,7 @@ static ssize_t ioc_weight_write(struct kernfs_open_file *of, char *buf,
struct blkg_conf_ctx ctx;
struct ioc_now now;
struct ioc_gq *iocg;
+ unsigned long flags;
u32 v;
int ret;
@@ -3195,11 +3196,11 @@ static ssize_t ioc_weight_write(struct kernfs_open_file *of, char *buf,
goto unprep;
}
- spin_lock(&iocg->ioc->lock);
+ spin_lock_irqsave(&iocg->ioc->lock, flags);
iocg->cfg_weight = v * WEIGHT_ONE;
ioc_now(iocg->ioc, &now);
weight_updated(iocg, &now);
- spin_unlock(&iocg->ioc->lock);
+ spin_unlock_irqrestore(&iocg->ioc->lock, flags);
ret = 0;
diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index cef02b6c5fa9..30e23fee4f15 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -639,6 +639,7 @@ static void blkcg_iolatency_exit(struct rq_qos *rqos)
timer_shutdown_sync(&blkiolat->timer);
flush_work(&blkiolat->enable_work);
blkcg_deactivate_policy(rqos->disk, &blkcg_policy_iolatency);
+ flush_work(&blkiolat->enable_work);
kfree(blkiolat);
}
@@ -811,16 +812,18 @@ static void iolatency_clear_scaling(struct blkcg_gq *blkg)
if (blkg->parent) {
struct iolatency_grp *iolat = blkg_to_lat(blkg->parent);
struct child_latency_info *lat_info;
+ unsigned long flags;
+
if (!iolat)
return;
lat_info = &iolat->child_lat;
- spin_lock(&lat_info->lock);
+ spin_lock_irqsave(&lat_info->lock, flags);
atomic_set(&lat_info->scale_cookie, DEFAULT_SCALE_COOKIE);
lat_info->last_scale_event = 0;
lat_info->scale_grp = NULL;
lat_info->scale_lat = 0;
- spin_unlock(&lat_info->lock);
+ spin_unlock_irqrestore(&lat_info->lock, flags);
}
}
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 7bca2805404f..ef3edd5a4785 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1777,10 +1777,10 @@ void blk_throtl_cancel_bios(struct gendisk *disk)
if (!blk_throtl_activated(q))
return;
- spin_lock_irq(&q->queue_lock);
- spin_lock(&td->lock);
+ mutex_lock(&q->blkcg_mutex);
+ spin_lock_irq(&td->lock);
/*
- * queue_lock is held, rcu lock is not needed here technically.
+ * blkcg_mutex is held, rcu lock is not needed here technically.
* However, rcu lock is still held to emphasize that following
* path need RCU protection and to prevent warning from lockdep.
*/
@@ -1797,8 +1797,8 @@ void blk_throtl_cancel_bios(struct gendisk *disk)
tg_cancel_writeback_bios(blkg_to_tg(blkg), cancel_bios);
}
rcu_read_unlock();
- spin_unlock(&td->lock);
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&td->lock);
+ mutex_unlock(&q->blkcg_mutex);
for (rw = READ; rw <= WRITE; rw++) {
struct bio *bio;
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 4/8] blk-cgroup: remove blkg radix tree preloading
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
` (2 preceding siblings ...)
2026-07-24 12:30 ` [RFC PATCH v2 3/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 5/8] blk-cgroup: allocate blkgs in blkg_create Yu Kuai
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
blkg creation is now serialized by q->blkcg_mutex and no longer runs
under q->queue_lock, and the next patch moves the blkg allocation itself
into blkg_create(). With the allocation no longer happening under
blkcg->lock, the radix_tree_preload() dance used to keep
radix_tree_insert() from sleeping while blkcg->lock is held is no longer
needed.
Remove the preload calls and the associated unwind path. This might
affect the nowait case when the first thread for blkcg and queue pair
to issue the first IO. However this is a super cold path, it's
impossible for a real world latency sensitive load to always create a
new blkcg, issue an io with nowait and then remove the blkcg.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 03e1d4d34921..8fdc42ab153b 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -419,7 +419,6 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
pol->pd_init_fn(blkg->pd[i]);
}
- /* insert */
spin_lock_irq(&blkcg->lock);
ret = radix_tree_insert(&blkcg->blkg_tree, disk->queue->id, blkg);
if (likely(!ret)) {
@@ -874,16 +873,10 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
goto fail_unlock;
}
- if (radix_tree_preload(GFP_KERNEL)) {
- blkg_free(new_blkg);
- ret = -ENOMEM;
- goto fail_unlock;
- }
-
if (!blkcg_policy_enabled(q, pol)) {
blkg_free(new_blkg);
ret = -EOPNOTSUPP;
- goto fail_preloaded;
+ goto fail_unlock;
}
rcu_read_lock();
@@ -895,12 +888,10 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
blkg = blkg_create(pos, disk, new_blkg);
if (IS_ERR(blkg)) {
ret = PTR_ERR(blkg);
- goto fail_preloaded;
+ goto fail_unlock;
}
}
- radix_tree_preload_end();
-
if (pos == blkcg)
goto success;
}
@@ -908,8 +899,6 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
ctx->blkg = blkg;
return 0;
-fail_preloaded:
- radix_tree_preload_end();
fail_unlock:
mutex_unlock(&q->blkcg_mutex);
/*
@@ -1447,7 +1436,6 @@ int blkcg_init_disk(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
struct blkcg_gq *new_blkg, *blkg;
- bool preloaded;
/*
* If the queue is shared across disk rebind (e.g., SCSI), the
@@ -1465,8 +1453,6 @@ int blkcg_init_disk(struct gendisk *disk)
if (!new_blkg)
return -ENOMEM;
- preloaded = !radix_tree_preload(GFP_KERNEL);
-
/* Make sure the root blkg exists. */
mutex_lock(&q->blkcg_mutex);
blkg = blkg_create(&blkcg_root, disk, new_blkg);
@@ -1474,16 +1460,12 @@ int blkcg_init_disk(struct gendisk *disk)
goto err_unlock;
q->root_blkg = blkg;
- if (preloaded)
- radix_tree_preload_end();
mutex_unlock(&q->blkcg_mutex);
return 0;
err_unlock:
mutex_unlock(&q->blkcg_mutex);
- if (preloaded)
- radix_tree_preload_end();
return PTR_ERR(blkg);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 5/8] blk-cgroup: allocate blkgs in blkg_create
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
` (3 preceding siblings ...)
2026-07-24 12:30 ` [RFC PATCH v2 4/8] blk-cgroup: remove blkg radix tree preloading Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 6/8] blk-cgroup: share blkg creation between lookup and config prep Yu Kuai
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
Move blkg allocation into blkg_create() and have it take a gfp_t mask, so
that the caller controls whether creation may sleep. blkg_create() now
always allocates the blkg itself instead of sometimes receiving a
preallocated one, which lets the lookup and config paths drop their open-
coded preallocation and retry loops.
blkg_lookup_create() and the root-blkg setup use GFP_NOIO (or GFP_KERNEL
for the root) so they do not recurse into IO reclaim; the nowait submit
path, added later, will use GFP_ATOMIC.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 86 +++++++++-------------------------------------
1 file changed, 16 insertions(+), 70 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 8fdc42ab153b..345c02a4ac32 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -370,14 +370,10 @@ static struct blkcg_gq *blkg_alloc(struct blkcg *blkcg, struct gendisk *disk,
return NULL;
}
-/*
- * If @new_blkg is %NULL, this function tries to allocate a new one as
- * necessary using %GFP_NOWAIT. @new_blkg is always consumed on return.
- */
static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
- struct blkcg_gq *new_blkg)
+ gfp_t gfp_mask)
{
- struct blkcg_gq *blkg;
+ struct blkcg_gq *blkg = NULL;
int i, ret;
lockdep_assert_held(&disk->queue->blkcg_mutex);
@@ -388,15 +384,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
goto err_free_blkg;
}
- /* allocate */
- if (!new_blkg) {
- new_blkg = blkg_alloc(blkcg, disk, GFP_NOWAIT);
- if (unlikely(!new_blkg)) {
- ret = -ENOMEM;
- goto err_free_blkg;
- }
+ blkg = blkg_alloc(blkcg, disk, gfp_mask);
+ if (unlikely(!blkg)) {
+ ret = -ENOMEM;
+ goto err_free_blkg;
}
- blkg = new_blkg;
/* link parent */
if (blkcg_parent(blkcg)) {
@@ -446,8 +438,8 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
return ERR_PTR(ret);
err_free_blkg:
- if (new_blkg)
- blkg_free(new_blkg);
+ if (blkg)
+ blkg_free(blkg);
return ERR_PTR(ret);
}
@@ -504,7 +496,7 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
}
rcu_read_unlock();
- blkg = blkg_create(pos, disk, NULL);
+ blkg = blkg_create(pos, disk, GFP_NOIO);
if (IS_ERR(blkg)) {
blkg = ret_blkg;
break;
@@ -857,7 +849,6 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
while (true) {
struct blkcg *pos = blkcg;
struct blkcg *parent;
- struct blkcg_gq *new_blkg;
parent = blkcg_parent(blkcg);
rcu_read_lock();
@@ -867,14 +858,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
}
rcu_read_unlock();
- new_blkg = blkg_alloc(pos, disk, GFP_NOIO);
- if (unlikely(!new_blkg)) {
- ret = -ENOMEM;
- goto fail_unlock;
- }
-
if (!blkcg_policy_enabled(q, pol)) {
- blkg_free(new_blkg);
ret = -EOPNOTSUPP;
goto fail_unlock;
}
@@ -882,10 +866,8 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
rcu_read_lock();
blkg = blkg_lookup(pos, q);
rcu_read_unlock();
- if (blkg) {
- blkg_free(new_blkg);
- } else {
- blkg = blkg_create(pos, disk, new_blkg);
+ if (!blkg) {
+ blkg = blkg_create(pos, disk, GFP_NOIO);
if (IS_ERR(blkg)) {
ret = PTR_ERR(blkg);
goto fail_unlock;
@@ -1435,7 +1417,7 @@ void blkg_init_queue(struct request_queue *q)
int blkcg_init_disk(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
- struct blkcg_gq *new_blkg, *blkg;
+ struct blkcg_gq *blkg;
/*
* If the queue is shared across disk rebind (e.g., SCSI), the
@@ -1449,13 +1431,9 @@ int blkcg_init_disk(struct gendisk *disk)
*/
wait_var_event(&q->root_blkg, !READ_ONCE(q->root_blkg));
- new_blkg = blkg_alloc(&blkcg_root, disk, GFP_KERNEL);
- if (!new_blkg)
- return -ENOMEM;
-
/* Make sure the root blkg exists. */
mutex_lock(&q->blkcg_mutex);
- blkg = blkg_create(&blkcg_root, disk, new_blkg);
+ blkg = blkg_create(&blkcg_root, disk, GFP_KERNEL);
if (IS_ERR(blkg))
goto err_unlock;
q->root_blkg = blkg;
@@ -1558,8 +1536,7 @@ static void blkg_free_policy_data(struct blkcg_gq *blkg,
int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
{
struct request_queue *q = disk->queue;
- struct blkg_policy_data *pd_prealloc = NULL;
- struct blkcg_gq *blkg, *pinned_blkg = NULL;
+ struct blkcg_gq *blkg;
unsigned int memflags;
int ret;
@@ -1577,7 +1554,6 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
if (queue_is_mq(q))
memflags = blk_mq_freeze_queue(q);
-retry:
mutex_lock(&q->blkcg_mutex);
/* blkg_list is pushed at the head, reverse walk to initialize parents first */
@@ -1589,34 +1565,9 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
if (hlist_unhashed(&blkg->blkcg_node))
continue;
- /* If prealloc matches, use it; otherwise try GFP_NOWAIT. */
- if (blkg == pinned_blkg) {
- pd = pd_prealloc;
- pd_prealloc = NULL;
- } else {
- pd = pol->pd_alloc_fn(disk, blkg->blkcg, GFP_NOWAIT);
- }
-
- if (!pd) {
- /*
- * GFP_NOWAIT failed. Free the existing one and
- * prealloc for @blkg w/ GFP_KERNEL.
- */
- if (pinned_blkg)
- blkg_put(pinned_blkg);
- blkg_get(blkg);
- pinned_blkg = blkg;
-
- mutex_unlock(&q->blkcg_mutex);
-
- if (pd_prealloc)
- pol->pd_free_fn(pd_prealloc);
- pd_prealloc = pol->pd_alloc_fn(disk, blkg->blkcg,
- GFP_KERNEL);
- if (pd_prealloc)
- goto retry;
+ pd = pol->pd_alloc_fn(disk, blkg->blkcg, GFP_NOIO);
+ if (!pd)
goto enomem;
- }
spin_lock_irq(&blkg->blkcg->lock);
@@ -1641,15 +1592,10 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
mutex_unlock(&q->blkcg_mutex);
if (queue_is_mq(q))
blk_mq_unfreeze_queue(q, memflags);
- if (pinned_blkg)
- blkg_put(pinned_blkg);
- if (pd_prealloc)
- pol->pd_free_fn(pd_prealloc);
return ret;
enomem:
/* alloc failed, take down everything */
- mutex_lock(&q->blkcg_mutex);
list_for_each_entry(blkg, &q->blkg_list, q_node)
blkg_free_policy_data(blkg, pol);
ret = -ENOMEM;
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 6/8] blk-cgroup: share blkg creation between lookup and config prep
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
` (4 preceding siblings ...)
2026-07-24 12:30 ` [RFC PATCH v2 5/8] blk-cgroup: allocate blkgs in blkg_create Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 7/8] bfq: avoid blkg lookup from locked cgroup update Yu Kuai
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
blkg_conf_prep() open-codes the same parent walk and blkg creation that
blkg_lookup_create() now performs. Give blkg_lookup_create() an out
parameter for the created/found blkg and have it report whether the target
blkg was created or found (returning the closest existing blkg in the out
parameter on failure), then have blkg_conf_prep() use the helper and treat
errors as config failures.
This keeps the bio association path's closest-blkg fallback and removes the
duplicate config path loop.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 71 +++++++++++++---------------------------------
1 file changed, 19 insertions(+), 52 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 345c02a4ac32..d611cf701f14 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -447,17 +447,19 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
* blkg_lookup_create - lookup blkg, try to create one if not there
* @blkcg: blkcg of interest
* @disk: gendisk of interest
+ * @gfp_mask: allocation mask to use
+ * @blkgp: out parameter for the target blkg, or closest blkg on failure
*
* Lookup blkg for the @blkcg - @disk pair. If it doesn't exist, try to
* create one. blkg creation is performed recursively from blkcg_root such
* that all non-root blkg's have access to the parent blkg. This function
* must be called with @disk->queue->blkcg_mutex held.
*
- * Returns the blkg or the closest blkg if blkg_create() fails as it walks
- * down from root.
+ * On success, *@blkgp points to the target blkg and 0 is returned. On
+ * failure, *@blkgp points to the closest blkg and the errno is returned.
*/
-static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
- struct gendisk *disk)
+static int blkg_lookup_create(struct blkcg *blkcg, struct gendisk *disk,
+ gfp_t gfp_mask, struct blkcg_gq **blkgp)
{
struct request_queue *q = disk->queue;
struct blkcg_gq *blkg;
@@ -469,7 +471,8 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
blkg != rcu_dereference(blkcg->blkg_hint))
rcu_assign_pointer(blkcg->blkg_hint, blkg);
rcu_read_unlock();
- return blkg;
+ *blkgp = blkg;
+ return 0;
}
rcu_read_unlock();
@@ -496,16 +499,16 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
}
rcu_read_unlock();
- blkg = blkg_create(pos, disk, GFP_NOIO);
+ blkg = blkg_create(pos, disk, gfp_mask);
if (IS_ERR(blkg)) {
- blkg = ret_blkg;
- break;
+ *blkgp = ret_blkg;
+ return PTR_ERR(blkg);
+ }
+ if (pos == blkcg) {
+ *blkgp = blkg;
+ return 0;
}
- if (pos == blkcg)
- break;
}
-
- return blkg;
}
static void blkg_destroy(struct blkcg_gq *blkg)
@@ -838,46 +841,10 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
goto fail_unlock;
}
- blkg = blkg_lookup(blkcg, q);
- if (blkg)
- goto success;
-
- /*
- * Create blkgs walking down from blkcg_root to @blkcg, so that all
- * non-root blkgs have access to their parents.
- */
- while (true) {
- struct blkcg *pos = blkcg;
- struct blkcg *parent;
-
- parent = blkcg_parent(blkcg);
- rcu_read_lock();
- while (parent && !blkg_lookup(parent, q)) {
- pos = parent;
- parent = blkcg_parent(parent);
- }
- rcu_read_unlock();
-
- if (!blkcg_policy_enabled(q, pol)) {
- ret = -EOPNOTSUPP;
- goto fail_unlock;
- }
-
- rcu_read_lock();
- blkg = blkg_lookup(pos, q);
- rcu_read_unlock();
- if (!blkg) {
- blkg = blkg_create(pos, disk, GFP_NOIO);
- if (IS_ERR(blkg)) {
- ret = PTR_ERR(blkg);
- goto fail_unlock;
- }
- }
+ ret = blkg_lookup_create(blkcg, disk, GFP_NOIO, &blkg);
+ if (ret)
+ goto fail_unlock;
- if (pos == blkcg)
- goto success;
- }
-success:
ctx->blkg = blkg;
return 0;
@@ -2022,7 +1989,7 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,
* time, hold lock to create new blkg.
*/
mutex_lock(&q->blkcg_mutex);
- blkg = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk);
+ blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk, GFP_NOIO, &blkg);
if (blkg)
blkg = blkg_lookup_tryget(blkg);
mutex_unlock(&q->blkcg_mutex);
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 7/8] bfq: avoid blkg lookup from locked cgroup update
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
` (5 preceding siblings ...)
2026-07-24 12:30 ` [RFC PATCH v2 6/8] blk-cgroup: share blkg creation between lookup and config prep Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-24 12:30 ` [RFC PATCH v2 8/8] block: fail nowait bio submit if blkg allocation can't sleep Yu Kuai
2026-07-26 7:08 ` [syzbot ci] Re: blk-cgroup: protect blkgs with blkcg_mutex syzbot ci
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
bfq_bio_bfqg() is called while bfqd->lock is held from the merge and
request insertion paths. It walks bio->bi_blkg and its parent chain to
find the closest online BFQ group, and re-associates the bio by looking
the chosen blkg up again through bio_associate_blkg_from_css().
Now that blkg creation runs under q->blkcg_mutex from the sleepable submit
path, bio_associate_blkg_from_css() can sleep on a lookup miss, which BFQ
must not do while holding bfqd->lock. The blkg BFQ wants is already in
hand from the ancestry walk, so update bio->bi_blkg by swapping references
to that existing blkg directly instead of looking it up again by css.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-cgroup.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 42614aa78cd4..8a3ff9510386 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -604,6 +604,16 @@ static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg)
}
}
+static void bfq_bio_update_blkg(struct bio *bio, struct blkcg_gq *blkg)
+{
+ if (bio->bi_blkg == blkg)
+ return;
+
+ blkg_get(blkg);
+ blkg_put(bio->bi_blkg);
+ bio->bi_blkg = blkg;
+}
+
struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)
{
struct blkcg_gq *blkg = bio->bi_blkg;
@@ -616,13 +626,13 @@ struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)
}
bfqg = blkg_to_bfqg(blkg);
if (bfqg->pd.online) {
- bio_associate_blkg_from_css(bio, &blkg->blkcg->css);
+ bfq_bio_update_blkg(bio, blkg);
return bfqg;
}
blkg = blkg->parent;
}
- bio_associate_blkg_from_css(bio,
- &bfqg_to_blkg(bfqd->root_group)->blkcg->css);
+ blkg = bfqg_to_blkg(bfqd->root_group);
+ bfq_bio_update_blkg(bio, blkg);
return bfqd->root_group;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 8/8] block: fail nowait bio submit if blkg allocation can't sleep
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
` (6 preceding siblings ...)
2026-07-24 12:30 ` [RFC PATCH v2 7/8] bfq: avoid blkg lookup from locked cgroup update Yu Kuai
@ 2026-07-24 12:30 ` Yu Kuai
2026-07-26 7:08 ` [syzbot ci] Re: blk-cgroup: protect blkgs with blkcg_mutex syzbot ci
8 siblings, 0 replies; 10+ messages in thread
From: Yu Kuai @ 2026-07-24 12:30 UTC (permalink / raw)
To: axboe, tj
Cc: hch, dongsheng.yang, cengku, josef, nilay, ming.lei, yukuai,
linux-block, cgroups
From: Yu Kuai <yukuai@fygo.io>
blkg association now happens from the submit path, which is allowed to sleep
when creating a missing blkg. REQ_NOWAIT I/O must not be made to sleep,
though: if a nowait bio is the first I/O for its cgroup/queue pair and the
blkg does not exist yet, creating it would require sleeping on
q->blkcg_mutex and allocating with a sleeping GFP.
blkg_tryget_closest() derives the nowait mode from the bio's REQ_NOWAIT flag.
For nowait, create the missing blkg without sleeping: mutex_trylock()
q->blkcg_mutex and allocate with GFP_ATOMIC. If that fails (atomic context,
contended mutex, or allocation failure), leave the bio unassociated and
return failure, so submit_bio()/submit_bio_noacct() complete the bio with
BLK_STS_AGAIN and the submitter can retry once the blkg exists.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 85 ++++++++++++++++++++++++++++++++++-----------
block/blk-core.c | 19 ++++++++--
include/linux/bio.h | 11 +++---
3 files changed, 86 insertions(+), 29 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index d611cf701f14..3c8efba2e69a 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -30,6 +30,7 @@
#include <linux/resume_user_mode.h>
#include <linux/psi.h>
#include <linux/part_stat.h>
+#include <linux/preempt.h>
#include "blk.h"
#include "blk-cgroup.h"
#include "blk-ioprio.h"
@@ -1984,6 +1985,30 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,
if (blkg)
return blkg;
+ if (bio->bi_opf & REQ_NOWAIT) {
+ int ret;
+
+ /*
+ * Nowait callers must not sleep on the mutex nor allocate with
+ * sleeping GFPs. Trylock the mutex and create the missing blkg
+ * atomically; if the mutex is contended, the caller is atomic,
+ * or blkg allocation fails, return NULL so the caller can fail
+ * the bio and let the submitter retry once the blkg exists.
+ */
+ if (!preemptible() || !mutex_trylock(&q->blkcg_mutex))
+ return NULL;
+
+ ret = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk,
+ GFP_ATOMIC, &blkg);
+ if (ret)
+ blkg = NULL;
+ else if (blkg)
+ blkg = blkg_lookup_tryget(blkg);
+ mutex_unlock(&q->blkcg_mutex);
+
+ return blkg;
+ }
+
/*
* Fast path failed, we're probably issuing IO in this cgroup the first
* time, hold lock to create new blkg.
@@ -2010,19 +2035,33 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,
*
* A reference will be taken on the blkg and will be released when @bio is
* freed.
+ *
+ * If @bio is REQ_NOWAIT and associating requires creating a new blkg, this
+ * function does not sleep; when the blkg cannot be created atomically it
+ * returns %false with @bio left unassociated so the caller can fail the I/O.
+ *
+ * Return: %true if @bio is associated with a blkg, %false on nowait failure.
*/
-void bio_associate_blkg_from_css(struct bio *bio,
+bool bio_associate_blkg_from_css(struct bio *bio,
struct cgroup_subsys_state *css)
{
- if (bio->bi_blkg)
+ struct blkcg_gq *blkg;
+
+ if (bio->bi_blkg) {
blkg_put(bio->bi_blkg);
+ bio->bi_blkg = NULL;
+ }
if (css && css->parent) {
- bio->bi_blkg = blkg_tryget_closest(bio, css);
+ blkg = blkg_tryget_closest(bio, css);
+ if (!blkg)
+ return false;
+ bio->bi_blkg = blkg;
} else {
blkg_get(bdev_get_queue(bio->bi_bdev)->root_blkg);
bio->bi_blkg = bdev_get_queue(bio->bi_bdev)->root_blkg;
}
+ return true;
}
EXPORT_SYMBOL_GPL(bio_associate_blkg_from_css);
@@ -2030,32 +2069,38 @@ EXPORT_SYMBOL_GPL(bio_associate_blkg_from_css);
* bio_associate_blkg - associate a bio with a blkg
* @bio: target bio
*
- * Associate @bio with the blkg found from the bio's css and request_queue.
- * If one is not found, bio_lookup_blkg() creates the blkg. If a blkg is
- * already associated, the css is reused and association redone as the
- * request_queue may have changed.
+ * Associate @bio with the blkg found from the bio's css and request_queue,
+ * creating it if necessary. If a blkg is already associated, the css is
+ * reused and association redone as the request_queue may have changed.
+ *
+ * If @bio is REQ_NOWAIT, association does not sleep; if the blkg cannot be
+ * created without sleeping, %false is returned with @bio left unassociated.
+ *
+ * Return: %true on success, %false on nowait failure.
*/
-void bio_associate_blkg(struct bio *bio)
+bool bio_associate_blkg(struct bio *bio)
{
struct cgroup_subsys_state *css;
+ bool ret;
if (blk_op_is_passthrough(bio->bi_opf))
- return;
+ return true;
if (bio->bi_blkg) {
css = bio_blkcg_css(bio);
- bio_associate_blkg_from_css(bio, css);
- } else {
- rcu_read_lock();
- css = blkcg_css();
- if (!css_tryget_online(css))
- css = NULL;
- rcu_read_unlock();
-
- bio_associate_blkg_from_css(bio, css);
- if (css)
- css_put(css);
+ return bio_associate_blkg_from_css(bio, css);
}
+
+ rcu_read_lock();
+ css = blkcg_css();
+ if (!css_tryget_online(css))
+ css = NULL;
+ rcu_read_unlock();
+
+ ret = bio_associate_blkg_from_css(bio, css);
+ if (css)
+ css_put(css);
+ return ret;
}
EXPORT_SYMBOL_GPL(bio_associate_blkg);
diff --git a/block/blk-core.c b/block/blk-core.c
index 0a1d2b09a5d9..9b96dd802c92 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -832,8 +832,16 @@ void submit_bio_noacct(struct bio *bio)
* rely on bio->bi_blkg matching the queue of bio->bi_bdev.
*/
#ifdef CONFIG_BLK_CGROUP
- if (!bio->bi_blkg || bio->bi_blkg->q != q)
- bio_associate_blkg(bio);
+ if (!bio->bi_blkg || bio->bi_blkg->q != q) {
+ /*
+ * For REQ_NOWAIT, do not sleep to create a new blkg: fail the
+ * bio so the submitter retries once the blkg exists.
+ */
+ if (!bio_associate_blkg(bio)) {
+ bio_endio_status(bio, BLK_STS_AGAIN);
+ return;
+ }
+ }
#endif
/*
@@ -979,8 +987,13 @@ void submit_bio(struct bio *bio)
* bio->bi_blkg, so associate the blkg (for new I/O, the first time) before
* it runs. This is the sleepable entry point for new I/O; remapped bios
* that reach submit_bio_noacct() directly are reassociated there.
+ * For REQ_NOWAIT I/O, fail the bio instead of sleeping if a new blkg
+ * cannot be allocated.
*/
- bio_associate_blkg(bio);
+ if (!bio_associate_blkg(bio)) {
+ bio_endio_status(bio, BLK_STS_AGAIN);
+ return;
+ }
bio_set_ioprio(bio);
submit_bio_noacct(bio);
}
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 7a7509c8a59a..570b62e676dc 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -503,17 +503,16 @@ static inline void bio_release_pages(struct bio *bio, bool mark_dirty)
disk_devt((bio)->bi_bdev->bd_disk)
#ifdef CONFIG_BLK_CGROUP
-void bio_associate_blkg(struct bio *bio);
-void bio_associate_blkg_from_css(struct bio *bio,
+bool bio_associate_blkg(struct bio *bio);
+bool bio_associate_blkg_from_css(struct bio *bio,
struct cgroup_subsys_state *css);
void bio_clone_blkg_association(struct bio *dst, struct bio *src);
void bio_disassociate_blkg(struct bio *bio);
void blkcg_punt_bio_submit(struct bio *bio);
#else /* CONFIG_BLK_CGROUP */
-static inline void bio_associate_blkg(struct bio *bio) { }
-static inline void bio_associate_blkg_from_css(struct bio *bio,
- struct cgroup_subsys_state *css)
-{ }
+static inline bool bio_associate_blkg(struct bio *bio) { return true; }
+static inline bool bio_associate_blkg_from_css(struct bio *bio,
+ struct cgroup_subsys_state *css) { return true; }
static inline void bio_clone_blkg_association(struct bio *dst,
struct bio *src) { }
static inline void bio_disassociate_blkg(struct bio *bio) { }
--
2.51.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [syzbot ci] Re: blk-cgroup: protect blkgs with blkcg_mutex
2026-07-24 12:30 [RFC PATCH v2 0/8] blk-cgroup: protect blkgs with blkcg_mutex Yu Kuai
` (7 preceding siblings ...)
2026-07-24 12:30 ` [RFC PATCH v2 8/8] block: fail nowait bio submit if blkg allocation can't sleep Yu Kuai
@ 2026-07-26 7:08 ` syzbot ci
8 siblings, 0 replies; 10+ messages in thread
From: syzbot ci @ 2026-07-26 7:08 UTC (permalink / raw)
To: axboe, cengku, cgroups, dongsheng.yang, hch, josef, linux-block,
ming.lei, nilay, tj, yukuai, yukuai
Cc: syzbot, syzkaller-bugs
syzbot ci has tested the following series
[v2] blk-cgroup: protect blkgs with blkcg_mutex
https://lore.kernel.org/all/20260724123037.3004560-1-yukuai@kernel.org
* [RFC PATCH v2 1/8] block: associate blkg in submit_bio instead of bio_set_dev
* [RFC PATCH v2 2/8] blk-throttle: protect throttle state with td lock
* [RFC PATCH v2 3/8] blk-cgroup: protect blkgs with blkcg_mutex
* [RFC PATCH v2 4/8] blk-cgroup: remove blkg radix tree preloading
* [RFC PATCH v2 5/8] blk-cgroup: allocate blkgs in blkg_create
* [RFC PATCH v2 6/8] blk-cgroup: share blkg creation between lookup and config prep
* [RFC PATCH v2 7/8] bfq: avoid blkg lookup from locked cgroup update
* [RFC PATCH v2 8/8] block: fail nowait bio submit if blkg allocation can't sleep
and found the following issue:
general protection fault in blkcg_punt_bio_submit
Full report is available here:
https://ci.syzbot.org/series/eece31b0-1f5d-4840-8a12-407d9a4d1c57
***
general protection fault in blkcg_punt_bio_submit
tree: axboe
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/axboe/linux.git
base: 093fbffe03f5c1bb9c10a9e5aa65b23250844403
arch: amd64
compiler: Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config: https://ci.syzbot.org/builds/2cc06765-4cf6-4329-9c0a-9fe326a5338e/config
syz repro: https://ci.syzbot.org/findings/38efdd1d-4179-4903-8ade-04afbce263b2/syz_repro
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
CPU: 0 UID: 0 PID: 26 Comm: kworker/u9:0 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Workqueue: btrfs-worker btrfs_work_helper
RIP: 0010:blkcg_punt_bio_submit+0x47/0x130 block/blk-cgroup.c:243
Code: ae 13 fd 4c 8d 73 50 4c 89 f0 48 c1 e8 03 80 3c 28 00 74 08 4c 89 f7 e8 07 06 7f fd 4d 8b 36 4d 8d 7e 30 4c 89 f8 48 c1 e8 03 <80> 3c 28 00 74 08 4c 89 ff e8 eb 05 7f fd 49 83 3f 00 0f 84 ab 00
RSP: 0000:ffffc90000a17a28 EFLAGS: 00010206
RAX: 0000000000000006 RBX: ffff888117071348 RCX: ffff8881032f5940
RDX: 0000000000000000 RSI: 0000000004000000 RDI: ffff888117071348
RBP: dffffc0000000000 R08: ffff8881bb74406f R09: 1ffff110376e880d
R10: dffffc0000000000 R11: ffffed10376e880e R12: ffff88816c5e6f08
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000030
FS: 0000000000000000(0000) GS:ffff88818d9cb000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f05d5660000 CR3: 0000000172eb2000 CR4: 00000000000006f0
Call Trace:
<TASK>
run_ordered_work fs/btrfs/async-thread.c:243 [inline]
btrfs_work_helper+0x577/0xc50 fs/btrfs/async-thread.c:322
process_one_work kernel/workqueue.c:3314 [inline]
process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3397
worker_thread+0xa47/0xfb0 kernel/workqueue.c:3478
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:blkcg_punt_bio_submit+0x47/0x130 block/blk-cgroup.c:243
Code: ae 13 fd 4c 8d 73 50 4c 89 f0 48 c1 e8 03 80 3c 28 00 74 08 4c 89 f7 e8 07 06 7f fd 4d 8b 36 4d 8d 7e 30 4c 89 f8 48 c1 e8 03 <80> 3c 28 00 74 08 4c 89 ff e8 eb 05 7f fd 49 83 3f 00 0f 84 ab 00
RSP: 0000:ffffc90000a17a28 EFLAGS: 00010206
RAX: 0000000000000006 RBX: ffff888117071348 RCX: ffff8881032f5940
RDX: 0000000000000000 RSI: 0000000004000000 RDI: ffff888117071348
RBP: dffffc0000000000 R08: ffff8881bb74406f R09: 1ffff110376e880d
R10: dffffc0000000000 R11: ffffed10376e880e R12: ffff88816c5e6f08
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000030
FS: 0000000000000000(0000) GS:ffff8882a8fcb000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007efceadf26a8 CR3: 0000000115190000 CR4: 00000000000006f0
----------------
Code disassembly (best guess):
0: ae scas %es:(%rdi),%al
1: 13 fd adc %ebp,%edi
3: 4c 8d 73 50 lea 0x50(%rbx),%r14
7: 4c 89 f0 mov %r14,%rax
a: 48 c1 e8 03 shr $0x3,%rax
e: 80 3c 28 00 cmpb $0x0,(%rax,%rbp,1)
12: 74 08 je 0x1c
14: 4c 89 f7 mov %r14,%rdi
17: e8 07 06 7f fd call 0xfd7f0623
1c: 4d 8b 36 mov (%r14),%r14
1f: 4d 8d 7e 30 lea 0x30(%r14),%r15
23: 4c 89 f8 mov %r15,%rax
26: 48 c1 e8 03 shr $0x3,%rax
* 2a: 80 3c 28 00 cmpb $0x0,(%rax,%rbp,1) <-- trapping instruction
2e: 74 08 je 0x38
30: 4c 89 ff mov %r15,%rdi
33: e8 eb 05 7f fd call 0xfd7f0623
38: 49 83 3f 00 cmpq $0x0,(%r15)
3c: 0f .byte 0xf
3d: 84 .byte 0x84
3e: ab stos %eax,%es:(%rdi)
***
If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
Tested-by: syzbot@syzkaller.appspotmail.com
---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.
To test a patch for this bug, please reply with `#syz test`
(should be on a separate line).
The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.
^ permalink raw reply [flat|nested] 10+ messages in thread