* [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
* [PATCH V4 8/9] null_blk: serialize configfs attribute stores with device setup
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>
The NULLB_DEVICE_ATTR _store takes no lock: apply_fn attributes
(submit_queues, poll_queues) get dev->NAME written again after apply_fn
returns, outside its lock; APPLY=NULL attributes are entirely lockless.
configfs only serializes stores per-open-file, so concurrent stores on
separate fds race.
For apply_fn attributes, once one store's apply_fn has reconfigured the
hardware, a second (losing) store can still overwrite dev->NAME
afterwards. This leaves dev->submit_queues out of sync with the live
queue count, which is later caught by the WARN_ON_ONCE() in
null_map_queues().
For !apply_fn attributes, power_store()'s null_add_dev() validates and
builds the device under "lock" but only sets CONFIGURED afterwards. A store
slipping in during this window can change a field mid-setup -- for example,
zone_nr_conv can be pushed above nr_zones after it has already been
clamped, leading to an out-of-bounds dev->zones[] access.
Take "lock" in the macro around the apply_fn call, the CONFIGURED test and
the field write, and move it out of nullb_apply_submit_queues()/
nullb_apply_poll_queues() so both paths are covered once. This serializes
stores with power_store's setup and with each other.
Also reset ret to 0 after the input parsing so that within the locked
section ret is purely a status code, rather than carrying the byte count
returned by nullb_device_##TYPE##_attr_store(). The field is then written
only on success via if (!ret), giving a single consistent rule for both the
apply_fn and the APPLY=NULL paths.
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 | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 340ecc0a331e..a411db6a8c40 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -358,17 +358,21 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \
int ret; \
\
ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\
if (ret < 0) \
return ret; \
+ ret = 0; \
+ mutex_lock(&lock); \
if (apply_fn) \
ret = apply_fn(dev, new_value); \
else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \
ret = -EBUSY; \
+ if (!ret) \
+ dev->NAME = new_value; \
+ mutex_unlock(&lock); \
if (ret < 0) \
return ret; \
- dev->NAME = new_value; \
return count; \
} \
CONFIGFS_ATTR(nullb_device_, NAME);
static int nullb_update_nr_hw_queues(struct nullb_device *dev,
@@ -428,29 +432,17 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
}
static int nullb_apply_submit_queues(struct nullb_device *dev,
unsigned int submit_queues)
{
- int ret;
-
- mutex_lock(&lock);
- ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
- mutex_unlock(&lock);
-
- return ret;
+ return nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
}
static int nullb_apply_poll_queues(struct nullb_device *dev,
unsigned int poll_queues)
{
- int ret;
-
- mutex_lock(&lock);
- ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
- mutex_unlock(&lock);
-
- return ret;
+ return nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
}
NULLB_DEVICE_ATTR(size, ulong, NULL);
NULLB_DEVICE_ATTR(completion_nsec, ulong, NULL);
NULLB_DEVICE_ATTR(submit_queues, uint, nullb_apply_submit_queues);
--
2.52.0
^ permalink raw reply related
* [PATCH V4 9/9] null_blk: serialize configfs attribute shows with the file-scope lock
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>
The _show callback in the NULLB_DEVICE_ATTR macro reads dev->NAME and the
_store path writes it. configfs does not serialize accesses across separate
open file descriptions (buffer->mutex is per-fd), and _show takes no lock,
so a concurrent read and write on the same attribute is a data race. The
_show readers also race against writes to these fields that run after the
configfs item becomes visible, e.g. in nullb_update_nr_hw_queues().
All of those writers now run under the file-scope lock: _store takes it
unconditionally, and the setup-side writers run under power_store() which
holds the same lock. The only remaining unsynchronized accesses are the
plain reads in _show. Rather than annotating every field with
READ_ONCE()/WRITE_ONCE() across files, simply take the file-scope lock in
_show (and in power_show) as well. This closes the remaining _show-vs-write
data races with a single lock and keeps the writers as plain assignments.
configfs attribute access is not on the I/O hot path, so taking the mutex
in _show is acceptable from a performance standpoint. The dev fields
written in null_alloc_dev() and dev->power in nullb_group_drop_item() need
no locking: the former runs from .make_group before the item is published,
and the latter is serialized by configfs frag_sem/frag_dead against
attribute show/store.
Suggested-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index a411db6a8c40..e829b502981b 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -343,12 +343,18 @@ static ssize_t nullb_device_bool_attr_store(bool *val, const char *page,
/* The following macro should only be used with TYPE = {uint, ulong, bool}. */
#define NULLB_DEVICE_ATTR(NAME, TYPE, APPLY) \
static ssize_t \
nullb_device_##NAME##_show(struct config_item *item, char *page) \
{ \
- return nullb_device_##TYPE##_attr_show( \
+ ssize_t ret; \
+ \
+ mutex_lock(&lock); \
+ ret = nullb_device_##TYPE##_attr_show( \
to_nullb_device(item)->NAME, page); \
+ mutex_unlock(&lock); \
+ \
+ return ret; \
} \
static ssize_t \
nullb_device_##NAME##_store(struct config_item *item, const char *page, \
size_t count) \
{ \
@@ -477,11 +483,17 @@ NULLB_DEVICE_ATTR(rotational, bool, NULL);
NULLB_DEVICE_ATTR(badblocks_once, bool, NULL);
NULLB_DEVICE_ATTR(badblocks_partial_io, bool, NULL);
static ssize_t nullb_device_power_show(struct config_item *item, char *page)
{
- return nullb_device_bool_attr_show(to_nullb_device(item)->power, page);
+ ssize_t ret;
+
+ mutex_lock(&lock);
+ ret = nullb_device_bool_attr_show(to_nullb_device(item)->power, page);
+ mutex_unlock(&lock);
+
+ return ret;
}
static ssize_t nullb_device_power_store(struct config_item *item,
const char *page, size_t count)
{
--
2.52.0
^ permalink raw reply related
* [PATCH V4 5/9] null_blk: free zones array on device power-off
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>
null_init_zoned_dev() allocates dev->zones when a zoned device is powered
on, but null_del_dev() never frees it on power-off; dev->zones is only
freed later in null_free_dev(), when the configfs directory is removed. If
the device is powered off and then on again, null_init_zoned_dev()
allocates a new array and overwrites the dev->zones pointer, leaking the
previous allocation each power cycle.
Free dev->zones in null_del_dev() via null_free_zoned_dev() to solve it.
And calling null_del_dev() in null_free_dev() is no longer necessary
because every caller already invokes null_del_dev() first: via
nullb_group_drop_item() before nullb_device_release(), in the
null_add_dev() error path of null_create_dev(), and in null_destroy_dev().
Remove the redundant call.
Fixes: ca4b2a011948 ("null_blk: add zone support")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index df85189f0b69..a75989220a2f 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -834,11 +834,10 @@ static struct nullb_device *null_alloc_dev(void)
static void null_free_dev(struct nullb_device *dev)
{
if (!dev)
return;
- null_free_zoned_dev(dev);
badblocks_exit(&dev->badblocks);
kfree(dev);
}
static enum hrtimer_restart null_cmd_timer_expired(struct hrtimer *timer)
@@ -1775,10 +1774,11 @@ static void null_del_dev(struct nullb *nullb)
atomic_long_set(&nullb->cur_bytes, LONG_MAX);
blk_mq_start_stopped_hw_queues(nullb->q, true);
}
put_disk(nullb->disk);
+ 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);
--
2.52.0
^ permalink raw reply related
* [PATCH V4 3/9] null_blk: move unregister_blkdev() after destroying dev in null_exit()
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>
In null_exit(), unregister_blkdev() is called before the null_blk instances
are destroyed, which is inconsistent with the cleanup order in null_init().
Move it after null_destroy_dev() so that teardown happens in the reverse
order of initialization.
No functional change intended.
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huaweicloud.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
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 4613035222cd..6cb213779cc5 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2192,19 +2192,19 @@ static void __exit null_exit(void)
{
struct nullb *nullb;
configfs_unregister_subsystem(&nullb_subsys);
- unregister_blkdev(null_major, "nullb");
-
mutex_lock(&lock);
while (!list_empty(&nullb_list)) {
nullb = list_entry(nullb_list.next, struct nullb, list);
null_destroy_dev(nullb);
}
mutex_unlock(&lock);
+ unregister_blkdev(null_major, "nullb");
+
if (tag_set.ops)
blk_mq_free_tag_set(&tag_set);
mutex_destroy(&lock);
}
--
2.52.0
^ permalink raw reply related
* [PATCH V4 1/9] null_blk: use DEFINE_MUTEX for the file-scope mutex
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(), mutex_init(&lock) currently happens after
configfs_register_subsystem(), which exposes the nullb subsystem to
userspace. A racing mkdir() into /sys/kernel/config/nullb/ can reach
null_find_dev_by_name() -> mutex_lock(&lock) before the mutex is
initialized, trigger warning:
[ 123.137788] DEBUG_LOCKS_WARN_ON(lock->magic != lock)
[ 123.137796] WARNING: kernel/locking/mutex.c:159 at mutex_lock+0x171/0x1c0, CPU#13: mkdir/1301
[ 123.140090] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4
......
[ 123.154926] Call Trace:
[ 123.155172] <TASK>
[ 123.155419] ? __pfx_mutex_lock+0x10/0x10
[ 123.156181] ? __pfx__raw_spin_lock+0x10/0x10
[ 123.156571] nullb_group_make_group+0x20/0x100 [null_blk]
[ 123.157011] configfs_mkdir+0x47b/0xc70
[ 123.157337] ? __pfx_configfs_mkdir+0x10/0x10
[ 123.157719] ? may_create_dentry+0x242/0x2e0
[ 123.158061] vfs_mkdir+0x2a9/0x6c0
[ 123.158352] filename_mkdirat+0x3dc/0x500
[ 123.158710] ? __pfx_filename_mkdirat+0x10/0x10
[ 123.159070] ? strncpy_from_user+0x3a/0x1d0
[ 123.159413] __x64_sys_mkdir+0x6b/0x90
[ 123.159760] do_syscall_64+0xea/0x600
Replace the runtime mutex_init(&lock) with a static DEFINE_MUTEX(lock)
declaration to fix this issue.
Fixes: 49c3b9266a71 ("block: null_blk: Improve device creation with configfs")
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/block/null_blk/main.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f8c0fd57e041..eba204b27785 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -64,11 +64,11 @@ struct nullb_page {
};
#define NULLB_PAGE_LOCK (MAP_SZ - 1)
#define NULLB_PAGE_FREE (MAP_SZ - 2)
static LIST_HEAD(nullb_list);
-static struct mutex lock;
+static DEFINE_MUTEX(lock);
static int null_major;
static DEFINE_IDA(nullb_indexes);
static struct blk_mq_tag_set tag_set;
enum {
@@ -2164,12 +2164,10 @@ static int __init null_init(void)
ret = configfs_register_subsystem(&nullb_subsys);
if (ret)
return ret;
- mutex_init(&lock);
-
null_major = register_blkdev(0, "nullb");
if (null_major < 0) {
ret = null_major;
goto err_conf;
}
--
2.52.0
^ permalink raw reply related
* [PATCH V4 0/9] null_blk: fix init/exit races and memleaks
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
From: Zizhi Wo <wozizhi@huawei.com>
This series fixes several issues in null_blk around lock initialization,
memory leaks, concurrent configfs access, and module init/exit.
Patches 1-4 are unchanged from v3.
Patch 5 fixes another memory leak of the zones array, and patch 6 is a
related cleanup.
Patch 7 fixes a NULL-ptr-deref of hctx when shrinking submit_queues on a
device that shares the global tag_set.
Patch 8 fixes a data race, similar to patch 5 in v3, but adjusts the
return value handling.
Patch 9 serializes configfs attribute shows to close the remaining
show-vs-write races.
See the individual patch descriptions for details.
Changes since v3:
- Added patch 5 (zones array memleak) and patch 6 (cleanup).
- Added patch 7 (NULL-ptr-deref on shared tag_set queue shrink).
- Patch 9: reworked the fix from v3's patch 6 to take the file-scope
lock in _store instead of scattering READ_ONCE/WRITE_ONCE.
https://lore.kernel.org/all/20260708073917.2172392-1-wozizhi@huaweicloud.com/
Changes since v2:
- Dropped the lock rename patch; the rename and locking rework will be
sent as a separate series (per Damien's suggestion).
- Patch 3: fixed the tense in the commit message.
- Patch 4: also update dev->NAME in the "!dev->nullb" path, which was
previously lost.
https://lore.kernel.org/all/20260707025542.1299859-1-wozizhi@huaweicloud.com/
Changes since v1:
- Added patches 4-6, and modify the lock name in patch 2.
https://lore.kernel.org/all/20260706123507.3809871-1-wozizhi@huaweicloud.com/
Zizhi Wo (9):
null_blk: use DEFINE_MUTEX for the file-scope mutex
null_blk: register configfs subsystem after creating default devices
null_blk: move unregister_blkdev() after destroying dev in null_exit()
null_blk: free global tag_set on init error path
null_blk: free zones array on device power-off
null_blk: clean up null_del_dev() to use cached dev pointer
null_blk: reject per-device queue resize for shared tag set
null_blk: serialize configfs attribute stores with device setup
null_blk: serialize configfs attribute shows with the file-scope lock
drivers/block/null_blk/main.c | 79 +++++++++++++++++++----------------
1 file changed, 44 insertions(+), 35 deletions(-)
--
2.52.0
^ permalink raw reply
* [PATCH V4 4/9] null_blk: free global tag_set on init error path
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>
If shared_tags is enabled, null_setup_tagset() allocates the global tag_set
via null_init_global_tag_set(). If device creation later fails, err_dev
destroys the default devices and calls unregister_blkdev(), but never frees
the global tag_set. Since module init failed, null_exit() is never invoked,
so the global tag_set's tags and maps are permanently leaked.
Free the global tag_set in err_dev, matching null_exit() which does
if (tag_set.ops) blk_mq_free_tag_set(&tag_set).
Fixes: 82f402fefa50 ("null_blk: add support for shared tags")
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 | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 6cb213779cc5..df85189f0b69 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2183,10 +2183,12 @@ static int __init null_init(void)
while (!list_empty(&nullb_list)) {
nullb = list_entry(nullb_list.next, struct nullb, list);
null_destroy_dev(nullb);
}
unregister_blkdev(null_major, "nullb");
+ if (tag_set.ops)
+ blk_mq_free_tag_set(&tag_set);
return ret;
}
static void __exit null_exit(void)
{
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v5] badblocks: fix infinite loop due to incorrect rounding and overflow
From: Coly Li @ 2026-07-09 10:25 UTC (permalink / raw)
To: Ramesh Adhikari; +Cc: axboe, gregkh, linux-block, lkp
In-Reply-To: <20260704171321.1956937-1-adhikari.resume@gmail.com>
On Sat, Jul 04, 2026 at 10:43:21PM +0800, Ramesh Adhikari wrote:
> The roundup() and rounddown() macros return the rounded value but
> do not modify their input in place. In _badblocks_set(),
> _badblocks_clear(), and badblocks_check(), the return values were
> being discarded, so s and target/next remained unrounded. Sectors
> were then calculated from these unrounded values, which could make
> sectors way too large (or zero), causing infinite loops in the
> re_insert/re_clear/re_check loops.
>
> This was confirmed with local syzkaller fuzzing against the nvdimm
> ioctl path (ND_IOCTL_CLEAR_ERROR -> nvdimm_clear_badblocks_region()
> -> badblocks_clear()), which reliably produces RCU stalls with the
> looping task caught mid-loop:
>
> rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> rcu: rcu_preempt kthread starved for 21001 jiffies! g40229 f0x0 RCU_GP_WAIT_FQS(5)
> ...
> Call Trace:
> badblocks_clear+0x259/0xb10
> nvdimm_clear_badblocks_region+0x165/0x1e0 [libnvdimm]
> device_for_each_child+0x11e/0x1a0
> nd_ioctl+0x1413/0x1750 [libnvdimm]
> __x64_sys_ioctl+0x18e/0x210
> do_syscall_64+0x102/0x5a0
>
> Fix this by properly capturing the return values of round_up()/
> round_down() (the power-of-two variants, since 1 << bb->shift is
> always a power of two -- roundup()/rounddown() use a division/
> modulo internally, which is undesirable here). Also add overflow
> checks (s > ULLONG_MAX - sectors) before the s + sectors addition
> in all three functions, and handle the case where sectors becomes
> zero after rounding.
>
> The overflow check is done unconditionally, before the bb->shift
> rounding block, rather than inside it. bb->shift == 0 is not just
> an initial state -- __badblocks_init() sets it to 0 by default, and
> drivers/md/md.c explicitly sets rdev->badblocks.shift back to 0 in
> several paths -- so s + sectors needs the same overflow guard
> whether or not rounding happens.
>
> Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
> ---
> v1-v3 chased individual len==0 symptoms in _badblocks_clear()/
> _badblocks_check() one call site at a time. Jens pointed out that
> approach wasn't finding the actual bug, just papering over spots as
> they were noticed. v4 was a full rewrite around the real root cause
> (roundup()/rounddown() discarding their return values), covering all
> three functions with proper overflow handling.
>
> Changes in v5:
> - Switch from roundup()/rounddown() (which use division/modulo on
> sector_t, a u64) to round_up()/round_down() (bitmask-based, since
> 1 << bb->shift is always a power of two). Fixes the v4 build
> failure kernel test robot reported on 32-bit (undefined reference
> to __aeabi_uldivmod on ARM, __umoddi3 on i386).
> - Move the s > ULLONG_MAX - sectors overflow check so it runs
> unconditionally in all three functions, instead of only inside
> the `if (bb->shift)` block. bb->shift == 0 is a real, common
> state (default init value, and explicitly set by drivers/md/md.c
> in several paths), so the overflow guard needs to apply there
> too, not just when rounding is active.
> - Build-tested locally (x86_64) and confirmed no residual div/mod
> symbols in the object file.
>
> Link to v4: https://lore.kernel.org/r/20260427151048.756072-1-adhikari.resume@gmail.com
>
Hi Ramesh,
Thanks for the fix up.
> block/badblocks.c | 40 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/block/badblocks.c b/block/badblocks.c
> index ece64e76fe8..e8484912532 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -853,15 +853,21 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
> /* Invalid sectors number */
> return false;
>
> + if (s > ULLONG_MAX - sectors)
> + return false;
> +
The original idea was to let caller to handle such overflow stuffs.
If you want to handle these things here, the above lines are not
enough indeed. You need to considering more conditions, I used to
unlike such codes, but now realize maybe such checks should be
added.
Here are some items in my brain,
- s + sectors overflow
- too big bb->shift
- 1 << bb->shift overflow
- the rounddown result even smaller then start s for large bb->shift
> if (bb->shift) {
> /* round the start down, and the end up */
> sector_t next = s + sectors;
>
> - rounddown(s, 1 << bb->shift);
> - roundup(next, 1 << bb->shift);
> + s = round_down(s, 1 << bb->shift);
> + next = round_up(next, 1 << bb->shift);
> sectors = next - s;
I suggest to divide the patch into two. The first one just contains
the above round_down and round_up fix. Another one check the rested
value ranges.
Also add Fixes tag to my origin badblocks rewrite commit, and add
a CC line to stable@vger.kernel.org
Thanks.
Coly Li
> }
>
> + if (sectors == 0)
> + return false;
> +
> write_seqlock_irqsave(&bb->lock, flags);
>
> bad.ack = acknowledged;
> @@ -1061,6 +1067,9 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
> /* Invalid sectors number */
> return false;
>
> + if (s > ULLONG_MAX - sectors)
> + return false;
> +
> if (bb->shift) {
> sector_t target;
>
> @@ -1071,11 +1080,17 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
> * isn't than to think a block is not bad when it is.
> */
> target = s + sectors;
> - roundup(s, 1 << bb->shift);
> - rounddown(target, 1 << bb->shift);
> - sectors = target - s;
> + s = round_up(s, 1 << bb->shift);
> + target = round_down(target, 1 << bb->shift);
> + if (target < s)
> + sectors = 0;
> + else
> + sectors = target - s;
> }
>
> + if (sectors == 0)
> + return false;
> +
> write_seqlock_irq(&bb->lock);
>
> bad.ack = true;
> @@ -1303,13 +1318,22 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
>
> WARN_ON(bb->shift < 0 || sectors == 0);
>
> + if (s > ULLONG_MAX - sectors)
> + return -EINVAL;
> +
> if (bb->shift > 0) {
> /* round the start down, and the end up */
> sector_t target = s + sectors;
>
> - rounddown(s, 1 << bb->shift);
> - roundup(target, 1 << bb->shift);
> - sectors = target - s;
> + s = round_down(s, 1 << bb->shift);
> + target = round_up(target, 1 << bb->shift);
> + if (target < s)
> + sectors = 0;
> + else
> + sectors = target - s;
> +
> + if (sectors == 0)
> + return 0;
> }
>
> retry:
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v2] xen-blkfront: fix double completion of split requests on resume
From: Roger Pau Monné @ 2026-07-09 10:27 UTC (permalink / raw)
To: Doruk Tan Ozturk
Cc: Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Jens Axboe, xen-devel, linux-block, linux-kernel
In-Reply-To: <20260709100853.7489-1-doruk@0sec.ai>
On Thu, Jul 09, 2026 at 12:08:53PM +0200, Doruk Tan Ozturk wrote:
> 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>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks, Roger.
^ permalink raw reply
* [PATCH v6 0/2] badblocks: fix infinite loop and validate sector range/shift
From: Ramesh Adhikari @ 2026-07-09 13:19 UTC (permalink / raw)
To: colyli, axboe; +Cc: gregkh, linux-block, stable, Ramesh Adhikari
In-Reply-To: <ak9CC591ivuQ4BP1@studio.local>
This replaces the single-patch v5 with a two-patch series, per Coly's
review.
v1-v4 chased symptoms of the same underlying bug (an RCU stall found
by syzkaller through the nvdimm ioctl path, in _badblocks_check() /
badblocks_check() looping with a non-advancing range) before landing
on the root cause in v4: rounddown()/roundup() don't modify their
argument in place, so 's'/'next'/'target' were never actually
rounded.
v5 folded the round_down()/round_up() fix together with overflow and
zero-length guards into one patch. Coly's review on v5 pointed out
that:
- the overflow check that was there wasn't sufficient on its own
(only one of several range-validity conditions), and
- the round fix and the validation should be separate patches,
since they're independently useful and one is safe to backport
on its own.
This series:
1/2 is exactly the round_down()/round_up() fix, nothing else. This
is what actually stops the infinite loop and also fixes the
32-bit build breakage kernel test robot reported on v1
(rounddown()/roundup() do 64-bit division/modulo on sector_t,
requiring libgcc helpers not linked into the kernel).
2/2 adds the range/shift validation Coly asked for: s+sectors
overflow, bb->shift too large to shift a sector_t by (bb->shift
is populated in drivers/md/md.c straight from an unvalidated
on-disk superblock byte), and detecting when round_up()/
round_down() themselves wrap near ULLONG_MAX.
Both are tagged Fixes: aa511ff8218b ("badblocks: switch to the
improved badblock handling code") and Cc: stable, since that's the
commit that introduced this code path.
Ramesh Adhikari (2):
badblocks: fix in-place round_up/round_down usage bug
badblocks: validate sector range and shift before rounding
block/badblocks.c | 52 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 45 insertions(+), 7 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v6 1/2] badblocks: fix in-place round_up/round_down usage bug
From: Ramesh Adhikari @ 2026-07-09 13:19 UTC (permalink / raw)
To: colyli, axboe
Cc: gregkh, linux-block, stable, Ramesh Adhikari, kernel test robot
In-Reply-To: <20260709131904.596684-1-adhikari.resume@gmail.com>
rounddown() and roundup() do not modify their first argument in
place; they return the rounded value. _badblocks_set(),
_badblocks_clear() and badblocks_check() were calling them as
bare statements and discarding the result, so 's' (and 'next'/
'target') were never actually rounded. Depending on the caller's
alignment this can leave 'sectors' unchanged or, in the reported
case, produce a range whose end never advances, causing
_badblocks_check()/badblocks_check() to loop with a non-advancing
cursor and stall the CPU (RCU stall) when called through the
nvdimm ioctl path via nvdimm_clear_badblocks_region().
rounddown()/roundup() also do division/modulo on the sector_t
(u64) operand, which requires libgcc helpers (__aeabi_uldivmod,
__umoddi3) that are not linked into the kernel on 32-bit builds,
breaking the build on arm/i386 (reported by kernel test robot).
Switch to round_down()/round_up() (include/linux/math.h), which
are mask-based, assign their result back to the variable being
rounded, and require no 64-bit division, fixing both the
non-rounding bug and the 32-bit build breakage.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604301231.IpPh4AiH-lkp@intel.com/
Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code")
Cc: stable@vger.kernel.org
Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
---
block/badblocks.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index ece64e76fe8..1f786b193fb 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -857,8 +857,8 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
/* round the start down, and the end up */
sector_t next = s + sectors;
- rounddown(s, 1 << bb->shift);
- roundup(next, 1 << bb->shift);
+ s = round_down(s, 1 << bb->shift);
+ next = round_up(next, 1 << bb->shift);
sectors = next - s;
}
@@ -1071,8 +1071,8 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
* isn't than to think a block is not bad when it is.
*/
target = s + sectors;
- roundup(s, 1 << bb->shift);
- rounddown(target, 1 << bb->shift);
+ s = round_up(s, 1 << bb->shift);
+ target = round_down(target, 1 << bb->shift);
sectors = target - s;
}
@@ -1307,8 +1307,8 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
/* round the start down, and the end up */
sector_t target = s + sectors;
- rounddown(s, 1 << bb->shift);
- roundup(target, 1 << bb->shift);
+ s = round_down(s, 1 << bb->shift);
+ target = round_up(target, 1 << bb->shift);
sectors = target - s;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v6 2/2] badblocks: validate sector range and shift before rounding
From: Ramesh Adhikari @ 2026-07-09 13:19 UTC (permalink / raw)
To: colyli, axboe; +Cc: gregkh, linux-block, stable, Ramesh Adhikari
In-Reply-To: <20260709131904.596684-1-adhikari.resume@gmail.com>
_badblocks_set(), _badblocks_clear() and badblocks_check() round
the caller-supplied [s, s+sectors) range to the current bb->shift
block size before touching the bad block table. That rounding
was not defensive against a few cases:
- s + sectors can overflow sector_t (u64), wrapping the range
end before it is ever compared against s.
- bb->shift is a plain 'int' field, populated in one case
(drivers/md/md.c, from the on-disk superblock's bblog_shift)
straight from an unvalidated byte with no upper bound. Shifting
by an amount >= the width of the shifted type is undefined
behaviour in C; "1 << bb->shift" was shifting an int literal,
so this was already undefined for bb->shift >= 32, let alone
the full 0-255 range bblog_shift allows.
- round_up()/round_down() rounding a value near ULLONG_MAX can
itself wrap back to a small value, so even with a valid shift
the rounded end of the range could end up smaller than the
rounded start, silently turning a small range into a huge one
(in _badblocks_clear()/badblocks_check(), which round the end
up) or losing the range entirely.
Add an explicit s+sectors overflow check, reject any bb->shift
that is too large to shift a sector_t by, cast the shift operand
to sector_t so the shift itself is never performed on a 32-bit
int, and detect post-rounding wrap by comparing the rounded
result back against the pre-rounding value.
Suggested-by: Coly Li <colyli@fygo.io>
Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code")
Cc: stable@vger.kernel.org
Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
---
block/badblocks.c | 52 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index 1f786b193fb..00a59729600 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -853,12 +853,23 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
/* Invalid sectors number */
return false;
+ if (s > ULLONG_MAX - sectors)
+ /* Range wraps past the end of sector_t */
+ return false;
+
if (bb->shift) {
/* round the start down, and the end up */
sector_t next = s + sectors;
- s = round_down(s, 1 << bb->shift);
- next = round_up(next, 1 << bb->shift);
+ if (bb->shift >= BITS_PER_LONG_LONG)
+ /* Corrupt/unsanitised shift value */
+ return false;
+
+ s = round_down(s, (sector_t)1 << bb->shift);
+ next = round_up(next, (sector_t)1 << bb->shift);
+ if (next <= s)
+ /* Rounding wrapped past the end of sector_t */
+ return false;
sectors = next - s;
}
@@ -1061,7 +1072,12 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
/* Invalid sectors number */
return false;
+ if (s > ULLONG_MAX - sectors)
+ /* Range wraps past the end of sector_t */
+ return false;
+
if (bb->shift) {
+ sector_t orig_s = s;
sector_t target;
/* When clearing we round the start up and the end down.
@@ -1070,10 +1086,21 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
* However it is better the think a block is bad when it
* isn't than to think a block is not bad when it is.
*/
+ if (bb->shift >= BITS_PER_LONG_LONG)
+ /* Corrupt/unsanitised shift value */
+ return false;
+
target = s + sectors;
- s = round_up(s, 1 << bb->shift);
- target = round_down(target, 1 << bb->shift);
- sectors = target - s;
+ s = round_up(s, (sector_t)1 << bb->shift);
+ target = round_down(target, (sector_t)1 << bb->shift);
+ if (s < orig_s || target < s)
+ /* Rounding wrapped, or range collapsed */
+ sectors = 0;
+ else
+ sectors = target - s;
+
+ if (sectors == 0)
+ return false;
}
write_seqlock_irq(&bb->lock);
@@ -1303,12 +1330,23 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
WARN_ON(bb->shift < 0 || sectors == 0);
+ if (s > ULLONG_MAX - sectors)
+ /* Range wraps past the end of sector_t */
+ return -EINVAL;
+
if (bb->shift > 0) {
/* round the start down, and the end up */
sector_t target = s + sectors;
- s = round_down(s, 1 << bb->shift);
- target = round_up(target, 1 << bb->shift);
+ if (bb->shift >= BITS_PER_LONG_LONG)
+ /* Corrupt/unsanitised shift value */
+ return -EINVAL;
+
+ s = round_down(s, (sector_t)1 << bb->shift);
+ target = round_up(target, (sector_t)1 << bb->shift);
+ if (target <= s)
+ /* Rounding wrapped past the end of sector_t */
+ return 0;
sectors = target - s;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] fs: report direct io constraints through file_getattr
From: Keith Busch @ 2026-07-09 13:46 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
In-Reply-To: <20260709071352.GA20180@lst.de>
On Thu, Jul 09, 2026 at 09:13:52AM +0200, Christoph Hellwig wrote:
> On Tue, Jul 07, 2026 at 06:18:43PM -0700, Keith Busch wrote:
>
> > + 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?
It tells you how many sub-sector vectors you can submit in your
readv/writev before it needs to add up to a logical block size.
Ex: 4k logical block size, 4 byte DMA, 256 max segments. You can define
4-byte iov's in your command, but you'll hit the max segment count
before you have a valid IO if they're all that small.
> > @@ -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?
I had to add a padding field to the struct to account for the implicit
hole in 64-bit and to ensure the struct is the same size for 32-bit.
It's a reserved field, so we have to ensure the current kernel doesn't
support any value here in case we define this field for something else
in the future.
^ permalink raw reply
* Re: [PATCH v6 1/2] badblocks: fix in-place round_up/round_down usage bug
From: Coly Li @ 2026-07-09 15:16 UTC (permalink / raw)
To: Ramesh Adhikari; +Cc: axboe, gregkh, linux-block, stable, kernel test robot
In-Reply-To: <20260709131904.596684-2-adhikari.resume@gmail.com>
On Thu, Jul 09, 2026 at 06:49:03PM +0800, Ramesh Adhikari wrote:
> rounddown() and roundup() do not modify their first argument in
> place; they return the rounded value. _badblocks_set(),
> _badblocks_clear() and badblocks_check() were calling them as
> bare statements and discarding the result, so 's' (and 'next'/
> 'target') were never actually rounded. Depending on the caller's
> alignment this can leave 'sectors' unchanged or, in the reported
> case, produce a range whose end never advances, causing
> _badblocks_check()/badblocks_check() to loop with a non-advancing
> cursor and stall the CPU (RCU stall) when called through the
> nvdimm ioctl path via nvdimm_clear_badblocks_region().
>
> rounddown()/roundup() also do division/modulo on the sector_t
> (u64) operand, which requires libgcc helpers (__aeabi_uldivmod,
> __umoddi3) that are not linked into the kernel on 32-bit builds,
> breaking the build on arm/i386 (reported by kernel test robot).
>
> Switch to round_down()/round_up() (include/linux/math.h), which
> are mask-based, assign their result back to the variable being
> rounded, and require no 64-bit division, fixing both the
> non-rounding bug and the 32-bit build breakage.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202604301231.IpPh4AiH-lkp@intel.com/
> Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
It looks good to me.
Reviewed-by: Coly Li <colyli@fygo.io>
Thanks.
Coly Li
> ---
> block/badblocks.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/block/badblocks.c b/block/badblocks.c
> index ece64e76fe8..1f786b193fb 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -857,8 +857,8 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
> /* round the start down, and the end up */
> sector_t next = s + sectors;
>
> - rounddown(s, 1 << bb->shift);
> - roundup(next, 1 << bb->shift);
> + s = round_down(s, 1 << bb->shift);
> + next = round_up(next, 1 << bb->shift);
> sectors = next - s;
> }
>
> @@ -1071,8 +1071,8 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
> * isn't than to think a block is not bad when it is.
> */
> target = s + sectors;
> - roundup(s, 1 << bb->shift);
> - rounddown(target, 1 << bb->shift);
> + s = round_up(s, 1 << bb->shift);
> + target = round_down(target, 1 << bb->shift);
> sectors = target - s;
> }
>
> @@ -1307,8 +1307,8 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
> /* round the start down, and the end up */
> sector_t target = s + sectors;
>
> - rounddown(s, 1 << bb->shift);
> - roundup(target, 1 << bb->shift);
> + s = round_down(s, 1 << bb->shift);
> + target = round_up(target, 1 << bb->shift);
> sectors = target - s;
> }
>
> --
> 2.43.0
^ permalink raw reply
* Re: [PATCH 2/2] dm-raid1: don't fail the mirror for invalid I/O errors
From: Benjamin Marzinski @ 2026-07-09 16:13 UTC (permalink / raw)
To: Mike Snitzer
Cc: Keith Busch, axboe, Mikulas Patocka, Keith Busch, dm-devel,
linux-block, Dr. David Alan Gilbert, Vjaceslavs Klimovs
In-Reply-To: <ak7cHXaWjVX6Uamj@redhat.com>
On Wed, Jul 08, 2026 at 07:24:14PM -0400, Benjamin Marzinski wrote:
> On Tue, Jul 07, 2026 at 03:44:54PM -0400, Mike Snitzer wrote:
> > On Mon, Jul 06, 2026 at 02:52:33PM -0600, Keith Busch wrote:
> > > On Wed, Jun 24, 2026 at 01:14:03PM +0200, Mikulas Patocka wrote:
> > So where does dm-raid1 and dm-crypt stand relative to these DIO memory
> > alignment changes? Inferring they are pretty exposed.
> >
> > > > BTW. I think that blk_path_error should also test for BLK_STS_INVAL and
> > > > return false, otherwise, dm-multipath would be suffering from this bug
> > > > too. Ben, could you test it?
> > >
> > > Good point.
> >
> > Would appreciate knowing if multipath exposed too.
>
> I can't make it happen. I can reproduce this using dm-mirror, but when I
> try with dm-multipath, it fails with BLK_STS_INVAL before it ever makes
> it into the request layer.
>
> blk_mq_submit_bio() -> __bio_split_to_limits() -> bio_split_rw() ->
> bio_split_rw_at() -> bio_split_io_at()
>
> bio_split_io_at() checks the bio_vec alignment and fails with -EINVAL,
> causing the bio to get failed.
However, as a general issue, I don't believe it's impossible to have an
unaligned bio_vec that returns false for bio_may_need_split(), so that
the splitting code will never get run. But if this is already getting
fixed earlier in the bio code, then the point is kind of moot.
In reference to changing blk_path_error(), multipath should only be
passing down requests that are valid on any of its paths, so it seems
reasonable to make the case that BLK_STS_INVAL is not a path error. But
if we are going to be checking the bios early against multipath's
limits, then it seems like the only reason we would get a BLK_STS_INVAL
is that multipath's limits don't work with the underlying path's. That
would likely mean there was a limit stacking bug, and who knows whether
another path would work better.
-Ben
> >
> > Thanks,
> > Mike
^ permalink raw reply
* [PATCH 00/12] drbd: Enable lock context analysis
From: Bart Van Assche @ 2026-07-09 21:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver,
Cc : Christoph Böhmwalder, Bart Van Assche
Hi Jens,
This patch series enables compile-time lock context analysis for DRBD. The goal
of this series is to allow the compiler to statically verify that locks are
acquired and released correctly, helping to prevent deadlocks and race
conditions. This version addresses feedback received on the v1 series, improves
the annotations, and refactors some conditional locking paths to use the modern
cleanup.h guard infrastructure.
Changes since v1:
- Extracted the DRBD patches from a larger series. See also
"[PATCH 00/27] Enable lock context analysis in drivers/block/"
(https://lore.kernel.org/all/cover.1781042470.git.bvanassche@acm.org/).
- Fixed the indentation of the arguments of
drbd_nl_put_dump_connections_result().
- Combined the two drbd_req_state() patches into a single patch.
Please consider these patches for the next merge window.
Thanks,
Bart.
Bart Van Assche (12):
drbd: Remove "extern" from function declarations
drbd: Retain one _get_ldev_if_state() implementation
drbd: Remove the get_ldev_if_state() macro
drbd: Remove the 'local' lock context
drbd: Simplify the bitmap locking functions.
drbd: Move two declarations
drbd: Pass 'resource' directly to complete_conflicting_writes()
drbd: Split drbd_nl_get_connections_dumpit()
drbd: Make a mutex_unlock() call unconditional
drbd: Rework locking in drbd_req_state()
drbd: Annotate drbd_bm_{lock,unlock}()
drbd: Enable lock context analysis
drivers/block/drbd/Makefile | 3 +
drivers/block/drbd/drbd_bitmap.c | 43 +-
drivers/block/drbd/drbd_config.h | 2 +-
drivers/block/drbd/drbd_int.h | 594 +++++++++++++------------
drivers/block/drbd/drbd_interval.h | 11 +-
drivers/block/drbd/drbd_main.c | 41 +-
drivers/block/drbd/drbd_nl.c | 117 ++---
drivers/block/drbd/drbd_receiver.c | 27 +-
drivers/block/drbd/drbd_req.c | 10 +-
drivers/block/drbd/drbd_req.h | 24 +-
drivers/block/drbd/drbd_state.c | 49 +-
drivers/block/drbd/drbd_state.h | 35 +-
drivers/block/drbd/drbd_state_change.h | 32 +-
drivers/block/drbd/drbd_worker.c | 6 +-
14 files changed, 508 insertions(+), 486 deletions(-)
^ permalink raw reply
* [PATCH 02/12] drbd: Retain one _get_ldev_if_state() implementation
From: Bart Van Assche @ 2026-07-09 21:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver,
Cc : Christoph Böhmwalder, Bart Van Assche,
Christoph Hellwig, Philipp Reisner, Lars Ellenberg,
Nathan Chancellor
In-Reply-To: <cover.1783632330.git.bvanassche@acm.org>
There are two slightly different _get_ldev_if_state() implementations
in the DRBD source code. Keep the version that is used. This does not
affect C=1 / C=2 builds because lock context checking is performed by
Clang instead of sparse since commit 5b63d0ae94cc ("compiler-context-
analysis: Remove Sparse support").
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/drbd/drbd_int.h | 7 ++-----
drivers/block/drbd/drbd_main.c | 19 -------------------
2 files changed, 2 insertions(+), 24 deletions(-)
diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index aa39c4d19133..bba52252fbac 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -2033,8 +2033,8 @@ static inline void put_ldev(struct drbd_device *device)
}
}
-#ifndef __CHECKER__
-static inline int _get_ldev_if_state(struct drbd_device *device, enum drbd_disk_state mins)
+static inline int _get_ldev_if_state(struct drbd_device *device,
+ enum drbd_disk_state mins)
{
int io_allowed;
@@ -2048,9 +2048,6 @@ static inline int _get_ldev_if_state(struct drbd_device *device, enum drbd_disk_
put_ldev(device);
return io_allowed;
}
-#else
-int _get_ldev_if_state(struct drbd_device *device, enum drbd_disk_state mins);
-#endif
/* this throttles on-the-fly application requests
* according to max_buffers settings;
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index a2a841c89201..dbf6e413db03 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -128,25 +128,6 @@ static const struct block_device_operations drbd_ops = {
.release = drbd_release,
};
-#ifdef __CHECKER__
-/* When checking with sparse, and this is an inline function, sparse will
- give tons of false positives. When this is a real functions sparse works.
- */
-int _get_ldev_if_state(struct drbd_device *device, enum drbd_disk_state mins)
-{
- int io_allowed;
-
- atomic_inc(&device->local_cnt);
- io_allowed = (device->state.disk >= mins);
- if (!io_allowed) {
- if (atomic_dec_and_test(&device->local_cnt))
- wake_up(&device->misc_wait);
- }
- return io_allowed;
-}
-
-#endif
-
/**
* tl_release() - mark as BARRIER_ACKED all requests in the corresponding transfer log epoch
* @connection: DRBD connection.
^ permalink raw reply related
* [PATCH 03/12] drbd: Remove the get_ldev_if_state() macro
From: Bart Van Assche @ 2026-07-09 21:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver,
Cc : Christoph Böhmwalder, Bart Van Assche,
Christoph Hellwig, Philipp Reisner, Lars Ellenberg
In-Reply-To: <cover.1783632330.git.bvanassche@acm.org>
The get_ldev_if_state() macro has been introduced because sparse does
not support something like __cond_acquires(). Remove this macro and
rename _get_ldev_if_state() into get_ldev_if_state().
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/drbd/drbd_int.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index bba52252fbac..49383c309865 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -2002,9 +2002,6 @@ static inline bool is_sync_state(enum drbd_conns connection_state)
*
* You have to call put_ldev() when finished working with device->ldev.
*/
-#define get_ldev_if_state(_device, _min_state) \
- (_get_ldev_if_state((_device), (_min_state)) ? \
- ({ __acquire(x); true; }) : false)
#define get_ldev(_device) get_ldev_if_state(_device, D_INCONSISTENT)
static inline void put_ldev(struct drbd_device *device)
@@ -2033,8 +2030,8 @@ static inline void put_ldev(struct drbd_device *device)
}
}
-static inline int _get_ldev_if_state(struct drbd_device *device,
- enum drbd_disk_state mins)
+static inline int get_ldev_if_state(struct drbd_device *device,
+ enum drbd_disk_state mins)
{
int io_allowed;
^ permalink raw reply related
* [PATCH 05/12] drbd: Simplify the bitmap locking functions.
From: Bart Van Assche @ 2026-07-09 21:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver,
Cc : Christoph Böhmwalder, Bart Van Assche,
Christoph Hellwig, Philipp Reisner, Lars Ellenberg
In-Reply-To: <cover.1783632330.git.bvanassche@acm.org>
Call mutex_lock() instead of mutex_trylock() followed by mutex_lock().
Remove the code that depends on device->bitmap == NULL because this
can't happen. All drbd_bm_{lock,unlock}() callers guarantee that the
device->bitmap pointer is valid.
Suggested-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/drbd/drbd_bitmap.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 1c68f1774e28..e332a96fe90c 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -124,22 +124,8 @@ static void __bm_print_lock_info(struct drbd_device *device, const char *func)
void drbd_bm_lock(struct drbd_device *device, char *why, enum bm_flag flags)
{
struct drbd_bitmap *b = device->bitmap;
- int trylock_failed;
- if (!b) {
- drbd_err(device, "FIXME no bitmap in drbd_bm_lock!?\n");
- return;
- }
-
- trylock_failed = !mutex_trylock(&b->bm_change);
-
- if (trylock_failed) {
- drbd_warn(device, "%s[%d] going to '%s' but bitmap already locked for '%s' by %s[%d]\n",
- current->comm, task_pid_nr(current),
- why, b->bm_why ?: "?",
- b->bm_task->comm, task_pid_nr(b->bm_task));
- mutex_lock(&b->bm_change);
- }
+ mutex_lock(&b->bm_change);
if (BM_LOCKED_MASK & b->bm_flags)
drbd_err(device, "FIXME bitmap already locked in bm_lock\n");
b->bm_flags |= flags & BM_LOCKED_MASK;
@@ -151,10 +137,6 @@ void drbd_bm_lock(struct drbd_device *device, char *why, enum bm_flag flags)
void drbd_bm_unlock(struct drbd_device *device)
{
struct drbd_bitmap *b = device->bitmap;
- if (!b) {
- drbd_err(device, "FIXME no bitmap in drbd_bm_unlock!?\n");
- return;
- }
if (!(BM_LOCKED_MASK & device->bitmap->bm_flags))
drbd_err(device, "FIXME bitmap not locked in bm_unlock\n");
^ permalink raw reply related
* [PATCH 06/12] drbd: Move two declarations
From: Bart Van Assche @ 2026-07-09 21:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver,
Cc : Christoph Böhmwalder, Bart Van Assche,
Christoph Hellwig, Philipp Reisner, Lars Ellenberg
In-Reply-To: <cover.1783632330.git.bvanassche@acm.org>
Move the resources_mutex declaration in front of the functions that
acquire and release this mutex. Move the drbd_wait_misc() declaration
past the struct drbd_device definition. This patch prepares for adding
lock context annotations.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/drbd/drbd_int.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 4b9d69adb9d4..1f3f2157df8b 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -195,7 +195,7 @@ struct drbd_device_work {
#include "drbd_interval.h"
-int drbd_wait_misc(struct drbd_device *, struct drbd_interval *);
+extern struct mutex resources_mutex;
void lock_all_resources(void);
void unlock_all_resources(void);
@@ -1094,6 +1094,7 @@ int drbd_bmio_set_n_write(struct drbd_device *device,
struct drbd_peer_device *peer_device);
int drbd_bmio_clear_n_write(struct drbd_device *device,
struct drbd_peer_device *peer_device);
+int drbd_wait_misc(struct drbd_device *device, struct drbd_interval *i);
/* Meta data layout
*
@@ -1363,8 +1364,6 @@ extern struct bio_set drbd_md_io_bio_set;
/* And a bio_set for cloning */
extern struct bio_set drbd_io_bio_set;
-extern struct mutex resources_mutex;
-
enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx,
unsigned int minor);
void drbd_destroy_device(struct kref *kref);
^ permalink raw reply related
* [PATCH 07/12] drbd: Pass 'resource' directly to complete_conflicting_writes()
From: Bart Van Assche @ 2026-07-09 21:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver,
Cc : Christoph Böhmwalder, Bart Van Assche,
Christoph Hellwig, Philipp Reisner, Lars Ellenberg
In-Reply-To: <cover.1783632330.git.bvanassche@acm.org>
Prepare for adding lock context annotations. Without this change, an
__assume_ctx_lock() statement would have to be added.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/drbd/drbd_req.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index 70f75ef07945..a851e43c94a8 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -951,7 +951,8 @@ static bool remote_due_to_read_balancing(struct drbd_device *device, sector_t se
*
* Only way out: remove the conflicting intervals from the tree.
*/
-static void complete_conflicting_writes(struct drbd_request *req)
+static void complete_conflicting_writes(struct drbd_resource *resource,
+ struct drbd_request *req)
{
DEFINE_WAIT(wait);
struct drbd_device *device = req->device;
@@ -974,9 +975,9 @@ static void complete_conflicting_writes(struct drbd_request *req)
/* Indicate to wake up device->misc_wait on progress. */
prepare_to_wait(&device->misc_wait, &wait, TASK_UNINTERRUPTIBLE);
i->waiting = true;
- spin_unlock_irq(&device->resource->req_lock);
+ spin_unlock_irq(&resource->req_lock);
schedule();
- spin_lock_irq(&device->resource->req_lock);
+ spin_lock_irq(&resource->req_lock);
}
finish_wait(&device->misc_wait, &wait);
}
@@ -1329,7 +1330,7 @@ static void drbd_send_and_submit(struct drbd_device *device, struct drbd_request
/* This may temporarily give up the req_lock,
* but will re-aquire it before it returns here.
* Needs to be before the check on drbd_suspended() */
- complete_conflicting_writes(req);
+ complete_conflicting_writes(resource, req);
/* no more giving up req_lock from now on! */
/* check for congestion, and potentially stop sending
^ permalink raw reply related
* [PATCH 08/12] drbd: Split drbd_nl_get_connections_dumpit()
From: Bart Van Assche @ 2026-07-09 21:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver,
Cc : Christoph Böhmwalder, Bart Van Assche, Philipp Reisner,
Lars Ellenberg
In-Reply-To: <cover.1783632330.git.bvanassche@acm.org>
Move the code between the 'put_result' and 'out' labels into a new
function. Prepare for making the following code unconditional:
if (resource)
mutex_unlock(&resource->conf_update);
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/drbd/drbd_nl.c | 98 ++++++++++++++++++++----------------
1 file changed, 55 insertions(+), 43 deletions(-)
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index c44b92856da3..ffcee2e2d1eb 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -3529,15 +3529,56 @@ int drbd_adm_dump_connections_done(struct netlink_callback *cb)
enum { SINGLE_RESOURCE, ITERATE_RESOURCES };
+static int drbd_nl_put_dump_connections_result(
+ struct sk_buff *skb, struct netlink_callback *cb,
+ struct drbd_resource *resource,
+ struct drbd_connection *connection, enum drbd_ret_code retcode)
+{
+ struct drbd_genlmsghdr *dh;
+
+ dh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+ &drbd_nl_family, NLM_F_MULTI,
+ DRBD_ADM_GET_CONNECTIONS);
+ if (!dh)
+ return -ENOMEM;
+ dh->ret_code = retcode;
+ dh->minor = -1U;
+ if (retcode == NO_ERROR) {
+ struct connection_statistics connection_statistics;
+ struct connection_info connection_info;
+ struct net_conf *net_conf;
+ int err;
+
+ err = nla_put_drbd_cfg_context(skb, resource, connection, NULL);
+ if (err)
+ return err;
+ net_conf = rcu_dereference(connection->net_conf);
+ if (net_conf) {
+ err = net_conf_to_skb(skb, net_conf);
+ if (err)
+ return err;
+ }
+ connection_to_info(&connection_info, connection);
+ err = connection_info_to_skb(skb, &connection_info);
+ if (err)
+ return err;
+ connection_statistics.conn_congested =
+ test_bit(NET_CONGESTED, &connection->flags);
+ err = connection_statistics_to_skb(skb, &connection_statistics);
+ if (err)
+ return err;
+ cb->args[2] = (long)connection;
+ }
+ genlmsg_end(skb, dh);
+ return 0;
+}
+
int drbd_nl_get_connections_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
{
struct nlattr *resource_filter;
struct drbd_resource *resource = NULL, *next_resource;
struct drbd_connection *connection;
- int err = 0, retcode;
- struct drbd_genlmsghdr *dh;
- struct connection_info connection_info;
- struct connection_statistics connection_statistics;
+ int err = 0;
rcu_read_lock();
resource = (struct drbd_resource *)cb->args[0];
@@ -3545,10 +3586,13 @@ int drbd_nl_get_connections_dumpit(struct sk_buff *skb, struct netlink_callback
resource_filter = find_cfg_context_attr(cb->nlh,
DRBD_A_DRBD_CFG_CONTEXT_CTX_RESOURCE_NAME);
if (resource_filter) {
- retcode = ERR_RES_NOT_KNOWN;
resource = drbd_find_resource(nla_data(resource_filter));
- if (!resource)
- goto put_result;
+ if (!resource) {
+ err = drbd_nl_put_dump_connections_result(
+ skb, cb, resource, NULL,
+ ERR_RES_NOT_KNOWN);
+ goto out;
+ }
cb->args[0] = (long)resource;
cb->args[1] = SINGLE_RESOURCE;
}
@@ -3579,8 +3623,9 @@ int drbd_nl_get_connections_dumpit(struct sk_buff *skb, struct netlink_callback
list_for_each_entry_continue_rcu(connection, &resource->connections, connections) {
if (!has_net_conf(connection))
continue;
- retcode = NO_ERROR;
- goto put_result; /* only one iteration */
+ err = drbd_nl_put_dump_connections_result(skb, cb, resource,
+ connection, NO_ERROR);
+ goto out; /* only one iteration */
}
no_more_connections:
@@ -3603,41 +3648,8 @@ int drbd_nl_get_connections_dumpit(struct sk_buff *skb, struct netlink_callback
cb->args[2] = 0;
goto next_resource;
}
- goto out; /* no more resources */
-
-put_result:
- dh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
- cb->nlh->nlmsg_seq, &drbd_nl_family,
- NLM_F_MULTI, DRBD_ADM_GET_CONNECTIONS);
- err = -ENOMEM;
- if (!dh)
- goto out;
- dh->ret_code = retcode;
- dh->minor = -1U;
- if (retcode == NO_ERROR) {
- struct net_conf *net_conf;
- err = nla_put_drbd_cfg_context(skb, resource, connection, NULL);
- if (err)
- goto out;
- net_conf = rcu_dereference(connection->net_conf);
- if (net_conf) {
- err = net_conf_to_skb(skb, net_conf);
- if (err)
- goto out;
- }
- connection_to_info(&connection_info, connection);
- err = connection_info_to_skb(skb, &connection_info);
- if (err)
- goto out;
- connection_statistics.conn_congested = test_bit(NET_CONGESTED, &connection->flags);
- err = connection_statistics_to_skb(skb, &connection_statistics);
- if (err)
- goto out;
- cb->args[2] = (long)connection;
- }
- genlmsg_end(skb, dh);
- err = 0;
+ /* no more resources */
out:
rcu_read_unlock();
^ 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