* [PATCH v2 0/2] scsi: sg: validate and clean up scatter_elem_sz handling
From: Yang Erkun @ 2026-07-09 4:32 UTC (permalink / raw)
To: bvanassche, dgilbert, James.Bottomley, martin.petersen, yukuai,
hch, axboe
Cc: linux-scsi, linux-block, yangerkun
v1->v2:
1. Split into two patches, patch 1 is the minimal bugfix that adds
validation to the scatter_elem_sz module parameter; patch 2 does the
refactoring
2. Rebased; no functional change versus v1.
v1:
https://patchwork.kernel.org/project/linux-scsi/patch/20260708063045.2008478-1-yangerkun@huawei.com/
Yang Erkun (2):
scsi: sg: validate scatter_elem_sz module parameter
scsi: sg: clean up scatter_elem_sz handling and module param
permissions
drivers/scsi/sg.c | 68 +++++++++++++++++++++++------------------------
1 file changed, 34 insertions(+), 34 deletions(-)
--
2.52.0
^ permalink raw reply
* [PATCH v2 1/2] scsi: sg: validate scatter_elem_sz module parameter
From: Yang Erkun @ 2026-07-09 4:32 UTC (permalink / raw)
To: bvanassche, dgilbert, James.Bottomley, martin.petersen, yukuai,
hch, axboe
Cc: linux-scsi, linux-block, yangerkun
In-Reply-To: <20260709043234.2447340-1-yangerkun@huawei.com>
echo -1 or 0 > /sys/module/sg/parameters/scatter_elem_sz
exec 4<> /dev/sg0
The above triggers the following UBSAN warning:
UBSAN: shift-out-of-bounds in drivers/scsi/sg.c:1888:13
shift exponent 64 is too large for 32-bit type 'int'
.....
Call Trace:
<TASK>
dump_stack_lvl+0x64/0x80
__ubsan_handle_shift_out_of_bounds+0x1d1/0x380
sg_build_indirect.cold+0x38/0x4b
sg_build_reserve+0x59/0x90
sg_add_sfp+0x151/0x240
sg_open+0x169/0x310
chrdev_open+0xbe/0x240
do_dentry_open+0x121/0x480
vfs_open+0x2e/0xf0
do_open+0x265/0x400
path_openat+0x110/0x2b0
do_file_open+0xe4/0x1a0
do_sys_openat2+0x7f/0xe0
__x64_sys_openat+0x56/0xa0
do_syscall_64+0xf5/0x640
entry_SYSCALL_64_after_hwframe+0x76/0x7e
The scatter_elem_sz module parameter currently lacks validation. Setting
it to -1/0 causes a left shift overflow in sg_build_indirect. Although
this overflow does not currently cause other problem, allowing
scatter_elem_sz to be set to -1/0 is not appropriate given its intended
purpose. Therefore, this patch uses module_param_call to add validation
checks: reject non-positive values and values whose order exceeds
MAX_PAGE_ORDER, and round the accepted value up to a power-of-two
multiple of PAGE_SIZE.
Fixes: 6460e75a104d ("[SCSI] sg: fixes for large page_size")
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
drivers/scsi/sg.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 74cd4e8a61c2..c56f8460c03f 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1622,7 +1622,33 @@ sg_remove_device(struct device *cl_dev)
kref_put(&sdp->d_ref, sg_device_destroy);
}
-module_param_named(scatter_elem_sz, scatter_elem_sz, int, S_IRUGO | S_IWUSR);
+static int scatter_elem_sz_set(const char *val, const struct kernel_param *kp)
+{
+ int ret, new_val, order;
+
+ ret = kstrtoint(val, 0, &new_val);
+ if (ret)
+ return ret;
+
+ if (new_val <= 0) {
+ pr_err("sg: scatter_elem_sz must be positive, got %d\n", new_val);
+ return -EINVAL;
+ }
+
+ order = get_order(new_val);
+ if (order > MAX_PAGE_ORDER) {
+ pr_err("sg: scatter_elem_sz too large (order %d > MAX_PAGE_ORDER %d)\n",
+ order, MAX_PAGE_ORDER);
+ return -EINVAL;
+ }
+
+ scatter_elem_sz = 1 << (PAGE_SHIFT + order);
+ return 0;
+}
+
+module_param_call(scatter_elem_sz, scatter_elem_sz_set, param_get_int,
+ &scatter_elem_sz, 0644);
+
module_param_named(allow_dio, sg_allow_dio, int, S_IRUGO | S_IWUSR);
static int def_reserved_size_set(const char *val, const struct kernel_param *kp)
--
2.52.0
^ permalink raw reply related
* [PATCH v2 2/2] scsi: sg: clean up scatter_elem_sz handling and module param permissions
From: Yang Erkun @ 2026-07-09 4:32 UTC (permalink / raw)
To: bvanassche, dgilbert, James.Bottomley, martin.petersen, yukuai,
hch, axboe
Cc: linux-scsi, linux-block, yangerkun
In-Reply-To: <20260709043234.2447340-1-yangerkun@huawei.com>
Now that scatter_elem_sz is validated and rounded up to a power-of-two
multiple of PAGE_SIZE by the .set callback at write time (and the
init-time value SG_SCATTER_SZ is always >= PAGE_SIZE), the runtime
fixup logic in sg_build_indirect() is redundant:
- The entry fixup (syncing scatter_elem_sz_prev to scatter_elem_sz)
was fragile -- it updated scatter_elem_sz_prev but not the local
variable "num", which is the exact source of the UBSAN warning
fixed by the previous patch.
- The in-loop fixup (updating scatter_elem_sz to ret_sz when
ret_sz > scatter_elem_sz_prev) is no longer needed because
scatter_elem_sz is always a power-of-two multiple of PAGE_SIZE,
making it always equal to ret_sz.
- The init_sg boot-time clamp is also removed as it can no longer
trigger (SG_SCATTER_SZ >= PAGE_SIZE, and the .set callback rejects
sub-PAGE_SIZE values).
Remove the scatter_elem_sz_prev variable, the entry/in-loop fixup, and
the boot-time clamp. Snapshot scatter_elem_sz into a local elem_sz for
get_order(), and switch the debug printk to "%s"/__func__ since the
"num" argument no longer exists.
While at it, replace S_IRUGO | S_IWUSR with 0644 for the allow_dio and
def_reserved_size module parameters, as checkpatch warns about symbolic
permissions on touched module_param lines.
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
drivers/scsi/sg.c | 40 +++++++---------------------------------
1 file changed, 7 insertions(+), 33 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c56f8460c03f..377c3e1dee7f 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -92,7 +92,6 @@ static int def_reserved_size = SG_DEF_RESERVED_SIZE;
static int sg_allow_dio = SG_ALLOW_DIO_DEF;
static int scatter_elem_sz = SG_SCATTER_SZ;
-static int scatter_elem_sz_prev = SG_SCATTER_SZ;
#define SG_SECTOR_SZ 512
@@ -1649,7 +1648,7 @@ static int scatter_elem_sz_set(const char *val, const struct kernel_param *kp)
module_param_call(scatter_elem_sz, scatter_elem_sz_set, param_get_int,
&scatter_elem_sz, 0644);
-module_param_named(allow_dio, sg_allow_dio, int, S_IRUGO | S_IWUSR);
+module_param_named(allow_dio, sg_allow_dio, int, 0644);
static int def_reserved_size_set(const char *val, const struct kernel_param *kp)
{
@@ -1675,8 +1674,7 @@ static const struct kernel_param_ops def_reserved_size_ops = {
.get = param_get_int,
};
-module_param_cb(def_reserved_size, &def_reserved_size_ops, &def_reserved_size,
- S_IRUGO | S_IWUSR);
+module_param_cb(def_reserved_size, &def_reserved_size_ops, &def_reserved_size, 0644);
MODULE_AUTHOR("Douglas Gilbert");
MODULE_DESCRIPTION("SCSI generic (sg) driver");
@@ -1694,11 +1692,6 @@ init_sg(void)
{
int rc;
- if (scatter_elem_sz < PAGE_SIZE) {
- scatter_elem_sz = PAGE_SIZE;
- scatter_elem_sz_prev = scatter_elem_sz;
- }
-
rc = register_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0),
SG_MAX_DEVS, "sg");
if (rc)
@@ -1880,9 +1873,10 @@ sg_build_sgat(Sg_scatter_hold * schp, const Sg_fd * sfp, int tablesize)
static int
sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size)
{
- int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems;
+ int ret_sz = 0, i, k, rem_sz, mx_sc_elems;
int sg_tablesize = sfp->parentdp->sg_tablesize;
int blk_size = buff_size, order;
+ int elem_sz = scatter_elem_sz;
gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO;
if (blk_size < 0)
@@ -1900,39 +1894,19 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size)
if (mx_sc_elems < 0)
return mx_sc_elems; /* most likely -ENOMEM */
- num = scatter_elem_sz;
- if (unlikely(num != scatter_elem_sz_prev)) {
- if (num < PAGE_SIZE) {
- scatter_elem_sz = PAGE_SIZE;
- scatter_elem_sz_prev = PAGE_SIZE;
- } else
- scatter_elem_sz_prev = num;
- }
-
- order = get_order(num);
+ order = get_order(elem_sz);
retry:
ret_sz = 1 << (PAGE_SHIFT + order);
for (k = 0, rem_sz = blk_size; rem_sz > 0 && k < mx_sc_elems;
k++, rem_sz -= ret_sz) {
-
- num = (rem_sz > scatter_elem_sz_prev) ?
- scatter_elem_sz_prev : rem_sz;
-
schp->pages[k] = alloc_pages(gfp_mask, order);
if (!schp->pages[k])
goto out;
- if (num == scatter_elem_sz_prev) {
- if (unlikely(ret_sz > scatter_elem_sz_prev)) {
- scatter_elem_sz = ret_sz;
- scatter_elem_sz_prev = ret_sz;
- }
- }
-
SCSI_LOG_TIMEOUT(5, sg_printk(KERN_INFO, sfp->parentdp,
- "sg_build_indirect: k=%d, num=%d, ret_sz=%d\n",
- k, num, ret_sz));
+ "%s: k=%d, ret_sz=%d\n",
+ __func__, k, ret_sz));
} /* end of for loop */
schp->page_order = order;
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v2] block: try slab allocation in bio_alloc_bioset() before mempool
From: Christoph Hellwig @ 2026-07-09 4:48 UTC (permalink / raw)
To: Joseph Qi
Cc: Jens Axboe, Christoph Hellwig, Baokun Li, linux-block,
linux-kernel
In-Reply-To: <20260709020145.4011533-1-joseph.qi@linux.alibaba.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [RFC PATCH v1 00/17] blk-cgroup: protect blkgs with blkcg_mutex
From: Christoph Hellwig @ 2026-07-09 6:09 UTC (permalink / raw)
To: Yu Kuai
Cc: Jens Axboe, Tejun Heo, Christoph Hellwig, Keith Busch,
Sagi Grimberg, Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Yu Kuai, Nilay Shroff, linux-block,
cgroups, linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260704195124.1375075-1-yukuai@kernel.org>
On Sun, Jul 05, 2026 at 03:51:07AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> This RFC moves queue-local blkg topology synchronization from
> q->queue_lock to q->blkcg_mutex.
>
> q->queue_lock is a hot block-layer spinlock used by request queue runtime
> paths,
I don't think it is hot any more. If it is in one of your workloads
we have a deep problem somewhere. That being said, futher removing
uses of his old catch-all lock is always good, hopefully we can
eventually remove it entirely.
So this series looks great to me conceptually, but I'm unfortunately
not a very qualified reviewer for the blk-cgroup code.
^ permalink raw reply
* Re: [RFC PATCH v1 01/17] nvme-multipath: retarget failedover bios from requeue work
From: Christoph Hellwig @ 2026-07-09 6:10 UTC (permalink / raw)
To: Yu Kuai
Cc: Jens Axboe, Tejun Heo, Christoph Hellwig, Keith Busch,
Sagi Grimberg, Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Yu Kuai, Nilay Shroff, linux-block,
cgroups, linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260704195124.1375075-2-yukuai@kernel.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [RFC PATCH v1 05/17] block: add bio_alloc_atomic() for atomic bio users
From: Christoph Hellwig @ 2026-07-09 6:12 UTC (permalink / raw)
To: Yu Kuai
Cc: Jens Axboe, Tejun Heo, Christoph Hellwig, Keith Busch,
Sagi Grimberg, Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Yu Kuai, Nilay Shroff, linux-block,
cgroups, linux-nvme, dm-devel,
linux-bcache@vger.kernel.org Joseph Qi
In-Reply-To: <20260704195124.1375075-6-yukuai@kernel.org>
On Sun, Jul 05, 2026 at 03:51:12AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Add bio_alloc_atomic() for callers that need a GFP_ATOMIC bio from the
> default bio set but cannot safely pass a bdev during allocation. The
> helper returns an unattached bio, leaving callers to set bi_bdev and
> attach blkcg state explicitly before submission.
>
> Use the helper for virtio-pmem flush child bios and OCFS2 heartbeat I/O.
> Both allocate bios from atomic paths and must avoid creating missing blkgs
> once blkg creation is protected by q->blkcg_mutex. virtio-pmem clones the
> parent bio's blkg association; OCFS2 binds heartbeat I/O to the root blkg.
Let's kill off the concept of atomic bio allocations instead.
Joseph already has an outstanding patch for nd_virtio that needs a little
bit more work, and ocfs2 should be easy enough as well.
^ permalink raw reply
* Re: [RFC PATCH v1 06/17] blk-cgroup: support non-blocking bio association
From: Christoph Hellwig @ 2026-07-09 6:13 UTC (permalink / raw)
To: Yu Kuai
Cc: Jens Axboe, Tejun Heo, Christoph Hellwig, Keith Busch,
Sagi Grimberg, Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Yu Kuai, Nilay Shroff, linux-block,
cgroups, linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260704195124.1375075-7-yukuai@kernel.org>
On Sun, Jul 05, 2026 at 03:51:13AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Allow bio association helpers to be called from non-blocking paths by
> returning whether the association succeeded and by taking a nowait argument.
> The normal callers pass nowait=false and keep the existing behavior of
> creating missing blkgs.
>
> For nowait=true, the helper only succeeds when the needed blkg already
> exists. This lets callers set or clone a bio's bdev without entering the
> sleepable missing-blkg creation path.
I think this is not needed once we kill nonblocking bio allocations,
but if not please have a clearly visible separate API for the
non-blocking version.
^ permalink raw reply
* Re: [RFC PATCH v1 07/17] block: support non-blocking bio allocation with a bdev
From: Christoph Hellwig @ 2026-07-09 6:15 UTC (permalink / raw)
To: Yu Kuai
Cc: Jens Axboe, Tejun Heo, Christoph Hellwig, Keith Busch,
Sagi Grimberg, Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Yu Kuai, Nilay Shroff, linux-block,
cgroups, linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260704195124.1375075-8-yukuai@kernel.org>
Maybe we should just move the blkcg allocation back to bio_submit where
we know we can sleep?
^ permalink raw reply
* Re: [RFC PATCH v1 15/17] blk-cgroup: remove blkg radix tree preloading
From: Christoph Hellwig @ 2026-07-09 6:18 UTC (permalink / raw)
To: Yu Kuai
Cc: Jens Axboe, Tejun Heo, Christoph Hellwig, Keith Busch,
Sagi Grimberg, Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Yu Kuai, Nilay Shroff, linux-block,
cgroups, linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260704195124.1375075-16-yukuai@kernel.org>
On Sun, Jul 05, 2026 at 03:51:22AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> blkg creation is now serialized by q->blkcg_mutex and no longer runs
> under q->queue_lock. The radix tree is initialized with GFP_NOWAIT, so
> radix_tree_insert() cannot sleep while blkcg->lock is held and the old
> preload dance is no longer needed.
>
> Remove the preload calls and the associated unwind path.
Isn't the GFP_NOWAIT a bit of a problem because it can fail way too
easy?
What about converting both the radix tree and list to an xarray
using the internal xarray to deal with sleeping allocations?
^ permalink raw reply
* [PATCH v8 0/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT
From: Ionut Nechita (Wind River) @ 2026-07-09 6:38 UTC (permalink / raw)
To: axboe, linux-block
Cc: bigeasy, bvanassche, clrkwllms, rostedt, ming.lei, muchun.song,
mkhalfella, chris.friesen, linux-kernel, linux-rt-devel,
linux-rt-users, stable, ionut_n2001, sunlightlinux,
Ionut Nechita (Wind River)
From: Ionut Nechita <ionut.nechita@windriver.com>
Hi Jens,
This is v8 of the fix for the PREEMPT_RT performance regression caused by
commit 6bda857bcbb86 ("block: fix ordering between checking
QUEUE_FLAG_QUIESCED request adding").
Changes since v7 (May 12):
- No code or commit-message-body changes; this is a tags-only respin.
Rebased on linux-next (next-20260708); the patch applies cleanly with
no context changes.
- Picked up Reviewed-by: Bart Van Assche <bvanassche@acm.org> on 1/1.
- Added an "Assisted-by:" trailer per the new AI contribution guidance in
Documentation/process/coding-assistants.rst, to record that Claude
assisted with this patch and that checkpatch was the analysis tool run.
As that document requires, the Signed-off-by / DCO certification is
and remains mine as the human submitter; no Signed-off-by was added by
the AI.
Changes since v6 (May 6):
- Reader-side barrier in blk_mq_run_hw_queue() changed from smp_rmb() to
smp_mb(). The race closed by commit 6bda857bcbb86 is a store-buffer
pattern: one CPU inserts a request and then reads the quiesce state,
another CPU unquiesces and then reads "has pending work". A full
barrier is needed on *both* sides, not just a read barrier on the
reader, so smp_mb() now pairs with the existing writer-side
smp_mb__after_atomic(). Thanks to Bart Van Assche for pointing out
that smp_rmb() was insufficient.
- Rewrote the in-code comments and the commit message to spell out which
ordering the removed q->queue_lock acquisitions provided and how it is
preserved.
The problem: on PREEMPT_RT, the spinlock_t q->queue_lock that commit
6bda857bcbb86 added to blk_mq_run_hw_queue() converts to a sleeping
rt_mutex. blk_mq_run_hw_queue() runs from every MSI-X IRQ thread and
hits that lock on the common "nothing pending" path, so all IRQ threads
serialise and go to D-state. On a Broadcom/LSI MegaRAID 12GSAS/PCIe
Secure SAS39xx (megaraid_sas, 128 MSI-X vectors, 120 hw queues),
throughput drops from 640 MB/s to 153 MB/s.
The fix takes the memory-barrier alternative and folds the quiesce
indicator into quiesce_depth itself: quiesce_depth becomes atomic_t,
QUEUE_FLAG_QUIESCED goes away, and no lock is left on the dispatch hot
path.
v7: https://lore.kernel.org/linux-block/20260512062815.10815-1-ionut.nechita@windriver.com/
Ionut Nechita (1):
block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention
on RT
block/blk-core.c | 1 +
block/blk-mq-debugfs.c | 1 -
block/blk-mq.c | 69 ++++++++++++++++++++++++++----------------
include/linux/blkdev.h | 9 ++++--
4 files changed, 50 insertions(+), 30 deletions(-)
base-commit: b9810cd75b9fb56a3425d391cba3f608502bd474
--
2.54.0
^ permalink raw reply
* [PATCH v8 1/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT
From: Ionut Nechita (Wind River) @ 2026-07-09 6:38 UTC (permalink / raw)
To: axboe, linux-block
Cc: bigeasy, bvanassche, clrkwllms, rostedt, ming.lei, muchun.song,
mkhalfella, chris.friesen, linux-kernel, linux-rt-devel,
linux-rt-users, stable, ionut_n2001, sunlightlinux, Ionut Nechita
In-Reply-To: <20260709063803.23538-1-ionut.nechita@windriver.com>
From: Ionut Nechita <ionut.nechita@windriver.com>
On PREEMPT_RT kernels, commit 6bda857bcbb86 ("block: fix ordering
between checking QUEUE_FLAG_QUIESCED request adding") causes a severe
throughput regression on systems with many MSI-X interrupt vectors.
That commit closed a store/load race between blk_mq_run_hw_queue() and
blk_mq_unquiesce_queue() by taking q->queue_lock around the requiesce
re-check in blk_mq_run_hw_queue(). Its changelog noted two ways to fix
the race -- (1) a pair of memory barriers, or (2) the queue_lock -- and
picked (2) because barriers are harder to maintain.
On RT, spinlock_t becomes a sleeping rt_mutex. blk_mq_run_hw_queue() is
called from every IRQ thread, and the re-check path is hit on the very
common "nothing pending" case, so all IRQ threads end up serialising on
the single q->queue_lock and block in D-state. On a Broadcom/LSI
MegaRAID 12GSAS/PCIe Secure SAS39xx (megaraid_sas, 128 MSI-X vectors,
120 hw queues) throughput drops from 640 MB/s to 153 MB/s.
Take approach (1) instead, and while at it turn quiesce_depth into the
single source of truth for the quiesce state:
- quiesce_depth becomes atomic_t and QUEUE_FLAG_QUIESCED is removed;
blk_queue_quiesced() is now "atomic_read(&q->quiesce_depth) > 0".
This also makes blk_queue_quiesced(), which is read locklessly from
the dispatch path, a clean atomic load instead of a plain-int read
racing with a spin_lock-protected int update.
- blk_mq_quiesce_queue_nowait() does an atomic_inc() followed by
smp_mb__after_atomic(). The spin_lock() it used to take only served
to publish the state change; every caller still follows the quiesce
with blk_mq_wait_quiesce_done() (synchronize_srcu()/synchronize_rcu()),
which is what actually drains in-flight dispatchers and makes the new
state globally visible. The barrier here just keeps the helper
self-contained for the few callers that defer that wait.
- blk_mq_unquiesce_queue() uses atomic_dec_if_positive() (so the
WARN-on-underflow check and the decrement are one atomic op) followed
by smp_mb__after_atomic() before blk_mq_run_hw_queues(). This is the
write side of the race fixed above: a full barrier between the
quiesce_depth store and the blk_mq_hctx_has_pending() load.
- blk_mq_run_hw_queue() drops the q->queue_lock around the requiesce
re-check and uses smp_mb() instead. This is the read side: a full
barrier between the just-inserted request (the store that makes
blk_mq_hctx_has_pending() true) and the quiesce-state load. A full
barrier is required on both sides -- this is a classic store-buffer
pattern -- so smp_mb()/smp_mb__after_atomic() rather than a read
barrier; with that, at least one of the two racing CPUs observes the
other's store and the hw queue is not left both un-quiesced and not
rerun.
No locking remains on the dispatch hot path.
Performance on the RT kernel and the hardware above:
- Before: 153 MB/s, IRQ threads in D-state on q->queue_lock
- After: 640 MB/s, no IRQ threads blocked
The non-RT path replaces a queue_lock acquire/release on the re-check
with an smp_mb(), so it should be no worse, and it also stops taking
q->queue_lock from blk_mq_run_hw_queue() entirely.
Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Fixes: 6bda857bcbb86 ("block: fix ordering between checking QUEUE_FLAG_QUIESCED request adding")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8 checkpatch
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
block/blk-core.c | 1 +
block/blk-mq-debugfs.c | 1 -
block/blk-mq.c | 69 ++++++++++++++++++++++++++----------------
include/linux/blkdev.h | 9 ++++--
4 files changed, 50 insertions(+), 30 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 365641266c9e..79669957f47e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -477,6 +477,7 @@ struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id)
mutex_init(&q->limits_lock);
mutex_init(&q->rq_qos_mutex);
spin_lock_init(&q->queue_lock);
+ atomic_set(&q->quiesce_depth, 0);
init_waitqueue_head(&q->mq_freeze_wq);
mutex_init(&q->mq_freeze_lock);
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 6754d8f9449c..567eedbbf50f 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -93,7 +93,6 @@ static const char *const blk_queue_flag_name[] = {
QUEUE_FLAG_NAME(INIT_DONE),
QUEUE_FLAG_NAME(STATS),
QUEUE_FLAG_NAME(REGISTERED),
- QUEUE_FLAG_NAME(QUIESCED),
QUEUE_FLAG_NAME(RQ_ALLOC_TIME),
QUEUE_FLAG_NAME(HCTX_ACTIVE),
QUEUE_FLAG_NAME(SQ_SCHED),
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c850330a32b..186bbcc8b927 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -260,12 +260,16 @@ EXPORT_SYMBOL_GPL(blk_mq_unfreeze_queue_non_owner);
*/
void blk_mq_quiesce_queue_nowait(struct request_queue *q)
{
- unsigned long flags;
-
- spin_lock_irqsave(&q->queue_lock, flags);
- if (!q->quiesce_depth++)
- blk_queue_flag_set(QUEUE_FLAG_QUIESCED, q);
- spin_unlock_irqrestore(&q->queue_lock, flags);
+ atomic_inc(&q->quiesce_depth);
+ /*
+ * Publish the quiesce_depth increment. Callers must follow this
+ * with blk_mq_wait_quiesce_done() (synchronize_srcu()/
+ * synchronize_rcu()), which is what actually guarantees that any
+ * in-flight dispatcher has finished and that later dispatchers see
+ * the queue as quiesced; the barrier here only keeps this helper
+ * self-contained for the few callers that defer the wait.
+ */
+ smp_mb__after_atomic();
}
EXPORT_SYMBOL_GPL(blk_mq_quiesce_queue_nowait);
@@ -314,21 +318,30 @@ EXPORT_SYMBOL_GPL(blk_mq_quiesce_queue);
*/
void blk_mq_unquiesce_queue(struct request_queue *q)
{
- unsigned long flags;
- bool run_queue = false;
+ int depth;
- spin_lock_irqsave(&q->queue_lock, flags);
- if (WARN_ON_ONCE(q->quiesce_depth <= 0)) {
- ;
- } else if (!--q->quiesce_depth) {
- blk_queue_flag_clear(QUEUE_FLAG_QUIESCED, q);
- run_queue = true;
- }
- spin_unlock_irqrestore(&q->queue_lock, flags);
+ depth = atomic_dec_if_positive(&q->quiesce_depth);
+ if (WARN_ON_ONCE(depth < 0))
+ return;
- /* dispatch requests which are inserted during quiescing */
- if (run_queue)
+ if (depth == 0) {
+ /*
+ * Full barrier between the quiesce_depth store above and the
+ * blk_mq_hctx_has_pending() load done from blk_mq_run_hw_queues()
+ * below. This pairs with the smp_mb() before the requiesce
+ * re-check in blk_mq_run_hw_queue(): of the two racing CPUs
+ * (one inserting a request and then re-checking quiesce state,
+ * the other unquiescing here and then checking for pending
+ * work) at least one sees the other's store, so the hw queue
+ * is not left with a request stranded on a now-running queue.
+ *
+ * atomic_dec_if_positive() already orders the decrement on
+ * success, but spell the barrier out so the pairing is obvious.
+ */
+ smp_mb__after_atomic();
+ /* dispatch requests which are inserted during quiescing */
blk_mq_run_hw_queues(q, true);
+ }
}
EXPORT_SYMBOL_GPL(blk_mq_unquiesce_queue);
@@ -2331,17 +2344,21 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
need_run = blk_mq_hw_queue_need_run(hctx);
if (!need_run) {
- unsigned long flags;
-
/*
- * Synchronize with blk_mq_unquiesce_queue(), because we check
- * if hw queue is quiesced locklessly above, we need the use
- * ->queue_lock to make sure we see the up-to-date status to
- * not miss rerunning the hw queue.
+ * Re-check after a full barrier. A request may have been
+ * inserted before this call, while a concurrent
+ * blk_mq_unquiesce_queue() drops quiesce_depth to zero and
+ * then runs the hw queues. This smp_mb() orders the request
+ * insert (the store that makes blk_mq_hctx_has_pending() true)
+ * before the requiesce-state load below, and pairs with the
+ * smp_mb__after_atomic() between the quiesce_depth store and
+ * the blk_mq_hctx_has_pending() load in blk_mq_unquiesce_queue()
+ * (and in blk_mq_quiesce_queue_nowait()). With a full barrier
+ * on both sides, at least one CPU observes the other's store,
+ * so the queue is not left both un-quiesced and not rerun.
*/
- spin_lock_irqsave(&hctx->queue->queue_lock, flags);
+ smp_mb();
need_run = blk_mq_hw_queue_need_run(hctx);
- spin_unlock_irqrestore(&hctx->queue->queue_lock, flags);
if (!need_run)
return;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index dbb549cdfb77..9e49ab9c78ae 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -525,7 +525,8 @@ struct request_queue {
spinlock_t queue_lock;
- int quiesce_depth;
+ /* Atomic quiesce depth - also serves as quiesced indicator (depth > 0) */
+ atomic_t quiesce_depth;
struct gendisk *disk;
@@ -670,7 +671,6 @@ enum {
QUEUE_FLAG_INIT_DONE, /* queue is initialized */
QUEUE_FLAG_STATS, /* track IO start and completion times */
QUEUE_FLAG_REGISTERED, /* queue has been registered to a disk */
- QUEUE_FLAG_QUIESCED, /* queue has been quiesced */
QUEUE_FLAG_RQ_ALLOC_TIME, /* record rq->alloc_time_ns */
QUEUE_FLAG_HCTX_ACTIVE, /* at least one blk-mq hctx is active */
QUEUE_FLAG_SQ_SCHED, /* single queue style io dispatch */
@@ -708,7 +708,10 @@ void blk_queue_flag_clear(unsigned int flag, struct request_queue *q);
#define blk_noretry_request(rq) \
((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
REQ_FAILFAST_DRIVER))
-#define blk_queue_quiesced(q) test_bit(QUEUE_FLAG_QUIESCED, &(q)->queue_flags)
+static inline bool blk_queue_quiesced(struct request_queue *q)
+{
+ return atomic_read(&q->quiesce_depth) > 0;
+}
#define blk_queue_pm_only(q) atomic_read(&(q)->pm_only)
#define blk_queue_registered(q) test_bit(QUEUE_FLAG_REGISTERED, &(q)->queue_flags)
#define blk_queue_sq_sched(q) test_bit(QUEUE_FLAG_SQ_SCHED, &(q)->queue_flags)
base-commit: b9810cd75b9fb56a3425d391cba3f608502bd474
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] fs: report direct io constraints through file_getattr
From: Christoph Hellwig @ 2026-07-09 7:13 UTC (permalink / raw)
To: Keith Busch
Cc: linux-block, linux-ext4, linux-f2fs-devel, linux-fsdevel,
linux-xfs, axboe, jack, brauner, cem, jaegeuk, aalbersh, tytso,
Keith Busch, Christoph Hellwig
In-Reply-To: <20260708011843.1036846-1-kbusch@meta.com>
On Tue, Jul 07, 2026 at 06:18:43PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> Memory alignment constraints for direct io can vary depending on the
> backing storage hardware. Provide support through file_getattr to report
> the attributes necessary for applications to know how to construct valid
> read and write requests.
This probably wants to be split in one patch for the new UAPI,
one for the helper and one for each user.
And especially the UAPI one needs a much more detailed commit log
explaining it, including why this duplicates some of the informastion
already in statx and documenting the semantics for all the fields.
Andrey, is there a man page or other official documentation for
file_setattr/file_getattr?
> +/*
> + * Handle DIO alignment for block devices via fileattr.
> + */
Maybe note that this purely about the block device constraints,
and file systems may expose additional ones?
> +void bdev_fileattr(const struct inode *inode, struct file_kattr *fa)
> +{
> + struct block_device *bdev;
> +
> + memset(fa, 0, sizeof(*fa));
> + fa->fsx_valid = true;
> + fa->flags_valid = true;
Doing the basic file_kattr initialization here feels dangerous
if we want to be able to call this from file system implementations.
I'd rather leave the initialization to the caller.
> +
> + bdev = blkdev_get_no_open(inode->i_rdev, false);
> + if (!bdev)
> + return;
.. and explicitly pass in the block device. ->i_rdev always is
i_sb->s_dev, which might not be the relevant backing device, e.g. for XFS
it could be that or the block device in m_rtdev_targp.
If i_rdev is i_sb->s_dev we can just use sb->s_bdev without needing a new
open.
> + fa->fsx_dio_mem_align = bdev_dma_alignment(bdev) + 1;
> + fa->fsx_dio_offset_align = bdev_logical_block_size(bdev);
> + fa->fsx_dio_read_offset_align = bdev_logical_block_size(bdev);
> + fa->fsx_dio_virt_boundary_align = bdev_virt_boundary_alignment(bdev);
> + fa->fsx_max_segments = bdev_max_segments(bdev);
How is the max_segments value defined in a way that is meaningful to
userspace?
> @@ -1005,6 +1006,27 @@ int ext4_fileattr_get(struct dentry *dentry, struct file_kattr *fa)
> if (ext4_has_feature_project(inode->i_sb))
> fa->fsx_projid = from_kprojid(&init_user_ns, ei->i_projid);
>
> + if (S_ISREG(inode->i_mode)) {
You'll probably want to split this into a helper to keep it easily
readable.
> + u32 dio_align = ext4_dio_alignment(inode);
> +
> + if (dio_align != 0) {
> + struct block_device *bdev = inode->i_sb->s_bdev;
> +
> + if (dio_align == 1) {
> + fa->fsx_dio_mem_align = bdev_dma_alignment(bdev) + 1;
> + fa->fsx_dio_offset_align = bdev_logical_block_size(bdev);
> + fa->fsx_dio_read_offset_align = bdev_logical_block_size(bdev);
> + } else {
> + fa->fsx_dio_mem_align = dio_align;
> + fa->fsx_dio_offset_align = dio_align;
> + fa->fsx_dio_read_offset_align = dio_align;
> + }
Call bdev_fileattr and override the relevant field as needed?
Question to the ext4 maintainers: why does ext4_dio_alignment
affect the in-memory alignment? If it does so, it should probably
also affect the virt boundry alignment..
> + if (S_ISREG(inode->i_mode)) {
> + unsigned int bsize = i_blocksize(inode);
> + struct block_device *bdev = inode->i_sb->s_bdev;
> +
> + if (!f2fs_force_buffered_io(inode, WRITE)) {
Same comments as for ext4. Also f2fs does support multiple devices,
but I'm not sure how data is placed on them, or if a file can be
on multiple devices. We'll need input from the f2fs
maintainers/contributors here.
> @@ -88,8 +89,12 @@ int vfs_fileattr_get(struct dentry *dentry, struct file_kattr *fa)
> struct inode *inode = d_inode(dentry);
> int error;
>
> - if (!inode->i_op->fileattr_get)
> - return -ENOIOCTLCMD;
> + if (!inode->i_op->fileattr_get) {
> + if (!S_ISBLK(inode->i_mode))
> + return -ENOIOCTLCMD;
> + bdev_fileattr(inode, fa);
> + return 0;
Don't we also want to fill out the attributes for block devices
on file systems that provide a ->fileattr_get?. Also if we fill
out something, we need the security_inode_file_getattr as well.
> /**
> @@ -145,6 +155,8 @@ static int file_attr_to_fileattr(const struct file_attr *fattr,
>
> if (fattr->fa_xflags & ~mask)
> return -EINVAL;
> + if (fattr->fa_pad)
> + return -EINVAL;
How is this related?
> + if (whichfork == XFS_DATA_FORK && S_ISREG(VFS_I(ip)->i_mode)) {
This probably wants a separate helper.
> + struct xfs_buftarg *target = xfs_inode_buftarg(ip);
> + struct block_device *bdev = target->bt_bdev;
.. and if the generic helpers gets an explicitl bdev we can use it
here and just override one value for for xfs_is_cow_inode().
> +static inline unsigned int bdev_virt_boundary_alignment(struct block_device *bdev)
Overly long line.
^ permalink raw reply
* Re: [PATCH] fs: report direct io constraints through file_getattr
From: Andrey Albershteyn @ 2026-07-09 8:51 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Keith Busch, linux-block, linux-ext4, linux-f2fs-devel,
linux-fsdevel, linux-xfs, axboe, jack, brauner, cem, jaegeuk,
aalbersh, tytso, Keith Busch
In-Reply-To: <20260709071352.GA20180@lst.de>
On 2026-07-09 09:13:52, Christoph Hellwig wrote:
> On Tue, Jul 07, 2026 at 06:18:43PM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> >
> > Memory alignment constraints for direct io can vary depending on the
> > backing storage hardware. Provide support through file_getattr to report
> > the attributes necessary for applications to know how to construct valid
> > read and write requests.
>
> This probably wants to be split in one patch for the new UAPI,
> one for the helper and one for each user.
>
> And especially the UAPI one needs a much more detailed commit log
> explaining it, including why this duplicates some of the informastion
> already in statx and documenting the semantics for all the fields.
>
> Andrey, is there a man page or other official documentation for
> file_setattr/file_getattr?
No, I had a draft in cover letter but haven't got to sending it to
man-pages. I will prepare a man page.
--
- Andrey
^ permalink raw reply
* Re: [PATCH] fs: report direct io constraints through file_getattr
From: Jan Kara @ 2026-07-09 9:14 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Keith Busch, linux-block, linux-ext4, linux-f2fs-devel,
linux-fsdevel, linux-xfs, axboe, jack, brauner, cem, jaegeuk,
aalbersh, tytso, Keith Busch, Eric Biggers
In-Reply-To: <20260709071352.GA20180@lst.de>
On Thu 09-07-26 09:13:52, Christoph Hellwig wrote:
> On Tue, Jul 07, 2026 at 06:18:43PM -0700, Keith Busch wrote:
> > + u32 dio_align = ext4_dio_alignment(inode);
> > +
> > + if (dio_align != 0) {
>
>
> > + struct block_device *bdev = inode->i_sb->s_bdev;
> > +
> > + if (dio_align == 1) {
> > + fa->fsx_dio_mem_align = bdev_dma_alignment(bdev) + 1;
> > + fa->fsx_dio_offset_align = bdev_logical_block_size(bdev);
> > + fa->fsx_dio_read_offset_align = bdev_logical_block_size(bdev);
> > + } else {
> > + fa->fsx_dio_mem_align = dio_align;
> > + fa->fsx_dio_offset_align = dio_align;
> > + fa->fsx_dio_read_offset_align = dio_align;
> > + }
>
> Call bdev_fileattr and override the relevant field as needed?
>
> Question to the ext4 maintainers: why does ext4_dio_alignment
> affect the in-memory alignment? If it does so, it should probably
> also affect the virt boundry alignment..
I guess that is mostly a historical accident. ext4_dio_alignment() returns
1 (iomap alignment is used and that's different for memory and file offset
alignment), 0 (dio not supported, memory and file offset alignment is
indeed the same), and blocksize (a special case which can happen for
fscrypt if it supports dio and where I believe memory alignment
requirements may be in fact different). I'm adding Eric to CC to answer
what actual requirements fscrypt has for memory buffers for direct IO. I'd
expect with inline encryption we would have the same requirements as
ordinary iomap direct IO and for other code paths I'm not sure... Eric?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 1/2] umh, treewide: Explicitly include linux/umh.h where needed
From: Petr Pavlu @ 2026-07-09 9:49 UTC (permalink / raw)
To: Michal Koutný
Cc: Tony Luck, Borislav Petkov, Thomas Gleixner, Ingo Molnar,
Dave Hansen, x86, H. Peter Anvin, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Johan Hovold, Alex Elder,
Greg Kroah-Hartman, Rafael J. Wysocki, Michal Januszewski,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Trond Myklebust, Anna Schumaker, Chuck Lever, Jeff Layton,
NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey, Mark Fasheh,
Joel Becker, Joseph Qi, Tejun Heo, Johannes Weiner,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Pavel Machek, Len Brown, Andrew Morton, Danilo Krummrich,
Nikolay Aleksandrov, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, David Howells,
Jarkko Sakkinen, Paul Moore, James Morris, Serge E. Hallyn,
Kentaro Takeda, Tetsuo Handa, linux-edac, linux-kernel, drbd-dev,
linux-block, greybus-dev, linuxppc-dev, linux-acpi, linux-fbdev,
dri-devel, linux-fsdevel, linux-nfs, ocfs2-devel, cgroups,
linux-modules, linux-pm, driver-core, bridge, netdev, keyrings,
linux-security-module
In-Reply-To: <ak6STbqZd-Q-c56v@localhost.localdomain>
On 7/8/26 8:13 PM, Michal Koutný wrote:
> Hi Petr.
>
> On Wed, Jul 08, 2026 at 05:44:29PM +0200, Petr Pavlu <petr.pavlu@suse.com> wrote:
>> diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
>> index a4337c9b5287..60eb994c32ae 100644
>> --- a/kernel/cgroup/cgroup-v1.c
>> +++ b/kernel/cgroup/cgroup-v1.c
>> @@ -16,6 +16,7 @@
>> #include <linux/pid_namespace.h>
>> #include <linux/cgroupstats.h>
>> #include <linux/fs_parser.h>
>> +#include <linux/umh.h>
>>
>> #include <trace/events/cgroup.h>
>
> There is kmod.h in here too but it's unnecessary, no module lazy loading
> in this code.
You're right. I'll remove the kmod.h include from
kernel/cgroup/cgroup-v1.c. I went through all the files again and it
seems this was the only place I missed.
--
Thanks,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 05/17] block: add bio_alloc_atomic() for atomic bio users
From: yu kuai @ 2026-07-09 9:49 UTC (permalink / raw)
To: Christoph Hellwig, yu kuai
Cc: Jens Axboe, Tejun Heo, Keith Busch, Sagi Grimberg,
Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Nilay Shroff, linux-block, cgroups,
linux-nvme, dm-devel, linux-bcache@vger.kernel.org Joseph Qi
In-Reply-To: <20260709061207.GC16504@lst.de>
Hi,
在 2026/7/9 14:12, Christoph Hellwig 写道:
> On Sun, Jul 05, 2026 at 03:51:12AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai@fygo.io>
>>
>> Add bio_alloc_atomic() for callers that need a GFP_ATOMIC bio from the
>> default bio set but cannot safely pass a bdev during allocation. The
>> helper returns an unattached bio, leaving callers to set bi_bdev and
>> attach blkcg state explicitly before submission.
>>
>> Use the helper for virtio-pmem flush child bios and OCFS2 heartbeat I/O.
>> Both allocate bios from atomic paths and must avoid creating missing blkgs
>> once blkg creation is protected by q->blkcg_mutex. virtio-pmem clones the
>> parent bio's blkg association; OCFS2 binds heartbeat I/O to the root blkg.
> Let's kill off the concept of atomic bio allocations instead.
> Joseph already has an outstanding patch for nd_virtio that needs a little
> bit more work, and ocfs2 should be easy enough as well.
Thanks for the notice and good to hear that. This will make this set much
simpler.
>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [RFC PATCH v1 15/17] blk-cgroup: remove blkg radix tree preloading
From: yu kuai @ 2026-07-09 9:57 UTC (permalink / raw)
To: Christoph Hellwig, yu kuai
Cc: Jens Axboe, Tejun Heo, Keith Busch, Sagi Grimberg,
Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Nilay Shroff, linux-block, cgroups,
linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260709061837.GF16504@lst.de>
Hi,
在 2026/7/9 14:18, Christoph Hellwig 写道:
> On Sun, Jul 05, 2026 at 03:51:22AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai@fygo.io>
>>
>> blkg creation is now serialized by q->blkcg_mutex and no longer runs
>> under q->queue_lock. The radix tree is initialized with GFP_NOWAIT, so
>> radix_tree_insert() cannot sleep while blkcg->lock is held and the old
>> preload dance is no longer needed.
>>
>> Remove the preload calls and the associated unwind path.
> Isn't the GFP_NOWAIT a bit of a problem because it can fail way too
> easy?
I think GFP_NOWAIT should not be a problem because it's only possible to
allocate blkg when the thread is issuing the first IO. And it's not a big
deal to fail nowait in this case because the caller should fall back to
sleepable context to issue this IO, and then blkg will be created. The following
nowait IO issued by this thread should no longer hit blkg allocation path
anymore.
>
> What about converting both the radix tree and list to an xarray
> using the internal xarray to deal with sleeping allocations?
>
--
Thanks,
Kuai
^ permalink raw reply
* [PATCH] block: rust: fix `Send` bound for `GenDisk`
From: Yuan Tan @ 2026-07-09 10:00 UTC (permalink / raw)
To: a.hindborg
Cc: miguel.ojeda.sandonis, boqun, linux-block, rust-for-linux,
zhiyunq, ardalan, pgovind2, dzueck, yuantan098, Yuan Tan
From: Andreas Hindborg <a.hindborg@kernel.org>
The `Send` implementation for `GenDisk<T>` was conditioned on `T: Send`.
This constrains the wrong type. `T` is the `Operations` implementation,
which is typically a zero-sized marker type that carries no data, so `T:
Send` says nothing about whether the data a `GenDisk` actually owns can be
moved to another thread.
A `GenDisk<T>` owns the queue data `T::QueueData` (stored as the
`gendisk`'s `queuedata` and dropped when the `GenDisk` is dropped) and an
`Arc<TagSet<T>>`. These are the values transferred when a `GenDisk` is sent
across a thread boundary, so the `Send` bound must constrain exactly them.
Bound `T::QueueData: Send` and `Arc<TagSet<T>>: Send` instead.
Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module")
Links: https://lore.kernel.org/all/cover.1780633578.git.ytan089@ucr.edu
Cc: stable@vger.kernel.org
Reported-by: Priya Bala Govindasamy <pgovind2@uci.edu>
Reported-by: Dylan Zueck <dzueck@uci.edu>
Reported-by: Yuan Tan <ytan089@ucr.edu>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Yuan Tan <ytan089@ucr.edu>
---
Changes in v4:
Following Miguel’s suggestion, I am resending Andreas’ v2 patch, but with
Priya’s and Dylan’s Reported-by tags added.
I noticed that this patch has not been merged yet, and I am not sure what
its current status is.
Link to v3:
- https://lore.kernel.org/all/20260611003220.3512652-1-yuantan098@gmail.com/
Changes in v3:
- Add Priya and Dylan's names to the `Reported-by` tags
Link to v2:
- https://lore.kernel.org/all/20260609-rnull-v6-19-rc5-send-v2-1-82c7404542e2@kernel.org/
Link to v1:
- https://lore.kernel.org/all/cover.1780633578.git.ytan089@ucr.edu/
I am a bit unsure how to handle this v3.
rust/kernel/block/mq/gen_disk.rs | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index fc97dd873974..4d49f33c6fab 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -212,8 +212,14 @@ pub struct GenDisk<T: Operations> {
}
// SAFETY: `GenDisk` is an owned pointer to a `struct gendisk` and an `Arc` to a
-// `TagSet` It is safe to send this to other threads as long as T is Send.
-unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
+// `TagSet`. It is safe to send this to other threads as long as these two are `Send`.
+unsafe impl<T> Send for GenDisk<T>
+where
+ T: Operations,
+ T::QueueData: Send,
+ Arc<TagSet<T>>: Send,
+{
+}
impl<T: Operations> Drop for GenDisk<T> {
fn drop(&mut self) {
--
2.43.2
^ permalink raw reply related
* Re: [RFC PATCH v1 07/17] block: support non-blocking bio allocation with a bdev
From: yu kuai @ 2026-07-09 10:02 UTC (permalink / raw)
To: Christoph Hellwig, yu kuai
Cc: Jens Axboe, Tejun Heo, Keith Busch, Sagi Grimberg,
Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Nilay Shroff, linux-block, cgroups,
linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260709061500.GE16504@lst.de>
Hi,
在 2026/7/9 14:15, Christoph Hellwig 写道:
> Maybe we should just move the blkcg allocation back to bio_submit where
> we know we can sleep?
This sounds good, I already switch blkg allocation just before submit_bio()
in some cases in this set.
Just one question, for nowait case, is it correct that we still can't sleep
during submit_bio()? If so, I still need to add nowait special case during
blkg creation. Or I can just return -AGAIN if blkg lookup failed, this is
much simplier.
>
--
Thanks,
Kuai
^ permalink raw reply
* [PATCH v2] xen-blkfront: fix double completion of split requests on resume
From: Doruk Tan Ozturk @ 2026-07-09 10:08 UTC (permalink / raw)
To: Roger Pau Monné, Juergen Gross
Cc: Stefano Stabellini, Oleksandr Tyshchenko, Jens Axboe, xen-devel,
linux-block, linux-kernel, Doruk Tan Ozturk
When a block request is too large for a single ring entry and the
backend does not support indirect descriptors, blkfront splits it across
two ring requests. This only happens when the frontend runs on a
64K-page kernel (e.g. arm64): there, even a single-page request may not
fit in one ring slot and must be split. blkif_ring_get_request() is
called twice and both shadow slots (shadow[id] and shadow[extra_id])
point at the *same* struct request, linked through associated_id.
blkif_completion() collapses the pair on the normal completion path,
recycling the second slot and completing the request once. The
suspend/resume walk in blkfront_resume() does not: it visits every
shadow slot with ->request set and calls blk_mq_end_request() or
re-queues ->request. For an in-flight split request it therefore
processes the shared struct request twice on resume/migration -- a
double completion.
Skip the secondary slot of a split request in the resume walk so each
logical request is processed exactly once. The secondary slot is the
linked one (associated_id != NO_ASSOCIATED_ID) that carries no
scatter-gather list (num_sg == 0); the first slot always keeps the sg
list. The bug is only reachable on suspend/resume or live migration of
such a guest, so it has no local reproducer.
Fixes: 6cc568339047 ("xen/blkfront: Handle non-indirect grant with 64KB pages")
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
drivers/block/xen-blkfront.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index f765970578f9..8dad7bf5f664 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2079,6 +2079,15 @@ static int blkfront_resume(struct xenbus_device *dev)
if (!shadow[j].request)
continue;
+ /*
+ * For requests split across multiple slots, process the
+ * underlying request only once: skip the linked, sg-less
+ * secondary slot.
+ */
+ if (shadow[j].associated_id != NO_ASSOCIATED_ID &&
+ shadow[j].num_sg == 0)
+ continue;
+
/*
* Get the bios in the request so we can re-queue them.
*/
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH v1 00/17] blk-cgroup: protect blkgs with blkcg_mutex
From: yu kuai @ 2026-07-09 10:08 UTC (permalink / raw)
To: Christoph Hellwig, yu kuai
Cc: Jens Axboe, Tejun Heo, Keith Busch, Sagi Grimberg,
Alasdair Kergon, Benjamin Marzinski, Mike Snitzer,
Mikulas Patocka, Dongsheng Yang, Zheng Gu, Coly Li,
Kent Overstreet, Josef Bacik, Nilay Shroff, linux-block, cgroups,
linux-nvme, dm-devel, linux-bcache
In-Reply-To: <20260709060959.GA16504@lst.de>
Hi,
在 2026/7/9 14:09, Christoph Hellwig 写道:
> On Sun, Jul 05, 2026 at 03:51:07AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai@fygo.io>
>>
>> This RFC moves queue-local blkg topology synchronization from
>> q->queue_lock to q->blkcg_mutex.
>>
>> q->queue_lock is a hot block-layer spinlock used by request queue runtime
>> paths,
> I don't think it is hot any more. If it is in one of your workloads
> we have a deep problem somewhere. That being said, futher removing
> uses of his old catch-all lock is always good, hopefully we can
> eventually remove it entirely.
>
> So this series looks great to me conceptually, but I'm unfortunately
> not a very qualified reviewer for the blk-cgroup code.
Thanks a lot for taking a look at this RFC set, it's very helpful.
It's true that blk-cgroup review is not active for a long time. I'd like to
help but I really don't have time to check every block layer patches.
>
--
Thanks,
Kuai
^ permalink raw reply
* [PATCH V4 6/9] null_blk: clean up null_del_dev() to use cached dev pointer
From: Zizhi Wo @ 2026-07-09 10:04 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
Replace remaining nullb->dev dereferences with the already-cached
local dev variable. No functional change.
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index a75989220a2f..6d30591abb28 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1767,11 +1767,11 @@ static void null_del_dev(struct nullb *nullb)
list_del_init(&nullb->list);
del_gendisk(nullb->disk);
- if (test_bit(NULLB_DEV_FL_THROTTLED, &nullb->dev->flags)) {
+ if (test_bit(NULLB_DEV_FL_THROTTLED, &dev->flags)) {
hrtimer_cancel(&nullb->bw_timer);
atomic_long_set(&nullb->cur_bytes, LONG_MAX);
blk_mq_start_stopped_hw_queues(nullb->q, true);
}
@@ -1779,11 +1779,11 @@ static void null_del_dev(struct nullb *nullb)
null_free_zoned_dev(dev);
if (nullb->tag_set == &nullb->__tag_set)
blk_mq_free_tag_set(nullb->tag_set);
kfree(nullb->queues);
if (null_cache_active(nullb))
- null_free_device_storage(nullb->dev, true);
+ null_free_device_storage(dev, true);
kfree(nullb);
dev->nullb = NULL;
}
static void null_config_discard(struct nullb *nullb, struct queue_limits *lim)
--
2.52.0
^ permalink raw reply related
* [PATCH V4 2/9] null_blk: register configfs subsystem after creating default devices
From: Zizhi Wo @ 2026-07-09 10:04 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
In null_init(), configfs_register_subsystem() currently runs before
register_blkdev(), so when null_blk is built as a module, a racing mkdir()
+ poweron from userspace can reach null_add_dev() while null_major is still
0. __add_disk() then hits WARN_ON(disk->minors) (major=0 with minors!=0)
and fails:
[root@fedora ~]# [ 2366.521436] WARNING: block/genhd.c:476 at __add_disk+0x8a7/0xde0,
[ 2366.523552] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
[ 2366.529081] CPU: 26 UID: 0 PID: 1600 Comm: sh Not tainted 7.2.0-rc1+ #66 PREEMPT(full)
......
[ 2366.547251] Call Trace:
[ 2366.547575] <TASK>
[ 2366.547831] ? _raw_spin_lock+0x84/0xe0
[ 2366.548260] add_disk_fwnode+0x114/0x560
[ 2366.548739] null_add_dev+0x102d/0x1b80 [null_blk]
[ 2366.549310] ? __pfx_null_add_dev+0x10/0x10 [null_blk]
[ 2366.549906] ? mutex_lock+0xde/0x1c0
[ 2366.550361] ? __pfx_mutex_lock+0x10/0x10
[ 2366.550827] nullb_device_power_store+0x1e7/0x280 [null_blk]
[ 2366.551499] ? __pfx_nullb_device_power_store+0x10/0x10 [null_blk]
[ 2366.552177] ? __kmalloc_cache_noprof+0x1f5/0x470
[ 2366.552748] ? configfs_write_iter+0x35c/0x4e0
[ 2366.553242] configfs_write_iter+0x286/0x4e0
[ 2366.553787] vfs_write+0x52d/0xd00
[ 2366.554169] ? __pfx_vfs_write+0x10/0x10
[ 2366.554679] ? __pfx___css_rstat_updated+0x10/0x10
[ 2366.555196] ? fdget_pos+0x1cf/0x4c0
[ 2366.555649] ksys_write+0xfc/0x1d0
......
Additionally, the err_dev path destroys all devices on nullb_list while
configfs is still registered. If a racing mkdir() + poweron puts a user
device on the list, null_destroy_dev()->null_free_dev() kfrees the user
device's nullb_device but /sys/kernel/config/nullb/<name> is still
reachable. Any userspace access to the item will trigger a UAF.
For simplicity, move configfs_register_subsystem() to the end to solve
the problems above.
Fixes: 3bf2bd20734e ("nullb: add configfs interface")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk/main.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index eba204b27785..4613035222cd 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2160,37 +2160,33 @@ static int __init null_init(void)
}
config_group_init(&nullb_subsys.su_group);
mutex_init(&nullb_subsys.su_mutex);
- ret = configfs_register_subsystem(&nullb_subsys);
- if (ret)
- return ret;
-
null_major = register_blkdev(0, "nullb");
- if (null_major < 0) {
- ret = null_major;
- goto err_conf;
- }
+ if (null_major < 0)
+ return null_major;
for (i = 0; i < nr_devices; i++) {
ret = null_create_dev();
if (ret)
goto err_dev;
}
+ ret = configfs_register_subsystem(&nullb_subsys);
+ if (ret)
+ goto err_dev;
+
pr_info("module loaded\n");
return 0;
err_dev:
while (!list_empty(&nullb_list)) {
nullb = list_entry(nullb_list.next, struct nullb, list);
null_destroy_dev(nullb);
}
unregister_blkdev(null_major, "nullb");
-err_conf:
- configfs_unregister_subsystem(&nullb_subsys);
return ret;
}
static void __exit null_exit(void)
{
--
2.52.0
^ permalink raw reply related
* [PATCH V4 7/9] null_blk: reject per-device queue resize for shared tag set
From: Zizhi Wo @ 2026-07-09 10:04 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
When shared_tags is enabled, null_setup_tagset() makes the device use the
global tag_set, whose driver_data stays NULL. null_map_queues() therefore
falls back to the module-wide g_submit_queues/g_poll_queues instead of any
per-device value.
Resizing submit_queues or poll_queues via configfs on such a device calls
blk_mq_update_nr_hw_queues() on the shared set, shrinking
set->nr_hw_queues. __blk_mq_realloc_hw_ctxs() only grows the
q->queue_hw_ctx[] allocation, so on shrink it merely exits and NULLs the
now-excess hctx slots. null_map_queues(), however, keeps mapping CPUs with
the unchanged g_submit_queues/g_poll_queues, so mq_map[] ends up pointing
at those NULLed hctx slots. blk_mq_map_swqueue() then dereferences the NULL
hctx (hctx->cpumask), crashing the kernel:
[ 460.218374] KASAN: null-ptr-deref in range [0x0000000000000098-0x000000000000009f]
[ 460.219003] CPU: 24 UID: 0 PID: 1492 Comm: sh Not tainted 7.2.0-rc2+ #67 PREEMPT(full)
[ 460.219792] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-4.fc41 04/01/2014
[ 460.220452] RIP: 0010:blk_mq_map_swqueue+0x4db/0x1430
......
[ 460.228977] Call Trace:
[ 460.229175] <TASK>
[ 460.229354] blk_mq_update_nr_hw_queues+0xd49/0x11c0
[ 460.229779] ? __pfx_blk_mq_update_nr_hw_queues+0x10/0x10
[ 460.230200] nullb_update_nr_hw_queues+0x1a9/0x370 [null_blk]
[ 460.230694] nullb_device_submit_queues_store+0xd9/0x170 [null_blk]
[ 460.231190] ? __pfx_nullb_device_submit_queues_store+0x10/0x10 [null_blk]
[ 460.231776] ? configfs_write_iter+0x35c/0x4e0
[ 460.232122] configfs_write_iter+0x286/0x4e0
[ 460.232460] vfs_write+0x52d/0xd00
[ 460.232779] ? __x64_sys_openat+0x108/0x1d0
[ 460.233106] ? __pfx_vfs_write+0x10/0x10
[ 460.233413] ? fdget_pos+0x1cf/0x4c0
[ 460.233745] ? fput_close+0x133/0x190
[ 460.234038] ? __pfx_expand_files+0x10/0x10
[ 460.234368] ksys_write+0xfc/0x1d0
Reproducer:
modprobe null_blk shared_tags=1 submit_queues=64 poll_queues=1
mkdir /sys/kernel/config/nullb/dev
echo 1 > /sys/kernel/config/nullb/dev/power
echo 1 > /sys/kernel/config/nullb/dev/submit_queues
A per-device resize of a shared tag set is meaningless anyway, so reject it
with -EINVAL in nullb_update_nr_hw_queues() when the device is bound to the
global tag_set.
Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 6d30591abb28..340ecc0a331e 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -380,10 +380,19 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
int ret, nr_hw_queues;
if (!dev->nullb)
return 0;
+ /*
+ * A shared tag_set is mapped via the module-wide queue counts, so a
+ * per-device resize is meaningless. On shrink it would also leave
+ * mq_map[] pointing at NULLed hctx slots, causing a NULL deref in
+ * blk_mq_map_swqueue(). Reject it.
+ */
+ if (dev->nullb->tag_set == &tag_set)
+ return -EINVAL;
+
/*
* Make sure at least one submit queue exists.
*/
if (!submit_queues)
return -EINVAL;
--
2.52.0
^ permalink raw reply related
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