[PATCH v6 0/2] Fix a recently introduced deadlock

[PATCH v6 0/2] Fix a recently introduced deadlock

[Bart Van Assche]

Hi Jens,

This patch series fixes a recently introduced deadlock triggered by modifying
request queue sysfs attributes if the dm-multipath queue_if_no_path attribute
is set. Please consider this patch series for the next merge window.

Thanks,

Bart.

Changes compared to v5:
 - Added a new patch for include/linux/backing-dev-defs.h.
 - Removed the data_race() invocations and added __data_racy annotations
   instead.

Changes compared to v4:
 - Use WRITE_ONCE() to update bdi->ra_pages.
 - Move a data_race() annotation from queue_io_timeout_store() into
   blk_queue_rq_timeout().

Changes compared to v3:
 - Added two data_race() annotations.

Changes compared to v2:
 - Dropped the controversial patch "block: Restrict the duration of sysfs
   attribute changes".

Changes compared to v1:
 - Added patch "block: Restrict the duration of sysfs attribute changes".
 - Remove queue freezing from more sysfs callbacks.

Bart Van Assche (2):
  fs: Add the __data_racy annotation to backing_dev_info.ra_pages
  block: Remove queue freezing from several sysfs store callbacks

 block/blk-sysfs.c                | 26 ++++++++------------------
 include/linux/backing-dev-defs.h |  4 +++-
 include/linux/blkdev.h           |  2 +-
 3 files changed, 12 insertions(+), 20 deletions(-)

[PATCH v6 1/2] fs: Add the __data_racy annotation to backing_dev_info.ra_pages

[Bart Van Assche]

Some but not all .ra_pages changes happen while block layer I/O is paused
with blk_mq_freeze_queue(). Filesystems may read .ra_pages even while
block layer I/O is paused, e.g. from inside their .fadvise callback.
Annotating all .ra_pages reads with READ_ONCE() would be cumbersome.
Hence, add the __data_racy annotatation to the .ra_pages member
variable.

Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/linux/backing-dev-defs.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index c5c9d89c73ed..30f4bd9ff7c8 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -168,7 +168,9 @@ struct backing_dev_info {
 	u64 id;
 	struct rb_node rb_node; /* keyed by ->id */
 	struct list_head bdi_list;
-	unsigned long ra_pages;	/* max readahead in PAGE_SIZE units */
+	/* max readahead in PAGE_SIZE units */
+	unsigned long __data_racy ra_pages;
+
 	unsigned long io_pages;	/* max allowed IO size */
 
 	struct kref refcnt;	/* Reference counter for the structure */

[PATCH v6 2/2] block: Remove queue freezing from several sysfs store callbacks

[Bart Van Assche]

Freezing the request queue from inside sysfs store callbacks may cause a
deadlock in combination with the dm-multipath driver and the
queue_if_no_path option. Additionally, freezing the request queue slows
down system boot on systems where sysfs attributes are set synchronously.

Fix this by removing the blk_mq_freeze_queue() / blk_mq_unfreeze_queue()
calls from the store callbacks that do not strictly need these callbacks.
Add the __data_racy annotation to request_queue.rq_timeout to suppress
KCSAN data race reports about the rq_timeout reads.

This patch may cause a small delay in applying the new settings.

For all the attributes affected by this patch, I/O will complete
correctly whether the old or the new value of the attribute is used.

This patch affects the following sysfs attributes:
* io_poll_delay
* io_timeout
* nomerges
* read_ahead_kb
* rq_affinity

Here is an example of a deadlock triggered by running test srp/002
if this patch is not applied:

task:multipathd
Call Trace:
 <TASK>
 __schedule+0x8c1/0x1bf0
 schedule+0xdd/0x270
 schedule_preempt_disabled+0x1c/0x30
 __mutex_lock+0xb89/0x1650
 mutex_lock_nested+0x1f/0x30
 dm_table_set_restrictions+0x823/0xdf0
 __bind+0x166/0x590
 dm_swap_table+0x2a7/0x490
 do_resume+0x1b1/0x610
 dev_suspend+0x55/0x1a0
 ctl_ioctl+0x3a5/0x7e0
 dm_ctl_ioctl+0x12/0x20
 __x64_sys_ioctl+0x127/0x1a0
 x64_sys_call+0xe2b/0x17d0
 do_syscall_64+0x96/0x3a0
 entry_SYSCALL_64_after_hwframe+0x4b/0x53
 </TASK>
task:(udev-worker)
Call Trace:
 <TASK>
 __schedule+0x8c1/0x1bf0
 schedule+0xdd/0x270
 blk_mq_freeze_queue_wait+0xf2/0x140
 blk_mq_freeze_queue_nomemsave+0x23/0x30
 queue_ra_store+0x14e/0x290
 queue_attr_store+0x23e/0x2c0
 sysfs_kf_write+0xde/0x140
 kernfs_fop_write_iter+0x3b2/0x630
 vfs_write+0x4fd/0x1390
 ksys_write+0xfd/0x230
 __x64_sys_write+0x76/0xc0
 x64_sys_call+0x276/0x17d0
 do_syscall_64+0x96/0x3a0
 entry_SYSCALL_64_after_hwframe+0x4b/0x53
 </TASK>

Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Nilay Shroff <nilay@linux.ibm.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: stable@vger.kernel.org
Fixes: af2814149883 ("block: freeze the queue in queue_attr_store")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-sysfs.c      | 26 ++++++++------------------
 include/linux/blkdev.h |  2 +-
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 76c47fe9b8d6..8684c57498cc 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -143,21 +143,22 @@ queue_ra_store(struct gendisk *disk, const char *page, size_t count)
 {
 	unsigned long ra_kb;
 	ssize_t ret;
-	unsigned int memflags;
 	struct request_queue *q = disk->queue;
 
 	ret = queue_var_store(&ra_kb, page, count);
 	if (ret < 0)
 		return ret;
 	/*
-	 * ->ra_pages is protected by ->limits_lock because it is usually
-	 * calculated from the queue limits by queue_limits_commit_update.
+	 * The ->ra_pages change below is protected by ->limits_lock because it
+	 * is usually calculated from the queue limits by
+	 * queue_limits_commit_update().
+	 *
+	 * bdi->ra_pages reads are not serialized against bdi->ra_pages writes.
+	 * Use WRITE_ONCE() to write bdi->ra_pages once.
 	 */
 	mutex_lock(&q->limits_lock);
-	memflags = blk_mq_freeze_queue(q);
-	disk->bdi->ra_pages = ra_kb >> (PAGE_SHIFT - 10);
+	WRITE_ONCE(disk->bdi->ra_pages, ra_kb >> (PAGE_SHIFT - 10));
 	mutex_unlock(&q->limits_lock);
-	blk_mq_unfreeze_queue(q, memflags);
 
 	return ret;
 }
@@ -375,21 +376,18 @@ static ssize_t queue_nomerges_store(struct gendisk *disk, const char *page,
 				    size_t count)
 {
 	unsigned long nm;
-	unsigned int memflags;
 	struct request_queue *q = disk->queue;
 	ssize_t ret = queue_var_store(&nm, page, count);
 
 	if (ret < 0)
 		return ret;
 
-	memflags = blk_mq_freeze_queue(q);
 	blk_queue_flag_clear(QUEUE_FLAG_NOMERGES, q);
 	blk_queue_flag_clear(QUEUE_FLAG_NOXMERGES, q);
 	if (nm == 2)
 		blk_queue_flag_set(QUEUE_FLAG_NOMERGES, q);
 	else if (nm)
 		blk_queue_flag_set(QUEUE_FLAG_NOXMERGES, q);
-	blk_mq_unfreeze_queue(q, memflags);
 
 	return ret;
 }
@@ -409,7 +407,6 @@ queue_rq_affinity_store(struct gendisk *disk, const char *page, size_t count)
 #ifdef CONFIG_SMP
 	struct request_queue *q = disk->queue;
 	unsigned long val;
-	unsigned int memflags;
 
 	ret = queue_var_store(&val, page, count);
 	if (ret < 0)
@@ -421,7 +418,6 @@ queue_rq_affinity_store(struct gendisk *disk, const char *page, size_t count)
 	 * are accessed individually using atomic test_bit operation. So we
 	 * don't grab any lock while updating these flags.
 	 */
-	memflags = blk_mq_freeze_queue(q);
 	if (val == 2) {
 		blk_queue_flag_set(QUEUE_FLAG_SAME_COMP, q);
 		blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, q);
@@ -432,7 +428,6 @@ queue_rq_affinity_store(struct gendisk *disk, const char *page, size_t count)
 		blk_queue_flag_clear(QUEUE_FLAG_SAME_COMP, q);
 		blk_queue_flag_clear(QUEUE_FLAG_SAME_FORCE, q);
 	}
-	blk_mq_unfreeze_queue(q, memflags);
 #endif
 	return ret;
 }
@@ -446,11 +441,9 @@ static ssize_t queue_poll_delay_store(struct gendisk *disk, const char *page,
 static ssize_t queue_poll_store(struct gendisk *disk, const char *page,
 				size_t count)
 {
-	unsigned int memflags;
 	ssize_t ret = count;
 	struct request_queue *q = disk->queue;
 
-	memflags = blk_mq_freeze_queue(q);
 	if (!(q->limits.features & BLK_FEAT_POLL)) {
 		ret = -EINVAL;
 		goto out;
@@ -459,7 +452,6 @@ static ssize_t queue_poll_store(struct gendisk *disk, const char *page,
 	pr_info_ratelimited("writes to the poll attribute are ignored.\n");
 	pr_info_ratelimited("please use driver specific parameters instead.\n");
 out:
-	blk_mq_unfreeze_queue(q, memflags);
 	return ret;
 }
 
@@ -472,7 +464,7 @@ static ssize_t queue_io_timeout_show(struct gendisk *disk, char *page)
 static ssize_t queue_io_timeout_store(struct gendisk *disk, const char *page,
 				  size_t count)
 {
-	unsigned int val, memflags;
+	unsigned int val;
 	int err;
 	struct request_queue *q = disk->queue;
 
@@ -480,9 +472,7 @@ static ssize_t queue_io_timeout_store(struct gendisk *disk, const char *page,
 	if (err || val == 0)
 		return -EINVAL;
 
-	memflags = blk_mq_freeze_queue(q);
 	blk_queue_rq_timeout(q, msecs_to_jiffies(val));
-	blk_mq_unfreeze_queue(q, memflags);
 
 	return count;
 }
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 2fff8a80dbd2..cb4ba09959ee 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -495,7 +495,7 @@ struct request_queue {
 	 */
 	unsigned long		queue_flags;
 
-	unsigned int		rq_timeout;
+	unsigned int __data_racy rq_timeout;
 
 	unsigned int		queue_depth;
 

Re: [PATCH v6 2/2] block: Remove queue freezing from several sysfs store callbacks

[Nilay Shroff]

Hi Bart,

On 11/15/25 2:34 AM, Bart Van Assche wrote:
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 2fff8a80dbd2..cb4ba09959ee 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -495,7 +495,7 @@ struct request_queue {
>  	 */
>  	unsigned long		queue_flags;
>  
> -	unsigned int		rq_timeout;
> +	unsigned int __data_racy rq_timeout;
>  
>  	unsigned int		queue_depth;

This change look good to me however as I mentioned earlier, 
introducing __data_racy would break the kernel build. So 
are you going to raise a separate bug report to fix it? 

  AS      .tmp_vmlinux2.kallsyms.o
  LD      vmlinux.unstripped
  BTFIDS  vmlinux.unstripped
WARN: multiple IDs found for 'task_struct': 116, 10183 - using 116
WARN: multiple IDs found for 'module': 190, 10190 - using 190
WARN: multiple IDs found for 'vm_area_struct': 324, 10227 - using 324
WARN: multiple IDs found for 'inode': 956, 10314 - using 956
WARN: multiple IDs found for 'path': 989, 10344 - using 989
WARN: multiple IDs found for 'file': 765, 10375 - using 765
WARN: multiple IDs found for 'cgroup': 1030, 10409 - using 1030
WARN: multiple IDs found for 'seq_file': 1358, 10593 - using 1358
WARN: multiple IDs found for 'bpf_prog': 2054, 10984 - using 2054
WARN: multiple IDs found for 'bpf_map': 2134, 11012 - using 2134
[...]
[...]
make[2]: *** [scripts/Makefile.vmlinux:72: vmlinux.unstripped] Error 255
make[2]: *** Deleting file 'vmlinux.unstripped'
make[1]: *** [/home/src/linux/Makefile:1242: vmlinux] Error 2
make: *** [Makefile:248: __sub-make] Error 2

Thanks,
--Nilay

Re: [PATCH v6 2/2] block: Remove queue freezing from several sysfs store callbacks

[Bart Van Assche]

On 11/17/25 1:01 AM, Nilay Shroff wrote:
> This change look good to me however as I mentioned earlier,
> introducing __data_racy would break the kernel build. So
> are you going to raise a separate bug report to fix it?
> 
>    AS      .tmp_vmlinux2.kallsyms.o
>    LD      vmlinux.unstripped
>    BTFIDS  vmlinux.unstripped
> WARN: multiple IDs found for 'task_struct': 116, 10183 - using 116
> WARN: multiple IDs found for 'module': 190, 10190 - using 190
> WARN: multiple IDs found for 'vm_area_struct': 324, 10227 - using 324
> WARN: multiple IDs found for 'inode': 956, 10314 - using 956
> WARN: multiple IDs found for 'path': 989, 10344 - using 989
> WARN: multiple IDs found for 'file': 765, 10375 - using 765
> WARN: multiple IDs found for 'cgroup': 1030, 10409 - using 1030
> WARN: multiple IDs found for 'seq_file': 1358, 10593 - using 1358
> WARN: multiple IDs found for 'bpf_prog': 2054, 10984 - using 2054
> WARN: multiple IDs found for 'bpf_map': 2134, 11012 - using 2134
> [...]
> [...]
> make[2]: *** [scripts/Makefile.vmlinux:72: vmlinux.unstripped] Error 255
> make[2]: *** Deleting file 'vmlinux.unstripped'
> make[1]: *** [/home/src/linux/Makefile:1242: vmlinux] Error 2
> make: *** [Makefile:248: __sub-make] Error 2

The kernel build is already broken without my patch series. Anyway, I
have reported this. In the kernel documentation I found the following:

**Please do NOT report BPF issues to bugzilla.kernel.org since it
is a guarantee that the reported issue will be overlooked.**

So I sent an email to the BPF mailing list reporting that the kernel
build fails if both CONFIG_DEBUG_INFO_BTF and CONFIG_KCSAN are enabled
for Linus' master branch (commit e7c375b18160 ("Merge tag
'vfs-6.18-rc7.fixes' of gitolite.kernel.org:pub/scm/linux/kernel/git/
vfs/vfs")). See also
https://lore.kernel.org/bpf/2412725b-916c-47bd-91c3-c2d57e3e6c7b@acm.org/.

Bart.

Re: [PATCH v6 2/2] block: Remove queue freezing from several sysfs store callbacks

[Nilay Shroff]

On 11/18/25 2:14 AM, Bart Van Assche wrote:
> On 11/17/25 1:01 AM, Nilay Shroff wrote:
>> This change look good to me however as I mentioned earlier,
>> introducing __data_racy would break the kernel build. So
>> are you going to raise a separate bug report to fix it?
>>
>>    AS      .tmp_vmlinux2.kallsyms.o
>>    LD      vmlinux.unstripped
>>    BTFIDS  vmlinux.unstripped
>> WARN: multiple IDs found for 'task_struct': 116, 10183 - using 116
>> WARN: multiple IDs found for 'module': 190, 10190 - using 190
>> WARN: multiple IDs found for 'vm_area_struct': 324, 10227 - using 324
>> WARN: multiple IDs found for 'inode': 956, 10314 - using 956
>> WARN: multiple IDs found for 'path': 989, 10344 - using 989
>> WARN: multiple IDs found for 'file': 765, 10375 - using 765
>> WARN: multiple IDs found for 'cgroup': 1030, 10409 - using 1030
>> WARN: multiple IDs found for 'seq_file': 1358, 10593 - using 1358
>> WARN: multiple IDs found for 'bpf_prog': 2054, 10984 - using 2054
>> WARN: multiple IDs found for 'bpf_map': 2134, 11012 - using 2134
>> [...]
>> [...]
>> make[2]: *** [scripts/Makefile.vmlinux:72: vmlinux.unstripped] Error 255
>> make[2]: *** Deleting file 'vmlinux.unstripped'
>> make[1]: *** [/home/src/linux/Makefile:1242: vmlinux] Error 2
>> make: *** [Makefile:248: __sub-make] Error 2
> 
> The kernel build is already broken without my patch series. Anyway, I
> have reported this. In the kernel documentation I found the following:
> 
> **Please do NOT report BPF issues to bugzilla.kernel.org since it
> is a guarantee that the reported issue will be overlooked.**
> 
> So I sent an email to the BPF mailing list reporting that the kernel
> build fails if both CONFIG_DEBUG_INFO_BTF and CONFIG_KCSAN are enabled
> for Linus' master branch (commit e7c375b18160 ("Merge tag
> 'vfs-6.18-rc7.fixes' of gitolite.kernel.org:pub/scm/linux/kernel/git/
> vfs/vfs")). See also
> https://lore.kernel.org/bpf/2412725b-916c-47bd-91c3-c2d57e3e6c7b@acm.org/.
> 
Okay sounds good. Though I couldn't recreate this without __data_racy
on my platform. Anyways, I will review your patchset.

Thanks,
--Nilay

Re: [PATCH v6 1/2] fs: Add the __data_racy annotation to backing_dev_info.ra_pages

[Nilay Shroff]

On 11/15/25 2:34 AM, Bart Van Assche wrote:
> Some but not all .ra_pages changes happen while block layer I/O is paused
> with blk_mq_freeze_queue(). Filesystems may read .ra_pages even while
> block layer I/O is paused, e.g. from inside their .fadvise callback.
> Annotating all .ra_pages reads with READ_ONCE() would be cumbersome.
> Hence, add the __data_racy annotatation to the .ra_pages member
> variable.
> 
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Nilay Shroff <nilay@linux.ibm.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH v6 2/2] block: Remove queue freezing from several sysfs store callbacks

[Nilay Shroff]

On 11/15/25 2:34 AM, Bart Van Assche wrote:
> Freezing the request queue from inside sysfs store callbacks may cause a
> deadlock in combination with the dm-multipath driver and the
> queue_if_no_path option. Additionally, freezing the request queue slows
> down system boot on systems where sysfs attributes are set synchronously.
> 
> Fix this by removing the blk_mq_freeze_queue() / blk_mq_unfreeze_queue()
> calls from the store callbacks that do not strictly need these callbacks.
> Add the __data_racy annotation to request_queue.rq_timeout to suppress
> KCSAN data race reports about the rq_timeout reads.
> 
> This patch may cause a small delay in applying the new settings.
> 
> For all the attributes affected by this patch, I/O will complete
> correctly whether the old or the new value of the attribute is used.
> 
> This patch affects the following sysfs attributes:
> * io_poll_delay
> * io_timeout
> * nomerges
> * read_ahead_kb
> * rq_affinity

Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH v6 0/2] Fix a recently introduced deadlock

[Jens Axboe]

On Fri, 14 Nov 2025 13:04:05 -0800, Bart Van Assche wrote:
> This patch series fixes a recently introduced deadlock triggered by modifying
> request queue sysfs attributes if the dm-multipath queue_if_no_path attribute
> is set. Please consider this patch series for the next merge window.
> 
> Thanks,
> 
> Bart.
> 
> [...]

Applied, thanks!

[1/2] fs: Add the __data_racy annotation to backing_dev_info.ra_pages
      commit: 335a0927a9ccc2694b8d6f61b656597af2784df5
[2/2] block: Remove queue freezing from several sysfs store callbacks
      commit: 77148836a0876c9573555c966104808de33e1332

Best regards,
-- 
Jens Axboe