* Re: [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
From: Joseph Qi @ 2026-07-08 9:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, linux-kernel, Baokun Li
In-Reply-To: <20260708084547.GA3591@lst.de>
On 7/8/26 4:45 PM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2026 at 09:14:13AM +0800, Joseph Qi wrote:
>> This causes virtio-pmem flush (async_pmem_flush) to fail with -ENOMEM
>> whenever the percpu bio cache happens to be empty (common right after
>> boot), making the device effectively unmountable:
>
> Please fix that to not sure GFP_ATOMIC instead. Flushes are used
> in file system writeabck and must not use potential failing allocations.
>
> Even if you slightly increase the chance of it not failing, it still
> can and this code is simply broken.
>
Looks sane. So commit b520c4eef83d exposed the bug but not introduced it.
>> Fix this by restructuring the allocation so that the percpu cache is
>> tried first when applicable, and the slab allocation is always attempted
>> as a common fallback path when the bio is still NULL.
>
> I don;
>
>>
>> Fixes: b520c4eef83d ("block: split bio_alloc_bioset more clearly into a fast and slowpath")
>> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>> ---
>> block/bio.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/block/bio.c b/block/bio.c
>> index f2a5f4d0a9672..9e7939861b94a 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -553,6 +553,11 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
>> */
>> opf |= REQ_ALLOC_CACHE;
>> bio = bio_alloc_percpu_cache(bs);
>> + if (!bio) {
>> + p = kmem_cache_alloc(bs->bio_slab, gfp);
>> + if (p)
>> + bio = p + bs->front_pad;
>> + }
>
> But even if we wanted this, this code should not be duplicated by
> share the common version.
This is because I want to keep REQ_ALLOC_CACHE set.
Thanks for comments. I'll try to fix it in virtio-pmem driver.
Thanks,
Joseph
^ permalink raw reply
* Re: [PATCH V3 5/6] null_blk: serialize configfs attribute stores with device setup
From: Zizhi Wo @ 2026-07-08 9:11 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1
In-Reply-To: <20260708073917.2172392-6-wozizhi@huaweicloud.com>
在 2026/7/8 15:39, Zizhi Wo 写道:
> 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 the loser can overwrite dev->NAME after the
> winner's apply_fn reconfigured hardware, mismatching dev->submit_queues
> with the live queue count (null_map_queues WARN_ON_ONCE). For APPLY=NULL
> attributes, a store during power_store's null_add_dev() (validates and
> builds the device under "lock" but sets CONFIGURED only afterwards) can
> change a field mid-setup -- e.g. zone_nr_conv pushed above nr_zones after
> clamping, causing 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.
>
> _show still takes no lock; its data races are handled by READ_ONCE/
> WRITE_ONCE in a follow-up.
>
> 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 | 21 ++++++---------------
> 1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index df85189f0b69..9e0002e4aeec 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -360,13 +360,16 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \
> ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\
> if (ret < 0) \
> return ret; \
> + 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) \
Sorry for the noise. The condition here should be ret >= 0; I'll fix
this in the next version. :(
> + dev->NAME = new_value; \
> + mutex_unlock(&lock); \
> if (ret < 0) \
> return ret; \
> - dev->NAME = new_value; \
> return count; \
> } \
> CONFIGFS_ATTR(nullb_device_, NAME);
> @@ -421,25 +424,13 @@ 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);
^ permalink raw reply
* Re: [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
From: Christoph Hellwig @ 2026-07-08 8:45 UTC (permalink / raw)
To: Joseph Qi
Cc: Jens Axboe, Christoph Hellwig, linux-block, linux-kernel,
Baokun Li
In-Reply-To: <20260708011413.1710118-1-joseph.qi@linux.alibaba.com>
On Wed, Jul 08, 2026 at 09:14:13AM +0800, Joseph Qi wrote:
> This causes virtio-pmem flush (async_pmem_flush) to fail with -ENOMEM
> whenever the percpu bio cache happens to be empty (common right after
> boot), making the device effectively unmountable:
Please fix that to not sure GFP_ATOMIC instead. Flushes are used
in file system writeabck and must not use potential failing allocations.
Even if you slightly increase the chance of it not failing, it still
can and this code is simply broken.
> Fix this by restructuring the allocation so that the percpu cache is
> tried first when applicable, and the slab allocation is always attempted
> as a common fallback path when the bio is still NULL.
I don;
>
> Fixes: b520c4eef83d ("block: split bio_alloc_bioset more clearly into a fast and slowpath")
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> block/bio.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/block/bio.c b/block/bio.c
> index f2a5f4d0a9672..9e7939861b94a 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -553,6 +553,11 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
> */
> opf |= REQ_ALLOC_CACHE;
> bio = bio_alloc_percpu_cache(bs);
> + if (!bio) {
> + p = kmem_cache_alloc(bs->bio_slab, gfp);
> + if (p)
> + bio = p + bs->front_pad;
> + }
But even if we wanted this, this code should not be duplicated by
share the common version.
^ permalink raw reply
* Re: blktests: call for agreement to relicense GPL-2.0 files
From: Christoph Hellwig @ 2026-07-08 8:06 UTC (permalink / raw)
To: Shin'ichiro Kawasaki
Cc: linux-block, Bart Van Assche, Ming Lei, Ming Lei, Keith Busch,
Yi Zhang, Christoph Hellwig, Li Zhijian, Akinobu Mita,
John Pittman
In-Reply-To: <akyeX0hlcrnOC0nT@shinmob>
As a contributor I'm fine with all the specified options.
^ permalink raw reply
* [PATCH V3 2/6] null_blk: register configfs subsystem after creating default devices
From: Zizhi Wo @ 2026-07-08 7:39 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260708073917.2172392-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>
---
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
@@ -2162,15 +2162,9 @@ 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();
@@ -2178,6 +2172,10 @@ static int __init null_init(void)
goto err_dev;
}
+ ret = configfs_register_subsystem(&nullb_subsys);
+ if (ret)
+ goto err_dev;
+
pr_info("module loaded\n");
return 0;
@@ -2187,8 +2185,6 @@ static int __init null_init(void)
null_destroy_dev(nullb);
}
unregister_blkdev(null_major, "nullb");
-err_conf:
- configfs_unregister_subsystem(&nullb_subsys);
return ret;
}
--
2.52.0
^ permalink raw reply related
* [PATCH V3 1/6] null_blk: use DEFINE_MUTEX for the file-scope mutex
From: Zizhi Wo @ 2026-07-08 7:39 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260708073917.2172392-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
@@ -66,7 +66,7 @@ struct nullb_page {
#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;
@@ -2166,8 +2166,6 @@ static int __init null_init(void)
if (ret)
return ret;
- mutex_init(&lock);
-
null_major = register_blkdev(0, "nullb");
if (null_major < 0) {
ret = null_major;
--
2.52.0
^ permalink raw reply related
* [PATCH V3 5/6] null_blk: serialize configfs attribute stores with device setup
From: Zizhi Wo @ 2026-07-08 7:39 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260708073917.2172392-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 the loser can overwrite dev->NAME after the
winner's apply_fn reconfigured hardware, mismatching dev->submit_queues
with the live queue count (null_map_queues WARN_ON_ONCE). For APPLY=NULL
attributes, a store during power_store's null_add_dev() (validates and
builds the device under "lock" but sets CONFIGURED only afterwards) can
change a field mid-setup -- e.g. zone_nr_conv pushed above nr_zones after
clamping, causing 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.
_show still takes no lock; its data races are handled by READ_ONCE/
WRITE_ONCE in a follow-up.
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 | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index df85189f0b69..9e0002e4aeec 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -360,13 +360,16 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \
ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\
if (ret < 0) \
return ret; \
+ 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);
@@ -421,25 +424,13 @@ 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);
--
2.52.0
^ permalink raw reply related
* [PATCH V3 0/6] null_blk: fix init/exit races and memleaks
From: Zizhi Wo @ 2026-07-08 7:39 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
This series fixes several issues in null_blk around lock initialization,
concurrent configfs access, and module init/exit.
Patch 1 fixes the uninitialized mutex. Following Bart's suggestion, the
fix now uses DEFINE_MUTEX(). This series no longer renames the lock;
following Damien Le Moal's suggestion, the rename and locking rework will
be sent as a separate series.
Patch 2 fixes configfs registration concurrency.
Patch 3 reorders resource release in null_exit() to match null_init().
Patch 4 fixes a global tag_set leak on the null_init() error path.
Patch 5 fixes a mid-setup race where a store changes a field before
CONFIGURED is set, causing out-of-bounds dev->zones[] access
Patch 6 adds READ_ONCE()/WRITE_ONCE() to the configfs attribute
reads/writes that race with each other.
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 5: Mofify the lock position in v2 patch 4 to fix a mid-setup race.
- Added new patch 6.
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 (6):
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: serialize configfs attribute stores with device setup
null_blk: mark racy configfs attribute accesses with
READ_ONCE/WRITE_ONCE
drivers/block/null_blk/main.c | 92 +++++++++++++++-------------------
drivers/block/null_blk/zoned.c | 18 +++----
2 files changed, 49 insertions(+), 61 deletions(-)
--
2.52.0
^ permalink raw reply
* [PATCH V3 3/6] null_blk: move unregister_blkdev() after destroying dev in null_exit()
From: Zizhi Wo @ 2026-07-08 7:39 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260708073917.2172392-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>
---
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
@@ -2194,8 +2194,6 @@ static void __exit null_exit(void)
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);
@@ -2203,6 +2201,8 @@ static void __exit null_exit(void)
}
mutex_unlock(&lock);
+ unregister_blkdev(null_major, "nullb");
+
if (tag_set.ops)
blk_mq_free_tag_set(&tag_set);
--
2.52.0
^ permalink raw reply related
* [PATCH V3 6/6] null_blk: mark racy configfs attribute accesses with READ_ONCE/WRITE_ONCE
From: Zizhi Wo @ 2026-07-08 7:39 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260708073917.2172392-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 dev->NAME. 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. Mark the _show read with READ_ONCE() and the _store write with
WRITE_ONCE() in the macro. The non-macro "power" attribute has the same
issue between power_show and power_store and is marked the same way.
The same _show readers also race against writes to these fields outside
_store that run after the configfs item becomes visible:
1. nullb_update_nr_hw_queues() (submit_queues, poll_queues),
2. null_validate_conf() (queue_mode, submit_queues, poll_queues, irqmode,
blocking, cache_size, mbps),
3. null_config_discard() (discard),
4. null_init_zoned_dev() (zone_capacity, zone_nr_conv,
zone_append_max_sectors, zone_max_active, zone_max_open),
5. null_add_dev() (index).
These run under the file-scope lock (taken in the macro for _store, and in
power_store for setup), but _show does not take that lock, so a plain write
still races with the READ_ONCE() read. Mark all of them with WRITE_ONCE().
Writes in null_alloc_dev() are intentionally left as plain assignments: it
runs from the .make_group callback before the configfs item is published,
so _show cannot run concurrently with it. The dev->power write in
nullb_group_drop_item() is also left plain: configfs takes frag_sem for
write and sets frag_dead during rmdir, and drop_item runs only after that,
so attribute show/store (frag_sem readers) cannot be concurrent with it.
The setup-side reads in null_validate_conf()/null_config_discard()/etc.
no longer race with _store now that _store takes the file-scope lock; those
reads are therefore left plain. This patch closes the remaining
_show-vs-write data races.
Suggested-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 47 +++++++++++++++++-----------------
drivers/block/null_blk/zoned.c | 18 ++++++-------
2 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 9e0002e4aeec..fd3c993a67b2 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -346,7 +346,7 @@ static ssize_t \
nullb_device_##NAME##_show(struct config_item *item, char *page) \
{ \
return nullb_device_##TYPE##_attr_show( \
- to_nullb_device(item)->NAME, page); \
+ READ_ONCE(to_nullb_device(item)->NAME), page); \
} \
static ssize_t \
nullb_device_##NAME##_store(struct config_item *item, const char *page, \
@@ -366,7 +366,7 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \
else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \
ret = -EBUSY; \
if (!ret) \
- dev->NAME = new_value; \
+ WRITE_ONCE(dev->NAME, new_value); \
mutex_unlock(&lock); \
if (ret < 0) \
return ret; \
@@ -404,8 +404,8 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
*/
dev->prev_submit_queues = dev->submit_queues;
dev->prev_poll_queues = dev->poll_queues;
- dev->submit_queues = submit_queues;
- dev->poll_queues = poll_queues;
+ WRITE_ONCE(dev->submit_queues, submit_queues);
+ WRITE_ONCE(dev->poll_queues, poll_queues);
set = dev->nullb->tag_set;
nr_hw_queues = submit_queues + poll_queues;
@@ -414,8 +414,8 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
if (ret) {
/* on error, revert the queue numbers */
- dev->submit_queues = dev->prev_submit_queues;
- dev->poll_queues = dev->prev_poll_queues;
+ WRITE_ONCE(dev->submit_queues, dev->prev_submit_queues);
+ WRITE_ONCE(dev->poll_queues, dev->prev_poll_queues);
}
return ret;
@@ -469,7 +469,8 @@ 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);
+ return nullb_device_bool_attr_show(
+ READ_ONCE(to_nullb_device(item)->power), page);
}
static ssize_t nullb_device_power_store(struct config_item *item,
@@ -496,11 +497,11 @@ static ssize_t nullb_device_power_store(struct config_item *item,
}
set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
- dev->power = newp;
+ WRITE_ONCE(dev->power, newp);
ret = count;
} else if (dev->power && !newp) {
if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
- dev->power = newp;
+ WRITE_ONCE(dev->power, newp);
null_del_dev(dev->nullb);
}
clear_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
@@ -1783,13 +1784,13 @@ static void null_config_discard(struct nullb *nullb, struct queue_limits *lim)
return;
if (!nullb->dev->memory_backed) {
- nullb->dev->discard = false;
+ WRITE_ONCE(nullb->dev->discard, false);
pr_info("discard option is ignored without memory backing\n");
return;
}
if (nullb->dev->zoned) {
- nullb->dev->discard = false;
+ WRITE_ONCE(nullb->dev->discard, false);
pr_info("discard option is ignored in zoned mode\n");
return;
}
@@ -1881,31 +1882,31 @@ static int null_validate_conf(struct nullb_device *dev)
}
if (dev->queue_mode == NULL_Q_BIO) {
pr_err("BIO-based IO path is no longer available, using blk-mq instead.\n");
- dev->queue_mode = NULL_Q_MQ;
+ WRITE_ONCE(dev->queue_mode, NULL_Q_MQ);
}
if (dev->use_per_node_hctx) {
if (dev->submit_queues != nr_online_nodes)
- dev->submit_queues = nr_online_nodes;
+ WRITE_ONCE(dev->submit_queues, nr_online_nodes);
} else if (dev->submit_queues > nr_cpu_ids)
- dev->submit_queues = nr_cpu_ids;
+ WRITE_ONCE(dev->submit_queues, nr_cpu_ids);
else if (dev->submit_queues == 0)
- dev->submit_queues = 1;
+ WRITE_ONCE(dev->submit_queues, 1);
dev->prev_submit_queues = dev->submit_queues;
if (dev->poll_queues > g_poll_queues)
- dev->poll_queues = g_poll_queues;
+ WRITE_ONCE(dev->poll_queues, g_poll_queues);
dev->prev_poll_queues = dev->poll_queues;
- dev->irqmode = min_t(unsigned int, dev->irqmode, NULL_IRQ_TIMER);
+ WRITE_ONCE(dev->irqmode, min_t(unsigned int, dev->irqmode, NULL_IRQ_TIMER));
/* Do memory allocation, so set blocking */
if (dev->memory_backed)
- dev->blocking = true;
+ WRITE_ONCE(dev->blocking, true);
else /* cache is meaningless */
- dev->cache_size = 0;
- dev->cache_size = min_t(unsigned long, ULONG_MAX / 1024 / 1024,
- dev->cache_size);
- dev->mbps = min_t(unsigned int, 1024 * 40, dev->mbps);
+ WRITE_ONCE(dev->cache_size, 0);
+ WRITE_ONCE(dev->cache_size, min_t(unsigned long, ULONG_MAX / 1024 / 1024,
+ dev->cache_size));
+ WRITE_ONCE(dev->mbps, min_t(unsigned int, 1024 * 40, dev->mbps));
if (dev->zoned &&
(!dev->zone_size || !is_power_of_2(dev->zone_size))) {
@@ -2015,7 +2016,7 @@ static int null_add_dev(struct nullb_device *dev)
goto out_cleanup_disk;
nullb->index = rv;
- dev->index = rv;
+ WRITE_ONCE(dev->index, rv);
if (config_item_name(&dev->group.cg_item)) {
/* Use configfs dir name as the device name */
diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 384bdce6a9b7..75613eaf3ea9 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -66,7 +66,7 @@ int null_init_zoned_dev(struct nullb_device *dev,
}
if (!dev->zone_capacity)
- dev->zone_capacity = dev->zone_size;
+ WRITE_ONCE(dev->zone_capacity, dev->zone_size);
if (dev->zone_capacity > dev->zone_size) {
pr_err("zone capacity (%lu MB) larger than zone size (%lu MB)\n",
@@ -99,29 +99,29 @@ int null_init_zoned_dev(struct nullb_device *dev,
spin_lock_init(&dev->zone_res_lock);
if (dev->zone_nr_conv >= dev->nr_zones) {
- dev->zone_nr_conv = dev->nr_zones - 1;
+ WRITE_ONCE(dev->zone_nr_conv, dev->nr_zones - 1);
pr_info("changed the number of conventional zones to %u",
dev->zone_nr_conv);
}
- dev->zone_append_max_sectors =
- min(ALIGN_DOWN(dev->zone_append_max_sectors,
- dev->blocksize >> SECTOR_SHIFT),
- zone_capacity_sects);
+ WRITE_ONCE(dev->zone_append_max_sectors,
+ min(ALIGN_DOWN(dev->zone_append_max_sectors,
+ dev->blocksize >> SECTOR_SHIFT),
+ zone_capacity_sects));
/* Max active zones has to be < nbr of seq zones in order to be enforceable */
if (dev->zone_max_active >= dev->nr_zones - dev->zone_nr_conv) {
- dev->zone_max_active = 0;
+ WRITE_ONCE(dev->zone_max_active, 0);
pr_info("zone_max_active limit disabled, limit >= zone count\n");
}
/* Max open zones has to be <= max active zones */
if (dev->zone_max_active && dev->zone_max_open > dev->zone_max_active) {
- dev->zone_max_open = dev->zone_max_active;
+ WRITE_ONCE(dev->zone_max_open, dev->zone_max_active);
pr_info("changed the maximum number of open zones to %u\n",
dev->zone_max_open);
} else if (dev->zone_max_open >= dev->nr_zones - dev->zone_nr_conv) {
- dev->zone_max_open = 0;
+ WRITE_ONCE(dev->zone_max_open, 0);
pr_info("zone_max_open limit disabled, limit >= zone count\n");
}
dev->need_zone_res_mgmt = dev->zone_max_active || dev->zone_max_open;
--
2.52.0
^ permalink raw reply related
* [PATCH V3 4/6] null_blk: free global tag_set on init error path
From: Zizhi Wo @ 2026-07-08 7:39 UTC (permalink / raw)
To: axboe, dlemoal, nilay, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260708073917.2172392-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>
---
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
@@ -2185,6 +2185,8 @@ static int __init null_init(void)
null_destroy_dev(nullb);
}
unregister_blkdev(null_major, "nullb");
+ if (tag_set.ops)
+ blk_mq_free_tag_set(&tag_set);
return ret;
}
--
2.52.0
^ permalink raw reply related
* Re: [PATCH] block: rnull: use vertical import style
From: Link Mauve @ 2026-07-08 7:40 UTC (permalink / raw)
To: Guru Das Srinagesh
Cc: Andreas Hindborg, Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-block,
rust-for-linux, linux-kernel, llvm
In-Reply-To: <20260705-rfl-vert-imp-2-v1-1-e6ab69c55095@gurudas.dev>
On Sun, Jul 05, 2026 at 06:36:48PM -0700, Guru Das Srinagesh wrote:
> Convert `use` imports to vertical layout for better readability and
> maintainability.
>
> Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
This one is:
Reviewed-by: Link Mauve <linkmauve@linkmauve.fr>
Thanks!
> ---
> Came across a recent commit bc58905eb07 ("samples: rust_misc_device: use
> vertical import style") and found a few more locations that could
> benefit from this cleanup. No functional changes.
>
> Separating out patches per-subsystem as per the review feedback in [0].
>
> Tested via:
>
> $ make LLVM=1 rustfmtcheck || echo "fail"
> $
>
> Initially thought that rust-for-linux/rust-block-next was the right base
> branch to use, which is why I didn't group this with [1] based on
> rust-next that I sent out a short while ago. Using rust-next as base for
> this patch.
>
> [0]: https://lore.kernel.org/lkml/20260628-b4-rust-vertical-imports-v1-0-98bc71d4810b@gurudas.dev/
> [1]: https://lore.kernel.org/lkml/20260705-rfl-vert-imp-v1-1-2f07946a905f@gurudas.dev/
> ---
> drivers/block/rnull/configfs.rs | 25 ++++++++++++++++++++-----
> drivers/block/rnull/rnull.rs | 13 ++++++++++---
> 2 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 7c2eb5c0b722..2149b47db697 100644
> --- a/drivers/block/rnull/configfs.rs
> +++ b/drivers/block/rnull/configfs.rs
> @@ -1,15 +1,30 @@
> // SPDX-License-Identifier: GPL-2.0
>
> -use super::{NullBlkDevice, THIS_MODULE};
> +use super::{
> + NullBlkDevice,
> + THIS_MODULE, //
> +};
> use kernel::{
> - block::mq::gen_disk::{GenDisk, GenDiskBuilder},
> - configfs::{self, AttributeOperations},
> + block::mq::gen_disk::{
> + GenDisk,
> + GenDiskBuilder, //
> + },
> + configfs::{
> + self,
> + AttributeOperations, //
> + },
> configfs_attrs,
> - fmt::{self, Write as _},
> + fmt::{
> + self,
> + Write as _, //
> + },
> new_mutex,
> page::PAGE_SIZE,
> prelude::*,
> - str::{kstrtobool_bytes, CString},
> + str::{
> + kstrtobool_bytes,
> + CString, //
> + },
> sync::Mutex,
> };
>
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index 0ca8715febe8..d58d2c4c5f63 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -10,12 +10,19 @@
> self,
> mq::{
> self,
> - gen_disk::{self, GenDisk},
> - Operations, TagSet,
> + gen_disk::{
> + self,
> + GenDisk, //
> + },
> + Operations,
> + TagSet, //
> },
> },
> prelude::*,
> - sync::{aref::ARef, Arc},
> + sync::{
> + aref::ARef,
> + Arc, //
> + },
> };
>
> module! {
>
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260705-rfl-vert-imp-2-9bba7706d3f0
>
> Best regards,
> --
> Guru Das Srinagesh <linux@gurudas.dev>
>
>
--
Link Mauve
^ permalink raw reply
* [PATCH] scsi: sg: validate and round up scatter_elem_sz module parameter
From: Yang Erkun @ 2026-07-08 6:30 UTC (permalink / raw)
To: dgilbert, James.Bottomley, martin.petersen, yukuai, hch, axboe
Cc: linux-scsi, linux-block, yangerkun
echo -1 or 0 > /sys/module/sg/parameters/scatter_elem_sz
exec 4<> /dev/sg0
The preferred shell trigger 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.
Additionally, the code handling scatter_elem_sz is refactored for
clarity: the scatter_elem_sz_prev variable and the runtime fixup logic
in sg_build_indirect() are removed. 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. The in-loop fixup (updating
scatter_elem_sz to ret_sz when ret_sz > scatter_elem_sz_prev) is no
longer needed because the .set callback rounds up scatter_elem_sz to 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 SG_SCATTER_SZ (32768)
is always >= PAGE_SIZE.
Fixes: 6460e75a104d ("[SCSI] sg: fixes for large page_size")
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
drivers/scsi/sg.c | 68 +++++++++++++++++++++++------------------------
1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 74cd4e8a61c2..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
@@ -1622,8 +1621,34 @@ 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);
-module_param_named(allow_dio, sg_allow_dio, 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, 0644);
static int def_reserved_size_set(const char *val, const struct kernel_param *kp)
{
@@ -1649,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");
@@ -1668,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)
@@ -1854,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)
@@ -1874,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] blk-cgroup: clear blkg->pd[] with WRITE_ONCE() in blkcg_deactivate_policy()
From: Guopeng Zhang @ 2026-07-08 5:59 UTC (permalink / raw)
To: Tao Cui, Tejun Heo, Josef Bacik, Jens Axboe
Cc: Yu Kuai, cgroups, linux-block, linux-kernel, Guopeng Zhang
In-Reply-To: <f2f3f741-3420-4dbb-92e1-1598370922d8@linux.dev>
在 2026/7/8 13:29, Tao Cui 写道:
> FYI, this WRITE_ONCE() change is also in Yu Kuai's in-flight series that
> factors the pd teardown loop into a helper:
>
> https://lore.kernel.org/all/20260625025739.2459651-5-yukuai@kernel.org/
>
> That series isn't merged yet (v3, 2026-06-25), so this minimal Fixes
> patch is still useful in the meantime -- just flagging the overlap.
>
hello，
Thanks for the review, and thanks for pointing out the overlap.
If Yu's series gets merged first, this patch can simply be dropped.
Thanks,
Guopeng
> 在 2026/7/8 13:10, Tao Cui 写道:
>>
>>
>> 在 2026/7/7 20:58, Guopeng Zhang 写道:
>>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>>
>>> blkcg_activate_policy() installs blkg->pd[] entries with WRITE_ONCE()
>>> and also uses WRITE_ONCE() when clearing them on its error path.
>>> blkg_to_pd() is used by RCU readers and reads the same array with
>>> READ_ONCE().
>>>
>>> blkcg_deactivate_policy() clears the entry with a plain store. Use
>>> WRITE_ONCE() there as well.
>>>
>>> Fixes: 56cc24f59c14 ("blk-cgroup: don't nest queue_lock under rcu in blkcg_print_blkgs()")
>>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>> ---
>>> block/blk-cgroup.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
>>> index d2a1f5903f24..a1dd69f99f5c 100644
>>> --- a/block/blk-cgroup.c
>>> +++ b/block/blk-cgroup.c
>>> @@ -1691,7 +1691,7 @@ void blkcg_deactivate_policy(struct gendisk *disk,
>>> if (blkg->pd[pol->plid]->online && pol->pd_offline_fn)
>>> pol->pd_offline_fn(blkg->pd[pol->plid]);
>>> pol->pd_free_fn(blkg->pd[pol->plid]);
>>> - blkg->pd[pol->plid] = NULL;
>>> + WRITE_ONCE(blkg->pd[pol->plid], NULL);
>>> }
>>> spin_unlock(&blkcg->lock);
>>> }
>>
>> Reviewed-by: Tao Cui <cuitao@kylinos.cn>
>>
>
^ permalink raw reply
* Re: [PATCH] blk-cgroup: clear blkg->pd[] with WRITE_ONCE() in blkcg_deactivate_policy()
From: Tao Cui @ 2026-07-08 5:29 UTC (permalink / raw)
To: Guopeng Zhang, Tejun Heo, Josef Bacik, Jens Axboe
Cc: cui.tao, Yu Kuai, cgroups, linux-block, linux-kernel,
Guopeng Zhang
In-Reply-To: <841e91db-a51e-4df5-8cfc-e3762a2fecf1@linux.dev>
FYI, this WRITE_ONCE() change is also in Yu Kuai's in-flight series that
factors the pd teardown loop into a helper:
https://lore.kernel.org/all/20260625025739.2459651-5-yukuai@kernel.org/
That series isn't merged yet (v3, 2026-06-25), so this minimal Fixes
patch is still useful in the meantime -- just flagging the overlap.
在 2026/7/8 13:10, Tao Cui 写道:
>
>
> 在 2026/7/7 20:58, Guopeng Zhang 写道:
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> blkcg_activate_policy() installs blkg->pd[] entries with WRITE_ONCE()
>> and also uses WRITE_ONCE() when clearing them on its error path.
>> blkg_to_pd() is used by RCU readers and reads the same array with
>> READ_ONCE().
>>
>> blkcg_deactivate_policy() clears the entry with a plain store. Use
>> WRITE_ONCE() there as well.
>>
>> Fixes: 56cc24f59c14 ("blk-cgroup: don't nest queue_lock under rcu in blkcg_print_blkgs()")
>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> ---
>> block/blk-cgroup.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
>> index d2a1f5903f24..a1dd69f99f5c 100644
>> --- a/block/blk-cgroup.c
>> +++ b/block/blk-cgroup.c
>> @@ -1691,7 +1691,7 @@ void blkcg_deactivate_policy(struct gendisk *disk,
>> if (blkg->pd[pol->plid]->online && pol->pd_offline_fn)
>> pol->pd_offline_fn(blkg->pd[pol->plid]);
>> pol->pd_free_fn(blkg->pd[pol->plid]);
>> - blkg->pd[pol->plid] = NULL;
>> + WRITE_ONCE(blkg->pd[pol->plid], NULL);
>> }
>> spin_unlock(&blkcg->lock);
>> }
>
> Reviewed-by: Tao Cui <cuitao@kylinos.cn>
>
^ permalink raw reply
* Re: [PATCH] blk-cgroup: clear blkg->pd[] with WRITE_ONCE() in blkcg_deactivate_policy()
From: Tao Cui @ 2026-07-08 5:10 UTC (permalink / raw)
To: Guopeng Zhang, Tejun Heo, Josef Bacik, Jens Axboe
Cc: cui.tao, Yu Kuai, cgroups, linux-block, linux-kernel,
Guopeng Zhang
In-Reply-To: <20260707125814.1978139-1-guopeng.zhang@linux.dev>
在 2026/7/7 20:58, Guopeng Zhang 写道:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> blkcg_activate_policy() installs blkg->pd[] entries with WRITE_ONCE()
> and also uses WRITE_ONCE() when clearing them on its error path.
> blkg_to_pd() is used by RCU readers and reads the same array with
> READ_ONCE().
>
> blkcg_deactivate_policy() clears the entry with a plain store. Use
> WRITE_ONCE() there as well.
>
> Fixes: 56cc24f59c14 ("blk-cgroup: don't nest queue_lock under rcu in blkcg_print_blkgs()")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> block/blk-cgroup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index d2a1f5903f24..a1dd69f99f5c 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -1691,7 +1691,7 @@ void blkcg_deactivate_policy(struct gendisk *disk,
> if (blkg->pd[pol->plid]->online && pol->pd_offline_fn)
> pol->pd_offline_fn(blkg->pd[pol->plid]);
> pol->pd_free_fn(blkg->pd[pol->plid]);
> - blkg->pd[pol->plid] = NULL;
> + WRITE_ONCE(blkg->pd[pol->plid], NULL);
> }
> spin_unlock(&blkcg->lock);
> }
Reviewed-by: Tao Cui <cuitao@kylinos.cn>
^ permalink raw reply
* Re: [PATCH] blk-iolatency: flush enable work after policy deactivation
From: Tao Cui @ 2026-07-08 5:03 UTC (permalink / raw)
To: Cen Zhang, Tejun Heo, Josef Bacik, Jens Axboe
Cc: cui.tao, cgroups, linux-block, linux-kernel, baijiaju1990
In-Reply-To: <20260621135916.2657247-1-zzzccc427@gmail.com>
在 2026/6/21 21:59, Cen Zhang 写道:
> A blk-iolatency rq-qos teardown can free struct blk_iolatency while a
> freshly queued enable_work callback still references it. The observed
> failure is:
>
> blkcg_iolatency_exit() flushes enable_work before deactivating the
> iolatency policy. However, blkcg_deactivate_policy() calls
> iolatency_pd_offline() for online policy data, and iolatency_pd_offline()
> clears min_lat_nsec through iolatency_set_min_lat_nsec(). If this clears
> the last nonzero latency target, enable_cnt reaches zero and schedules
> enable_work again after the flush has already returned.
>
> The buggy scenario involves two paths, with each column showing the order
> within that path:
>
> blkcg_iolatency_exit() path: system_wq worker path:
> 1. Flush old enable_work. 1. enable_work is idle.
> 2. Deactivate the policy. 2. no worker owns it.
> 3. Offline queues new enable_work. 3. work item becomes pending.
> 4. Free blkiolat. 4. worker later runs the item.
> 5. Owner storage is gone. 5. worker dereferences blkiolat.
>
> Flush enable_work again after blkcg_deactivate_policy() returns and before
> freeing blkiolat. Policy offline callbacks have completed at that point,
> so the second drain covers the late queueing path without changing the
> normal enable/disable accounting rules.
>
Acked-by: Tao Cui <cuitao@kylinos.cn>
The first flush_work() above blkcg_deactivate_policy() is now redundant
and can be dropped.
> Validation reproduced this kernel report:
> BUG: KASAN: slab-use-after-free in assign_work+0x2a/0x150
>
> Call Trace:
> <TASK>
> dump_stack_lvl+0x53/0x70
> print_report+0xd0/0x630
> ? __pfx__raw_spin_lock_irqsave+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __virt_addr_valid+0xea/0x1a0
> ? assign_work+0x2a/0x150
> kasan_report+0xce/0x100
> ? assign_work+0x2a/0x150
> assign_work+0x2a/0x150
> worker_thread+0x1b7/0x500
> ? __pfx_worker_thread+0x10/0x10
> kthread+0x192/0x1d0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x2ac/0x3c0
> ? __pfx_ret_from_fork+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __switch_to+0x2d5/0x6e0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> Allocated by task 470:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x14/0x30
> __kasan_kmalloc+0x8f/0xa0
> iolatency_set_limit+0x301/0x450
> cgroup_file_write+0x178/0x2e0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Freed by task 611:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x14/0x30
> kasan_save_free_info+0x3b/0x60
> __kasan_slab_free+0x43/0x70
> kfree+0x131/0x390
> rq_qos_exit+0x5d/0x90
> __del_gendisk+0x394/0x490
> del_gendisk+0xa1/0xe0
> virtblk_remove+0x41/0xd0
> virtio_dev_remove+0x63/0xe0
> device_release_driver_internal+0x246/0x2e0
> unbind_store+0xa9/0xb0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Last potentially related work creation:
> kasan_save_stack+0x33/0x60
> kasan_record_aux_stack+0x8c/0xa0
> __queue_work+0x42a/0x800
> queue_work_on+0x5d/0x70
> iolatency_set_min_lat_nsec+0x196/0x230
> iolatency_pd_offline+0x1f/0x40
> blkcg_deactivate_policy+0x194/0x270
> blkcg_iolatency_exit+0x33/0x40
> rq_qos_exit+0x5d/0x90
> __del_gendisk+0x394/0x490
> del_gendisk+0xa1/0xe0
> virtblk_remove+0x41/0xd0
> virtio_dev_remove+0x63/0xe0
> device_release_driver_internal+0x246/0x2e0
> unbind_store+0xa9/0xb0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Second to last potentially related work creation:
> kasan_save_stack+0x33/0x60
> kasan_record_aux_stack+0x8c/0xa0
> __queue_work+0x42a/0x800
> queue_work_on+0x5d/0x70
> iolatency_set_min_lat_nsec+0x196/0x230
> iolatency_set_limit+0x3f1/0x450
> cgroup_file_write+0x178/0x2e0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Fixes: 8a177a36da6c ("blk-iolatency: Fix inflight count imbalances and IO hangs on offline")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> block/blk-iolatency.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
> index 1aaee6fb0f59..a0bdd8a5c94c 100644
> --- a/block/blk-iolatency.c
> +++ b/block/blk-iolatency.c
> @@ -639,6 +639,11 @@ static void blkcg_iolatency_exit(struct rq_qos *rqos)
> timer_shutdown_sync(&blkiolat->timer);
> flush_work(&blkiolat->enable_work);
> blkcg_deactivate_policy(rqos->disk, &blkcg_policy_iolatency);
> + /*
> + * blkcg_deactivate_policy() invokes iolatency_pd_offline(), which may
> + * queue enable_work again when it clears the last latency target.
> + */
> + flush_work(&blkiolat->enable_work);
> kfree(blkiolat);
> }
>
^ permalink raw reply
* Re: blktests: call for agreement to relicense GPL-2.0 files
From: Zhijian Li (Fujitsu) @ 2026-07-08 2:34 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, linux-block@vger.kernel.org,
Bart Van Assche, Ming Lei, Ming Lei, Keith Busch, Yi Zhang,
Christoph Hellwig, Akinobu Mita, John Pittman
In-Reply-To: <akyeX0hlcrnOC0nT@shinmob>
On 07/07/2026 14:38, Shin'ichiro Kawasaki wrote:
> 1) block/029
> Author: Bart Van Assche<bvanassche@acm.org>
> Contributors: Yi Zhang<yi.zhang@redhat.com>
> Christoph Hellwig<hch@lst.de>
> Li Zhijian<lizhijian@fujitsu.com>
>
> 2) block/030
> Author: Bart Van Assche<bvanassche@acm.org>
> Contributors: Christoph Hellwig<hch@lst.de>
> Li Zhijian<lizhijian@fujitsu.com>
> Akinobu Mita<akinobu.mita@gmail.com>
Ack for switching to GPL-3.0+
^ permalink raw reply
* [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
From: Joseph Qi @ 2026-07-08 1:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: Christoph Hellwig, linux-block, linux-kernel, Baokun Li
bio_alloc_bioset() was restructured in commit b520c4eef83d ("block:
split bio_alloc_bioset more clearly into a fast and slowpath") to
separate fast and slow paths. However, the restructuring introduced a
regression for GFP_ATOMIC callers when the per-CPU bio cache is enabled.
Before the restructuring, a percpu cache miss would fall through to the
mempool allocation path, which internally performs a slab allocation.
After the restructuring, a percpu cache miss leaves bio as NULL and
falls through to:
if (unlikely(!bio)) {
if (!(saved_gfp & __GFP_DIRECT_RECLAIM))
return NULL;
...
}
This immediately returns NULL for GFP_ATOMIC callers without ever
attempting a slab allocation, even when there is plenty of free memory.
The comment says "non-blocking mempool allocations just go back to the
slab allocation", but for the percpu-cache path the slab was never tried
in the first place.
This causes virtio-pmem flush (async_pmem_flush) to fail with -ENOMEM
whenever the percpu bio cache happens to be empty (common right after
boot), making the device effectively unmountable:
Buffer I/O error on dev pmem0, logical block 0, lost sync page write
Fix this by restructuring the allocation so that the percpu cache is
tried first when applicable, and the slab allocation is always attempted
as a common fallback path when the bio is still NULL.
Fixes: b520c4eef83d ("block: split bio_alloc_bioset more clearly into a fast and slowpath")
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
block/bio.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/block/bio.c b/block/bio.c
index f2a5f4d0a9672..9e7939861b94a 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -553,6 +553,11 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
*/
opf |= REQ_ALLOC_CACHE;
bio = bio_alloc_percpu_cache(bs);
+ if (!bio) {
+ p = kmem_cache_alloc(bs->bio_slab, gfp);
+ if (p)
+ bio = p + bs->front_pad;
+ }
} else {
opf &= ~REQ_ALLOC_CACHE;
p = kmem_cache_alloc(bs->bio_slab, gfp);
--
2.39.3
^ permalink raw reply related
* Re: [PATCH 2/2] dm-raid1: don't fail the mirror for invalid I/O errors
From: Mike Snitzer @ 2026-07-07 19:44 UTC (permalink / raw)
To: Keith Busch, axboe
Cc: Mikulas Patocka, Benjamin Marzinski, Keith Busch, dm-devel,
linux-block, Dr. David Alan Gilbert, Vjaceslavs Klimovs
In-Reply-To: <akwVkSnXtyl6rBdh@kbusch-mbp>
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:
> > This approach is OK, I will stage the patches when 7.2-rc1 comes out and
> > when I'll fork the dm git branches.
> >
> > I suggest one change - it is kind of hacky when multiple I/O completion
> > callbacks write into io->orig_bio->bi_status concurrently - so it would be
> > better to not do it and maintain and return separate bit mask for
> > non-retryable errors.
> >
> > For example:
> >
> > static void complete_io(struct io *io)
> > {
> > unsigned long error_bits = io->error_bits;
> > unsigned long nonretryable_error_bits = io->nonretryable_error_bits;
> > io_notify_fn fn = io->callback;
> > void *context = io->context;
> >
> > if (io->vma_invalidate_size)
> > invalidate_kernel_vmap_range(io->vma_invalidate_address,
> > io->vma_invalidate_size);
> >
> > mempool_free(io, &io->client->pool);
> > fn(error_bits, nonretryable_error_bits, context);
> > }
> >
> > static void dec_count(struct io *io, unsigned int region, blk_status_t error)
> > {
> > if (unlikely(error == BLK_STS_NOTSUPP) || unlikely(error == BLK_STS_INVAL))
> > set_bit(region, &io->nonretryable_error_bits);
> > else if (unlikely(error != BLK_STS_OK))
> > set_bit(region, &io->error_bits);
> >
> > if (atomic_dec_and_test(&io->count))
> > complete_io(io);
> > }
> >
> > Please send the updated patch that uses this approach.
>
> Sure thing, I can get started on that. Though I think it's largely
> obviated if we get the block layer to handle things early rather than
> submit malformed bio's, and this will accomplish that:
>
> https://lore.kernel.org/dm-devel/20260624170905.3972095-1-kbusch@meta.com/
Jens hasn't picked that series up yet. Jens?
> But I can certainly respin this series as it provides a more indepth
> defence.
>
> I also owe an update on the relaxed dm-crypt direct-io memory alignment
> as well, as that series fell through the cracks on me for the previous
> merge window.
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.
Thanks,
Mike
^ permalink raw reply
* Re: blktests: call for agreement to relicense GPL-2.0 files
From: Keith Busch @ 2026-07-07 16:55 UTC (permalink / raw)
To: Shin'ichiro Kawasaki
Cc: linux-block, Bart Van Assche, Ming Lei, Ming Lei, Yi Zhang,
Christoph Hellwig, Li Zhijian, Akinobu Mita, John Pittman
In-Reply-To: <akyeX0hlcrnOC0nT@shinmob>
On Tue, Jul 07, 2026 at 03:38:51PM +0900, Shin'ichiro Kawasaki wrote:
> 5) block/042
> Author: Keith Busch <kbusch@kernel.org>
> Contributors: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> John Pittman <jpittman@redhat.com>
>
> 6) block/043
> Author: Keith Busch <kbusch@kernel.org>
> Contributors: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>
> 7) src/dio-offsets.c
> Author: Keith Busch <kbusch@kernel.org>
> Contributors: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Bart Van Assche <bvanassche@acm.org>
Ack for switching to GPL-3.0+
^ permalink raw reply
* Re: blktests: call for agreement to relicense GPL-2.0 files
From: Bart Van Assche @ 2026-07-07 16:53 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, linux-block, Ming Lei, Ming Lei,
Keith Busch, Yi Zhang, Christoph Hellwig, Li Zhijian,
Akinobu Mita, John Pittman
In-Reply-To: <akyeX0hlcrnOC0nT@shinmob>
On 7/6/26 11:38 PM, Shin'ichiro Kawasaki wrote:
> To the authors and the contributors,
>
> I would like to call for your responses. Please let us know if you agree to
> modify the license of the files from GPL-2.0 to GPL-2.0+ (or GPL-3.0+).
I'm fine with making this change.
Thanks,
Bart.
^ permalink raw reply
* Re: blktests: call for agreement to relicense GPL-2.0 files
From: Ming Lei @ 2026-07-07 16:23 UTC (permalink / raw)
To: Shin'ichiro Kawasaki
Cc: linux-block, Bart Van Assche, Ming Lei, Keith Busch, Yi Zhang,
Christoph Hellwig, Li Zhijian, Akinobu Mita, John Pittman
In-Reply-To: <akyeX0hlcrnOC0nT@shinmob>
On Tue, Jul 7, 2026 at 1:39 AM Shin'ichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> Hello blktests contributors,
>
> Recent discussion unveiled a license inconsistency [1]. Currently, each file of
> blktests has one of the licenses: GPL-2.0, GPL-2.0+ or GPL-3.0+. Majority of the
> blktests code has GPL-3.0+, so the effective main license is GPL-3.0+ (e.g.,
> check, common/rc). However, GPL-2.0 and GPL-3.0 are not compatible with each
> other [2]. Because the majority of blktests is GPL-3.0+, blktests cannot contain
> GPL-2.0 files. All files shall be GPL-2.0+ or GPL-3.0+.
>
> [1] https://lore.kernel.org/linux-block/20260626045650.GA8752@lst.de/
> [2] https://www.gnu.org/licenses/gpl-faq.html#v2v3Compatibility
>
> As of July 2026, seven files below have GPL-2.0. It is desired to change their
> license to GPL-2.0+ (or GPL-3.0+). The list below shows Authors and Contributors
> of each file. "Author" means the original file creator who set the license
> GPL-2.0. "Contributor" means those who made changes to the files later on.
>
> 1) block/029
> Author: Bart Van Assche <bvanassche@acm.org>
> Contributors: Yi Zhang <yi.zhang@redhat.com>
> Christoph Hellwig <hch@lst.de>
> Li Zhijian <lizhijian@fujitsu.com>
>
> 2) block/030
> Author: Bart Van Assche <bvanassche@acm.org>
> Contributors: Christoph Hellwig <hch@lst.de>
> Li Zhijian <lizhijian@fujitsu.com>
> Akinobu Mita <akinobu.mita@gmail.com>
>
> 3) block/031
> Author: Bart Van Assche <bvanassche@acm.org>
> Contributors: Yi Zhang <yi.zhang@redhat.com>
> Christoph Hellwig <hch@lst.de>
> Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>
> 4) block/040
> Author: Ming Lei <ming.lei@redhat.com>
Ack for switching block/040 from GPL-2.0 to GPL-3.0+.
Thanks
Ming Lei
^ permalink raw reply
* [PATCH 6.12.y 2/2] block: fix queue freeze vs limits lock order in sysfs store methods
From: Konstantin Andreev @ 2026-07-07 15:37 UTC (permalink / raw)
To: stable
Cc: linux-block, linux-kernel, Christoph Hellwig, Ming Lei,
Damien Le Moal, Martin K. Petersen, Nilay Shroff,
Johannes Thumshirn, Jens Axboe
In-Reply-To: <20260707153729.1834723-1-andreev@swemel.ru>
From: Christoph Hellwig <hch@lst.de>
[ Upstream commit c99f66e4084a62a2cc401c4704a84328aeddc9ec ]
queue_attr_store() always freezes a device queue before calling the
attribute store operation. For attributes that control queue limits, the
store operation will also lock the queue limits with a call to
queue_limits_start_update(). However, some drivers (e.g. SCSI sd) may
need to issue commands to a device to obtain limit values from the
hardware with the queue limits locked. This creates a potential ABBA
deadlock situation if a user attempts to modify a limit (thus freezing
the device queue) while the device driver starts a revalidation of the
device queue limits.
Avoid such deadlock by not freezing the queue before calling the
->store_limit() method in struct queue_sysfs_entry and instead use the
queue_limits_commit_update_frozen helper to freeze the queue after taking
the limits lock.
This also removes taking the sysfs lock for the store_limit method as
it doesn't protect anything here, but creates even more nesting.
Hopefully it will go away from the actual sysfs methods entirely soon.
(commit log adapted from a similar patch from Damien Le Moal)
Fixes: ff956a3be95b ("block: use queue_limits_commit_update in queue_discard_max_store")
Fixes: 0327ca9d53bf ("block: use queue_limits_commit_update in queue_max_sectors_store")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Link: https://lore.kernel.org/r/20250110054726.1499538-7-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
Backport of the CVE-2025-21807 fix to 6.12.y LTS
(the fix first appeared in mainline v6.14-rc1).
This CVE was previously backported to the v6.13.12 stable branch on 2025-02-08,
but was not ported to LTS kernels.
block/blk-sysfs.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index c9e572ae0b9c..a5b6bcdfb9aa 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -684,22 +684,24 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
return res;
}
- mutex_lock(&q->sysfs_lock);
- blk_mq_freeze_queue(q);
if (entry->store_limit) {
struct queue_limits lim = queue_limits_start_update(q);
res = entry->store_limit(disk, page, length, &lim);
if (res < 0) {
queue_limits_cancel_update(q);
- } else {
- res = queue_limits_commit_update(q, &lim);
- if (!res)
- res = length;
+ return res;
}
- } else {
- res = entry->store(disk, page, length);
+
+ res = queue_limits_commit_update_frozen(q, &lim);
+ if (res)
+ return res;
+ return length;
}
+
+ mutex_lock(&q->sysfs_lock);
+ blk_mq_freeze_queue(q);
+ res = entry->store(disk, page, length);
blk_mq_unfreeze_queue(q);
mutex_unlock(&q->sysfs_lock);
return res;
--
2.47.3
^ permalink raw reply related
* [PATCH 6.12.y 1/2] block: add a store_limit operations for sysfs entries
From: Konstantin Andreev @ 2026-07-07 15:37 UTC (permalink / raw)
To: stable
Cc: linux-block, linux-kernel, Christoph Hellwig, Ming Lei,
Damien Le Moal, Martin K. Petersen, Nilay Shroff,
Johannes Thumshirn, John Garry, Jens Axboe
From: Christoph Hellwig <hch@lst.de>
[ Upstream commit a16230649ce27f8ac7dd8a5b079d9657aa96de16 ]
De-duplicate the code for updating queue limits by adding a store_limit
method that allows having common code handle the actual queue limits
update.
Note that this is a pure refactoring patch and does not address the
existing freeze vs limits lock order problem in the refactored code,
which will be addressed next.
[6.12.y-backport note: dropped "iostats_passthrough"
queue limit parameter from the upstream commit,
as this parameter does not exist in the 6.12 kernel branch]
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Link: https://lore.kernel.org/r/20250110054726.1499538-6-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Stable-dep-of: c99f66e4084a ("block: fix queue freeze vs limits lock order in sysfs store methods")
Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
The prerequisite patch to backport of the CVE-2025-21807 fix to 6.12.y LTS
(see Stable-dep-of)
The patch set applies on top of linux-6.12.y tag v6.12.95
block/blk-sysfs.c | 108 +++++++++++++++++++++++-----------------------
1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 6a38f312e385..c9e572ae0b9c 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -25,6 +25,8 @@ struct queue_sysfs_entry {
ssize_t (*show)(struct gendisk *disk, char *page);
int (*load_module)(struct gendisk *disk, const char *page, size_t count);
ssize_t (*store)(struct gendisk *disk, const char *page, size_t count);
+ int (*store_limit)(struct gendisk *disk, const char *page,
+ size_t count, struct queue_limits *lim);
};
static ssize_t
@@ -152,13 +154,11 @@ QUEUE_SYSFS_SHOW_CONST(discard_zeroes_data, 0)
QUEUE_SYSFS_SHOW_CONST(write_same_max, 0)
QUEUE_SYSFS_SHOW_CONST(poll_delay, -1)
-static ssize_t queue_max_discard_sectors_store(struct gendisk *disk,
- const char *page, size_t count)
+static int queue_max_discard_sectors_store(struct gendisk *disk,
+ const char *page, size_t count, struct queue_limits *lim)
{
unsigned long max_discard_bytes;
- struct queue_limits lim;
ssize_t ret;
- int err;
ret = queue_var_store(&max_discard_bytes, page, count);
if (ret < 0)
@@ -170,12 +170,8 @@ static ssize_t queue_max_discard_sectors_store(struct gendisk *disk,
if ((max_discard_bytes >> SECTOR_SHIFT) > UINT_MAX)
return -EINVAL;
- lim = queue_limits_start_update(disk->queue);
- lim.max_user_discard_sectors = max_discard_bytes >> SECTOR_SHIFT;
- err = queue_limits_commit_update(disk->queue, &lim);
- if (err)
- return err;
- return ret;
+ lim->max_user_discard_sectors = max_discard_bytes >> SECTOR_SHIFT;
+ return 0;
}
/*
@@ -190,30 +186,24 @@ static ssize_t queue_zone_append_max_show(struct gendisk *disk, char *page)
SECTOR_SHIFT);
}
-static ssize_t
-queue_max_sectors_store(struct gendisk *disk, const char *page, size_t count)
+static int
+queue_max_sectors_store(struct gendisk *disk, const char *page, size_t count,
+ struct queue_limits *lim)
{
unsigned long max_sectors_kb;
- struct queue_limits lim;
ssize_t ret;
- int err;
ret = queue_var_store(&max_sectors_kb, page, count);
if (ret < 0)
return ret;
- lim = queue_limits_start_update(disk->queue);
- lim.max_user_sectors = max_sectors_kb << 1;
- err = queue_limits_commit_update(disk->queue, &lim);
- if (err)
- return err;
- return ret;
+ lim->max_user_sectors = max_sectors_kb << 1;
+ return 0;
}
static ssize_t queue_feature_store(struct gendisk *disk, const char *page,
- size_t count, blk_features_t feature)
+ size_t count, struct queue_limits *lim, blk_features_t feature)
{
- struct queue_limits lim;
unsigned long val;
ssize_t ret;
@@ -221,15 +211,11 @@ static ssize_t queue_feature_store(struct gendisk *disk, const char *page,
if (ret < 0)
return ret;
- lim = queue_limits_start_update(disk->queue);
if (val)
- lim.features |= feature;
+ lim->features |= feature;
else
- lim.features &= ~feature;
- ret = queue_limits_commit_update(disk->queue, &lim);
- if (ret)
- return ret;
- return count;
+ lim->features &= ~feature;
+ return 0;
}
#define QUEUE_SYSFS_FEATURE(_name, _feature) \
@@ -238,10 +224,10 @@ static ssize_t queue_##_name##_show(struct gendisk *disk, char *page) \
return sprintf(page, "%u\n", \
!!(disk->queue->limits.features & _feature)); \
} \
-static ssize_t queue_##_name##_store(struct gendisk *disk, \
- const char *page, size_t count) \
+static int queue_##_name##_store(struct gendisk *disk, \
+ const char *page, size_t count, struct queue_limits *lim) \
{ \
- return queue_feature_store(disk, page, count, _feature); \
+ return queue_feature_store(disk, page, count, lim, _feature); \
}
QUEUE_SYSFS_FEATURE(rotational, BLK_FEAT_ROTATIONAL)
@@ -381,12 +367,10 @@ static ssize_t queue_wc_show(struct gendisk *disk, char *page)
return sprintf(page, "write through\n");
}
-static ssize_t queue_wc_store(struct gendisk *disk, const char *page,
- size_t count)
+static int queue_wc_store(struct gendisk *disk, const char *page,
+ size_t count, struct queue_limits *lim)
{
- struct queue_limits lim;
bool disable;
- int err;
if (!strncmp(page, "write back", 10)) {
disable = false;
@@ -397,15 +381,11 @@ static ssize_t queue_wc_store(struct gendisk *disk, const char *page,
return -EINVAL;
}
- lim = queue_limits_start_update(disk->queue);
if (disable)
- lim.flags |= BLK_FLAG_WRITE_CACHE_DISABLED;
+ lim->flags |= BLK_FLAG_WRITE_CACHE_DISABLED;
else
- lim.flags &= ~BLK_FLAG_WRITE_CACHE_DISABLED;
- err = queue_limits_commit_update(disk->queue, &lim);
- if (err)
- return err;
- return count;
+ lim->flags &= ~BLK_FLAG_WRITE_CACHE_DISABLED;
+ return 0;
}
#define QUEUE_RO_ENTRY(_prefix, _name) \
@@ -421,6 +401,13 @@ static struct queue_sysfs_entry _prefix##_entry = { \
.store = _prefix##_store, \
};
+#define QUEUE_LIM_RW_ENTRY(_prefix, _name) \
+static struct queue_sysfs_entry _prefix##_entry = { \
+ .attr = { .name = _name, .mode = 0644 }, \
+ .show = _prefix##_show, \
+ .store_limit = _prefix##_store, \
+}
+
#define QUEUE_RW_LOAD_MODULE_ENTRY(_prefix, _name) \
static struct queue_sysfs_entry _prefix##_entry = { \
.attr = { .name = _name, .mode = 0644 }, \
@@ -431,7 +418,7 @@ static struct queue_sysfs_entry _prefix##_entry = { \
QUEUE_RW_ENTRY(queue_requests, "nr_requests");
QUEUE_RW_ENTRY(queue_ra, "read_ahead_kb");
-QUEUE_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
+QUEUE_LIM_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
QUEUE_RO_ENTRY(queue_max_hw_sectors, "max_hw_sectors_kb");
QUEUE_RO_ENTRY(queue_max_segments, "max_segments");
QUEUE_RO_ENTRY(queue_max_integrity_segments, "max_integrity_segments");
@@ -447,7 +434,7 @@ QUEUE_RO_ENTRY(queue_io_opt, "optimal_io_size");
QUEUE_RO_ENTRY(queue_max_discard_segments, "max_discard_segments");
QUEUE_RO_ENTRY(queue_discard_granularity, "discard_granularity");
QUEUE_RO_ENTRY(queue_max_hw_discard_sectors, "discard_max_hw_bytes");
-QUEUE_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
+QUEUE_LIM_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
QUEUE_RO_ENTRY(queue_discard_zeroes_data, "discard_zeroes_data");
QUEUE_RO_ENTRY(queue_atomic_write_max_sectors, "atomic_write_max_bytes");
@@ -470,7 +457,7 @@ QUEUE_RW_ENTRY(queue_nomerges, "nomerges");
QUEUE_RW_ENTRY(queue_rq_affinity, "rq_affinity");
QUEUE_RW_ENTRY(queue_poll, "io_poll");
QUEUE_RW_ENTRY(queue_poll_delay, "io_poll_delay");
-QUEUE_RW_ENTRY(queue_wc, "write_cache");
+QUEUE_LIM_RW_ENTRY(queue_wc, "write_cache");
QUEUE_RO_ENTRY(queue_fua, "fua");
QUEUE_RO_ENTRY(queue_dax, "dax");
QUEUE_RW_ENTRY(queue_io_timeout, "io_timeout");
@@ -483,10 +470,10 @@ static struct queue_sysfs_entry queue_hw_sector_size_entry = {
.show = queue_logical_block_size_show,
};
-QUEUE_RW_ENTRY(queue_rotational, "rotational");
-QUEUE_RW_ENTRY(queue_iostats, "iostats");
-QUEUE_RW_ENTRY(queue_add_random, "add_random");
-QUEUE_RW_ENTRY(queue_stable_writes, "stable_writes");
+QUEUE_LIM_RW_ENTRY(queue_rotational, "rotational");
+QUEUE_LIM_RW_ENTRY(queue_iostats, "iostats");
+QUEUE_LIM_RW_ENTRY(queue_add_random, "add_random");
+QUEUE_LIM_RW_ENTRY(queue_stable_writes, "stable_writes");
#ifdef CONFIG_BLK_WBT
static ssize_t queue_var_store64(s64 *var, const char *page)
@@ -683,7 +670,7 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
struct request_queue *q = disk->queue;
ssize_t res;
- if (!entry->store)
+ if (!entry->store_limit && !entry->store)
return -EIO;
/*
@@ -697,11 +684,24 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
return res;
}
- blk_mq_freeze_queue(q);
mutex_lock(&q->sysfs_lock);
- res = entry->store(disk, page, length);
- mutex_unlock(&q->sysfs_lock);
+ blk_mq_freeze_queue(q);
+ if (entry->store_limit) {
+ struct queue_limits lim = queue_limits_start_update(q);
+
+ res = entry->store_limit(disk, page, length, &lim);
+ if (res < 0) {
+ queue_limits_cancel_update(q);
+ } else {
+ res = queue_limits_commit_update(q, &lim);
+ if (!res)
+ res = length;
+ }
+ } else {
+ res = entry->store(disk, page, length);
+ }
blk_mq_unfreeze_queue(q);
+ mutex_unlock(&q->sysfs_lock);
return res;
}
--
2.47.3
^ 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