From nobody Mon Sep 28 09:59:42 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 30F8730DD00; Sun, 23 Aug 2026 15:29:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787498987; cv=none; b=Ava6xyJ22yXJPLYpfmz4LhQuHJ8/C41eq87e3udHMvCJ6mXvQUKJfr20+McH9hd3zs9mrUfyxhWxCfEGT1GnJWQrCzrmqguarF+ikH0GnxfxWG+Oe/lKrD0XBWicassUqTOTvEvxSnywJqsTsLgVoX+a+8YhtzkQ2oy0JxE1KHg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787498987; c=relaxed/simple; bh=vTimXbnqV249kP52RAAyLMwrcCBOog1Sw5F4yoZMlO8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=rCXI/NHYSt3Hf3KzELJVGqVU42hZf8FlkaZ2xboMPLaTDwzZOizsDA/0e3TnqYY6Uv+Fa4eU+CkkxEouAgpladHTp2Adeanv1PsLXVqppl88OITm2t1yidfUJoE4r82iX6h6wC8jhcFZ/GprBet7qgqCSjIgxyl1XWciDXZLxdU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=DdqbPaJp; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="DdqbPaJp" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8E3011F00A3A; Sun, 23 Aug 2026 15:29:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787498985; bh=Q6DK2HsdZTqxAvuZnTy1Sfa9gNOhsZSg4TnUdkLSdB8=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=DdqbPaJpNnuyG/FbAkt5F9fqAI8cV/oz8Lbya61BXXLyTtqtEwf/BsKBEVZ6DTkFM fJxva+MyJL80OoBnZYGvCIGEnjl2GVbgzpWB2jeDTrChfijVEvp3+pWco84FMitR8D +2g4/bYdqamtHh2PPL9wBHz+gv+mmFlo3sFLPSvaONyIXjABO5ch5uvAQ2U9sIh1nq qo+73MmekdFUp/CuA7pVIM/ZNZe0jFQ761T4fSaJCXlcS341CYNRt00FafNSu9m80w G/TLcACLbNDs5Y4StaX/BInucW8ScltR7IfWdt3YZG6v3nTksi6IjhetGoZnvUaOg2 6wR891i/XmaRg== From: Yu Kuai To: Jens Axboe , Tejun Heo , Josef Bacik , Sebastian Andrzej Siewior , Clark Williams , Steven Rostedt Cc: Yu Kuai , Christoph Hellwig , Nilay Shroff , Tao Cui , Hannes Reinecke , linux-block@vger.kernel.org, cgroups@vger.kernel.org, linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [RFC PATCH v3 1/6] blk-cgroup: call pd_free_fn() outside spinlocks Date: Sun, 23 Aug 2026 23:29:20 +0800 Message-ID: <20260823152926.1043863-2-yukuai@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260823152926.1043863-1-yukuai@kernel.org> References: <20260823152926.1043863-1-yukuai@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Yu Kuai blkcg_policy_teardown_pds() calls pd_free_fn() while holding both q->queue_lock and blkcg->lock. This is not safe for policies such as iocost, whose ioc_pd_free() calls hrtimer_cancel(). On PREEMPT_RT the hrtimer cancellation slow path can sleep while waiting for a soft hrtimer callback to finish. Keep the offline callback and policy data detachment protected by the existing spinlocks, but tear down one policy data object at a time and drop the locks before invoking pd_free_fn(). q->blkcg_mutex serializes the operation against blkg_free_workfn(), so the associated blkg remains valid while the callback runs. Fixes: 7caa47151ab2 ("blkcg: implement blk-iocost") Signed-off-by: Yu Kuai --- block/blk-cgroup.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 1bd91223367c..5b51be2fefc1 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1548,33 +1548,51 @@ struct cgroup_subsys io_cgrp_subsys =3D { .depends_on =3D 1 << memory_cgrp_id, #endif }; EXPORT_SYMBOL_GPL(io_cgrp_subsys); =20 -/* - * Tear down per-blkg policy data for @pol on @q. - */ -static void blkcg_policy_teardown_pds(struct request_queue *q, - const struct blkcg_policy *pol) +static struct blkg_policy_data * +blkcg_policy_detach_pd(struct request_queue *q, + const struct blkcg_policy *pol) { + struct blkg_policy_data *pd =3D NULL; struct blkcg_gq *blkg; =20 + lockdep_assert_held(&q->blkcg_mutex); + + spin_lock_irq(&q->queue_lock); list_for_each_entry(blkg, &q->blkg_list, q_node) { struct blkcg *blkcg =3D blkg->blkcg; - struct blkg_policy_data *pd; =20 spin_lock(&blkcg->lock); pd =3D blkg->pd[pol->plid]; if (pd) { if (pd->online && pol->pd_offline_fn) pol->pd_offline_fn(pd); pd->online =3D false; - pol->pd_free_fn(pd); WRITE_ONCE(blkg->pd[pol->plid], NULL); } spin_unlock(&blkcg->lock); + + if (pd) + break; } + spin_unlock_irq(&q->queue_lock); + + return pd; +} + +/* + * Tear down per-blkg policy data for @pol on @q. + */ +static void blkcg_policy_teardown_pds(struct request_queue *q, + const struct blkcg_policy *pol) +{ + struct blkg_policy_data *pd; + + while ((pd =3D blkcg_policy_detach_pd(q, pol))) + pol->pd_free_fn(pd); } =20 /** * blkcg_activate_policy - activate a blkcg policy on a gendisk * @disk: gendisk of interest @@ -1687,13 +1705,11 @@ int blkcg_activate_policy(struct gendisk *disk, con= st struct blkcg_policy *pol) pol->pd_free_fn(pd_prealloc); return ret; =20 enomem: /* alloc failed, take down everything */ - spin_lock_irq(&q->queue_lock); blkcg_policy_teardown_pds(q, pol); - spin_unlock_irq(&q->queue_lock); ret =3D -ENOMEM; goto out; } EXPORT_SYMBOL_GPL(blkcg_activate_policy); =20 @@ -1719,12 +1735,13 @@ void blkcg_deactivate_policy(struct gendisk *disk, =20 mutex_lock(&q->blkcg_mutex); spin_lock_irq(&q->queue_lock); =20 __clear_bit(pol->plid, q->blkcg_pols); - blkcg_policy_teardown_pds(q, pol); spin_unlock_irq(&q->queue_lock); + + blkcg_policy_teardown_pds(q, pol); mutex_unlock(&q->blkcg_mutex); =20 if (queue_is_mq(q)) blk_mq_unfreeze_queue(q, memflags); } --=20 2.51.0 From nobody Mon Sep 28 09:59:42 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0FD2438F651; Sun, 23 Aug 2026 15:29:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787498997; cv=none; b=pX8Y3Nc3RDXLFmKrECXaYP107mLhH2y4l2eoq5iRUXPlZC9Ztk7gEFMj/QRz1LdhKs6ZornCktWEZMly1kZoUhaCUzDF/3C7ACTGRhZE2Z0FZCDzjOyHijK6S1CBzTFgCX7QzHALlnJDcKc1ALf40bwsVW4fY4hytyNqM8h5BE4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787498997; c=relaxed/simple; bh=1U4e9V38ZzDnZ/zD8kFocqVQ6dW4tN9HZ91X0pi6iO4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=NR2whJyzn3Cqil5f1DOZcL8UWh6gGrD4zMqL2LPI/dyrS6ssLoTOf9WxMVu05RbS/conzw5+/yMqkZ6wdyihRUJ5SfKl1fHNAVs75GVEb3T6GfrN+Cw6ves1afb1T5S6AlJR+6rLMAl3E9n1acmzS0tLHpa9CkkVsgX4gwUw3AU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=nUciHw8v; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="nUciHw8v" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C755F1F000E9; Sun, 23 Aug 2026 15:29:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787498992; bh=El/XT7hdwqWl1/0Red4A0YufWLRO62Hp/16joI5fn1w=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=nUciHw8vR2Ek2Iou6QaO9HjG0c3x9ZPE5008+lNW6rUvK9JwxieMz6uj1I0LtbElZ Epxd+dqqapdjjFYmVs2BnaxQ+9Uk12Rvb7aZqoPGxRIjlrUzYogmEwLn+YtSin3BsA OOZTnE9eBaJlzFcgQZ3TuB/25GWP5FjBnq4lSf1uhbdDUc/eMTbm8VQkswq5n0GLvR Zxju3pYFLWzWfOu23tQxwTILVWjWvekN4kRN7sgECgnMC98M90UCgi8+7dIvWBydma cgPjwqB7cIN6rQ2ECMzQ3JxHd7Vu/fL1GHpoLTE3GrulkZJRs/pSixAralp2jKDfCH y0NXSTya667/g== From: Yu Kuai To: Jens Axboe , Tejun Heo , Josef Bacik , Sebastian Andrzej Siewior , Clark Williams , Steven Rostedt Cc: Yu Kuai , Christoph Hellwig , Nilay Shroff , Tao Cui , Hannes Reinecke , linux-block@vger.kernel.org, cgroups@vger.kernel.org, linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [RFC PATCH v3 2/6] blk-throttle: protect throttle state with td lock Date: Sun, 23 Aug 2026 23:29:21 +0800 Message-ID: <20260823152926.1043863-3-yukuai@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260823152926.1043863-1-yukuai@kernel.org> References: <20260823152926.1043863-1-yukuai@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Yu Kuai 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. blkg_destroy_all() offlines policy data, but the data is freed asynchronously. A per-group pending timer can therefore outlive blk_throtl_exit(), which frees td before the policy data is released. Previously, the timer callback took queue_lock and checked q->root_blkg before dereferencing td, so a late per-group callback exited after teardown. Once the callback takes td->lock, it dereferences td before that check and can access freed memory. Shut down all per-group and top-level timers before freeing td. Use timer_shutdown_sync() instead of timer_delete_sync() because this is final teardown: it waits for active callbacks and prevents any later mod_timer() from rearming the timer. Use the same shutdown semantics before freeing a throtl_grp. Signed-off-by: Yu Kuai --- 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 3828c3857900..2ff30700e84e 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -28,10 +28,13 @@ static struct workqueue_struct *kthrotld_workqueue; =20 #define rb_entry_tg(node) rb_entry((node), struct throtl_grp, rb_node) =20 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; =20 struct request_queue *queue; =20 @@ -344,15 +347,20 @@ static void tg_update_has_rules(struct throtl_grp *tg) } =20 static void throtl_pd_online(struct blkg_policy_data *pd) { struct throtl_grp *tg =3D pd_to_tg(pd); + struct throtl_data *td =3D 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); } =20 static void tg_release(struct rcu_head *rcu) { struct blkg_policy_data *pd =3D @@ -366,11 +374,11 @@ static void tg_release(struct rcu_head *rcu) =20 static void throtl_pd_free(struct blkg_policy_data *pd) { struct throtl_grp *tg =3D pd_to_tg(pd); =20 - timer_delete_sync(&tg->service_queue.pending_timer); + timer_shutdown_sync(&tg->service_queue.pending_timer); call_rcu(&pd->rcu_head, tg_release); } =20 static struct throtl_grp * throtl_rb_first(struct throtl_service_queue *parent_sq) @@ -1140,13 +1148,13 @@ static void throtl_pending_timer_fn(struct timer_li= st *t) if (tg) q =3D tg->pd.blkg->q; else q =3D td->queue; =20 - spin_lock_irq(&q->queue_lock); + spin_lock_irq(&td->lock); =20 - if (!q->root_blkg) + if (!READ_ONCE(q->root_blkg)) goto out_unlock; =20 again: parent_sq =3D sq->parent_sq; dispatched =3D false; @@ -1166,13 +1174,13 @@ static void throtl_pending_timer_fn(struct timer_li= st *t) =20 if (throtl_schedule_next_dispatch(sq, false)) break; =20 /* 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); } =20 if (!dispatched) goto out_unlock; =20 @@ -1191,11 +1199,11 @@ static void throtl_pending_timer_fn(struct timer_li= st *t) } else { /* reached the top-level, queue issuing */ queue_work(kthrotld_workqueue, &td->dispatch_work); } out_unlock: - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&td->lock); } =20 /** * blk_throtl_dispatch_work_fn - work function for throtl_data->dispatch_w= ork * @work: work item being executed @@ -1207,23 +1215,22 @@ static void throtl_pending_timer_fn(struct timer_li= st *t) static void blk_throtl_dispatch_work_fn(struct work_struct *work) { struct throtl_data *td =3D container_of(work, struct throtl_data, dispatch_work); struct throtl_service_queue *td_sq =3D &td->service_queue; - struct request_queue *q =3D td->queue; struct bio_list bio_list_on_stack; struct bio *bio; struct blk_plug plug; int rw; =20 bio_list_init(&bio_list_on_stack); =20 - spin_lock_irq(&q->queue_lock); + spin_lock_irq(&td->lock); for (rw =3D READ; rw <=3D WRITE; rw++) while ((bio =3D 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); =20 if (!bio_list_empty(&bio_list_on_stack)) { blk_start_plug(&plug); while ((bio =3D bio_list_pop(&bio_list_on_stack))) submit_bio_noacct_nocheck(bio, false); @@ -1297,11 +1304,11 @@ static void tg_conf_updated(struct throtl_grp *tg, = bool global) continue; } rcu_read_unlock(); =20 /* - * 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 * that a group's limit are dropped suddenly and we don't want to * account recently dispatched IO with new low rate. @@ -1325,10 +1332,11 @@ static int blk_throtl_init(struct gendisk *disk) td =3D kzalloc_node(sizeof(*td), GFP_KERNEL, q->node); if (!td) return -ENOMEM; =20 INIT_WORK(&td->dispatch_work, blk_throtl_dispatch_work_fn); + spin_lock_init(&td->lock); throtl_service_queue_init(&td->service_queue); =20 memflags =3D blk_mq_freeze_queue(disk->queue); blk_mq_quiesce_queue(disk->queue); =20 @@ -1379,18 +1387,20 @@ static ssize_t tg_set_conf(struct kernfs_open_file = *of, goto unprep; if (!v) v =3D U64_MAX; =20 tg =3D blkg_to_tg(ctx.blkg); + spin_lock_irq(&tg->td->lock); tg_update_carryover(tg); =20 if (is_u64) *(u64 *)((void *)tg + of_cft(of)->private) =3D v; else *(unsigned int *)((void *)tg + of_cft(of)->private) =3D v; =20 tg_conf_updated(tg, false); + spin_unlock_irq(&tg->td->lock); ret =3D 0; =20 unprep: blkg_conf_unprep(&ctx); =20 @@ -1561,10 +1571,11 @@ static ssize_t tg_set_limit(struct kernfs_open_file= *of, ret =3D blkg_conf_prep(blkcg, &blkcg_policy_throtl, &ctx); if (ret) goto close_bdev; =20 tg =3D blkg_to_tg(ctx.blkg); + spin_lock_irq(&tg->td->lock); tg_update_carryover(tg); =20 v[0] =3D tg->bps[READ]; v[1] =3D tg->bps[WRITE]; v[2] =3D tg->iops[READ]; @@ -1584,15 +1595,15 @@ static ssize_t tg_set_limit(struct kernfs_open_file= *of, =20 ret =3D -EINVAL; p =3D tok; strsep(&p, "=3D"); if (!p || (sscanf(p, "%llu", &val) !=3D 1 && strcmp(p, "max"))) - goto unprep; + goto unlock; =20 ret =3D -ERANGE; if (!val) - goto unprep; + goto unlock; =20 ret =3D -EINVAL; if (!strcmp(tok, "rbps")) v[0] =3D val; else if (!strcmp(tok, "wbps")) @@ -1600,20 +1611,24 @@ static ssize_t tg_set_limit(struct kernfs_open_file= *of, else if (!strcmp(tok, "riops")) v[2] =3D min_t(u64, val, UINT_MAX); else if (!strcmp(tok, "wiops")) v[3] =3D min_t(u64, val, UINT_MAX); else - goto unprep; + goto unlock; } =20 tg->bps[READ] =3D v[0]; tg->bps[WRITE] =3D v[1]; tg->iops[READ] =3D v[2]; tg->iops[WRITE] =3D v[3]; =20 tg_conf_updated(tg, false); + spin_unlock_irq(&tg->td->lock); ret =3D 0; + goto unprep; +unlock: + spin_unlock_irq(&tg->td->lock); unprep: blkg_conf_unprep(&ctx); close_bdev: blkg_conf_close_bdev(&ctx); return ret ?: nbytes; @@ -1634,10 +1649,32 @@ static void throtl_shutdown_wq(struct request_queue= *q) struct throtl_data *td =3D q->td; =20 cancel_work_sync(&td->dispatch_work); } =20 +static void throtl_shutdown_timers(struct request_queue *q) +{ + struct throtl_data *td =3D 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 =3D 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 =3D &tg->service_queue; =20 if (tg->flags & THROTL_TG_CANCELING) @@ -1667,11 +1704,17 @@ static void tg_flush_bios(struct throtl_grp *tg) throtl_schedule_next_dispatch(sq->parent_sq, true); } =20 static void throtl_pd_offline(struct blkg_policy_data *pd) { - tg_flush_bios(pd_to_tg(pd)); + struct throtl_grp *tg =3D pd_to_tg(pd); + struct throtl_data *td =3D tg->td; + unsigned long flags; + + spin_lock_irqsave(&td->lock, flags); + tg_flush_bios(tg); + spin_unlock_irqrestore(&td->lock, flags); } =20 struct blkcg_policy blkcg_policy_throtl =3D { .dfl_cftypes =3D throtl_files, .legacy_cftypes =3D throtl_legacy_files, @@ -1723,19 +1766,21 @@ static void tg_cancel_writeback_bios(struct throtl_= grp *tg, } =20 void blk_throtl_cancel_bios(struct gendisk *disk) { struct request_queue *q =3D disk->queue; + struct throtl_data *td =3D q->td; struct cgroup_subsys_state *pos_css; struct blkcg_gq *blkg; struct bio_list cancel_bios[2] =3D { }; int rw; =20 if (!blk_throtl_activated(q)) return; =20 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 * path need RCU protection and to prevent warning from lockdep. */ @@ -1750,10 +1795,11 @@ void blk_throtl_cancel_bios(struct gendisk *disk) * del_gendisk. */ tg_cancel_writeback_bios(blkg_to_tg(blkg), cancel_bios); } rcu_read_unlock(); + spin_unlock(&td->lock); spin_unlock_irq(&q->queue_lock); =20 for (rw =3D READ; rw <=3D WRITE; rw++) { struct bio *bio; while ((bio =3D bio_list_pop(&cancel_bios[rw]))) @@ -1789,21 +1835,20 @@ static bool tg_within_limit(struct throtl_grp *tg, = struct bio *bio, bool rw) return tg_dispatch_time(tg, bio) =3D=3D 0; } =20 bool __blk_throtl_bio(struct bio *bio) { - struct request_queue *q =3D bdev_get_queue(bio->bi_bdev); struct blkcg_gq *blkg =3D bio_blkg(bio); struct throtl_qnode *qn =3D NULL; struct throtl_grp *tg =3D blkg_to_tg(blkg); struct throtl_service_queue *sq; bool rw =3D bio_data_dir(bio); bool throttled =3D false; struct throtl_data *td =3D tg->td; =20 rcu_read_lock(); - spin_lock_irq(&q->queue_lock); + spin_lock_irq(&td->lock); sq =3D &tg->service_queue; =20 while (true) { if (tg_within_limit(tg, bio, rw)) { /* within limits, let's charge and dispatch directly */ @@ -1875,30 +1920,32 @@ bool __blk_throtl_bio(struct bio *bio) tg_update_disptime(tg); throtl_schedule_next_dispatch(tg->service_queue.parent_sq, true); } =20 out_unlock: - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&td->lock); =20 rcu_read_unlock(); return throttled; } =20 void blk_throtl_exit(struct gendisk *disk) { struct request_queue *q =3D disk->queue; + struct throtl_data *td =3D q->td; =20 /* * blkg_destroy_all() already deactivate throtl policy, just check and * free throtl data. */ - if (!q->td) + if (!td) return; =20 - timer_delete_sync(&q->td->service_queue.pending_timer); + throtl_shutdown_timers(q); throtl_shutdown_wq(q); - kfree(q->td); + q->td =3D NULL; + kfree(td); } =20 static int __init throtl_init(void) { kthrotld_workqueue =3D alloc_workqueue("kthrotld", WQ_MEM_RECLAIM | WQ_PE= RCPU, 0); --=20 2.51.0 From nobody Mon Sep 28 09:59:42 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D5DF825C804; Sun, 23 Aug 2026 15:29:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499004; cv=none; b=cdxeOaByJZyTtoSWp5sZ9e6l1nnLg/7mIvtS/00Y+PgvbgmHbpoecyNYEKI+LaMSJOVZUgjFOhAaa7CvC1AohfofO7WuaTvrZc1nN9usuIcoKn17mrPBg/bR4v7nmbw4aIh2CFmY+jVkKQwXU8ZUee/3crXLaCrL8aksq8QvpLQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499004; c=relaxed/simple; bh=cXMMhSiywBOMMr7eQqERnj5XwBgqPgSFkFehkaXMIr4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=agffq8cAKwwn5Jn0qVLqwM3RW3YT28Jzi0OSq/h98tnMQBeO5o4lEFD09hTEifUfsGIFcRiAOgH1WpUtaoyAVS7KOt3NZbpsqqCkrQfi+LrzmXz58AzmGoe2lLa2Hi6Bc6+oyDjTMGSiLeD8i5ysTmM6uC7AFUfJyNbOtKS3Qi0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=VuVQZf7v; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="VuVQZf7v" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 47F241F00A3A; Sun, 23 Aug 2026 15:29:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787498998; bh=hyG3XfLInuMHvbuGa7WkSBQFReMcSCRzPSASGqxw0Bc=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=VuVQZf7vq7nZ05LTJMmvItO8e2X/OEeptkuXIqX7glTOVvStT0ewu7SZAsBA3P0e/ hAFrtBBVRHSAEooMqAmXC4pZovq0cNq0XWgfXEPIQrhmbFRYDePpppSrS2FOBukKAT C2qFSQFy+3o6rtKnVoNHuNnlDmCq8aZWtLlT7JL5TkgAA2/wbF4HZME4clk/xbGxDM AgX5DNFw63pl4MMp3PSJnpxEHLzQseJYyHR5pX1w3gAEDxhJKiWMcJZkr4HYoDMa3/ XFjL7ES+Vq4DFd2t+JIowcUMwsY1m3RSuftTMue+NQbXDaXQJp46CI4FS6ntzRnWDg 5Wk+DLCOBTzCw== From: Yu Kuai To: Jens Axboe , Tejun Heo , Josef Bacik , Sebastian Andrzej Siewior , Clark Williams , Steven Rostedt Cc: Yu Kuai , Christoph Hellwig , Nilay Shroff , Tao Cui , Hannes Reinecke , linux-block@vger.kernel.org, cgroups@vger.kernel.org, linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [RFC PATCH v3 3/6] blk-cgroup: protect blkgs with blkcg_mutex Date: Sun, 23 Aug 2026 23:29:22 +0800 Message-ID: <20260823152926.1043863-4-yukuai@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260823152926.1043863-1-yukuai@kernel.org> References: <20260823152926.1043863-1-yukuai@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Yu Kuai 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 topology synchronization to q->blkcg_mutex. Hold it while creating and destroying blkgs, while preparing and undoing configuration, and while activating or deactivating policies. Keep the common bio_blkg() lookup on an RCU fast path so I/O for an existing blkg does not acquire blkcg_mutex. Only take the mutex when the blkg hierarchy needs to be created. 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 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(). Signed-off-by: Yu Kuai --- block/bfq-cgroup.c | 10 ++- block/blk-cgroup.c | 153 +++++++++++++++++------------------------- block/blk-cgroup.h | 11 ++- block/blk-iocost.c | 8 ++- block/blk-iolatency.c | 7 +- block/blk-throttle.c | 10 +-- 6 files changed, 87 insertions(+), 112 deletions(-) diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index 4a3975f9ff74..d64cea475d7b 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -426,11 +426,11 @@ static void bfqg_stats_xfer_dead(struct bfq_group *bf= qg) if (!bfqg) /* root_group */ return; =20 parent =3D bfqg_parent(bfqg); =20 - lockdep_assert_held(&bfqg_to_blkg(bfqg)->q->queue_lock); + lockdep_assert_held(&bfqg_to_blkg(bfqg)->q->blkcg_mutex); =20 if (unlikely(!parent)) return; =20 bfqg_stats_add_aux(&parent->stats, &bfqg->stats); @@ -876,11 +876,11 @@ static void bfq_reparent_active_queues(struct bfq_dat= a *bfqd, /** * bfq_pd_offline - deactivate the entity associated with @pd, * 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) { struct bfq_service_tree *st; @@ -949,22 +949,20 @@ void bfq_end_wr_async(struct bfq_data *bfqd) { struct request_queue *q =3D bfqd->queue; struct blkcg_gq *blkg; =20 mutex_lock(&q->blkcg_mutex); - spin_lock_irq(&q->queue_lock); - spin_lock(&bfqd->lock); + spin_lock_irq(&bfqd->lock); =20 list_for_each_entry(blkg, &q->blkg_list, q_node) { struct bfq_group *bfqg =3D blkg_to_bfqg(blkg); =20 bfq_end_wr_async_queues(bfqd, bfqg); } bfq_end_wr_async_queues(bfqd, bfqd->root_group); =20 - spin_unlock(&bfqd->lock); - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&bfqd->lock); mutex_unlock(&q->blkcg_mutex); } =20 static int bfq_io_show_weight_legacy(struct seq_file *sf, void *v) { diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 5b51be2fefc1..0f34a80a726d 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -62,12 +62,10 @@ static LIST_HEAD(all_blkcgs); /* protected by blkcg_po= l_mutex */ =20 bool blkcg_debug_stats =3D false; =20 static DEFINE_RAW_SPINLOCK(blkg_stat_lock); =20 -#define BLKG_DESTROY_BATCH_SIZE 64 - const struct rhashtable_params blkg_hash_params =3D { .key_len =3D sizeof_field(struct blkcg_gq, blkcg_id), .key_offset =3D offsetof(struct blkcg_gq, blkcg_id), .head_offset =3D offsetof(struct blkcg_gq, q_hash_node), .automatic_shrinking =3D true, @@ -139,15 +137,13 @@ static void blkg_free_workfn(struct work_struct *work) for (i =3D 0; i < BLKCG_MAX_POLS; i++) if (blkg->pd[i]) 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); if (list_empty(&q->blkg_list)) wake_up_var(&q->blkg_list); - spin_unlock_irq(&q->queue_lock); mutex_unlock(&q->blkcg_mutex); =20 /* * Release blkcg css ref only after blkg is removed from q->blkg_list, * so concurrent iterators won't see a blkg with a freed blkcg. @@ -397,11 +393,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blk= cg, struct gendisk *disk, struct blkcg_gq *new_blkg) { struct blkcg_gq *blkg; int i, ret; =20 - lockdep_assert_held(&disk->queue->queue_lock); + lockdep_assert_held(&disk->queue->blkcg_mutex); =20 /* request_queue is dying, do not create/recreate a blkg */ if (blk_queue_dying(disk->queue)) { ret =3D -ENODEV; goto err_free_blkg; @@ -417,16 +413,19 @@ static struct blkcg_gq *blkg_create(struct blkcg *blk= cg, struct gendisk *disk, } blkg =3D new_blkg; =20 /* link parent */ if (blkcg_parent(blkcg)) { + rcu_read_lock(); blkg->parent =3D blkg_lookup(blkcg_parent(blkcg), disk->queue); if (WARN_ON_ONCE(!blkg->parent)) { + rcu_read_unlock(); ret =3D -ENODEV; goto err_free_blkg; } blkg_get(blkg->parent); + rcu_read_unlock(); } =20 /* invoke per-policy init */ for (i =3D 0; i < BLKCG_MAX_POLS; i++) { struct blkcg_policy *pol =3D blkcg_policy[i]; @@ -434,11 +433,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blk= cg, struct gendisk *disk, if (blkg->pd[i] && pol->pd_init_fn) pol->pd_init_fn(blkg->pd[i]); } =20 /* insert */ - spin_lock(&blkcg->lock); + spin_lock_irq(&blkcg->lock); ret =3D rhashtable_insert_fast(&disk->queue->blkg_hash, &blkg->q_hash_node, blkg_hash_params); if (likely(!ret)) { hlist_add_head_rcu(&blkg->blkcg_node, &blkcg->blkg_list); list_add(&blkg->q_node, &disk->queue->blkg_list); @@ -452,11 +451,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blk= cg, struct gendisk *disk, blkg->pd[i]->online =3D true; } } blkg->online =3D true; } - spin_unlock(&blkcg->lock); + spin_unlock_irq(&blkcg->lock); =20 if (!ret) return blkg; =20 /* @blkg failed fully initialized, use the usual release path */ @@ -485,13 +484,12 @@ static struct blkcg_gq *blkg_lookup_tryget(struct blk= cg_gq *blkg) * @blkcg: blkcg of interest * @disk: gendisk of interest * * 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. - * - * Must be called with @disk->queue->queue_lock held. + * 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 closest blkg with an extra reference acquired. If * blkg_create() fails while walking down from root, the returned blkg may * belong to an ancestor of @blkcg. This function never returns %NULL. */ @@ -499,11 +497,11 @@ static struct blkcg_gq *blkg_lookup_create(struct blk= cg *blkcg, struct gendisk *disk) { struct request_queue *q =3D disk->queue; struct blkcg_gq *blkg; =20 - lockdep_assert_held(&q->queue_lock); + lockdep_assert_held(&q->blkcg_mutex); =20 rcu_read_lock(); blkg =3D blkg_lookup(blkcg, q); if (blkg) { blkg =3D blkg_lookup_tryget(blkg); @@ -520,20 +518,22 @@ static struct blkcg_gq *blkg_lookup_create(struct blk= cg *blkcg, while (true) { struct blkcg *pos =3D blkcg; struct blkcg *parent =3D blkcg_parent(blkcg); struct blkcg_gq *ret_blkg =3D q->root_blkg; =20 + rcu_read_lock(); while (parent) { blkg =3D blkg_lookup(parent, q); if (blkg) { /* remember closest blkg */ ret_blkg =3D blkg; break; } pos =3D parent; parent =3D blkcg_parent(parent); } + rcu_read_unlock(); =20 blkg =3D blkg_create(pos, disk, NULL); if (IS_ERR(blkg)) { blkg =3D ret_blkg; break; @@ -548,11 +548,11 @@ static struct blkcg_gq *blkg_lookup_create(struct blk= cg *blkcg, static void blkg_destroy(struct blkcg_gq *blkg) { struct blkcg *blkcg =3D blkg->blkcg; int i; =20 - lockdep_assert_held(&blkg->q->queue_lock); + lockdep_assert_held(&blkg->q->blkcg_mutex); lockdep_assert_held(&blkcg->lock); =20 /* * blkg stays on the queue list until blkg_free_workfn(), see details in * blkg_free_workfn(), hence this function can be called from @@ -585,37 +585,22 @@ static void blkg_destroy(struct blkcg_gq *blkg) =20 static void blkg_destroy_all(struct gendisk *disk) { struct request_queue *q =3D disk->queue; struct blkcg_gq *blkg; - int count =3D BLKG_DESTROY_BATCH_SIZE; int i; =20 -restart: mutex_lock(&q->blkcg_mutex); - spin_lock_irq(&q->queue_lock); list_for_each_entry(blkg, &q->blkg_list, q_node) { struct blkcg *blkcg =3D blkg->blkcg; =20 if (hlist_unhashed(&blkg->blkcg_node)) continue; =20 - spin_lock(&blkcg->lock); + spin_lock_irq(&blkcg->lock); blkg_destroy(blkg); - spin_unlock(&blkcg->lock); - - /* - * in order to avoid holding the spin lock for too long, release - * it when a batch of blkgs are destroyed. - */ - if (!(--count)) { - count =3D BLKG_DESTROY_BATCH_SIZE; - spin_unlock_irq(&q->queue_lock); - mutex_unlock(&q->blkcg_mutex); - cond_resched(); - goto restart; - } + spin_unlock_irq(&blkcg->lock); } =20 /* * Mark policy deactivated since policy offline has been done, and * the free is scheduled, so future blkcg_deactivate_policy() can @@ -627,11 +612,10 @@ static void blkg_destroy_all(struct gendisk *disk) if (pol) __clear_bit(pol->plid, q->blkcg_pols); } =20 q->root_blkg =3D NULL; - spin_unlock_irq(&q->queue_lock); mutex_unlock(&q->blkcg_mutex); } =20 static void blkg_iostat_set(struct blkg_iostat *dst, struct blkg_iostat *s= rc) { @@ -843,12 +827,12 @@ EXPORT_SYMBOL_GPL(blkg_conf_open_bdev); * accordingly. On success, @ctx->body points to the part of @ctx->input * following MAJ:MIN, @ctx->bdev points to the target block device and * @ctx->blkg to the blkg being configured. * * blkg_conf_open_bdev() must be called on @ctx beforehand. On success, th= is - * 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) { struct gendisk *disk; @@ -862,18 +846,19 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct = blkcg_policy *pol, disk =3D ctx->bdev->bd_disk; q =3D disk->queue; =20 /* Prevent concurrent with blkcg_deactivate_policy() */ mutex_lock(&q->blkcg_mutex); - spin_lock_irq(&q->queue_lock); =20 if (!blkcg_policy_enabled(q, pol)) { ret =3D -EOPNOTSUPP; goto fail_unlock; } =20 + rcu_read_lock(); blkg =3D blkg_lookup(blkcg, q); + rcu_read_unlock(); if (blkg) goto success; =20 /* * Create blkgs walking down from blkcg_root to @blkcg, so that all @@ -883,33 +868,32 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct = blkcg_policy *pol, struct blkcg *pos =3D blkcg; struct blkcg *parent; struct blkcg_gq *new_blkg; =20 parent =3D blkcg_parent(blkcg); + rcu_read_lock(); while (parent && !blkg_lookup(parent, q)) { pos =3D parent; parent =3D blkcg_parent(parent); } - - /* Drop locks to do new blkg allocation with GFP_KERNEL. */ - spin_unlock_irq(&q->queue_lock); + rcu_read_unlock(); =20 new_blkg =3D blkg_alloc(pos, disk, GFP_NOIO); if (unlikely(!new_blkg)) { ret =3D -ENOMEM; - goto fail_exit; + goto fail_unlock; } =20 - spin_lock_irq(&q->queue_lock); - if (!blkcg_policy_enabled(q, pol)) { blkg_free(new_blkg); ret =3D -EOPNOTSUPP; goto fail_unlock; } =20 + rcu_read_lock(); blkg =3D blkg_lookup(pos, q); + rcu_read_unlock(); if (blkg) { blkg_free(new_blkg); } else { blkg =3D blkg_create(pos, disk, new_blkg); if (IS_ERR(blkg)) { @@ -920,17 +904,14 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct = blkcg_policy *pol, =20 if (pos =3D=3D blkcg) goto success; } success: - mutex_unlock(&q->blkcg_mutex); ctx->blkg =3D blkg; return 0; =20 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 * short msleep(). It isn't strictly necessary but queue * can be bypassing for some time and it's always nice to @@ -949,11 +930,11 @@ EXPORT_SYMBOL_GPL(blkg_conf_prep); * @ctx: blkg_conf_ctx initialized with blkg_conf_init() */ 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 =3D NULL; } EXPORT_SYMBOL_GPL(blkg_conf_unprep); =20 /** @@ -1269,12 +1250,13 @@ static struct blkcg_gq *blkcg_get_first_blkg(struct= blkcg *blkcg) =20 /** * 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 dan= cing. + * blkgs should be removed while holding both q->blkcg_mutex and blkcg->lo= ck. + * 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 all= owing * blkcg_css_free to eventually be called. * * This is the blkcg counterpart of ioc_release_fn(). */ @@ -1285,17 +1267,17 @@ static void blkcg_destroy_blkgs(struct blkcg *blkcg) might_sleep(); =20 while ((blkg =3D blkcg_get_first_blkg(blkcg))) { struct request_queue *q =3D blkg->q; =20 - spin_lock_irq(&q->queue_lock); - spin_lock(&blkcg->lock); + mutex_lock(&q->blkcg_mutex); + spin_lock_irq(&blkcg->lock); =20 blkg_destroy(blkg); =20 - spin_unlock(&blkcg->lock); - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&blkcg->lock); + mutex_unlock(&q->blkcg_mutex); =20 blkg_put(blkg); cond_resched(); } } @@ -1499,22 +1481,21 @@ int blkcg_init_disk(struct gendisk *disk) new_blkg =3D blkg_alloc(&blkcg_root, disk, GFP_KERNEL); if (!new_blkg) return -ENOMEM; =20 /* 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 =3D blkg_create(&blkcg_root, disk, new_blkg); if (IS_ERR(blkg)) goto err_unlock; q->root_blkg =3D blkg; - spin_unlock_irq(&q->queue_lock); + mutex_unlock(&q->blkcg_mutex); =20 return 0; =20 err_unlock: - spin_unlock_irq(&q->queue_lock); + mutex_unlock(&q->blkcg_mutex); return PTR_ERR(blkg); } =20 void blkcg_exit_disk(struct gendisk *disk) { @@ -1548,51 +1529,37 @@ struct cgroup_subsys io_cgrp_subsys =3D { .depends_on =3D 1 << memory_cgrp_id, #endif }; EXPORT_SYMBOL_GPL(io_cgrp_subsys); =20 -static struct blkg_policy_data * -blkcg_policy_detach_pd(struct request_queue *q, - const struct blkcg_policy *pol) +/* + * Tear down per-blkg policy data for @pol on @q. + */ +static void blkcg_policy_teardown_pds(struct request_queue *q, + const struct blkcg_policy *pol) { - struct blkg_policy_data *pd =3D NULL; struct blkcg_gq *blkg; =20 lockdep_assert_held(&q->blkcg_mutex); =20 - spin_lock_irq(&q->queue_lock); list_for_each_entry(blkg, &q->blkg_list, q_node) { struct blkcg *blkcg =3D blkg->blkcg; + struct blkg_policy_data *pd; =20 - spin_lock(&blkcg->lock); + spin_lock_irq(&blkcg->lock); pd =3D blkg->pd[pol->plid]; if (pd) { if (pd->online && pol->pd_offline_fn) pol->pd_offline_fn(pd); pd->online =3D false; WRITE_ONCE(blkg->pd[pol->plid], NULL); } - spin_unlock(&blkcg->lock); + spin_unlock_irq(&blkcg->lock); =20 if (pd) - break; + pol->pd_free_fn(pd); } - spin_unlock_irq(&q->queue_lock); - - return pd; -} - -/* - * Tear down per-blkg policy data for @pol on @q. - */ -static void blkcg_policy_teardown_pds(struct request_queue *q, - const struct blkcg_policy *pol) -{ - struct blkg_policy_data *pd; - - while ((pd =3D blkcg_policy_detach_pd(q, pol))) - pol->pd_free_fn(pd); } =20 /** * blkcg_activate_policy - activate a blkcg policy on a gendisk * @disk: gendisk of interest @@ -1600,13 +1567,13 @@ static void blkcg_policy_teardown_pds(struct reques= t_queue *q, * * Activate @pol on @disk. Requires %GFP_KERNEL context. @disk goes thro= ugh * bypass mode to populate its blkgs with policy_data for @pol. * * Activation happens with @disk bypassed, so nobody would be accessing bl= kgs - * 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_enable= d() + * 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. */ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy = *pol) @@ -1631,12 +1598,10 @@ int blkcg_activate_policy(struct gendisk *disk, con= st struct blkcg_policy *pol) if (queue_is_mq(q)) memflags =3D blk_mq_freeze_queue(q); =20 mutex_lock(&q->blkcg_mutex); retry: - spin_lock_irq(&q->queue_lock); - /* blkg_list is pushed at the head, reverse walk to initialize parents fi= rst */ list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) { struct blkg_policy_data *pd; =20 if (blkg->pd[pol->plid]) @@ -1661,23 +1626,24 @@ int blkcg_activate_policy(struct gendisk *disk, con= st struct blkcg_policy *pol) if (pinned_blkg) blkg_put(pinned_blkg); blkg_get(blkg); pinned_blkg =3D blkg; =20 - spin_unlock_irq(&q->queue_lock); + mutex_unlock(&q->blkcg_mutex); =20 if (pd_prealloc) pol->pd_free_fn(pd_prealloc); pd_prealloc =3D pol->pd_alloc_fn(disk, blkg->blkcg, GFP_KERNEL); + mutex_lock(&q->blkcg_mutex); if (pd_prealloc) goto retry; else goto enomem; } =20 - spin_lock(&blkg->blkcg->lock); + spin_lock_irq(&blkg->blkcg->lock); =20 pd->blkg =3D blkg; pd->plid =3D pol->plid; WRITE_ONCE(blkg->pd[pol->plid], pd); =20 @@ -1686,17 +1652,16 @@ int blkcg_activate_policy(struct gendisk *disk, con= st struct blkcg_policy *pol) =20 if (pol->pd_online_fn) pol->pd_online_fn(pd); pd->online =3D true; =20 - spin_unlock(&blkg->blkcg->lock); + spin_unlock_irq(&blkg->blkcg->lock); } =20 __set_bit(pol->plid, q->blkcg_pols); ret =3D 0; =20 - 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) @@ -1732,15 +1697,12 @@ void blkcg_deactivate_policy(struct gendisk *disk, =20 if (queue_is_mq(q)) memflags =3D blk_mq_freeze_queue(q); =20 mutex_lock(&q->blkcg_mutex); - spin_lock_irq(&q->queue_lock); =20 __clear_bit(pol->plid, q->blkcg_pols); - spin_unlock_irq(&q->queue_lock); - blkcg_policy_teardown_pds(q, pol); mutex_unlock(&q->blkcg_mutex); =20 if (queue_is_mq(q)) blk_mq_unfreeze_queue(q, memflags); @@ -2162,23 +2124,34 @@ struct blkcg_gq *bio_blkg(struct bio *bio) { struct blkcg *blkcg =3D bio_blkcg(bio); struct gendisk *disk; struct request_queue *q; struct blkcg_gq *blkg; + int ret; =20 if (!blkcg || !bio->bi_bdev) return NULL; =20 if (bio_flagged(bio, BIO_BLKG_REF)) return bio_pinned_blkg(bio); =20 disk =3D bio->bi_bdev->bd_disk; q =3D disk->queue; =20 - spin_lock_irq(&q->queue_lock); + rcu_read_lock(); + blkg =3D blkg_lookup(blkcg, q); + if (blkg) + blkg =3D blkg_lookup_tryget(blkg); + rcu_read_unlock(); + if (blkg) { + bio_set_blkg_ref(bio, blkg); + return blkg; + } + + mutex_lock(&q->blkcg_mutex); blkg =3D blkg_lookup_create(blkcg, disk); - spin_unlock_irq(&q->queue_lock); + mutex_unlock(&q->blkcg_mutex); =20 bio_set_blkg_ref(bio, blkg); return blkg; } EXPORT_SYMBOL_GPL(bio_blkg); diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 1925420154c1..dcba4eda9826 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -68,11 +68,11 @@ struct blkcg_gq { struct blkcg_gq *parent; =20 /* reference count */ struct percpu_ref refcnt; =20 - /* 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; =20 struct blkg_iostat_set __percpu *iostat_cpu; struct blkg_iostat_set iostat; =20 @@ -229,13 +229,13 @@ struct blkg_conf_ctx { void blkg_conf_init(struct blkg_conf_ctx *ctx, char *input); 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); =20 /** * bio_issue_as_root_blkg - see if this bio needs to be issued as root blkg @@ -388,13 +388,12 @@ static inline void bio_clear_blkcg(struct bio *bio) * @d_blkg: loop cursor pointing to the current descendant * @pos_css: used for iteration * @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. 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) \ css_for_each_descendant_pre((pos_css), &(p_blkg)->blkcg->css) \ if (((d_blkg) =3D blkg_lookup(css_to_blkcg(pos_css), \ diff --git a/block/blk-iocost.c b/block/blk-iocost.c index 57f2b4d4af20..31419add4340 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -2773,11 +2773,12 @@ static void ioc_rqos_throttle(struct rq_qos *rqos, = struct bio *bio) } =20 static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, struct bio *bio) { - struct ioc_gq *iocg =3D blkg_to_iocg(bio_blkg(bio)); + struct blkcg_gq *blkg =3D bio_blkg_lookup(rq->bio); + struct ioc_gq *iocg =3D blkg_to_iocg(blkg); struct ioc *ioc =3D rqos_to_ioc(rqos); sector_t bio_end =3D bio_end_sector(bio); struct ioc_now now; u64 vtime, abs_cost, cost; unsigned long flags; @@ -3152,10 +3153,11 @@ static ssize_t ioc_weight_write(struct kernfs_open_= file *of, char *buf, struct blkcg *blkcg =3D css_to_blkcg(of_css(of)); struct ioc_cgrp *iocc =3D blkcg_to_iocc(blkcg); struct blkg_conf_ctx ctx; struct ioc_now now; struct ioc_gq *iocg; + unsigned long flags; u32 v; int ret; =20 if (!strchr(buf, ':')) { struct blkcg_gq *blkg; @@ -3204,15 +3206,15 @@ static ssize_t ioc_weight_write(struct kernfs_open_= file *of, char *buf, goto unprep; if (v < CGROUP_WEIGHT_MIN || v > CGROUP_WEIGHT_MAX) goto unprep; } =20 - spin_lock(&iocg->ioc->lock); + spin_lock_irqsave(&iocg->ioc->lock, flags); iocg->cfg_weight =3D v * WEIGHT_ONE; ioc_now(iocg->ioc, &now); weight_updated(iocg, &now); - spin_unlock(&iocg->ioc->lock); + spin_unlock_irqrestore(&iocg->ioc->lock, flags); =20 ret =3D 0; =20 unprep: blkg_conf_unprep(&ctx); diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c index 7220edafd72b..b4bed73b645b 100644 --- a/block/blk-iolatency.c +++ b/block/blk-iolatency.c @@ -640,10 +640,11 @@ static void blkcg_iolatency_exit(struct rq_qos *rqos) struct blk_iolatency *blkiolat =3D BLKIOLATENCY(rqos); =20 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); } =20 static const struct rq_qos_ops blkcg_iolatency_ops =3D { .throttle =3D blkcg_iolatency_throttle, @@ -812,20 +813,22 @@ static void iolatency_set_min_lat_nsec(struct blkcg_g= q *blkg, u64 val) static void iolatency_clear_scaling(struct blkcg_gq *blkg) { if (blkg->parent) { struct iolatency_grp *iolat =3D blkg_to_lat(blkg->parent); struct child_latency_info *lat_info; + unsigned long flags; + if (!iolat) return; =20 lat_info =3D &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 =3D 0; lat_info->scale_grp =3D NULL; lat_info->scale_lat =3D 0; - spin_unlock(&lat_info->lock); + spin_unlock_irqrestore(&lat_info->lock, flags); } } =20 static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 2ff30700e84e..045eeab38646 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1775,14 +1775,14 @@ void blk_throtl_cancel_bios(struct gendisk *disk) int rw; =20 if (!blk_throtl_activated(q)) return; =20 - 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. */ rcu_read_lock(); blkg_for_each_descendant_post(blkg, pos_css, q->root_blkg) { @@ -1795,12 +1795,12 @@ void blk_throtl_cancel_bios(struct gendisk *disk) * del_gendisk. */ 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); =20 for (rw =3D READ; rw <=3D WRITE; rw++) { struct bio *bio; while ((bio =3D bio_list_pop(&cancel_bios[rw]))) bio_io_error(bio); --=20 2.51.0 From nobody Mon Sep 28 09:59:42 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 03D0938331B; Sun, 23 Aug 2026 15:30:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499008; cv=none; b=f5MCwCw/45e1NFrIw7vurm38NJ+F9SFulz7tTN9qAS2BK89Dw2uwQT2y71clJGfQ9//IZEtLH0JiosZgpkR2q5bDs6bzpeVV50XBAlNugZ/mQc+ncQCnGv2buxWpxfA8Nvqmg3lSUOgX/VIR+XMZ0mxjT/Nol1AgTtIQJZpfhE8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499008; c=relaxed/simple; bh=1H++vHCuZ3wTy09c/SM9msODiffQlxRb89vI7PIU3DE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=u9VJL1Cp5zUz3olEHnv6G7BtiujKt9CZo5jOHTagWk0hdURgP641uPtXMFhXegFnInhYc9v6nlLLq2jZm/Dkh9H2SXnWk75VG2eGd3LyAu6AiT1Es4oQB8FLRRoNHi9f9PFrxodWftJbpAf355PtHrs1fFZE6xebfZxocYgueA8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=i032EMwa; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="i032EMwa" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0D5611F000E9; Sun, 23 Aug 2026 15:29:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787499003; bh=1IQwcC3opStUF2dM1dI8wiAomf7Ot1O3sOeHkKWVCi4=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=i032EMwafzEf+D5w6IFnJ0vDy+/9eY4EsGscTOg7iIlppboJjnP3/Y9bS5eb0yVq7 vNCoCqMWP87dLb9/KToAe5RbbnbUoNmX3jmBZBOB5F4aRsYYkWsxcQKY6WXT/6Soh1 BDvb/pgJlPlqnpftDkyraBgdBEvRliXtwUazLWVtJdrTEm62jH65rHS7fy0XWQ/yMv keLfJXsGQdf+7+TLXTawYFuihg1y5ORc1sthz6mltBs51s5CE/9iqdqVMEoub+KlBy 4BkiK2dgLbIiOlexALS89b1nRL6mH75b83rkjiOPmO+DPXC45pd9r5s8jzc81Xw/7v KFwJubfoGDaOQ== From: Yu Kuai To: Jens Axboe , Tejun Heo , Josef Bacik , Sebastian Andrzej Siewior , Clark Williams , Steven Rostedt Cc: Yu Kuai , Christoph Hellwig , Nilay Shroff , Tao Cui , Hannes Reinecke , linux-block@vger.kernel.org, cgroups@vger.kernel.org, linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [RFC PATCH v3 4/6] blk-cgroup: allocate blkgs in blkg_create Date: Sun, 23 Aug 2026 23:29:23 +0800 Message-ID: <20260823152926.1043863-5-yukuai@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260823152926.1043863-1-yukuai@kernel.org> References: <20260823152926.1043863-1-yukuai@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Yu Kuai 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 policy path added later will use GFP_ATOMIC. Signed-off-by: Yu Kuai --- block/blk-cgroup.c | 48 +++++++++++++--------------------------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 0f34a80a726d..33fba781017b 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -383,37 +383,29 @@ static struct blkcg_gq *blkg_alloc(struct blkcg *blkc= g, struct gendisk *disk, out_free_blkg: kfree(blkg); return NULL; } =20 -/* - * 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 *d= isk, - struct blkcg_gq *new_blkg) + gfp_t gfp_mask) { - struct blkcg_gq *blkg; + struct blkcg_gq *blkg =3D NULL; int i, ret; =20 lockdep_assert_held(&disk->queue->blkcg_mutex); =20 /* request_queue is dying, do not create/recreate a blkg */ if (blk_queue_dying(disk->queue)) { ret =3D -ENODEV; goto err_free_blkg; } =20 - /* allocate */ - if (!new_blkg) { - new_blkg =3D blkg_alloc(blkcg, disk, GFP_NOWAIT); - if (unlikely(!new_blkg)) { - ret =3D -ENOMEM; - goto err_free_blkg; - } + blkg =3D blkg_alloc(blkcg, disk, gfp_mask); + if (unlikely(!blkg)) { + ret =3D -ENOMEM; + goto err_free_blkg; } - blkg =3D new_blkg; =20 /* link parent */ if (blkcg_parent(blkcg)) { rcu_read_lock(); blkg->parent =3D blkg_lookup(blkcg_parent(blkcg), disk->queue); @@ -461,12 +453,12 @@ static struct blkcg_gq *blkg_create(struct blkcg *blk= cg, struct gendisk *disk, /* @blkg failed fully initialized, use the usual release path */ percpu_ref_kill(&blkg->refcnt); return ERR_PTR(ret); =20 err_free_blkg: - if (new_blkg) - blkg_free(new_blkg); + if (blkg) + blkg_free(blkg); return ERR_PTR(ret); } =20 /* * The root blkg holds a live reference while the disk is active, so walki= ng @@ -531,11 +523,11 @@ static struct blkcg_gq *blkg_lookup_create(struct blk= cg *blkcg, pos =3D parent; parent =3D blkcg_parent(parent); } rcu_read_unlock(); =20 - blkg =3D blkg_create(pos, disk, NULL); + blkg =3D blkg_create(pos, disk, GFP_NOIO); if (IS_ERR(blkg)) { blkg =3D ret_blkg; break; } if (pos =3D=3D blkcg) @@ -865,39 +857,29 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct = blkcg_policy *pol, * non-root blkgs have access to their parents. */ while (true) { struct blkcg *pos =3D blkcg; struct blkcg *parent; - struct blkcg_gq *new_blkg; =20 parent =3D blkcg_parent(blkcg); rcu_read_lock(); while (parent && !blkg_lookup(parent, q)) { pos =3D parent; parent =3D blkcg_parent(parent); } rcu_read_unlock(); =20 - new_blkg =3D blkg_alloc(pos, disk, GFP_NOIO); - if (unlikely(!new_blkg)) { - ret =3D -ENOMEM; - goto fail_unlock; - } - if (!blkcg_policy_enabled(q, pol)) { - blkg_free(new_blkg); ret =3D -EOPNOTSUPP; goto fail_unlock; } =20 rcu_read_lock(); blkg =3D blkg_lookup(pos, q); rcu_read_unlock(); - if (blkg) { - blkg_free(new_blkg); - } else { - blkg =3D blkg_create(pos, disk, new_blkg); + if (!blkg) { + blkg =3D blkg_create(pos, disk, GFP_NOIO); if (IS_ERR(blkg)) { ret =3D PTR_ERR(blkg); goto fail_unlock; } } @@ -1466,27 +1448,23 @@ void blkg_exit_queue(struct request_queue *q) } =20 int blkcg_init_disk(struct gendisk *disk) { struct request_queue *q =3D disk->queue; - struct blkcg_gq *new_blkg, *blkg; + struct blkcg_gq *blkg; =20 /* * If the queue is shared across disk rebind (e.g., SCSI), the * previous disk's blkcg state is cleaned up asynchronously via * disk_release() -> blkcg_exit_disk(). Wait for all old blkgs to be * removed from the queue list before setting up new blkcg state. */ wait_var_event(&q->blkg_list, list_empty_careful(&q->blkg_list)); =20 - new_blkg =3D blkg_alloc(&blkcg_root, disk, GFP_KERNEL); - if (!new_blkg) - return -ENOMEM; - /* Make sure the root blkg exists. */ mutex_lock(&q->blkcg_mutex); - blkg =3D blkg_create(&blkcg_root, disk, new_blkg); + blkg =3D blkg_create(&blkcg_root, disk, GFP_KERNEL); if (IS_ERR(blkg)) goto err_unlock; q->root_blkg =3D blkg; mutex_unlock(&q->blkcg_mutex); =20 --=20 2.51.0 From nobody Mon Sep 28 09:59:42 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 62B3D38BF7F; Sun, 23 Aug 2026 15:30:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499015; cv=none; b=ccS4ywHwehd3oLoqK6ujG/P0UH7YU1rvMi+69v4fujdDExI2RyjDxpXc7KcK2NcobgiWchY6meVZkSlDadIGHn1RUUoOzRk/Ee173GSQYnKG8Hc3FFA6o2n9FBCktjAT+aULnD44Ohv+3xAkkEQI6vWjDBWuSeLgpx1a9555X3o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499015; c=relaxed/simple; bh=lZLXyzGI3BPPufsSN+QptIf0HcFnvcIkm/66+VsJYs8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=RqL9qsY2W1sh4HbZUto9s+LWXGeCkr/rN74YQmrgE4zprMGTC/+lDh7yJdRXpAPjdYLLPKgb2UcGSt8AjCh9bhLAwxeEPlxpbTFGBtlY4aQzMmZA4FKtI9LnTY4/sQTGzswji0ljjRBAnaucB2Qiz7zuKjusZMBD3bpgp3JctyU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=MT7emHK4; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="MT7emHK4" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 35E711F00A3D; Sun, 23 Aug 2026 15:30:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787499009; bh=QZpqZ1N6Q8eI05SmOt+SeVXizm3rCf2SBy2S4yV0Ajk=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=MT7emHK4/o8bo/uYiquUF2bNOXCMkt/ntv6GyPL/3y6sqhQOs8joOq2kImmAkja04 3k47GPz4NCjcPgyC7ACWsauTcUgKrcy8Gg3vQVnR3nCRvuQyszKP+4+PfnLC3pSFaP GBmo8U7zN+KzhPk2wiSNbTcczc7K57Byt4i0V4fxKmfrvR0KwgP/z4Gb0oAHqL6kUf rVxSvLAsu/3RiI/0N+kWYZdOZJeTPfy1JbNWwSl947uSTB+t1kBFXP13/ZIp5yKQBf lll5JmHLIDhPc1wRz4Dp7Oi+3Y+gg8WlfTI9JuBK75HtLiUgg4bOTnD/G4OEMi0ejK KI9rh/34WTcrw== From: Yu Kuai To: Jens Axboe , Tejun Heo , Josef Bacik , Sebastian Andrzej Siewior , Clark Williams , Steven Rostedt Cc: Yu Kuai , Christoph Hellwig , Nilay Shroff , Tao Cui , Hannes Reinecke , linux-block@vger.kernel.org, cgroups@vger.kernel.org, linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [RFC PATCH v3 5/6] blk-cgroup: share blkg creation between lookup and config prep Date: Sun, 23 Aug 2026 23:29:24 +0800 Message-ID: <20260823152926.1043863-6-yukuai@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260823152926.1043863-1-yukuai@kernel.org> References: <20260823152926.1043863-1-yukuai@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Yu Kuai 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 --- block/blk-cgroup.c | 77 +++++++++++++--------------------------------- 1 file changed, 21 insertions(+), 56 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 33fba781017b..31afb433ab18 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -473,34 +473,36 @@ static struct blkcg_gq *blkg_lookup_tryget(struct blk= cg_gq *blkg) =20 /** * 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 closest blkg with an extra reference acquired. If - * blkg_create() fails while walking down from root, the returned blkg may - * belong to an ancestor of @blkcg. This function never returns %NULL. + * 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. + * The returned blkg does not have an extra reference acquired. */ -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 =3D disk->queue; struct blkcg_gq *blkg; =20 lockdep_assert_held(&q->blkcg_mutex); =20 rcu_read_lock(); blkg =3D blkg_lookup(blkcg, q); if (blkg) { - blkg =3D blkg_lookup_tryget(blkg); + *blkgp =3D blkg; rcu_read_unlock(); - return blkg; + return 0; } rcu_read_unlock(); =20 /* * Create blkgs walking down from blkcg_root to @blkcg, so that all @@ -523,20 +525,20 @@ static struct blkcg_gq *blkg_lookup_create(struct blk= cg *blkcg, pos =3D parent; parent =3D blkcg_parent(parent); } rcu_read_unlock(); =20 - blkg =3D blkg_create(pos, disk, GFP_NOIO); + blkg =3D blkg_create(pos, disk, gfp_mask); if (IS_ERR(blkg)) { - blkg =3D ret_blkg; - break; + *blkgp =3D ret_blkg; + return PTR_ERR(blkg); + } + if (pos =3D=3D blkcg) { + *blkgp =3D blkg; + return 0; } - if (pos =3D=3D blkcg) - break; } - - return blkg_lookup_tryget(blkg); } =20 static void blkg_destroy(struct blkcg_gq *blkg) { struct blkcg *blkcg =3D blkg->blkcg; @@ -844,52 +846,14 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct = blkcg_policy *pol, if (!blkcg_policy_enabled(q, pol)) { ret =3D -EOPNOTSUPP; goto fail_unlock; } =20 - rcu_read_lock(); - blkg =3D blkg_lookup(blkcg, q); - rcu_read_unlock(); - 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 =3D blkcg; - struct blkcg *parent; - - parent =3D blkcg_parent(blkcg); - rcu_read_lock(); - while (parent && !blkg_lookup(parent, q)) { - pos =3D parent; - parent =3D blkcg_parent(parent); - } - rcu_read_unlock(); - - if (!blkcg_policy_enabled(q, pol)) { - ret =3D -EOPNOTSUPP; - goto fail_unlock; - } - - rcu_read_lock(); - blkg =3D blkg_lookup(pos, q); - rcu_read_unlock(); - if (!blkg) { - blkg =3D blkg_create(pos, disk, GFP_NOIO); - if (IS_ERR(blkg)) { - ret =3D PTR_ERR(blkg); - goto fail_unlock; - } - } + ret =3D blkg_lookup_create(blkcg, disk, GFP_NOIO, &blkg); + if (ret) + goto fail_unlock; =20 - if (pos =3D=3D blkcg) - goto success; - } -success: ctx->blkg =3D blkg; return 0; =20 fail_unlock: mutex_unlock(&q->blkcg_mutex); @@ -2124,11 +2088,12 @@ struct blkcg_gq *bio_blkg(struct bio *bio) bio_set_blkg_ref(bio, blkg); return blkg; } =20 mutex_lock(&q->blkcg_mutex); - blkg =3D blkg_lookup_create(blkcg, disk); + blkg_lookup_create(blkcg, disk, GFP_NOIO, &blkg); + blkg =3D blkg_lookup_tryget(blkg); mutex_unlock(&q->blkcg_mutex); =20 bio_set_blkg_ref(bio, blkg); return blkg; } --=20 2.51.0 From nobody Mon Sep 28 09:59:42 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ADB0E3803C7; Sun, 23 Aug 2026 15:30:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499022; cv=none; b=l9eHt1SjdAM+m7EPwBbt+xn8h4J8NvmtVfA3AjmY+9jMybEzTSodovfm4TpkG2kImzmD3OSVmCRlnTaxX5y2/goW1hpnNhhw6JfhSIR73G7qAXATv9QOdmAppcpZw+od2p1q5TIU4y/IRjLpPXt6GMB2MfrtcfTaRK1Gao7xXMs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787499022; c=relaxed/simple; bh=4WVHel4pOdtiOTMyEKBQUpuaFKs5xSVxjmqmqLqy7vk=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=eNPQ5Xo4YzumkoOoXZii3P0B4TjuOd/Bd9fox+vuIDxThr8S11rfuEPd2idbsD6bXM6eMW64E91XeB9uKmeOaxmxr4FMfjtwz7I7rWNiTUVS+nZBT9aHkEwrLrCo+/RnWzHKXXj4V/UuwnwsAnsf6Pih6WaCXYyfchfyC5YaHxA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=bkLp3g46; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="bkLp3g46" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B4E5F1F000E9; Sun, 23 Aug 2026 15:30:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787499018; bh=+2h2b9GZqMfRQ8idPJK/91YPZMHFjGy6V7Lv0AmS1VE=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=bkLp3g462ZQuOKw3UmciKrFB1EBx5ynpzxxKW1G/wfmMOaBkcFczqrT6Jp4A8fehN dEMd5QR7ZkbQIlgwnMoGz2MT5tdIN7/dh+/cjKbpYwai+V9tocU2aNMLN5/lzXrR/a sDaWUu1gGXmeHUTZG8HXoMn+trStGH4HgxPB1ZsnPpTGGpQjYA+aFOdEk0VE6zB2Un O2tJx5hdTngCJfK1MmUsT5uc9A3ASBsaNqorjeLamJR4XEPqcCZ7qZ115nTAXiu0Xg 9Zu3YSG7rzaJ54HGUKEQfm5g/D6krMNy4zWsu1j1b0C9B0T8r2/A+6A77/P7pB3F5y SrQA7urefmyKw== From: Yu Kuai To: Jens Axboe , Tejun Heo , Josef Bacik , Sebastian Andrzej Siewior , Clark Williams , Steven Rostedt Cc: Yu Kuai , Christoph Hellwig , Nilay Shroff , Tao Cui , Hannes Reinecke , linux-block@vger.kernel.org, cgroups@vger.kernel.org, linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [RFC PATCH v3 6/6] blk-cgroup: make policy blkg creation nowait-safe Date: Sun, 23 Aug 2026 23:29:25 +0800 Message-ID: <20260823152926.1043863-7-yukuai@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260823152926.1043863-1-yukuai@kernel.org> References: <20260823152926.1043863-1-yukuai@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Yu Kuai bio_blkg() is called by blkcg policy paths when they need a queue-local blkg. Keep that allocation lazy instead of preparing every REQ_NOWAIT bio from submit_bio_noacct(). If a policy first needs a blkg for a REQ_NOWAIT bio, use mutex_trylock() and GFP_ATOMIC so the lookup never sleeps. If the mutex cannot be acquired, look up and pin the closest existing blkg in the hierarchy under RCU. The creation helper provides the same fallback if atomic allocation fails, so valid policy I/O always gets a blkg without blocking. Signed-off-by: Yu Kuai --- block/blk-cgroup.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 31afb433ab18..9895d6661070 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -28,10 +28,11 @@ #include #include #include #include #include +#include #include "blk.h" #include "blk-cgroup.h" #include "blk-ioprio.h" #include "blk-throttle.h" =20 @@ -469,10 +470,24 @@ static struct blkcg_gq *blkg_lookup_tryget(struct blk= cg_gq *blkg) while (!blkg_tryget(blkg)) blkg =3D blkg->parent; return blkg; } =20 +static struct blkcg_gq *blkg_lookup_closest(struct blkcg *blkcg, + struct request_queue *q) +{ + struct blkcg_gq *blkg; + + rcu_read_lock(); + while (!(blkg =3D blkg_lookup(blkcg, q))) + blkcg =3D blkcg_parent(blkcg); + blkg =3D blkg_lookup_tryget(blkg); + rcu_read_unlock(); + + return blkg; +} + /** * 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 @@ -2066,11 +2081,10 @@ struct blkcg_gq *bio_blkg(struct bio *bio) { struct blkcg *blkcg =3D bio_blkcg(bio); struct gendisk *disk; struct request_queue *q; struct blkcg_gq *blkg; - int ret; =20 if (!blkcg || !bio->bi_bdev) return NULL; =20 if (bio_flagged(bio, BIO_BLKG_REF)) @@ -2087,10 +2101,29 @@ struct blkcg_gq *bio_blkg(struct bio *bio) if (blkg) { bio_set_blkg_ref(bio, blkg); return blkg; } =20 + if (bio->bi_opf & REQ_NOWAIT) { + /* + * 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 cannot be acquired, skip allocation + * and pin the closest existing blkg instead. blkg_lookup_create() + * provides the same fallback if allocation fails. + */ + if (!preemptible() || !mutex_trylock(&q->blkcg_mutex)) { + blkg =3D blkg_lookup_closest(blkcg, q); + } else { + blkg_lookup_create(blkcg, disk, GFP_ATOMIC, &blkg); + blkg =3D blkg_lookup_tryget(blkg); + mutex_unlock(&q->blkcg_mutex); + } + bio_set_blkg_ref(bio, blkg); + return blkg; + } + mutex_lock(&q->blkcg_mutex); blkg_lookup_create(blkcg, disk, GFP_NOIO, &blkg); blkg =3D blkg_lookup_tryget(blkg); mutex_unlock(&q->blkcg_mutex); =20 --=20 2.51.0