* [PATCH v4] loop: Fix NULL pointer dereference in lo_rw_aio()
From: Tetsuo Handa @ 2026-06-07 10:54 UTC (permalink / raw)
To: Jens Axboe
Cc: Bart Van Assche, Christoph Hellwig, Damien Le Moal, Ming Lei,
linux-block, LKML, Andrew Morton, Linus Torvalds, linux-btrfs,
David Sterba, linux-fsdevel, Christian Brauner, Hillf Danton
In-Reply-To: <b27609f0-59f0-403d-90af-274c55df817e@I-love.SAKURA.ne.jp>
syzbot is reporting NULL pointer dereference in lo_rw_aio() [1][2].
An analysis by the Gemini AI collaborator [3] considers that this problem
is caused by a timing shift primarily exposed by commit 65565ca5f99b
("block: unify the synchronous bi_end_io callbacks"), along with helper
refactorings like commit 92c3737a2473 ("block: add a bio_submit_or_kill
helper").
But due to difficulty of reproducing this race, discussion about what is
happening and how to fix this problem is stalling. Also, we haven't
identified how many filesystems are subjected to this problem.
Therefore, this patch introduces a grace period for flushing pending I/O
requests (which should be a good thing from the perspective of defensive
programming) so that we won't hit NULL pointer dereference problem, and
also emits BUG: message in order to help filesystem developers identify
the caller of an I/O request that failed to wait for completion so that
filesystem developers can fix such caller to wait for completion.
Note that emitting BUG: message is enabled only if CONFIG_KCOV=y, for
this check is a waste of computation resources for almost all users.
Link: https://syzkaller.appspot.com/bug?extid=cd8a9a308e879a4e2c28 [1]
Link: https://syzkaller.appspot.com/bug?extid=bc273027d5643e48e5b3 [2]
Link: https://lkml.kernel.org/r/fbb3edda-f108-4e5b-acf2-266f043f8125@I-love.SAKURA.ne.jp [3]
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/block/loop.c | 82 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 80 insertions(+), 2 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..4ff254d8b623 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -85,8 +85,26 @@ struct loop_cmd {
struct bio_vec *bvec;
struct cgroup_subsys_state *blkcg_css;
struct cgroup_subsys_state *memcg_css;
+#ifdef CONFIG_KCOV
+ unsigned long stack_entries[30];
+ int stack_nr;
+ pid_t pid;
+ char comm[TASK_COMM_LEN];
+#endif
};
+static void loop_check_io_race(struct loop_device *lo, struct loop_cmd *cmd)
+{
+#ifdef CONFIG_KCOV
+ if (unlikely(data_race(READ_ONCE(lo->lo_state)) == Lo_rundown)) {
+ pr_err("BUG: %s/%u is doing I/O request on loop%d in Lo_rundown state.\n",
+ cmd->comm, cmd->pid, lo->lo_number);
+ printk("Call trace:\n");
+ stack_trace_print(cmd->stack_entries, cmd->stack_nr, 4);
+ }
+#endif
+}
+
#define LOOP_IDLE_WORKER_TIMEOUT (60 * HZ)
#define LOOP_DEFAULT_HW_Q_DEPTH 128
@@ -1747,8 +1765,59 @@ static void lo_release(struct gendisk *disk)
need_clear = (lo->lo_state == Lo_rundown);
mutex_unlock(&lo->lo_mutex);
- if (need_clear)
+ if (need_clear) {
+ /*
+ * Temporarily release disk->open_mutex in order to flush pending I/O
+ * requests before clearing the backing device.
+ *
+ * This is a layering violation. But since bdev->bd_disk->fops->release()
+ * (which is mapped to lo_release()) is the final function which
+ * blkdev_put_whole() from bdev_release() calls immediately before
+ * releasing disk->open_mutex, this changes nothing except opens a new
+ * race window for allowing disk->fops->open() (which is mapped to
+ * lo_open()) to be called.
+ *
+ * Even if lo_open() is called from blkdev_get_whole() due to this race,
+ * the Lo_rundown state guarantees that lo_open() will fail with -ENXIO.
+ * Thus, there will be effectively no change caused by this violation.
+ */
+ mutex_unlock(&lo->lo_disk->open_mutex);
+ /*
+ * Now that loop_queue_rq() sees lo->lo_state != Lo_bound,
+ * wait for already started loop_queue_rq() to complete.
+ */
+ synchronize_rcu();
+ /*
+ * Now that no more works are scheduled by loop_queue_rq(),
+ * wait for already scheduled works to complete.
+ */
+ drain_workqueue(lo->workqueue);
+ /*
+ * Now that no more AIO requests are scheduled by lo_rw_aio(),
+ * wait for already started AIO to complete.
+ *
+ * Due to synchronize_rcu() + drain_workqueue() sequence above,
+ * calling blk_mq_unfreeze_queue() immediately after blk_mq_freeze_queue()
+ * returns has to be safe, for loop_queue_rq() no longer schedules new
+ * lo_rw_aio() works and lo_rw_aio() no longer submits new AIO requests.
+ *
+ * Deferring blk_mq_unfreeze_queue() does not help because we are about
+ * to clear the backing device and drop the refcount for the backing device.
+ * There is nothing we can do if blk_mq_freeze_queue() fails to flush.
+ */
+ blk_mq_unfreeze_queue(lo->lo_queue, blk_mq_freeze_queue(lo->lo_queue));
+ /*
+ * Perform remaining cleanup, with disk->open_mutex held.
+ *
+ * The lo->lo_state should remain Lo_rundown despite we temporarily
+ * released disk->open_mutex, for I am the only and the last user of
+ * this loop device because lo_open() cannot succeed.
+ */
+ mutex_lock(&lo->lo_disk->open_mutex);
+ if (WARN_ON(data_race(READ_ONCE(lo->lo_state)) != Lo_rundown))
+ return;
__loop_clr_fd(lo);
+ }
}
static void lo_free_disk(struct gendisk *disk)
@@ -1855,10 +1924,18 @@ static blk_status_t loop_queue_rq(struct blk_mq_hw_ctx *hctx,
struct loop_cmd *cmd = blk_mq_rq_to_pdu(rq);
struct loop_device *lo = rq->q->queuedata;
+#ifdef CONFIG_KCOV
+ cmd->stack_nr = stack_trace_save(cmd->stack_entries, ARRAY_SIZE(cmd->stack_entries), 0);
+ cmd->pid = current->pid;
+ get_task_comm(cmd->comm, current);
+#endif
+
blk_mq_start_request(rq);
- if (data_race(READ_ONCE(lo->lo_state)) != Lo_bound)
+ if (data_race(READ_ONCE(lo->lo_state)) != Lo_bound) {
+ loop_check_io_race(lo, cmd);
return BLK_STS_IOERR;
+ }
switch (req_op(rq)) {
case REQ_OP_FLUSH:
@@ -1901,6 +1978,7 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
int ret = 0;
struct mem_cgroup *old_memcg = NULL;
+ loop_check_io_race(lo, cmd);
if (write && (lo->lo_flags & LO_FLAGS_READ_ONLY)) {
ret = -EIO;
goto failed;
--
2.47.3
^ permalink raw reply related
* [PATCH v2] virtio-blk: clamp zone report to the report buffer capacity
From: Michael Bommarito @ 2026-06-07 12:48 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Stefan Hajnoczi, Jens Axboe
Cc: Xuan Zhuo, virtualization, linux-block, linux-kernel
virtblk_report_zones() trusts the device-reported number of zones when
walking the report buffer:
nz = min_t(u64, virtio64_to_cpu(vblk->vdev, report->nr_zones),
nr_zones);
...
for (i = 0; i < nz && zone_idx < nr_zones; i++) {
ret = virtblk_parse_zone(vblk, &report->zones[i], ...);
The buffer is allocated by virtblk_alloc_report_buffer(), whose size is
capped by the queue's max hardware sectors and max segments and can
therefore hold fewer descriptors than nr_zones. nz is bounded only by
the device-supplied report->nr_zones and the requested nr_zones, never
by the buffer's descriptor capacity. At probe time the request count is
unbounded (blk_revalidate_disk_zones() calls report_zones() with
nr_zones == UINT_MAX), so the device-supplied report->nr_zones is the
sole gate: a device that reports more zones than fit in the buffer
drives the loop to read report->zones[i] past the end of the allocation.
A malicious or buggy virtio-blk device that reports an inflated nr_zones
triggers this during zone revalidation at probe. KASAN reports a
vmalloc-out-of-bounds read in virtblk_report_zones() against the report
buffer allocated a few lines earlier.
Clamp nz to the number of descriptors that actually fit in the report
buffer.
Fixes: 95bfec41bd3d ("virtio-blk: add support for zoned block devices")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2: drop the explanatory comment per Michael S. Tsirkin's review; the
clamp itself is unchanged.
drivers/block/virtio_blk.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b1c9a27..32bf3ba 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -689,6 +689,8 @@ static int virtblk_report_zones(struct gendisk *disk, sector_t sector,
nz = min_t(u64, virtio64_to_cpu(vblk->vdev, report->nr_zones),
nr_zones);
+ nz = min_t(u64, nz,
+ (buflen - sizeof(*report)) / sizeof(report->zones[0]));
if (!nz)
break;
base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8
--
2.53.0
^ permalink raw reply related
* [PATCH v2] zram: fix partial I/O gating on non-4K PAGE_SIZE
From: Jianyue Wu @ 2026-06-07 15:31 UTC (permalink / raw)
To: minchan
Cc: Jianyue Wu, Sergey Senozhatsky, Jens Axboe, linux-kernel,
linux-block
IS_ENABLED() mainly for CONFIG_* symbols. ZRAM_PARTIAL_IO is a macro
defined as 1 on non-4K builds, so IS_ENABLED(ZRAM_PARTIAL_IO) becomes
IS_ENABLED(1) and evaluates false.
Replace that check with PAGE_SIZE == 4096 and fold is_partial_io() into
one helper so partial-I/O policy stays consistent. PAGE_SIZE is a
build-time constant, so the PAGE_SIZE == 4096 checks fold away on the
configurations where partial I/O is supported.
Tested-on: Raspberry Pi 5 (BCM2712, 4 KiB and 16 KiB page kernels)
Signed-off-by: Jianyue Wu <wujianyue000@gmail.com>
---
To: Minchan Kim <minchan@kernel.org>
To: Sergey Senozhatsky <senozhatsky@chromium.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org
Cc: linux-block@vger.kernel.org
zram: fix partial I/O gating on non-4K PAGE_SIZE
On PAGE_SIZE > 4K, zram writeback can use sub-page bvec I/O. The
synchronous read_from_bdev() path is used for that case.
v1 used IS_ENABLED(ZRAM_PARTIAL_IO) where ZRAM_PARTIAL_IO is a local
macro defined as 1, so the check expands to IS_ENABLED(1) and is always
false. The WARN_ON_ONCE(!IS_ENABLED(...)) guard then rejects the sync
path with -EIO.
Replace that check with PAGE_SIZE == 4096 and fold is_partial_io() into
one helper so partial-I/O policy stays consistent. PAGE_SIZE is a
build-time constant, so the PAGE_SIZE == 4096 checks fold away on the
configurations where partial I/O is supported.
Testing (Raspberry Pi 5, BCM2712, rpi-6.12.y):
16 KiB kernel (6.12.92-v8-16k+):
- full-page I/O: PASS
- sub-page I/O: PASS
- writeback-backed read: PASS (bd_reads=100)
- no zram WARNING in dmesg
4 KiB kernel (6.12.92-v8-4k+):
- full-page I/O: PASS
- sub-page I/O: PASS (regression; partial path not used by design)
- writeback-backed read: PASS (bd_reads=161)
- no WARN_ON_ONCE(PAGE_SIZE == 4096) in dmesg
Writeback tests use a loop block device as backing_dev. On 4 KiB
builds partial-path success is not required by design because
is_partial_io() is always false when PAGE_SIZE == 4096.
---
Changes in v2:
- Use PAGE_SIZE == 4096 for the read_from_bdev() guard.
- Fold is_partial_io() into one helper.
- Expand commit message with root cause, impact, and build-time note.
- Add Raspberry Pi 5 validation on 4 KiB and 16 KiB kernels.
- Link to v1: https://lore.kernel.org/r/20260531-zram-fix-partial-io-config-check-on-akpm-v1-1-eb085d98faea@gmail.com
Jianyue Wu (1):
zram: fix partial I/O gating on non-4K PAGE_SIZE
---
drivers/block/zram/zram_drv.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 6e1330ce4bc1..ddea09afd8dd 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -216,18 +216,12 @@ static bool zram_can_store_page(struct zram *zram)
return !zram->limit_pages || alloced_pages <= zram->limit_pages;
}
-#if PAGE_SIZE != 4096
static inline bool is_partial_io(struct bio_vec *bvec)
{
+ if (PAGE_SIZE == 4096)
+ return false;
return bvec->bv_len != PAGE_SIZE;
}
-#define ZRAM_PARTIAL_IO 1
-#else
-static inline bool is_partial_io(struct bio_vec *bvec)
-{
- return false;
-}
-#endif
#if defined CONFIG_ZRAM_WRITEBACK || defined CONFIG_ZRAM_MULTI_COMP
struct zram_pp_slot {
@@ -1510,7 +1504,8 @@ static int read_from_bdev(struct zram *zram, struct page *page, u32 index,
{
atomic64_inc(&zram->stats.bd_reads);
if (!parent) {
- if (WARN_ON_ONCE(!IS_ENABLED(ZRAM_PARTIAL_IO)))
+ /* Sub-page I/O only exists on non-4K PAGE_SIZE builds. */
+ if (WARN_ON_ONCE(PAGE_SIZE == 4096))
return -EIO;
return read_from_bdev_sync(zram, page, index, blk_idx);
}
---
base-commit: 404fb4f38e8f38469dfff4df0205c9d18eeb1f57
change-id: 20260531-zram-fix-partial-io-config-check-on-akpm-c62b972416f8
Best regards,
--
Jianyue Wu <wujianyue000@gmail.com>
^ permalink raw reply related
* Re: [PATCH] block: Add WQ_PERCPU to alloc_workqueue users
From: Marco Crivellari @ 2026-06-07 18:50 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-kernel, linux-block, Tejun Heo, Lai Jiangshan,
Frederic Weisbecker, Sebastian Andrzej Siewior, Michal Hocko,
Damien Le Moal
In-Reply-To: <178068011170.856067.204306158214952007.b4-ty@b4>
On Fri, Jun 5, 2026 at 7:21 PM Jens Axboe <axboe@kernel.dk> wrote:
>
>
> On Thu, 04 Jun 2026 12:53:47 +0200, Marco Crivellari wrote:
> > This continues the effort to refactor workqueue APIs, which began with
> > the introduction of new workqueues and a new alloc_workqueue flag in:
> >
> > commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
> > commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
> >
> > The refactoring is going to alter the default behavior of
> > alloc_workqueue() to be unbound by default.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] block: Add WQ_PERCPU to alloc_workqueue users
> commit: 7e712f292e7f01e91d09e83eb7b9526f77f66c71
>
Many thanks!
--
Marco Crivellari
SUSE Labs
^ permalink raw reply
* Re: [PATCH 2/5] bfq: protect q->blkg_list iteration in bfq_end_wr_async() with blkcg_mutex
From: yu kuai @ 2026-06-08 2:48 UTC (permalink / raw)
To: Nilay Shroff, Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Bart Van Assche, linux-block,
cgroups, linux-kernel, yukuai
In-Reply-To: <a532857a-16d5-4bef-bbd1-3bc080363182@linux.ibm.com>
Hi,
在 2026/6/5 1:31, Nilay Shroff 写道:
> On 6/3/26 6:57 PM, Yu Kuai wrote:
>> bfq_end_wr_async() iterates q->blkg_list while only holding bfqd->lock,
>> but not blkcg_mutex. This can race with blkg_free_workfn() that removes
>> blkgs from the list while holding blkcg_mutex.
>>
>> Add blkcg_mutex protection in bfq_end_wr() before taking bfqd->lock to
>> ensure proper synchronization when iterating q->blkg_list.
>>
>> Signed-off-by: Yu Kuai <yukuai@fygo.io>
>> ---
>> block/bfq-cgroup.c | 3 ++-
>> block/bfq-iosched.c | 6 ++++++
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
>> index 37ab70930c8d..f765e767d36a 100644
>> --- a/block/bfq-cgroup.c
>> +++ b/block/bfq-cgroup.c
>> @@ -939,11 +939,12 @@ void bfq_end_wr_async(struct bfq_data *bfqd)
>> struct blkcg_gq *blkg;
>> list_for_each_entry(blkg, &bfqd->queue->blkg_list, q_node) {
>> struct bfq_group *bfqg = blkg_to_bfqg(blkg);
>> - bfq_end_wr_async_queues(bfqd, bfqg);
>> + if (bfqg)
>> + bfq_end_wr_async_queues(bfqd, bfqg);
>> }
>> bfq_end_wr_async_queues(bfqd, bfqd->root_group);
>> }
>> static int bfq_io_show_weight_legacy(struct seq_file *sf, void *v)
>> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
>> index 141c602d5e85..42ccfd0c6140 100644
>> --- a/block/bfq-iosched.c
>> +++ b/block/bfq-iosched.c
>> @@ -2643,10 +2643,13 @@ void bfq_end_wr_async_queues(struct bfq_data
>> *bfqd,
>> static void bfq_end_wr(struct bfq_data *bfqd)
>> {
>> struct bfq_queue *bfqq;
>> int i;
>> +#ifdef CONFIG_BFQ_GROUP_IOSCHED
>> + mutex_lock(&bfqd->queue->blkcg_mutex);
>> +#endif
>> spin_lock_irq(&bfqd->lock);
>> for (i = 0; i < bfqd->num_actuators; i++) {
>> list_for_each_entry(bfqq, &bfqd->active_list[i], bfqq_list)
>> bfq_bfqq_end_wr(bfqq);
>> @@ -2654,10 +2657,13 @@ static void bfq_end_wr(struct bfq_data *bfqd)
>> list_for_each_entry(bfqq, &bfqd->idle_list, bfqq_list)
>> bfq_bfqq_end_wr(bfqq);
>> bfq_end_wr_async(bfqd);
>> spin_unlock_irq(&bfqd->lock);
>> +#ifdef CONFIG_BFQ_GROUP_IOSCHED
>> + mutex_unlock(&bfqd->queue->blkcg_mutex);
>> +#endif
>> }
>
> The above change protects the q->blkg_list iteration in
> bfq_end_wr_async()
> against list removal in blkg_free_workfn(). However the blkg insertion in
> blkg_create() still doesn't use q->blkcg_mutex and so list traversal in
> bfq_end_wr_async() may still race with blkg_create().
>
> So I think we may also need to protect blkg insert in blkg_create() using
> q->blkcg_mutex.
Yes, this is done in another huge patchset, because currently blkg_create()
is protected by queue_lock and can be called under rcu, code refactor will
be required.
I'll send the first set soon.
>
> Thanks,
> --Nilay
--
Thanks,
Kuai
^ permalink raw reply
* [PATCH 0/8] blk-cgroup: remove queue_lock nesting from blkcg paths
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
From: Yu Kuai <yukuai@fygo.io>
Hi,
This series is the follow-up blk-cgroup locking cleanup on top of the
earlier blkg-list protection fixes, and prepares blk-cgroup to stop using
q->queue_lock as the global blkg lifetime/iteration lock.
The current queue_lock based protection is hard to maintain because
queue_lock is used from hardirq and softirq completion paths, while some
blkcg cgroup file paths also need to iterate blkgs, print policy data, or
create blkgs from RCU-protected contexts. This series first tightens the
blkcg-side lifetime rules:
- blkcg_print_stat() iterates blkgs under blkcg->lock with IRQs disabled.
- policy data freeing is delayed past an RCU grace period.
- blkcg_print_blkgs(), blkg lookup/create, bio association, page-IO
association, blkg destruction, and BFQ initialization stop nesting
queue_lock under RCU or blkcg->lock.
Using blkcg->lock and RCU for blkcg-owned lists/data keeps the lock order
local to blk-cgroup and avoids extending queue_lock into cgroup file
iteration paths. It also makes the subsequent conversion to q->blkcg_mutex
possible without carrying forward queue_lock's interrupt-context
constraints.
Yu Kuai (8):
blk-cgroup: protect iterating blkgs with blkcg->lock in
blkcg_print_stat()
blk-cgroup: delay freeing policy data after rcu grace period
blk-cgroup: don't nest queue_lock under rcu in blkcg_print_blkgs()
blk-cgroup: don't nest queue_lock under rcu in blkg_lookup_create()
blk-cgroup: don't nest queue_lock under rcu in bio_associate_blkg()
blk-cgroup: don't nest queue_lock under blkcg->lock in
blkcg_destroy_blkgs()
mm/page_io: don't nest queue_lock under rcu in
bio_associate_blkg_from_page()
block, bfq: don't grab queue_lock to initialize bfq
block/bfq-cgroup.c | 17 ++++-
block/bfq-iosched.c | 5 --
block/blk-cgroup-rwstat.c | 15 ++--
block/blk-cgroup.c | 151 ++++++++++++++++++++++----------------
block/blk-cgroup.h | 8 +-
block/blk-iocost.c | 22 ++++--
block/blk-iolatency.c | 10 ++-
block/blk-throttle.c | 13 +++-
mm/page_io.c | 7 +-
9 files changed, 158 insertions(+), 90 deletions(-)
base-commit: b23df513de562739af61fa61ba80ef5e8059a636
--
2.51.0
^ permalink raw reply
* [PATCH 1/8] blk-cgroup: protect iterating blkgs with blkcg->lock in blkcg_print_stat()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
blkcg_print_one_stat() will be called for each blkg:
- access blkg->iostat, which is freed from rcu callback
blkg_free_workfn();
- access policy data from pd_stat_fn(), which is freed from
pd_free_fn(), while pd_free_fn() can be called by removing blkcg or
deactivating policy;
Take blkcg->lock while iterating so the blkgs stay online and both
blkg->iostat and policy data for activated policies stay valid. Use
irq-safe locking because blkcg->lock can be nested under q->queue_lock,
which is used from IRQ completion paths.
Prepare to convert protecting blkgs from request_queue with mutex.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index c75b2a103bbc..b55c43f72bcb 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1241,17 +1241,14 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
if (!seq_css(sf)->parent)
blkcg_fill_root_iostats();
else
css_rstat_flush(&blkcg->css);
- rcu_read_lock();
- hlist_for_each_entry_rcu(blkg, &blkcg->blkg_list, blkcg_node) {
- spin_lock_irq(&blkg->q->queue_lock);
+ guard(spinlock_irq)(&blkcg->lock);
+ hlist_for_each_entry(blkg, &blkcg->blkg_list, blkcg_node)
blkcg_print_one_stat(blkg, sf);
- spin_unlock_irq(&blkg->q->queue_lock);
- }
- rcu_read_unlock();
+
return 0;
}
static struct cftype blkcg_files[] = {
{
--
2.51.0
^ permalink raw reply related
* [PATCH 2/8] blk-cgroup: delay freeing policy data after rcu grace period
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
Currently blkcg_print_blkgs() must hold RCU to iterate blkgs from a
blkcg, and prfill() must hold queue_lock to prevent policy data from
being freed by policy deactivation. As a consequence, queue_lock has to
be nested under RCU from blkcg_print_blkgs().
Delay freeing policy data until after an RCU grace period so prfill() can
be protected by RCU alone.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-cgroup.c | 9 ++++++++-
block/blk-cgroup.h | 2 ++
block/blk-iocost.c | 14 ++++++++++++--
block/blk-iolatency.c | 10 +++++++++-
block/blk-throttle.c | 13 +++++++++++--
5 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index f765e767d36a..56f60e36c799 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -548,17 +548,24 @@ static void bfq_pd_init(struct blkg_policy_data *pd)
bfqg->active_entities = 0;
bfqg->num_queues_with_pending_reqs = 0;
bfqg->rq_pos_tree = RB_ROOT;
}
-static void bfq_pd_free(struct blkg_policy_data *pd)
+static void bfqg_release(struct rcu_head *rcu)
{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
struct bfq_group *bfqg = pd_to_bfqg(pd);
bfqg_put(bfqg);
}
+static void bfq_pd_free(struct blkg_policy_data *pd)
+{
+ call_rcu(&pd->rcu_head, bfqg_release);
+}
+
static void bfq_pd_reset_stats(struct blkg_policy_data *pd)
{
struct bfq_group *bfqg = pd_to_bfqg(pd);
bfqg_stats_reset(&bfqg->stats);
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index 1cce3294634d..fd206d1fa3c9 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -138,10 +138,12 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css)
struct blkg_policy_data {
/* the blkg and policy id this per-policy data belongs to */
struct blkcg_gq *blkg;
int plid;
bool online;
+
+ struct rcu_head rcu_head;
};
/*
* Policies that need to keep per-blkcg data which is independent from any
* request_queue associated to it should implement cpd_alloc/free_fn()
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 0cca88a366dc..c136b1f46fcc 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3024,10 +3024,20 @@ static void ioc_pd_init(struct blkg_policy_data *pd)
spin_lock_irqsave(&ioc->lock, flags);
weight_updated(iocg, &now);
spin_unlock_irqrestore(&ioc->lock, flags);
}
+static void iocg_release(struct rcu_head *rcu)
+{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
+ struct ioc_gq *iocg = pd_to_iocg(pd);
+
+ free_percpu(iocg->pcpu_stat);
+ kfree(iocg);
+}
+
static void ioc_pd_free(struct blkg_policy_data *pd)
{
struct ioc_gq *iocg = pd_to_iocg(pd);
struct ioc *ioc = iocg->ioc;
unsigned long flags;
@@ -3048,12 +3058,12 @@ static void ioc_pd_free(struct blkg_policy_data *pd)
spin_unlock_irqrestore(&ioc->lock, flags);
hrtimer_cancel(&iocg->waitq_timer);
}
- free_percpu(iocg->pcpu_stat);
- kfree(iocg);
+
+ call_rcu(&pd->rcu_head, iocg_release);
}
static void ioc_pd_stat(struct blkg_policy_data *pd, struct seq_file *s)
{
struct ioc_gq *iocg = pd_to_iocg(pd);
diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index 53e8dd2dfa8a..c79056410cd9 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -1026,17 +1026,25 @@ static void iolatency_pd_offline(struct blkg_policy_data *pd)
iolatency_set_min_lat_nsec(blkg, 0);
iolatency_clear_scaling(blkg);
}
-static void iolatency_pd_free(struct blkg_policy_data *pd)
+static void iolat_release(struct rcu_head *rcu)
{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
struct iolatency_grp *iolat = pd_to_lat(pd);
+
free_percpu(iolat->stats);
kfree(iolat);
}
+static void iolatency_pd_free(struct blkg_policy_data *pd)
+{
+ call_rcu(&pd->rcu_head, iolat_release);
+}
+
static struct cftype iolatency_files[] = {
{
.name = "latency",
.flags = CFTYPE_NOT_ON_ROOT,
.seq_show = iolatency_print_limit,
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index cabf91f0d0dc..0f89fb03cdb6 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -351,20 +351,29 @@ static void throtl_pd_online(struct blkg_policy_data *pd)
* Update has_rules[] after a new group is brought online.
*/
tg_update_has_rules(tg);
}
-static void throtl_pd_free(struct blkg_policy_data *pd)
+static void tg_release(struct rcu_head *rcu)
{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
struct throtl_grp *tg = pd_to_tg(pd);
- timer_delete_sync(&tg->service_queue.pending_timer);
blkg_rwstat_exit(&tg->stat_bytes);
blkg_rwstat_exit(&tg->stat_ios);
kfree(tg);
}
+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);
+ call_rcu(&pd->rcu_head, tg_release);
+}
+
static struct throtl_grp *
throtl_rb_first(struct throtl_service_queue *parent_sq)
{
struct rb_node *n;
--
2.51.0
^ permalink raw reply related
* [PATCH 3/8] blk-cgroup: don't nest queue_lock under rcu in blkcg_print_blkgs()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
With previous modification to delay freeing policy data after an RCU grace
period, prfill() can run under RCU instead of taking queue_lock. However,
policy teardown can still clear blkg->pd[plid] after blkcg_print_blkgs()
observes the policy enabled bit.
Load policy data once with READ_ONCE() and skip the blkg if teardown
already cleared it. Do the same in recursive stat walks for descendant
blkgs. Remove the stale BFQ debug queue_lock assertion because
blkcg_print_blkgs() no longer calls prfill() with queue_lock held. This
also lets ioc_qos_prfill() and ioc_cost_model_prfill() use IRQ-safe
ioc->lock locking without re-enabling IRQs while queue_lock is still held.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-cgroup.c | 8 +++++---
block/blk-cgroup-rwstat.c | 15 +++++++++------
block/blk-cgroup.c | 22 +++++++++++++---------
block/blk-cgroup.h | 6 +++---
block/blk-iocost.c | 8 ++++----
5 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 56f60e36c799..904d9e0d9029 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -1146,20 +1146,22 @@ static u64 bfqg_prfill_stat_recursive(struct seq_file *sf,
struct blkcg_gq *blkg = pd_to_blkg(pd);
struct blkcg_gq *pos_blkg;
struct cgroup_subsys_state *pos_css;
u64 sum = 0;
- lockdep_assert_held(&blkg->q->queue_lock);
-
rcu_read_lock();
blkg_for_each_descendant_pre(pos_blkg, pos_css, blkg) {
+ struct blkg_policy_data *pd;
struct bfq_stat *stat;
if (!pos_blkg->online)
continue;
- stat = (void *)blkg_to_pd(pos_blkg, &blkcg_policy_bfq) + off;
+ pd = blkg_to_pd(pos_blkg, &blkcg_policy_bfq);
+ if (!pd)
+ continue;
+ stat = (void *)pd + off;
sum += bfq_stat_read(stat) + atomic64_read(&stat->aux_cnt);
}
rcu_read_unlock();
return __blkg_prfill_u64(sf, pd, sum);
diff --git a/block/blk-cgroup-rwstat.c b/block/blk-cgroup-rwstat.c
index a55fb0c53558..aae910713814 100644
--- a/block/blk-cgroup-rwstat.c
+++ b/block/blk-cgroup-rwstat.c
@@ -99,26 +99,29 @@ void blkg_rwstat_recursive_sum(struct blkcg_gq *blkg, struct blkcg_policy *pol,
{
struct blkcg_gq *pos_blkg;
struct cgroup_subsys_state *pos_css;
unsigned int i;
- lockdep_assert_held(&blkg->q->queue_lock);
+ WARN_ON_ONCE(!rcu_read_lock_held());
memset(sum, 0, sizeof(*sum));
- rcu_read_lock();
blkg_for_each_descendant_pre(pos_blkg, pos_css, blkg) {
struct blkg_rwstat *rwstat;
if (!pos_blkg->online)
continue;
- if (pol)
- rwstat = (void *)blkg_to_pd(pos_blkg, pol) + off;
- else
+ if (pol) {
+ struct blkg_policy_data *pd = blkg_to_pd(pos_blkg, pol);
+
+ if (!pd)
+ continue;
+ rwstat = (void *)pd + off;
+ } else {
rwstat = (void *)pos_blkg + off;
+ }
for (i = 0; i < BLKG_RWSTAT_NR; i++)
sum->cnt[i] += blkg_rwstat_read_counter(rwstat, i);
}
- rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(blkg_rwstat_recursive_sum);
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b55c43f72bcb..46fc65050c38 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -699,13 +699,13 @@ const char *blkg_dev_name(struct blkcg_gq *blkg)
* @data: data to be passed to @prfill
* @show_total: to print out sum of prfill return values or not
*
* This function invokes @prfill on each blkg of @blkcg if pd for the
* policy specified by @pol exists. @prfill is invoked with @sf, the
- * policy data and @data and the matching queue lock held. If @show_total
- * is %true, the sum of the return values from @prfill is printed with
- * "Total" label at the end.
+ * policy data and @data under RCU read lock. If @show_total is %true, the
+ * sum of the return values from @prfill is printed with "Total" label at the
+ * end.
*
* This is to be used to construct print functions for
* cftype->read_seq_string method.
*/
void blkcg_print_blkgs(struct seq_file *sf, struct blkcg *blkcg,
@@ -717,14 +717,18 @@ void blkcg_print_blkgs(struct seq_file *sf, struct blkcg *blkcg,
struct blkcg_gq *blkg;
u64 total = 0;
rcu_read_lock();
hlist_for_each_entry_rcu(blkg, &blkcg->blkg_list, blkcg_node) {
- spin_lock_irq(&blkg->q->queue_lock);
- if (blkcg_policy_enabled(blkg->q, pol))
- total += prfill(sf, blkg->pd[pol->plid], data);
- spin_unlock_irq(&blkg->q->queue_lock);
+ struct blkg_policy_data *pd;
+
+ if (!blkcg_policy_enabled(blkg->q, pol))
+ continue;
+
+ pd = blkg_to_pd(blkg, pol);
+ if (pd)
+ total += prfill(sf, pd, data);
}
rcu_read_unlock();
if (show_total)
seq_printf(sf, "Total %llu\n", (unsigned long long)total);
@@ -1591,11 +1595,11 @@ static void blkcg_policy_teardown_pds(struct request_queue *q,
if (pd) {
if (pd->online && pol->pd_offline_fn)
pol->pd_offline_fn(pd);
pd->online = false;
pol->pd_free_fn(pd);
- blkg->pd[pol->plid] = NULL;
+ WRITE_ONCE(blkg->pd[pol->plid], NULL);
}
spin_unlock(&blkcg->lock);
}
}
@@ -1683,11 +1687,11 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
spin_lock(&blkg->blkcg->lock);
pd->blkg = blkg;
pd->plid = pol->plid;
- blkg->pd[pol->plid] = pd;
+ WRITE_ONCE(blkg->pd[pol->plid], pd);
if (pol->pd_init_fn)
pol->pd_init_fn(pd);
if (pol->pd_online_fn)
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index fd206d1fa3c9..5402b4ff6f3f 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -279,13 +279,13 @@ static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg,
* @pol: policy of interest
*
* Return pointer to private data associated with the @blkg-@pol pair.
*/
static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg,
- struct blkcg_policy *pol)
+ const struct blkcg_policy *pol)
{
- return blkg ? blkg->pd[pol->plid] : NULL;
+ return blkg ? READ_ONCE(blkg->pd[pol->plid]) : NULL;
}
static inline struct blkcg_policy_data *blkcg_to_cpd(struct blkcg *blkcg,
struct blkcg_policy *pol)
{
@@ -488,11 +488,11 @@ static inline int blkcg_activate_policy(struct gendisk *disk,
const struct blkcg_policy *pol) { return 0; }
static inline void blkcg_deactivate_policy(struct gendisk *disk,
const struct blkcg_policy *pol) { }
static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg,
- struct blkcg_policy *pol) { return NULL; }
+ const struct blkcg_policy *pol) { return NULL; }
static inline struct blkcg_gq *pd_to_blkg(struct blkg_policy_data *pd) { return NULL; }
static inline void blkg_get(struct blkcg_gq *blkg) { }
static inline void blkg_put(struct blkcg_gq *blkg) { }
static inline void blk_cgroup_bio_start(struct bio *bio) { }
static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { return true; }
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index c136b1f46fcc..1f3f6e0f8901 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3188,11 +3188,11 @@ static u64 ioc_qos_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
struct ioc *ioc = pd_to_iocg(pd)->ioc;
if (!dname)
return 0;
- spin_lock(&ioc->lock);
+ spin_lock_irq(&ioc->lock);
seq_printf(sf, "%s enable=%d ctrl=%s rpct=%u.%02u rlat=%u wpct=%u.%02u wlat=%u min=%u.%02u max=%u.%02u\n",
dname, ioc->enabled, ioc->user_qos_params ? "user" : "auto",
ioc->params.qos[QOS_RPPM] / 10000,
ioc->params.qos[QOS_RPPM] % 10000 / 100,
ioc->params.qos[QOS_RLAT],
@@ -3201,11 +3201,11 @@ static u64 ioc_qos_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
ioc->params.qos[QOS_WLAT],
ioc->params.qos[QOS_MIN] / 10000,
ioc->params.qos[QOS_MIN] % 10000 / 100,
ioc->params.qos[QOS_MAX] / 10000,
ioc->params.qos[QOS_MAX] % 10000 / 100);
- spin_unlock(&ioc->lock);
+ spin_unlock_irq(&ioc->lock);
return 0;
}
static int ioc_qos_show(struct seq_file *sf, void *v)
{
@@ -3386,18 +3386,18 @@ static u64 ioc_cost_model_prfill(struct seq_file *sf,
u64 *u = ioc->params.i_lcoefs;
if (!dname)
return 0;
- spin_lock(&ioc->lock);
+ spin_lock_irq(&ioc->lock);
seq_printf(sf, "%s ctrl=%s model=linear "
"rbps=%llu rseqiops=%llu rrandiops=%llu "
"wbps=%llu wseqiops=%llu wrandiops=%llu\n",
dname, ioc->user_cost_model ? "user" : "auto",
u[I_LCOEF_RBPS], u[I_LCOEF_RSEQIOPS], u[I_LCOEF_RRANDIOPS],
u[I_LCOEF_WBPS], u[I_LCOEF_WSEQIOPS], u[I_LCOEF_WRANDIOPS]);
- spin_unlock(&ioc->lock);
+ spin_unlock_irq(&ioc->lock);
return 0;
}
static int ioc_cost_model_show(struct seq_file *sf, void *v)
{
--
2.51.0
^ permalink raw reply related
* [PATCH 4/8] blk-cgroup: don't nest queue_lock under rcu in blkg_lookup_create()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
Change this in two steps:
1) hold rcu lock and do blkg_lookup() from fast path;
2) hold queue_lock directly from slow path, and don't nest it under rcu
lock;
Prepare to convert protecting blkcg with blkcg_mutex instead of
queue_lock.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 57 +++++++++++++++++++++++++++++-----------------
1 file changed, 36 insertions(+), 21 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 46fc65050c38..e2896d582235 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -466,26 +466,21 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
struct gendisk *disk)
{
struct request_queue *q = disk->queue;
struct blkcg_gq *blkg;
- unsigned long flags;
-
- WARN_ON_ONCE(!rcu_read_lock_held());
- blkg = blkg_lookup(blkcg, q);
- if (blkg)
- return blkg;
-
- spin_lock_irqsave(&q->queue_lock, flags);
+ rcu_read_lock();
blkg = blkg_lookup(blkcg, q);
if (blkg) {
if (blkcg != &blkcg_root &&
blkg != rcu_dereference(blkcg->blkg_hint))
rcu_assign_pointer(blkcg->blkg_hint, blkg);
- goto found;
+ rcu_read_unlock();
+ return blkg;
}
+ rcu_read_unlock();
/*
* Create blkgs walking down from blkcg_root to @blkcg, so that all
* non-root blkgs have access to their parents. Returns the closest
* blkg to the intended blkg should blkg_create() fail.
@@ -513,12 +508,10 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
}
if (pos == blkcg)
break;
}
-found:
- spin_unlock_irqrestore(&q->queue_lock, flags);
return blkg;
}
static void blkg_destroy(struct blkcg_gq *blkg)
{
@@ -2098,10 +2091,22 @@ void blkcg_add_delay(struct blkcg_gq *blkg, u64 now, u64 delta)
return;
blkcg_scale_delay(blkg, now);
atomic64_add(delta, &blkg->delay_nsec);
}
+static inline struct blkcg_gq *blkg_lookup_tryget(struct blkcg_gq *blkg)
+{
+retry:
+ if (blkg_tryget(blkg))
+ return blkg;
+
+ blkg = blkg->parent;
+ if (blkg)
+ goto retry;
+
+ return NULL;
+}
/**
* blkg_tryget_closest - try and get a blkg ref on the closet blkg
* @bio: target bio
* @css: target css
*
@@ -2110,24 +2115,34 @@ void blkcg_add_delay(struct blkcg_gq *blkg, u64 now, u64 delta)
* up taking a reference on or %NULL if no reference was taken.
*/
static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,
struct cgroup_subsys_state *css)
{
- struct blkcg_gq *blkg, *ret_blkg = NULL;
+ struct request_queue *q = bio->bi_bdev->bd_queue;
+ struct blkcg *blkcg = css_to_blkcg(css);
+ struct blkcg_gq *blkg;
rcu_read_lock();
- blkg = blkg_lookup_create(css_to_blkcg(css), bio->bi_bdev->bd_disk);
- while (blkg) {
- if (blkg_tryget(blkg)) {
- ret_blkg = blkg;
- break;
- }
- blkg = blkg->parent;
- }
+ blkg = blkg_lookup(blkcg, q);
+ if (likely(blkg))
+ blkg = blkg_lookup_tryget(blkg);
rcu_read_unlock();
- return ret_blkg;
+ if (blkg)
+ return blkg;
+
+ /*
+ * 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);
+ blkg = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk);
+ if (blkg)
+ blkg = blkg_lookup_tryget(blkg);
+ spin_unlock_irq(&q->queue_lock);
+
+ return blkg;
}
/**
* bio_associate_blkg_from_css - associate a bio with a specified css
* @bio: target bio
--
2.51.0
^ permalink raw reply related
* [PATCH 5/8] blk-cgroup: don't nest queue_lock under rcu in bio_associate_blkg()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
If a bio is already associated with a blkg, the blkcg is already pinned
until the bio is done, so there is no need for RCU protection. Otherwise,
protect blkcg_css() with RCU independently. Prepare to protect blkcg with
blkcg_mutex instead of queue_lock.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index e2896d582235..8c9ca52a54f4 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -2186,20 +2186,24 @@ void bio_associate_blkg(struct bio *bio)
struct cgroup_subsys_state *css;
if (blk_op_is_passthrough(bio->bi_opf))
return;
- rcu_read_lock();
-
- if (bio->bi_blkg)
+ if (bio->bi_blkg) {
css = bio_blkcg_css(bio);
- else
+ 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);
-
- rcu_read_unlock();
+ bio_associate_blkg_from_css(bio, css);
+ if (css)
+ css_put(css);
+ }
}
EXPORT_SYMBOL_GPL(bio_associate_blkg);
/**
* bio_clone_blkg_association - clone blkg association from src to dst bio
--
2.51.0
^ permalink raw reply related
* [PATCH 6/8] blk-cgroup: don't nest queue_lock under blkcg->lock in blkcg_destroy_blkgs()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
The correct lock order is q->queue_lock before blkcg->lock, and in order
to prevent deadlock from blkcg_destroy_blkgs(), trylock is used for
q->queue_lock while blkcg->lock is already held, this is hacky.
Refactor blkcg_destroy_blkgs() to hold blkcg->lock only long enough to
get the first blkg and then release it. Then take q->queue_lock and
blkcg->lock in the correct order to destroy the blkg. This is a very cold
path, so the extra lock/unlock cycles are acceptable.
Also prepare to convert protecting blkcg with blkcg_mutex instead of
queue_lock.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 8c9ca52a54f4..d1f69a23c9d6 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1289,10 +1289,25 @@ struct list_head *blkcg_get_cgwb_list(struct cgroup_subsys_state *css)
*
* 3. Once the blkcg ref count goes to zero, blkcg_css_free() is called.
* This finally frees the blkcg.
*/
+static struct blkcg_gq *blkcg_get_first_blkg(struct blkcg *blkcg)
+{
+ struct blkcg_gq *blkg = NULL;
+
+ spin_lock_irq(&blkcg->lock);
+ if (!hlist_empty(&blkcg->blkg_list)) {
+ blkg = hlist_entry(blkcg->blkg_list.first, struct blkcg_gq,
+ blkcg_node);
+ blkg_get(blkg);
+ }
+ spin_unlock_irq(&blkcg->lock);
+
+ return blkg;
+}
+
/**
* 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
@@ -1302,36 +1317,28 @@ struct list_head *blkcg_get_cgwb_list(struct cgroup_subsys_state *css)
*
* This is the blkcg counterpart of ioc_release_fn().
*/
static void blkcg_destroy_blkgs(struct blkcg *blkcg)
{
- might_sleep();
+ struct blkcg_gq *blkg;
- spin_lock_irq(&blkcg->lock);
+ might_sleep();
- while (!hlist_empty(&blkcg->blkg_list)) {
- struct blkcg_gq *blkg = hlist_entry(blkcg->blkg_list.first,
- struct blkcg_gq, blkcg_node);
+ while ((blkg = blkcg_get_first_blkg(blkcg))) {
struct request_queue *q = blkg->q;
- if (need_resched() || !spin_trylock(&q->queue_lock)) {
- /*
- * Given that the system can accumulate a huge number
- * of blkgs in pathological cases, check to see if we
- * need to rescheduling to avoid softlockup.
- */
- spin_unlock_irq(&blkcg->lock);
- cond_resched();
- spin_lock_irq(&blkcg->lock);
- continue;
- }
+ spin_lock_irq(&q->queue_lock);
+ spin_lock(&blkcg->lock);
blkg_destroy(blkg);
- spin_unlock(&q->queue_lock);
- }
- spin_unlock_irq(&blkcg->lock);
+ spin_unlock(&blkcg->lock);
+ spin_unlock_irq(&q->queue_lock);
+
+ blkg_put(blkg);
+ cond_resched();
+ }
}
/**
* blkcg_pin_online - pin online state
* @blkcg_css: blkcg of interest
--
2.51.0
^ permalink raw reply related
* [PATCH 7/8] mm/page_io: don't nest queue_lock under rcu in bio_associate_blkg_from_page()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
Take a css reference under RCU, drop RCU, and then associate the bio with
the blkg. This avoids nesting queue_lock under RCU and prepares to protect
blkcg with blkcg_mutex instead of queue_lock.
Use css_tryget() instead of css_tryget_online() so swap writeback for
pages charged to a dying memcg still passes the dying css to
bio_associate_blkg_from_css(). That preserves the existing closest-live
ancestor fallback instead of charging those bios to the root blkg.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
mm/page_io.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/page_io.c b/mm/page_io.c
index 70cea9e24d2f..3b54c60c278e 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -315,12 +315,17 @@ static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio)
return;
rcu_read_lock();
memcg = folio_memcg(folio);
css = cgroup_e_css(memcg->css.cgroup, &io_cgrp_subsys);
- bio_associate_blkg_from_css(bio, css);
+ if (!css || !css_tryget(css))
+ css = NULL;
rcu_read_unlock();
+
+ bio_associate_blkg_from_css(bio, css);
+ if (css)
+ css_put(css);
}
#else
#define bio_associate_blkg_from_page(bio, folio) do { } while (0)
#endif /* CONFIG_MEMCG && CONFIG_BLK_CGROUP */
--
2.51.0
^ permalink raw reply related
* [PATCH 8/8] block, bfq: don't grab queue_lock to initialize bfq
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
The request_queue is frozen and quiesced while the elevator init_sched()
method runs, so queue_lock is not needed for BFQ cgroup initialization.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-iosched.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 42ccfd0c6140..5cabee2d4e7c 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -7207,14 +7207,11 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_queue *eq)
bfqd = kzalloc_node(sizeof(*bfqd), GFP_KERNEL, q->node);
if (!bfqd)
return -ENOMEM;
eq->elevator_data = bfqd;
-
- spin_lock_irq(&q->queue_lock);
q->elevator = eq;
- spin_unlock_irq(&q->queue_lock);
/*
* Our fallback bfqq if bfq_find_alloc_queue() runs into OOM issues.
* Grab a permanent reference to it, so that the normal code flow
* will not attempt to free it.
@@ -7243,11 +7240,10 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_queue *eq)
bfqd->num_actuators = 1;
/*
* If the disk supports multiple actuators, copy independent
* access ranges from the request queue structure.
*/
- spin_lock_irq(&q->queue_lock);
if (ia_ranges) {
/*
* Check if the disk ia_ranges size exceeds the current bfq
* actuator limit.
*/
@@ -7269,11 +7265,10 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_queue *eq)
/* Otherwise use single-actuator dev info */
if (bfqd->num_actuators == 1) {
bfqd->sector[0] = 0;
bfqd->nr_sectors[0] = get_capacity(q->disk);
}
- spin_unlock_irq(&q->queue_lock);
INIT_LIST_HEAD(&bfqd->dispatch);
hrtimer_setup(&bfqd->idle_slice_timer, bfq_idle_slice_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_REL);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 1/4] block: add a macro to initialize the status table
From: Christoph Hellwig @ 2026-06-08 5:09 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Christoph Hellwig, Jens Axboe, Jonathan Corbet, linux-block,
linux-doc, Keith Busch
In-Reply-To: <aiMZ-PXXQ-NxOHT4@casper.infradead.org>
On Fri, Jun 05, 2026 at 07:48:24PM +0100, Matthew Wilcox wrote:
> On Fri, Jun 05, 2026 at 08:44:27PM +0200, Christoph Hellwig wrote:
> > Prepare for adding a new value to the error table by adding a macro
> > to fill it.
>
> > +#define ENT(_tag, _errno, _desc) \
> > +[BLK_STS_##_tag] = { \
> > + .errno = _errno, \
> > + .name = _desc, \
>
> Bleh. I hate this. Before, I can grep for BLK_STS_NOSPC and find it.
You will still find BLK_STS_NOSPC itself in include/linux/blk_types.h
> After, I can't. Yes, I know we have a lot of such things already, but
> I don't like adding more.
I'm not a huge fan off CPP pasting, and especially thing we should never
use it to define global symbols (hi page/folio flag helpers!), and in
general try to avoid using them as much as possible. But I think here
the need to keep the names in sync with the tags exposed in debugfs is
more important than the grepability. Especially as this sits in _the_
core block file, so it can't be easily missed.
^ permalink raw reply
* configurable block error injection v3
From: Christoph Hellwig @ 2026-06-08 5:14 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc
Hi all,
this series adds a new configurable block error injection facility.
We already have a few to inject block errors, but unfortunately most
of them are either not very useful or hard to use, or both:
- The fail_make_request failure injection point can't distinguish
different commands, different ranges in the file and can only injection
plain I/O errors.
- the should_fail_bio 'dynamic' failure injection has all the same issues
as fail_make_request
- dm-error can only fail all command in the table using BLK_STS_IOERR
and requires setting up a new block device
- dm-flakey and dm-dust allow all kinds of configurability, but still
don't have good error selection, no good support for non-read/write
commands and are limited to the dm table alignment requirements,
which for zoned devices enforces setting them up for an entire zone.
They also once again require setting up a stacked block device,
which is really annoying in harnesses like xfstests
This series adds a new debugfs-based block layer error injection
that allows to configure what operations and ranges the injection
applied to, and what status to return. It also allows to configure a
failure ratio similar to the xfs errortag injection.
Changes since v2:
- improve the documentation a bit
- fix a spelling mistake in a comment
Changes since v1:
- drop the should_fail_bio removal and cleanup depending on it, as it's
used by eBPF programs and thus a hidden UABI.
- as a result split the code out to it's own Kconfig symbol
- various error handling fixed pointed out by Keith
- documentation spelling fixes pointed out by Randy
Diffstat:
Documentation/block/error-injection.rst | 59 ++++++
Documentation/block/index.rst | 1
block/Kconfig | 7
block/Makefile | 1
block/blk-core.c | 86 ++++++--
block/blk-sysfs.c | 4
block/blk.h | 15 +
block/error-injection.c | 308 ++++++++++++++++++++++++++++++++
block/genhd.c | 4
include/linux/blkdev.h | 6
10 files changed, 471 insertions(+), 20 deletions(-)
^ permalink raw reply
* [PATCH 1/4] block: add a macro to initialize the status table
From: Christoph Hellwig @ 2026-06-08 5:14 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Hannes Reinecke
In-Reply-To: <20260608051416.1205282-1-hch@lst.de>
Prepare for adding a new value to the error table by adding a macro
to fill it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
block/blk-core.c | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index b0f0a304ea0b..1614323282f1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -132,39 +132,44 @@ inline const char *blk_op_str(enum req_op op)
}
EXPORT_SYMBOL_GPL(blk_op_str);
+#define ENT(_tag, _errno, _desc) \
+[BLK_STS_##_tag] = { \
+ .errno = _errno, \
+ .name = _desc, \
+}
static const struct {
int errno;
const char *name;
} blk_errors[] = {
- [BLK_STS_OK] = { 0, "" },
- [BLK_STS_NOTSUPP] = { -EOPNOTSUPP, "operation not supported" },
- [BLK_STS_TIMEOUT] = { -ETIMEDOUT, "timeout" },
- [BLK_STS_NOSPC] = { -ENOSPC, "critical space allocation" },
- [BLK_STS_TRANSPORT] = { -ENOLINK, "recoverable transport" },
- [BLK_STS_TARGET] = { -EREMOTEIO, "critical target" },
- [BLK_STS_RESV_CONFLICT] = { -EBADE, "reservation conflict" },
- [BLK_STS_MEDIUM] = { -ENODATA, "critical medium" },
- [BLK_STS_PROTECTION] = { -EILSEQ, "protection" },
- [BLK_STS_RESOURCE] = { -ENOMEM, "kernel resource" },
- [BLK_STS_DEV_RESOURCE] = { -EBUSY, "device resource" },
- [BLK_STS_AGAIN] = { -EAGAIN, "nonblocking retry" },
- [BLK_STS_OFFLINE] = { -ENODEV, "device offline" },
+ ENT(OK, 0, ""),
+ ENT(NOTSUPP, -EOPNOTSUPP, "operation not supported"),
+ ENT(TIMEOUT, -ETIMEDOUT, "timeout"),
+ ENT(NOSPC, -ENOSPC, "critical space allocation"),
+ ENT(TRANSPORT, -ENOLINK, "recoverable transport"),
+ ENT(TARGET, -EREMOTEIO, "critical target"),
+ ENT(RESV_CONFLICT, -EBADE, "reservation conflict"),
+ ENT(MEDIUM, -ENODATA, "critical medium"),
+ ENT(PROTECTION, -EILSEQ, "protection"),
+ ENT(RESOURCE, -ENOMEM, "kernel resource"),
+ ENT(DEV_RESOURCE, -EBUSY, "device resource"),
+ ENT(AGAIN, -EAGAIN, "nonblocking retry"),
+ ENT(OFFLINE, -ENODEV, "device offline"),
/* device mapper special case, should not leak out: */
- [BLK_STS_DM_REQUEUE] = { -EREMCHG, "dm internal retry" },
+ ENT(DM_REQUEUE, -EREMCHG, "dm internal retry"),
/* zone device specific errors */
- [BLK_STS_ZONE_OPEN_RESOURCE] = { -ETOOMANYREFS, "open zones exceeded" },
- [BLK_STS_ZONE_ACTIVE_RESOURCE] = { -EOVERFLOW, "active zones exceeded" },
+ ENT(ZONE_OPEN_RESOURCE, -ETOOMANYREFS, "open zones exceeded"),
+ ENT(ZONE_ACTIVE_RESOURCE, -EOVERFLOW, "active zones exceeded"),
/* Command duration limit device-side timeout */
- [BLK_STS_DURATION_LIMIT] = { -ETIME, "duration limit exceeded" },
-
- [BLK_STS_INVAL] = { -EINVAL, "invalid" },
+ ENT(DURATION_LIMIT, -ETIME, "duration limit exceeded"),
+ ENT(INVAL, -EINVAL, "invalid"),
/* everything else not covered above: */
- [BLK_STS_IOERR] = { -EIO, "I/O" },
+ ENT(IOERR, -EIO, "I/O"),
};
+#undef ENT
blk_status_t errno_to_blk_status(int errno)
{
--
2.53.0
^ permalink raw reply related
* [PATCH 2/4] block: add a "tag" for block status codes
From: Christoph Hellwig @ 2026-06-08 5:14 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Hannes Reinecke
In-Reply-To: <20260608051416.1205282-1-hch@lst.de>
The full name of the status codes is not good for user interfaces as it
can contain white spaces. Add the name of the status code without the
BLK_STS_ prefix as a tag so that it can be used for user interfaces.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
block/blk-core.c | 28 ++++++++++++++++++++++++++++
block/blk.h | 2 ++
2 files changed, 30 insertions(+)
diff --git a/block/blk-core.c b/block/blk-core.c
index 1614323282f1..7aa9cd110bdd 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -135,10 +135,12 @@ EXPORT_SYMBOL_GPL(blk_op_str);
#define ENT(_tag, _errno, _desc) \
[BLK_STS_##_tag] = { \
.errno = _errno, \
+ .tag = __stringify(_tag), \
.name = _desc, \
}
static const struct {
int errno;
+ const char *tag;
const char *name;
} blk_errors[] = {
ENT(OK, 0, ""),
@@ -203,6 +205,32 @@ const char *blk_status_to_str(blk_status_t status)
return blk_errors[idx].name;
}
+const char *blk_status_to_tag(blk_status_t status)
+{
+ int idx = (__force int)status;
+
+ if (WARN_ON_ONCE(idx >= ARRAY_SIZE(blk_errors)))
+ return "<null>";
+ return blk_errors[idx].tag;
+}
+
+blk_status_t tag_to_blk_status(const char *tag)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(blk_errors); i++) {
+ if (blk_errors[i].tag &&
+ !strcmp(blk_errors[i].tag, tag))
+ return (__force blk_status_t)i;
+ }
+
+ /*
+ * Return BLK_STS_OK for mismatches as this function is intended to
+ * parse error status values.
+ */
+ return BLK_STS_OK;
+}
+
/**
* blk_sync_queue - cancel any pending callbacks on a queue
* @q: the queue
diff --git a/block/blk.h b/block/blk.h
index 1a2d9101bba0..0eb8e932ec66 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -50,6 +50,8 @@ struct blk_flush_queue *blk_alloc_flush_queue(int node, int cmd_size,
void blk_free_flush_queue(struct blk_flush_queue *q);
const char *blk_status_to_str(blk_status_t status);
+const char *blk_status_to_tag(blk_status_t status);
+blk_status_t tag_to_blk_status(const char *tag);
bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
bool blk_queue_start_drain(struct request_queue *q);
--
2.53.0
^ permalink raw reply related
* [PATCH 3/4] block: add a str_to_blk_op helper
From: Christoph Hellwig @ 2026-06-08 5:14 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Hannes Reinecke
In-Reply-To: <20260608051416.1205282-1-hch@lst.de>
Add a helper to find the REQ_OP_XYZ constant from the "XYZ" string.
This will be used for the error injection debugfs interface.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
block/blk-core.c | 10 ++++++++++
block/blk.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/block/blk-core.c b/block/blk-core.c
index 7aa9cd110bdd..aa90aad6da13 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -132,6 +132,16 @@ inline const char *blk_op_str(enum req_op op)
}
EXPORT_SYMBOL_GPL(blk_op_str);
+enum req_op str_to_blk_op(const char *op)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(blk_op_name); i++)
+ if (blk_op_name[i] && !strcmp(blk_op_name[i], op))
+ return (enum req_op)i;
+ return REQ_OP_LAST;
+}
+
#define ENT(_tag, _errno, _desc) \
[BLK_STS_##_tag] = { \
.errno = _errno, \
diff --git a/block/blk.h b/block/blk.h
index 0eb8e932ec66..e8b7d5517086 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -52,6 +52,7 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
const char *blk_status_to_str(blk_status_t status);
const char *blk_status_to_tag(blk_status_t status);
blk_status_t tag_to_blk_status(const char *tag);
+enum req_op str_to_blk_op(const char *op);
bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
bool blk_queue_start_drain(struct request_queue *q);
--
2.53.0
^ permalink raw reply related
* [PATCH 4/4] block: add configurable error injection
From: Christoph Hellwig @ 2026-06-08 5:14 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Hannes Reinecke
In-Reply-To: <20260608051416.1205282-1-hch@lst.de>
Add a new block error injection interface that allows to inject specific
status code for specific ranges.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
Documentation/block/error-injection.rst | 59 +++++
Documentation/block/index.rst | 1 +
block/Kconfig | 7 +
block/Makefile | 1 +
block/blk-core.c | 3 +
block/blk-sysfs.c | 4 +
block/blk.h | 12 +
block/error-injection.c | 308 ++++++++++++++++++++++++
block/genhd.c | 4 +
include/linux/blkdev.h | 6 +
10 files changed, 405 insertions(+)
create mode 100644 Documentation/block/error-injection.rst
create mode 100644 block/error-injection.c
diff --git a/Documentation/block/error-injection.rst b/Documentation/block/error-injection.rst
new file mode 100644
index 000000000000..a96b7af362c5
--- /dev/null
+++ b/Documentation/block/error-injection.rst
@@ -0,0 +1,59 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================
+Configurable Error Injection
+============================
+
+Overview
+--------
+
+Configurable error injection allows injecting specific block layer status codes
+for ranges of a block device. Errors can be injected unconditionally, or with a
+given probability.
+
+To use configurable error injection, CONFIG_BLK_ERROR_INJECTION must be enabled.
+
+The only interface is the error_injection debugfs file, which is created for
+each registered gendisk. Writes to this file are used to create or delete rules
+and reads return a list of the current error injection sites.
+
+Options
+-------
+
+The following options specify the operations:
+
+=================== =======================================================
+add add a new rule
+removeall remove all existing rules
+=================== =======================================================
+
+The following options specify the details of the rule for the add operation:
+
+=================== =======================================================
+op=<string> block layer operation this rule applies to. This uses
+ the XYZ for each REQ_OP_XYZ operation, e.g. READ, WRITE
+ or DISCARD. Mandatory.
+status=<string> Status to return. This uses XYZ for each BLK_STS_XYZ
+ code, e.g. IOERR or MEDIUM. Mandatory.
+start=<number> First block layer sector the rule applies to.
+ Optional, defaults to 0.
+nr_sectors=<number> Number of sectors this rule applies.
+ Optional, defaults to the remainder of the device.
+chance=<number> Only return a failure with a likelihood of 1/chance.
+ Optional, defaults to 1 (always).
+=================== =======================================================
+
+Example
+-------
+
+Return BLK_STS_IOERR for one in 10 reads of sector 0 of /dev/nvme0n1:
+
+ $ echo 'add,op=READ,start=0,status=IOERR,chance=10' > /sys/kernel/debug/block/nvme0n1/error_injection
+
+Return BLK_STS_MEDIUM for every write to /dev/nvme0n1:
+
+ $ echo 'add,op=WRITE,start=0,status=MEDIUM' > /sys/kernel/debug/block/nvme0n1/error_injection
+
+Remove all rules for /dev/nvme0n1:
+
+ $ echo 'removeall' > /sys/kernel/debug/block/nvme0n1/error_injection
diff --git a/Documentation/block/index.rst b/Documentation/block/index.rst
index 9fea696f9daa..bfa1bbd31ddf 100644
--- a/Documentation/block/index.rst
+++ b/Documentation/block/index.rst
@@ -22,3 +22,4 @@ Block
switching-sched
writeback_cache_control
ublk
+ error-injection
diff --git a/block/Kconfig b/block/Kconfig
index 15027963472d..7651b86eed56 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -221,6 +221,13 @@ config BLOCK_HOLDER_DEPRECATED
config BLK_MQ_STACKING
bool
+config BLK_ERROR_INJECTION
+ bool "Enable block layer error injection"
+ help
+ Enable inserting arbitrary block errors through a debugfs interface.
+
+ See Documentation/block/error-injection.rst for details.
+
source "block/Kconfig.iosched"
endif # BLOCK
diff --git a/block/Makefile b/block/Makefile
index 54130faacc21..e7bd320e3d69 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -13,6 +13,7 @@ obj-y := bdev.o fops.o bio.o elevator.o blk-core.o blk-sysfs.o \
genhd.o ioprio.o badblocks.o partitions/ blk-rq-qos.o \
disk-events.o blk-ia-ranges.o early-lookup.o
+obj-$(CONFIG_BLK_ERROR_INJECTION) += error-injection.o
obj-$(CONFIG_BLK_DEV_BSG_COMMON) += bsg.o
obj-$(CONFIG_BLK_DEV_BSGLIB) += bsg-lib.o
obj-$(CONFIG_BLK_CGROUP) += blk-cgroup.o
diff --git a/block/blk-core.c b/block/blk-core.c
index aa90aad6da13..268735582ef1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -767,6 +767,9 @@ static void __submit_bio_noacct_mq(struct bio *bio)
void submit_bio_noacct_nocheck(struct bio *bio, bool split)
{
+ if (unlikely(blk_error_inject(bio)))
+ return;
+
blk_cgroup_bio_start(bio);
if (!bio_flagged(bio, BIO_TRACE_COMPLETION)) {
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index f22c1f253eb3..8a0c2be48a31 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -933,6 +933,8 @@ static void blk_debugfs_remove(struct gendisk *disk)
blk_debugfs_lock_nomemsave(q);
blk_trace_shutdown(q);
+ if (IS_ENABLED(CONFIG_BLK_ERROR_INJECTION))
+ blk_error_injection_exit(disk);
debugfs_remove_recursive(q->debugfs_dir);
q->debugfs_dir = NULL;
q->sched_debugfs_dir = NULL;
@@ -963,6 +965,8 @@ int blk_register_queue(struct gendisk *disk)
memflags = blk_debugfs_lock(q);
q->debugfs_dir = debugfs_create_dir(disk->disk_name, blk_debugfs_root);
+ if (IS_ENABLED(CONFIG_BLK_ERROR_INJECTION))
+ blk_error_injection_init(disk);
if (queue_is_mq(q))
blk_mq_debugfs_register(q);
blk_debugfs_unlock(q, memflags);
diff --git a/block/blk.h b/block/blk.h
index e8b7d5517086..10df23b2cb90 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -660,6 +660,18 @@ static inline bool should_fail_request(struct block_device *part,
}
#endif /* CONFIG_FAIL_MAKE_REQUEST */
+void blk_error_injection_init(struct gendisk *disk);
+void blk_error_injection_exit(struct gendisk *disk);
+bool __blk_error_inject(struct bio *bio);
+static inline bool blk_error_inject(struct bio *bio)
+{
+ if (!IS_ENABLED(CONFIG_BLK_ERROR_INJECTION))
+ return false;
+ if (!test_bit(GD_ERROR_INJECT, &bio->bi_bdev->bd_disk->state))
+ return false;
+ return __blk_error_inject(bio);
+}
+
/*
* Optimized request reference counting. Ideally we'd make timeouts be more
* clever, as that's the only reason we need references at all... But until
diff --git a/block/error-injection.c b/block/error-injection.c
new file mode 100644
index 000000000000..3ca4ad297683
--- /dev/null
+++ b/block/error-injection.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 Christoph Hellwig.
+ */
+#include <linux/debugfs.h>
+#include <linux/blkdev.h>
+#include <linux/parser.h>
+#include <linux/seq_file.h>
+#include "blk.h"
+
+struct blk_error_inject {
+ struct list_head entry;
+ sector_t start;
+ sector_t end;
+ enum req_op op;
+ blk_status_t status;
+
+ /* only inject every 1 / chance times */
+ unsigned int chance;
+};
+
+bool __blk_error_inject(struct bio *bio)
+{
+ struct gendisk *disk = bio->bi_bdev->bd_disk;
+ struct blk_error_inject *inj;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(inj, &disk->error_injection_list, entry) {
+ if (bio->bi_iter.bi_sector <= inj->end &&
+ bio_end_sector(bio) > inj->start &&
+ bio_op(bio) == inj->op) {
+ blk_status_t status = inj->status;
+
+ if (inj->chance > 1 &&
+ (get_random_u32() % inj->chance) != 0)
+ continue;
+
+ pr_info_ratelimited("%pg: injecting %s error for %s at sector %llu:%u\n",
+ disk->part0,
+ blk_status_to_str(status),
+ blk_op_str(inj->op),
+ bio->bi_iter.bi_sector,
+ bio_sectors(bio));
+ rcu_read_unlock();
+ bio_endio_status(bio, status);
+ return true;
+ }
+ }
+ rcu_read_unlock();
+ return false;
+}
+
+static int error_inject_add(struct gendisk *disk, enum req_op op,
+ sector_t start, u64 nr_sectors, blk_status_t status,
+ unsigned int chance)
+{
+ struct blk_error_inject *inj;
+ int error = -EINVAL;
+
+ if (op == REQ_OP_LAST)
+ return -EINVAL;
+ if (status == BLK_STS_OK)
+ return -EINVAL;
+
+ inj = kzalloc_obj(*inj);
+ if (!inj)
+ return -ENOMEM;
+
+ if (nr_sectors) {
+ if (U64_MAX - nr_sectors < start)
+ goto out_free_inj;
+ inj->end = start + nr_sectors - 1;
+ } else {
+ inj->end = U64_MAX;
+ }
+
+ inj->op = op;
+ inj->start = start;
+ inj->status = status;
+ inj->chance = chance;
+
+ pr_debug_ratelimited("%pg: adding %s injection for %s at sector %llu:%llu\n",
+ disk->part0, blk_status_to_str(status),
+ blk_op_str(op),
+ start, nr_sectors);
+
+ /*
+ * Add to the front of the list so that newer entries can partially
+ * override other entries. This also intentionally allows duplicate
+ * entries as there is no real reason to reject them.
+ */
+ mutex_lock(&disk->error_injection_lock);
+ if (!disk_live(disk)) {
+ mutex_unlock(&disk->error_injection_lock);
+ error = -ENODEV;
+ goto out_free_inj;
+ }
+ list_add_rcu(&inj->entry, &disk->error_injection_list);
+ set_bit(GD_ERROR_INJECT, &disk->state);
+ mutex_unlock(&disk->error_injection_lock);
+ return 0;
+
+out_free_inj:
+ kfree(inj);
+ return error;
+}
+
+static void error_inject_removall(struct gendisk *disk)
+{
+ struct blk_error_inject *inj;
+
+ mutex_lock(&disk->error_injection_lock);
+ clear_bit(GD_ERROR_INJECT, &disk->state);
+ while ((inj = list_first_entry_or_null(&disk->error_injection_list,
+ struct blk_error_inject, entry))) {
+ list_del_rcu(&inj->entry);
+ mutex_unlock(&disk->error_injection_lock);
+
+ kfree_rcu_mightsleep(inj);
+
+ mutex_lock(&disk->error_injection_lock);
+ }
+ mutex_unlock(&disk->error_injection_lock);
+}
+
+enum options {
+ Opt_add = (1u << 0),
+ Opt_removeall = (1u << 1),
+
+ Opt_op = (1u << 16),
+ Opt_start = (1u << 17),
+ Opt_nr_sectors = (1u << 18),
+ Opt_status = (1u << 19),
+ Opt_chance = (1u << 20),
+
+ Opt_invalid,
+};
+
+static const match_table_t opt_tokens = {
+ { Opt_add, "add", },
+ { Opt_removeall, "removeall", },
+ { Opt_op, "op=%s", },
+ { Opt_start, "start=%u" },
+ { Opt_nr_sectors, "nr_sectors=%u" },
+ { Opt_status, "status=%s" },
+ { Opt_chance, "chance=%u" },
+ { Opt_invalid, NULL, },
+};
+
+static int match_op(substring_t *args, enum req_op *op)
+{
+ const char *tag;
+
+ tag = match_strdup(args);
+ if (!tag)
+ return -ENOMEM;
+ *op = str_to_blk_op(tag);
+ if (*op == REQ_OP_LAST)
+ pr_warn("invalid op '%s'\n", tag);
+ kfree(tag);
+ return 0;
+}
+
+static int match_status(substring_t *args, blk_status_t *status)
+{
+ const char *tag;
+
+ tag = match_strdup(args);
+ if (!tag)
+ return -ENOMEM;
+ *status = tag_to_blk_status(tag);
+ if (!*status)
+ pr_warn("invalid status '%s'\n", tag);
+ kfree(tag);
+ return 0;
+}
+
+static ssize_t blk_error_injection_parse_options(struct gendisk *disk,
+ char *options)
+{
+ enum { Unset, Add, Removeall } action = Unset;
+ unsigned int option_mask = 0, chance = 1;
+ enum req_op op = REQ_OP_LAST;
+ u64 start = 0, nr_sectors = 0;
+ blk_status_t status = BLK_STS_OK;
+ substring_t args[MAX_OPT_ARGS];
+ char *p;
+
+ while ((p = strsep(&options, ",\n")) != NULL) {
+ int error = 0;
+ ssize_t token;
+
+ if (!*p)
+ continue;
+ token = match_token(p, opt_tokens, args);
+ option_mask |= token;
+ switch (token) {
+ case Opt_add:
+ if (action != Unset)
+ return -EINVAL;
+ action = Add;
+ break;
+ case Opt_removeall:
+ if (action != Unset)
+ return -EINVAL;
+ action = Removeall;
+ break;
+ case Opt_op:
+ error = match_op(args, &op);
+ break;
+ case Opt_start:
+ error = match_u64(args, &start);
+ break;
+ case Opt_nr_sectors:
+ error = match_u64(args, &nr_sectors);
+ break;
+ case Opt_status:
+ error = match_status(args, &status);
+ break;
+ case Opt_chance:
+ error = match_uint(args, &chance);
+ if (!error && chance == 0)
+ error = -EINVAL;
+ break;
+ default:
+ pr_warn("unknown parameter or missing value '%s'\n", p);
+ error = -EINVAL;
+ }
+ if (error)
+ return error;
+ }
+
+ switch (action) {
+ case Add:
+ return error_inject_add(disk, op, start, nr_sectors, status,
+ chance);
+ case Removeall:
+ if (option_mask & ~Opt_removeall)
+ return -EINVAL;
+ error_inject_removall(disk);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t blk_error_injection_write(struct file *file,
+ const char __user *ubuf, size_t count, loff_t *pos)
+{
+ struct gendisk *disk = file_inode(file)->i_private;
+ char *options;
+ int error;
+
+ options = memdup_user_nul(ubuf, count);
+ if (IS_ERR(options))
+ return PTR_ERR(options);
+ error = blk_error_injection_parse_options(disk, options);
+ kfree(options);
+
+ if (error)
+ return error;
+ return count;
+}
+
+static int blk_error_injection_show(struct seq_file *s, void *private)
+{
+ struct gendisk *disk = s->private;
+ struct blk_error_inject *inj;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(inj, &disk->error_injection_list, entry) {
+ seq_printf(s, "%llu:%llu status=%s,chance=%u",
+ inj->start, inj->end,
+ blk_status_to_tag(inj->status), inj->chance);
+ seq_putc(s, '\n');
+ }
+ rcu_read_unlock();
+ return 0;
+}
+
+static int blk_error_injection_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, blk_error_injection_show, inode->i_private);
+}
+
+static int blk_error_injection_release(struct inode *inode, struct file *file)
+{
+ return single_release(inode, file);
+}
+
+static const struct file_operations blk_error_injection_fops = {
+ .owner = THIS_MODULE,
+ .write = blk_error_injection_write,
+ .read = seq_read,
+ .open = blk_error_injection_open,
+ .release = blk_error_injection_release,
+};
+
+void blk_error_injection_init(struct gendisk *disk)
+{
+ debugfs_create_file("error_injection", 0600, disk->queue->debugfs_dir,
+ disk, &blk_error_injection_fops);
+}
+
+void blk_error_injection_exit(struct gendisk *disk)
+{
+ error_inject_removall(disk);
+}
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e9..f84b6a355b57 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1485,6 +1485,10 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
INIT_LIST_HEAD(&disk->slave_bdevs);
+#endif
+#ifdef CONFIG_BLK_ERROR_INJECTION
+ mutex_init(&disk->error_injection_lock);
+ INIT_LIST_HEAD(&disk->error_injection_list);
#endif
mutex_init(&disk->rqos_state_mutex);
kobject_init(&disk->queue_kobj, &blk_queue_ktype);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 57e84d59a642..5070851cf924 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -176,6 +176,7 @@ struct gendisk {
#define GD_SUPPRESS_PART_SCAN 5
#define GD_OWNS_QUEUE 6
#define GD_ZONE_APPEND_USED 7
+#define GD_ERROR_INJECT 8
struct mutex open_mutex; /* open/close mutex */
unsigned open_partitions; /* number of open partitions */
@@ -227,6 +228,11 @@ struct gendisk {
*/
struct blk_independent_access_ranges *ia_ranges;
+#ifdef CONFIG_BLK_ERROR_INJECTION
+ struct mutex error_injection_lock;
+ struct list_head error_injection_list;
+#endif
+
struct mutex rqos_state_mutex; /* rqos state change mutex */
};
--
2.53.0
^ permalink raw reply related
* [PATCH] rust: block: require `Sync` for `Operations::QueueData`
From: Andreas Hindborg @ 2026-06-08 8:24 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Jens Axboe, Daniel Almeida
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
The queue data installed in a `GenDisk` is stored in the request queue and
handed back to the driver as a shared borrow through the `queue_rq` and
`commit_rqs` callbacks. Both callbacks obtain that borrow via
`ForeignOwnable::borrow` and may execute concurrently on several CPUs,
since the block layer runs one hardware queue per CPU. That means a shared
reference to the same queue data can be live on multiple threads at once,
which is only sound when the referent is `Sync`.
The initial `GenDisk` private data support omitted this bound, so a
driver could install a non-`Sync` type as queue data and then access
it concurrently from multiple CPUs without synchronization. Add a
`Sync` bound to the `QueueData` associated type to rule that out.
Fixes: 90d952fac8ac ("rust: block: add `GenDisk` private data support")
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/block/mq/operations.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 8ad46129a52c..89029f468f44 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -30,7 +30,7 @@
pub trait Operations: Sized {
/// Data associated with the `struct request_queue` that is allocated for
/// the `GenDisk` associated with this `Operations` implementation.
- type QueueData: ForeignOwnable;
+ type QueueData: ForeignOwnable + Sync;
/// Called by the kernel to queue a request with the driver. If `is_last` is
/// `false`, the driver is allowed to defer committing the request.
---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260608-queue-data-sync-80b66ab312ac
Best regards,
--
Andreas Hindborg <a.hindborg@kernel.org>
^ permalink raw reply related
* [PATCH] block: fix arg type in `blk_mq_update_nr_hw_queues`
From: Andreas Hindborg @ 2026-06-08 8:39 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-kernel, Andreas Hindborg
The type of the argument `nr_hw_queues` in the function
`blk_mq_update_nr_hw_queues` is a signed integer. This is wrong,
considering the field `nr_hw_queues` of `struct blk_mq_tag_set` is
unsigned. Thus, change the type of the parameter to unsigned.
Cascade the change to downstream functions.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
block/blk-mq.c | 13 +++++++------
include/linux/blk-mq.h | 2 +-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4c5c16cce4f8..5eb9b52f3146 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4816,10 +4816,10 @@ static void blk_mq_update_queue_map(struct blk_mq_tag_set *set)
static struct blk_mq_tags **blk_mq_prealloc_tag_set_tags(
struct blk_mq_tag_set *set,
- int new_nr_hw_queues)
+ unsigned int new_nr_hw_queues)
{
struct blk_mq_tags **new_tags;
- int i;
+ unsigned int i;
if (set->nr_hw_queues >= new_nr_hw_queues)
return NULL;
@@ -5134,12 +5134,12 @@ static int blk_mq_elv_switch_none(struct request_queue *q,
}
static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
- int nr_hw_queues)
+ unsigned int nr_hw_queues)
{
struct request_queue *q;
- int prev_nr_hw_queues = set->nr_hw_queues;
+ unsigned int prev_nr_hw_queues = set->nr_hw_queues;
unsigned int memflags;
- int i;
+ unsigned int i;
struct xarray elv_tbl;
struct blk_mq_tags **new_tags;
bool queues_frozen = false;
@@ -5234,7 +5234,8 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
__blk_mq_free_map_and_rqs(set, i);
}
-void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues)
+void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
+ unsigned int nr_hw_queues)
{
down_write(&set->update_nr_hwq_lock);
mutex_lock(&set->tag_list_lock);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..f73b4a1f16db 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -974,7 +974,7 @@ unsigned int blk_mq_num_online_queues(unsigned int max_queues);
void blk_mq_map_queues(struct blk_mq_queue_map *qmap);
void blk_mq_map_hw_queues(struct blk_mq_queue_map *qmap,
struct device *dev, unsigned int offset);
-void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
+void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, unsigned int nr_hw_queues);
void blk_mq_quiesce_queue_nowait(struct request_queue *q);
---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260608-update-hw-nodes-arg-940ecec0380a
Best regards,
--
Andreas Hindborg <a.hindborg@kernel.org>
^ permalink raw reply related
* Re: [PATCH RFC 1/8] fs, block: move blk_mode_t and fop_flags_t into <linux/types.h>
From: Jan Kara @ 2026-06-08 9:57 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Jan Kara, Jens Axboe, Alexander Viro,
linux-block, linux-kernel, linux-fsdevel, Carlos Maiolino,
linux-xfs, Chris Mason, David Sterba, linux-btrfs,
Theodore Ts'o, linux-ext4, Gao Xiang, linux-erofs
In-Reply-To: <20260602-work-super-bdev_holder_global-v1-1-bb0fd82f3861@kernel.org>
On Tue 02-06-26 12:10:07, Christian Brauner wrote:
> blk_mode_t and fop_flags_t are both plain 'unsigned int __bitwise' flag
> typedefs, exactly like the gfp_t, slab_flags_t and fmode_t that already
> live in <linux/types.h>. Move them there so they are available
> everywhere without having to drag in a subsystem header.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Makes sense. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> include/linux/blkdev.h | 2 --
> include/linux/fs.h | 2 --
> include/linux/types.h | 2 ++
> 3 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 890128cdea1c..c8494d64a69d 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -126,8 +126,6 @@ struct blk_integrity {
> unsigned char pi_tuple_size;
> };
>
> -typedef unsigned int __bitwise blk_mode_t;
> -
> /* open for reading */
> #define BLK_OPEN_READ ((__force blk_mode_t)(1 << 0))
> /* open for writing */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 11559c513dfb..e9346be8470f 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1921,8 +1921,6 @@ struct dir_context {
> struct io_uring_cmd;
> struct offset_ctx;
>
> -typedef unsigned int __bitwise fop_flags_t;
> -
> struct file_operations {
> struct module *owner;
> fop_flags_t fop_flags;
> diff --git a/include/linux/types.h b/include/linux/types.h
> index 608050dbca6a..ef026585420b 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -163,6 +163,8 @@ typedef u32 dma_addr_t;
> typedef unsigned int __bitwise gfp_t;
> typedef unsigned int __bitwise slab_flags_t;
> typedef unsigned int __bitwise fmode_t;
> +typedef unsigned int __bitwise blk_mode_t;
> +typedef unsigned int __bitwise fop_flags_t;
>
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> typedef u64 phys_addr_t;
>
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH RFC 3/8] fs: refuse to claim any frozen block device
From: Jan Kara @ 2026-06-08 10:01 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Jan Kara, Jens Axboe, Alexander Viro,
linux-block, linux-kernel, linux-fsdevel, Carlos Maiolino,
linux-xfs, Chris Mason, David Sterba, linux-btrfs,
Theodore Ts'o, linux-ext4, Gao Xiang, linux-erofs
In-Reply-To: <20260602-work-super-bdev_holder_global-v1-3-bb0fd82f3861@kernel.org>
On Tue 02-06-26 12:10:09, Christian Brauner wrote:
> setup_bdev_super() already refuses to bring a filesystem up on a frozen
> block device but only for the primary device. Now that filesystems claim
> every device through fs_bdev_file_open_by_{dev,path}(), do that check
> once in the registration helper so it covers all of them.
>
> Drop the now-redundant check from setup_bdev_super().
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
> fs/super.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index e0174d5819a0..cea743f699e4 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1690,6 +1690,17 @@ static int fs_bdev_register(struct file *bdev_file, struct super_block *sb)
> sb->s_count++;
> spin_unlock(&sb_lock);
>
> + /*
> + * Don't bring a filesystem up on a frozen device. The entry is already
> + * published, so a freeze either is seen here or finds it and waits in
> + * super_lock() until this mount is born or (on -EBUSY) dies. The mount
> + * aborts, so the entry is torn down without rebalancing @fs_bdev_active.
> + */
> + if (atomic_read(&file_bdev(bdev_file)->bd_fsfreeze_count) > 0) {
> + fs_bdev_holder_put(h);
> + return -EBUSY;
> + }
> +
> return 0;
> }
Shouldn't this check be common also for the branch where we only increase
the refcount? Or is a filesystem where a superblock claims the bdev
multiple times and can get frozen inbetween too insane?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH RFC 2/8] fs: add a global device to super block hash table
From: Jan Kara @ 2026-06-08 10:14 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Jan Kara, Jens Axboe, Alexander Viro,
linux-block, linux-kernel, linux-fsdevel, Carlos Maiolino,
linux-xfs, Chris Mason, David Sterba, linux-btrfs,
Theodore Ts'o, linux-ext4, Gao Xiang, linux-erofs
In-Reply-To: <20260602-work-super-bdev_holder_global-v1-2-bb0fd82f3861@kernel.org>
On Tue 02-06-26 12:10:08, Christian Brauner wrote:
> fs_holder_ops recovers the owning superblock from bdev->bd_holder, which
> forces the holder to be exactly one superblock and prevents several
> superblocks from sharing one block device. That's what erofs is doing.
>
> Introduce a global dev_t-keyed rhltable mapping each block device to the
> superblock(s) using it. The holder argument becomes purely the block
> layer's exclusivity token (a superblock, or a file_system_type for
> shared devices) and is no longer needed by the fs specific callbacks.
>
> Registration keeps one entry per (device, superblock). When a filesystem
> claims a device it already uses (xfs with its log on the data device), no
> second entry is added, so each superblock is acted on once.
>
> Each table entry holds a passive reference (s_count) on its superblock,
> so the struct stays valid for as long as the entry is reachable. The
> callbacks look the device up in the table and act on every superblock
> using it:
>
> Unlinking an entry is deferred to the last unpin, so a cursor never
> resumes from a removed node. After this it's possible to act on all
> superblocks that share a given device.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Looks good! One comment below:
> static void fs_bdev_mark_dead(struct block_device *bdev, bool surprise)
> {
> - struct super_block *sb;
> + struct fs_bdev_holder *h;
> + dev_t dev = bdev->bd_dev;
>
> - sb = bdev_super_lock(bdev, false);
> - if (!sb)
> - return;
> + mutex_unlock(&bdev->bd_holder_lock);
The moment we drop bd_holder_lock, there's nothing which prevents the bdev
owner from changing. So this can lead to a situation where we miss calling
->mark_dead callback of the new holder. Similarly for all the other holder
ops. I didn't find a situation where it would actually matter so I think
we're fine but it's a potential catch. Anyway, feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
>
> - if (sb->s_op->remove_bdev) {
> - int ret;
> + for (h = fs_bdev_first(dev); h; h = fs_bdev_next(h)) {
> + struct super_block *sb = h->sb;
>
> - ret = sb->s_op->remove_bdev(sb, bdev);
> - if (!ret) {
> - super_unlock_shared(sb);
> - return;
> + if (!super_lock_shared(sb))
> + continue;
> + if (sb->s_root && (sb->s_flags & SB_ACTIVE)) {
> + if (!sb->s_op->remove_bdev ||
> + sb->s_op->remove_bdev(sb, bdev)) {
> + if (!surprise)
> + sync_filesystem(sb);
> + shrink_dcache_sb(sb);
> + evict_inodes(sb);
> + if (sb->s_op->shutdown)
> + sb->s_op->shutdown(sb);
> + }
> }
> - /* Fallback to shutdown. */
> + super_unlock_shared(sb);
> }
> -
> - if (!surprise)
> - sync_filesystem(sb);
> - shrink_dcache_sb(sb);
> - evict_inodes(sb);
> - if (sb->s_op->shutdown)
> - sb->s_op->shutdown(sb);
> -
> - super_unlock_shared(sb);
> }
>
> static void fs_bdev_sync(struct block_device *bdev)
> {
> - struct super_block *sb;
> + struct fs_bdev_holder *h;
> + dev_t dev = bdev->bd_dev;
>
> - sb = bdev_super_lock(bdev, false);
> - if (!sb)
> - return;
> + mutex_unlock(&bdev->bd_holder_lock);
>
> - sync_filesystem(sb);
> - super_unlock_shared(sb);
> -}
> + for (h = fs_bdev_first(dev); h; h = fs_bdev_next(h)) {
> + struct super_block *sb = h->sb;
>
> -static struct super_block *get_bdev_super(struct block_device *bdev)
> -{
> - bool active = false;
> - struct super_block *sb;
> -
> - sb = bdev_super_lock(bdev, true);
> - if (sb) {
> - active = atomic_inc_not_zero(&sb->s_active);
> - super_unlock_excl(sb);
> + if (!super_lock_shared(sb))
> + continue;
> + if (sb->s_root && (sb->s_flags & SB_ACTIVE))
> + sync_filesystem(sb);
> + super_unlock_shared(sb);
> }
> - if (!active)
> - return NULL;
> - return sb;
> }
>
> /**
> - * fs_bdev_freeze - freeze owning filesystem of block device
> + * fs_bdev_freeze - freeze every superblock using a block device
> * @bdev: block device
> *
> - * Freeze the filesystem that owns this block device if it is still
> - * active.
> - *
> - * A filesystem that owns multiple block devices may be frozen from each
> - * block device and won't be unfrozen until all block devices are
> - * unfrozen. Each block device can only freeze the filesystem once as we
> - * nest freezes for block devices in the block layer.
> + * Freeze each live superblock using @bdev. A superblock owning several block
> + * devices is frozen once per device and stays frozen until all are thawed; the
> + * block layer nests these freezes so the count stays balanced.
> *
> - * Return: If the freeze was successful zero is returned. If the freeze
> - * failed a negative error code is returned.
> + * Return: 0, or the error from the one superblock on a single-fs device. When
> + * several superblocks share @bdev a per-superblock failure is swallowed
> + * (see below), but a sync_blockdev() failure is always reported.
> */
> static int fs_bdev_freeze(struct block_device *bdev)
> {
> - struct super_block *sb;
> - int error = 0;
> + dev_t dev = bdev->bd_dev;
> + struct fs_bdev_holder *h;
> + unsigned int count = 0;
> + int error = 0, err;
>
> lockdep_assert_held(&bdev->bd_fsfreeze_mutex);
>
> - sb = get_bdev_super(bdev);
> - if (!sb)
> - return -EINVAL;
> + mutex_unlock(&bdev->bd_holder_lock);
>
> - if (sb->s_op->freeze_super)
> - error = sb->s_op->freeze_super(sb,
> - FREEZE_MAY_NEST | FREEZE_HOLDER_USERSPACE, NULL);
> - else
> - error = freeze_super(sb,
> - FREEZE_MAY_NEST | FREEZE_HOLDER_USERSPACE, NULL);
> + for (h = fs_bdev_first(dev); h; h = fs_bdev_next(h)) {
> + if (!atomic_inc_not_zero(&h->sb->s_active))
> + continue;
> + err = fs_super_freeze(h->sb);
> + if (err && !error)
> + error = err;
> + deactivate_super(h->sb);
> + count++;
> + }
> +
> + /*
> + * When several superblocks share the device, keep it frozen even if some
> + * of them failed to freeze and swallow the error: rolling the rest back
> + * via thaw_super() can fail too, so neither is a clear win. A single
> + * filesystem (count == 1) still reports its error.
> + */
> + if (error && count > 1)
> + error = 0;
> if (!error)
> error = sync_blockdev(bdev);
> - deactivate_super(sb);
> return error;
> }
>
> /**
> - * fs_bdev_thaw - thaw owning filesystem of block device
> + * fs_bdev_thaw - thaw every superblock using a block device
> * @bdev: block device
> *
> - * Thaw the filesystem that owns this block device.
> + * The counterpart to fs_bdev_freeze(): thaw each live superblock using @bdev.
> + * A zero return does not imply a superblock is fully unfrozen; it may have been
> + * frozen more than once (by the kernel or via another device).
> *
> - * A filesystem that owns multiple block devices may be frozen from each
> - * block device and won't be unfrozen until all block devices are
> - * unfrozen. Each block device can only freeze the filesystem once as we
> - * nest freezes for block devices in the block layer.
> - *
> - * Return: If the thaw was successful zero is returned. If the thaw
> - * failed a negative error code is returned. If this function
> - * returns zero it doesn't mean that the filesystem is unfrozen
> - * as it may have been frozen multiple times (kernel may hold a
> - * freeze or might be frozen from other block devices).
> + * Return: 0, or the first error on a single-fs device; a shared device swallows
> + * per-superblock errors, as fs_bdev_freeze() does.
> */
> static int fs_bdev_thaw(struct block_device *bdev)
> {
> - struct super_block *sb;
> - int error;
> + dev_t dev = bdev->bd_dev;
> + struct fs_bdev_holder *h;
> + unsigned int count = 0;
> + int error = 0, err;
>
> lockdep_assert_held(&bdev->bd_fsfreeze_mutex);
>
> - /*
> - * The block device may have been frozen before it was claimed by a
> - * filesystem. Concurrently another process might try to mount that
> - * frozen block device and has temporarily claimed the block device for
> - * that purpose causing a concurrent fs_bdev_thaw() to end up here. The
> - * mounter is already about to abort mounting because they still saw an
> - * elevanted bdev->bd_fsfreeze_count so get_bdev_super() will return
> - * NULL in that case.
> - */
> - sb = get_bdev_super(bdev);
> - if (!sb)
> - return -EINVAL;
> + mutex_unlock(&bdev->bd_holder_lock);
>
> - if (sb->s_op->thaw_super)
> - error = sb->s_op->thaw_super(sb,
> - FREEZE_MAY_NEST | FREEZE_HOLDER_USERSPACE, NULL);
> - else
> - error = thaw_super(sb,
> - FREEZE_MAY_NEST | FREEZE_HOLDER_USERSPACE, NULL);
> - deactivate_super(sb);
> + for (h = fs_bdev_first(dev); h; h = fs_bdev_next(h)) {
> + if (!atomic_inc_not_zero(&h->sb->s_active))
> + continue;
> + err = fs_super_thaw(h->sb);
> + if (err && !error)
> + error = err;
> + deactivate_super(h->sb);
> + count++;
> + }
> +
> + /* Shared device: swallow per-superblock errors, like fs_bdev_freeze(). */
> + if (error && count > 1)
> + error = 0;
> return error;
> }
>
> @@ -1602,6 +1651,131 @@ const struct blk_holder_ops fs_holder_ops = {
> };
> EXPORT_SYMBOL_GPL(fs_holder_ops);
>
> +static int fs_bdev_register(struct file *bdev_file, struct super_block *sb)
> +{
> + dev_t dev = file_bdev(bdev_file)->bd_dev;
> + struct rhlist_head *list, *pos;
> + struct fs_bdev_holder *h;
> + int err;
> +
> + /*
> + * A superblock may claim one device more than once (xfs with its log on
> + * the data device). Keep a single entry per (device, superblock) and
> + * count the claims in @fs_bdev_active; the entry lives until the last one
> + * is released.
> + */
> + scoped_guard(rcu) {
> + list = rhltable_lookup(&fs_bdev_supers, &dev, fs_bdev_params);
> + rhl_for_each_entry_rcu(h, pos, list, node)
> + if (h->sb == sb && refcount_inc_not_zero(&h->fs_bdev_active))
> + return 0;
> + }
> +
> + h = kmalloc(sizeof(*h), GFP_KERNEL);
> + if (!h)
> + return -ENOMEM;
> + h->dev = dev;
> + h->sb = sb;
> + refcount_set(&h->fs_bdev_passive, 1);
> + refcount_set(&h->fs_bdev_active, 1);
> +
> + err = rhltable_insert(&fs_bdev_supers, &h->node, fs_bdev_params);
> + if (err) {
> + kfree(h);
> + return err;
> + }
> +
> + /* The sb->s_count ref keeps @h->sb valid for as long as the entry exists. */
> + spin_lock(&sb_lock);
> + sb->s_count++;
> + spin_unlock(&sb_lock);
> +
> + return 0;
> +}
> +
> +/**
> + * fs_bdev_file_open_by_dev - claim a block device on behalf of a superblock
> + * @dev: block device number
> + * @mode: open mode
> + * @holder: block-layer exclusivity token (a superblock, or the file_system_type
> + * when the device may be shared by several superblocks of that type)
> + * @sb: superblock to drive fs_holder_ops events for
> + *
> + * Open @dev with &fs_holder_ops and register that @sb uses it, so device
> + * removal/sync/freeze/thaw are propagated to @sb (and any other superblock
> + * sharing @dev). Must be paired with fs_bdev_file_release().
> + *
> + * Return: an opened block-device file or an ERR_PTR().
> + */
> +struct file *fs_bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> + struct super_block *sb)
> +{
> + struct file *bdev_file;
> + int err;
> +
> + bdev_file = bdev_file_open_by_dev(dev, mode, holder, &fs_holder_ops);
> + if (IS_ERR(bdev_file))
> + return bdev_file;
> +
> + err = fs_bdev_register(bdev_file, sb);
> + if (err) {
> + bdev_fput(bdev_file);
> + return ERR_PTR(err);
> + }
> + return bdev_file;
> +}
> +EXPORT_SYMBOL_GPL(fs_bdev_file_open_by_dev);
> +
> +struct file *fs_bdev_file_open_by_path(const char *path, blk_mode_t mode,
> + void *holder, struct super_block *sb)
> +{
> + struct file *bdev_file;
> + int err;
> +
> + bdev_file = bdev_file_open_by_path(path, mode, holder, &fs_holder_ops);
> + if (IS_ERR(bdev_file))
> + return bdev_file;
> +
> + err = fs_bdev_register(bdev_file, sb);
> + if (err) {
> + bdev_fput(bdev_file);
> + return ERR_PTR(err);
> + }
> + return bdev_file;
> +}
> +EXPORT_SYMBOL_GPL(fs_bdev_file_open_by_path);
> +
> +/**
> + * fs_bdev_file_release - release a block device claimed for a superblock
> + * @bdev_file: file returned by fs_bdev_file_open_by_{dev,path}()
> + * @sb: superblock the device was claimed for
> + *
> + * Drop one claim on the {dev, @sb} entry; the last claim unregisters it (a
> + * pinning cursor defers the actual unlink). Then close the block device.
> + */
> +void fs_bdev_file_release(struct file *bdev_file, struct super_block *sb)
> +{
> + dev_t dev = file_bdev(bdev_file)->bd_dev;
> + struct fs_bdev_holder *h, *found = NULL;
> + struct rhlist_head *list, *pos;
> +
> + rcu_read_lock();
> + list = rhltable_lookup(&fs_bdev_supers, &dev, fs_bdev_params);
> + rhl_for_each_entry_rcu(h, pos, list, node) {
> + if (h->sb != sb)
> + continue;
> + /* At most one entry per (dev, sb); the last claim drops the bias. */
> + if (refcount_dec_and_test(&h->fs_bdev_active))
> + found = h;
> + break;
> + }
> + rcu_read_unlock();
> + if (found)
> + fs_bdev_holder_put(found);
> + bdev_fput(bdev_file);
> +}
> +EXPORT_SYMBOL_GPL(fs_bdev_file_release);
> +
> int setup_bdev_super(struct super_block *sb, int sb_flags,
> struct fs_context *fc)
> {
> @@ -1609,7 +1783,7 @@ int setup_bdev_super(struct super_block *sb, int sb_flags,
> struct file *bdev_file;
> struct block_device *bdev;
>
> - bdev_file = bdev_file_open_by_dev(sb->s_dev, mode, sb, &fs_holder_ops);
> + bdev_file = fs_bdev_file_open_by_dev(sb->s_dev, mode, sb, sb);
> if (IS_ERR(bdev_file)) {
> if (fc)
> errorf(fc, "%s: Can't open blockdev", fc->source);
> @@ -1623,7 +1797,7 @@ int setup_bdev_super(struct super_block *sb, int sb_flags,
> * writable from userspace even for a read-only block device.
> */
> if ((mode & BLK_OPEN_WRITE) && bdev_read_only(bdev)) {
> - bdev_fput(bdev_file);
> + fs_bdev_file_release(bdev_file, sb);
> return -EACCES;
> }
>
> @@ -1634,7 +1808,7 @@ int setup_bdev_super(struct super_block *sb, int sb_flags,
> if (atomic_read(&bdev->bd_fsfreeze_count) > 0) {
> if (fc)
> warnf(fc, "%pg: Can't mount, blockdev is frozen", bdev);
> - bdev_fput(bdev_file);
> + fs_bdev_file_release(bdev_file, sb);
> return -EBUSY;
> }
> spin_lock(&sb_lock);
> @@ -1725,7 +1899,7 @@ void kill_block_super(struct super_block *sb)
> generic_shutdown_super(sb);
> if (bdev) {
> sync_blockdev(bdev);
> - bdev_fput(sb->s_bdev_file);
> + fs_bdev_file_release(sb->s_bdev_file, sb);
> }
> }
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index c8494d64a69d..43d37c02febf 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1760,13 +1760,6 @@ struct blk_holder_ops {
> int (*thaw)(struct block_device *bdev);
> };
>
> -/*
> - * For filesystems using @fs_holder_ops, the @holder argument passed to
> - * helpers used to open and claim block devices via
> - * bd_prepare_to_claim() must point to a superblock.
> - */
> -extern const struct blk_holder_ops fs_holder_ops;
> -
> /*
> * Return the correct open flags for blkdev_get_by_* for super block flags
> * as stored in sb->s_flags.
> diff --git a/include/linux/fs/super.h b/include/linux/fs/super.h
> index f21ffbb6dea5..721d842e3b24 100644
> --- a/include/linux/fs/super.h
> +++ b/include/linux/fs/super.h
> @@ -235,4 +235,11 @@ int freeze_super(struct super_block *super, enum freeze_holder who,
> int thaw_super(struct super_block *super, enum freeze_holder who,
> const void *freeze_owner);
>
> +struct file;
> +struct file *fs_bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> + struct super_block *sb);
> +struct file *fs_bdev_file_open_by_path(const char *path, blk_mode_t mode,
> + void *holder, struct super_block *sb);
> +void fs_bdev_file_release(struct file *bdev_file, struct super_block *sb);
> +
> #endif /* _LINUX_FS_SUPER_H */
>
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox