Linux block layer
 help / color / mirror / Atom feed
* [PATCH] block: assign caller-specific lockdep class to disk->open_mutex
@ 2026-05-30 13:45 Tetsuo Handa
  2026-05-30 21:15 ` Bart Van Assche
  2026-05-30 22:50 ` [PATCH] " Hillf Danton
  0 siblings, 2 replies; 13+ messages in thread
From: Tetsuo Handa @ 2026-05-30 13:45 UTC (permalink / raw)
  To: Jens Axboe, linux-block, LKML
  Cc: Bart Van Assche, Andrew Morton, Ming Lei, Damien Le Moal,
	Christoph Hellwig, Qu Wenruo, Hillf Danton

The block core currently allocates a single monolithic lockdep key for
disk->open_mutex across all callers. This single key conflates locking
hierarchies between independent block streams. For example, if a stacked
driver like loop flushes its internal workqueues inside lo_release() while
holding its own open_mutex, lockdep views this as a potential ABBA deadlock
against the underlying storage stack, leading to numerous circular
dependency splats [2][3][4][5][6].

To reduce false-positives structurally, this patch splits the global
monolithic lock class into distinct, per-caller during disk allocation;
by changing "lock_class_key" into a 2-element array:
  - lkclass[0]: Used for the legacy "(bio completion)" map.
  - lkclass[1]: Assigned to target caller's disk->open_mutex.

This patch was tested by adding drain_workqueue() to __loop_clr_fd() during
testing of a patch for [1], and actually helped stopping [2][4][6].
Even if our final solution for [1] does not call drain_workqueue() with
disk->open_mutex held, keeping locking chains simpler and shorter should
be a good change.

Link: https://syzkaller.appspot.com/bug?extid=cd8a9a308e879a4e2c28 [1]
Link: https://syzkaller.appspot.com/bug?extid=2f62807dc3239b8f584e [2]
Link: https://syzkaller.appspot.com/bug?extid=c4e9d077bcc86bee08dc [3]
Link: https://syzkaller.appspot.com/bug?extid=0f427123ae84b3ba6dc7 [4]
Link: https://syzkaller.appspot.com/bug?extid=4feabfc9641267769c97 [5]
Link: https://syzkaller.appspot.com/bug?extid=fb0ff9bfe34ad282ebd4 [6]
Suggested-by: AI Mode in Google Search (no mail address)
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 block/blk-mq.c         | 4 ++--
 block/blk.h            | 2 +-
 block/genhd.c          | 8 ++++----
 drivers/scsi/sd.c      | 4 ++--
 drivers/scsi/sr.c      | 4 ++--
 include/linux/blk-mq.h | 8 ++++----
 include/linux/blkdev.h | 6 +++---
 7 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 28c2d931e75e..01a15ac40754 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4492,7 +4492,7 @@ EXPORT_SYMBOL(blk_mq_destroy_queue);
 
 struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata,
-		struct lock_class_key *lkclass)
+		struct lock_class_key lkclass[2])
 {
 	struct request_queue *q;
 	struct gendisk *disk;
@@ -4513,7 +4513,7 @@ struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 EXPORT_SYMBOL(__blk_mq_alloc_disk);
 
 struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
-		struct lock_class_key *lkclass)
+		struct lock_class_key lkclass[2])
 {
 	struct gendisk *disk;
 
diff --git a/block/blk.h b/block/blk.h
index b998a7761faf..1744748f9b68 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -614,7 +614,7 @@ void drop_partition(struct block_device *part);
 void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors);
 
 struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
-		struct lock_class_key *lkclass);
+		struct lock_class_key lkclass[2]);
 struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id);
 
 int disk_scan_partitions(struct gendisk *disk, blk_mode_t mode);
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e9..303bd5e619e7 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1444,7 +1444,7 @@ dev_t part_devt(struct gendisk *disk, u8 partno)
 }
 
 struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
-		struct lock_class_key *lkclass)
+		struct lock_class_key lkclass[2])
 {
 	struct gendisk *disk;
 
@@ -1467,7 +1467,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 		goto out_free_bdi;
 
 	disk->node_id = node_id;
-	mutex_init(&disk->open_mutex);
+	mutex_init_with_key(&disk->open_mutex, &lkclass[1]);
 	xa_init(&disk->part_tbl);
 	if (xa_insert(&disk->part_tbl, 0, disk->part0, GFP_KERNEL))
 		goto out_destroy_part_tbl;
@@ -1482,7 +1482,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 	device_initialize(disk_to_dev(disk));
 	inc_diskseq(disk);
 	q->disk = disk;
-	lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
+	lockdep_init_map(&disk->lockdep_map, "(bio completion)", &lkclass[0], 0);
 #ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 	INIT_LIST_HEAD(&disk->slave_bdevs);
 #endif
@@ -1506,7 +1506,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 }
 
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
-		struct lock_class_key *lkclass)
+		struct lock_class_key lkclass[2])
 {
 	struct queue_limits default_lim = { };
 	struct request_queue *q;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 599e75f33334..d8a1bbd4f19e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -112,7 +112,7 @@ static DEFINE_MUTEX(sd_mutex_lock);
 static mempool_t *sd_page_pool;
 static mempool_t *sd_large_page_pool;
 static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
-static struct lock_class_key sd_bio_compl_lkclass;
+static struct lock_class_key sd_bio_compl_lkclass[2];
 
 static const char *sd_cache_types[] = {
 	"write through", "none", "write back",
@@ -4021,7 +4021,7 @@ static int sd_probe(struct scsi_device *sdp)
 		goto out;
 
 	gd = blk_mq_alloc_disk_for_queue(sdp->request_queue,
-					 &sd_bio_compl_lkclass);
+					 sd_bio_compl_lkclass);
 	if (!gd)
 		goto out_free;
 
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index c36c54ecd354..421b8bd37db0 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -106,7 +106,7 @@ static struct scsi_driver sr_template = {
 static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG];
 static DEFINE_SPINLOCK(sr_index_lock);
 
-static struct lock_class_key sr_bio_compl_lkclass;
+static struct lock_class_key sr_bio_compl_lkclass[2];
 
 static int sr_open(struct cdrom_device_info *, int);
 static void sr_release(struct cdrom_device_info *);
@@ -634,7 +634,7 @@ static int sr_probe(struct scsi_device *sdev)
 		goto fail;
 
 	disk = blk_mq_alloc_disk_for_queue(sdev->request_queue,
-					   &sr_bio_compl_lkclass);
+					   sr_bio_compl_lkclass);
 	if (!disk)
 		goto fail_free;
 	mutex_init(&cd->lock);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..57d805c78827 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -726,15 +726,15 @@ enum {
 
 struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata,
-		struct lock_class_key *lkclass);
+		struct lock_class_key lkclass[2]);
 #define blk_mq_alloc_disk(set, lim, queuedata)				\
 ({									\
-	static struct lock_class_key __key;				\
+	static struct lock_class_key __key[2];				\
 									\
-	__blk_mq_alloc_disk(set, lim, queuedata, &__key);		\
+	__blk_mq_alloc_disk(set, lim, queuedata, __key);		\
 })
 struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
-		struct lock_class_key *lkclass);
+		struct lock_class_key lkclass[2]);
 struct request_queue *blk_mq_alloc_queue(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata);
 int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 890128cdea1c..3cd2056cde28 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -974,7 +974,7 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate);
 
 void put_disk(struct gendisk *disk);
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
-		struct lock_class_key *lkclass);
+		struct lock_class_key lkclass[2]);
 
 /**
  * blk_alloc_disk - allocate a gendisk structure
@@ -990,9 +990,9 @@ struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
  */
 #define blk_alloc_disk(lim, node_id)					\
 ({									\
-	static struct lock_class_key __key;				\
+	static struct lock_class_key __key[2];				\
 									\
-	__blk_alloc_disk(lim, node_id, &__key);				\
+	__blk_alloc_disk(lim, node_id, __key);				\
 })
 
 int __register_blkdev(unsigned int major, const char *name,
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] block: assign caller-specific lockdep class to disk->open_mutex
  2026-05-30 13:45 [PATCH] block: assign caller-specific lockdep class to disk->open_mutex Tetsuo Handa
@ 2026-05-30 21:15 ` Bart Van Assche
  2026-06-01  7:11   ` Christoph Hellwig
  2026-05-30 22:50 ` [PATCH] " Hillf Danton
  1 sibling, 1 reply; 13+ messages in thread
From: Bart Van Assche @ 2026-05-30 21:15 UTC (permalink / raw)
  To: Tetsuo Handa, Jens Axboe, linux-block, LKML
  Cc: Andrew Morton, Ming Lei, Damien Le Moal, Christoph Hellwig,
	Qu Wenruo, Hillf Danton

On 5/30/26 6:45 AM, Tetsuo Handa wrote:
> -	static struct lock_class_key __key;				\
> +	static struct lock_class_key __key[2];				\
The two elements of this array have different roles. From the point of
view of code readability and maintainability it's probably much better
to make this a struct with two named members rather than a two-element
array.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] block: assign caller-specific lockdep class to disk->open_mutex
  2026-05-30 13:45 [PATCH] block: assign caller-specific lockdep class to disk->open_mutex Tetsuo Handa
  2026-05-30 21:15 ` Bart Van Assche
@ 2026-05-30 22:50 ` Hillf Danton
  1 sibling, 0 replies; 13+ messages in thread
From: Hillf Danton @ 2026-05-30 22:50 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Jens Axboe, linux-block, LKML, Bart Van Assche, Boqun Feng,
	Andrew Morton, Ming Lei, Damien Le Moal, Christoph Hellwig,
	Qu Wenruo, Hillf Danton

On Sat, 30 May 2026 22:45:55 +0900 Tetsuo Handa wrote:
> The block core currently allocates a single monolithic lockdep key for
> disk->open_mutex across all callers. This single key conflates locking
> hierarchies between independent block streams. For example, if a stacked
> driver like loop flushes its internal workqueues inside lo_release() while
> holding its own open_mutex, lockdep views this as a potential ABBA deadlock
> against the underlying storage stack, leading to numerous circular
> dependency splats [2][3][4][5][6].
> 
> To reduce false-positives structurally, this patch splits the global
> monolithic lock class into distinct, per-caller during disk allocation;
> by changing "lock_class_key" into a 2-element array:
>   - lkclass[0]: Used for the legacy "(bio completion)" map.
>   - lkclass[1]: Assigned to target caller's disk->open_mutex.
> 
I wonder how this works given e966eaeeb623 ("locking/lockdep: Remove the
cross-release locking checks").

> This patch was tested by adding drain_workqueue() to __loop_clr_fd() during
> testing of a patch for [1], and actually helped stopping [2][4][6].
> Even if our final solution for [1] does not call drain_workqueue() with
> disk->open_mutex held, keeping locking chains simpler and shorter should
> be a good change.
> 
> Link: https://syzkaller.appspot.com/bug?extid=cd8a9a308e879a4e2c28 [1]
> Link: https://syzkaller.appspot.com/bug?extid=2f62807dc3239b8f584e [2]
> Link: https://syzkaller.appspot.com/bug?extid=c4e9d077bcc86bee08dc [3]
> Link: https://syzkaller.appspot.com/bug?extid=0f427123ae84b3ba6dc7 [4]
> Link: https://syzkaller.appspot.com/bug?extid=4feabfc9641267769c97 [5]
> Link: https://syzkaller.appspot.com/bug?extid=fb0ff9bfe34ad282ebd4 [6]
> Suggested-by: AI Mode in Google Search (no mail address)
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  block/blk-mq.c         | 4 ++--
>  block/blk.h            | 2 +-
>  block/genhd.c          | 8 ++++----
>  drivers/scsi/sd.c      | 4 ++--
>  drivers/scsi/sr.c      | 4 ++--
>  include/linux/blk-mq.h | 8 ++++----
>  include/linux/blkdev.h | 6 +++---
>  7 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 28c2d931e75e..01a15ac40754 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4492,7 +4492,7 @@ EXPORT_SYMBOL(blk_mq_destroy_queue);
>  
>  struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
>  		struct queue_limits *lim, void *queuedata,
> -		struct lock_class_key *lkclass)
> +		struct lock_class_key lkclass[2])
>  {
>  	struct request_queue *q;
>  	struct gendisk *disk;
> @@ -4513,7 +4513,7 @@ struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
>  EXPORT_SYMBOL(__blk_mq_alloc_disk);
>  
>  struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
> -		struct lock_class_key *lkclass)
> +		struct lock_class_key lkclass[2])
>  {
>  	struct gendisk *disk;
>  
> diff --git a/block/blk.h b/block/blk.h
> index b998a7761faf..1744748f9b68 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -614,7 +614,7 @@ void drop_partition(struct block_device *part);
>  void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors);
>  
>  struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
> -		struct lock_class_key *lkclass);
> +		struct lock_class_key lkclass[2]);
>  struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id);
>  
>  int disk_scan_partitions(struct gendisk *disk, blk_mode_t mode);
> diff --git a/block/genhd.c b/block/genhd.c
> index 7d6854fd28e9..303bd5e619e7 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1444,7 +1444,7 @@ dev_t part_devt(struct gendisk *disk, u8 partno)
>  }
>  
>  struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
> -		struct lock_class_key *lkclass)
> +		struct lock_class_key lkclass[2])
>  {
>  	struct gendisk *disk;
>  
> @@ -1467,7 +1467,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
>  		goto out_free_bdi;
>  
>  	disk->node_id = node_id;
> -	mutex_init(&disk->open_mutex);
> +	mutex_init_with_key(&disk->open_mutex, &lkclass[1]);
>  	xa_init(&disk->part_tbl);
>  	if (xa_insert(&disk->part_tbl, 0, disk->part0, GFP_KERNEL))
>  		goto out_destroy_part_tbl;
> @@ -1482,7 +1482,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
>  	device_initialize(disk_to_dev(disk));
>  	inc_diskseq(disk);
>  	q->disk = disk;
> -	lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
> +	lockdep_init_map(&disk->lockdep_map, "(bio completion)", &lkclass[0], 0);
>  #ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
>  	INIT_LIST_HEAD(&disk->slave_bdevs);
>  #endif
> @@ -1506,7 +1506,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
>  }
>  
>  struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
> -		struct lock_class_key *lkclass)
> +		struct lock_class_key lkclass[2])
>  {
>  	struct queue_limits default_lim = { };
>  	struct request_queue *q;
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 599e75f33334..d8a1bbd4f19e 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -112,7 +112,7 @@ static DEFINE_MUTEX(sd_mutex_lock);
>  static mempool_t *sd_page_pool;
>  static mempool_t *sd_large_page_pool;
>  static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
> -static struct lock_class_key sd_bio_compl_lkclass;
> +static struct lock_class_key sd_bio_compl_lkclass[2];
>  
>  static const char *sd_cache_types[] = {
>  	"write through", "none", "write back",
> @@ -4021,7 +4021,7 @@ static int sd_probe(struct scsi_device *sdp)
>  		goto out;
>  
>  	gd = blk_mq_alloc_disk_for_queue(sdp->request_queue,
> -					 &sd_bio_compl_lkclass);
> +					 sd_bio_compl_lkclass);
>  	if (!gd)
>  		goto out_free;
>  
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index c36c54ecd354..421b8bd37db0 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -106,7 +106,7 @@ static struct scsi_driver sr_template = {
>  static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG];
>  static DEFINE_SPINLOCK(sr_index_lock);
>  
> -static struct lock_class_key sr_bio_compl_lkclass;
> +static struct lock_class_key sr_bio_compl_lkclass[2];
>  
>  static int sr_open(struct cdrom_device_info *, int);
>  static void sr_release(struct cdrom_device_info *);
> @@ -634,7 +634,7 @@ static int sr_probe(struct scsi_device *sdev)
>  		goto fail;
>  
>  	disk = blk_mq_alloc_disk_for_queue(sdev->request_queue,
> -					   &sr_bio_compl_lkclass);
> +					   sr_bio_compl_lkclass);
>  	if (!disk)
>  		goto fail_free;
>  	mutex_init(&cd->lock);
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 18a2388ba581..57d805c78827 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -726,15 +726,15 @@ enum {
>  
>  struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
>  		struct queue_limits *lim, void *queuedata,
> -		struct lock_class_key *lkclass);
> +		struct lock_class_key lkclass[2]);
>  #define blk_mq_alloc_disk(set, lim, queuedata)				\
>  ({									\
> -	static struct lock_class_key __key;				\
> +	static struct lock_class_key __key[2];				\
>  									\
> -	__blk_mq_alloc_disk(set, lim, queuedata, &__key);		\
> +	__blk_mq_alloc_disk(set, lim, queuedata, __key);		\
>  })
>  struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
> -		struct lock_class_key *lkclass);
> +		struct lock_class_key lkclass[2]);
>  struct request_queue *blk_mq_alloc_queue(struct blk_mq_tag_set *set,
>  		struct queue_limits *lim, void *queuedata);
>  int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 890128cdea1c..3cd2056cde28 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -974,7 +974,7 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate);
>  
>  void put_disk(struct gendisk *disk);
>  struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
> -		struct lock_class_key *lkclass);
> +		struct lock_class_key lkclass[2]);
>  
>  /**
>   * blk_alloc_disk - allocate a gendisk structure
> @@ -990,9 +990,9 @@ struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
>   */
>  #define blk_alloc_disk(lim, node_id)					\
>  ({									\
> -	static struct lock_class_key __key;				\
> +	static struct lock_class_key __key[2];				\
>  									\
> -	__blk_alloc_disk(lim, node_id, &__key);				\
> +	__blk_alloc_disk(lim, node_id, __key);				\
>  })
>  
>  int __register_blkdev(unsigned int major, const char *name,
> -- 
> 2.47.3
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] block: assign caller-specific lockdep class to disk->open_mutex
  2026-05-30 21:15 ` Bart Van Assche
@ 2026-06-01  7:11   ` Christoph Hellwig
  2026-06-03  6:25     ` [PATCH v2] " Tetsuo Handa
  0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2026-06-01  7:11 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Tetsuo Handa, Jens Axboe, linux-block, LKML, Andrew Morton,
	Ming Lei, Damien Le Moal, Christoph Hellwig, Qu Wenruo,
	Hillf Danton

On Sat, May 30, 2026 at 02:15:04PM -0700, Bart Van Assche wrote:
> On 5/30/26 6:45 AM, Tetsuo Handa wrote:
>> -	static struct lock_class_key __key;				\
>> +	static struct lock_class_key __key[2];				\
> The two elements of this array have different roles. From the point of
> view of code readability and maintainability it's probably much better
> to make this a struct with two named members rather than a two-element
> array.

Exactly.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-01  7:11   ` Christoph Hellwig
@ 2026-06-03  6:25     ` Tetsuo Handa
  2026-06-03 11:54       ` [PATCH v3] " Tetsuo Handa
  0 siblings, 1 reply; 13+ messages in thread
From: Tetsuo Handa @ 2026-06-03  6:25 UTC (permalink / raw)
  To: Christoph Hellwig, Bart Van Assche, Jens Axboe
  Cc: linux-block, LKML, Andrew Morton, Ming Lei, Damien Le Moal,
	Qu Wenruo, Hillf Danton, Miguel Ojeda

The block core currently allocates a single monolithic lockdep key for
disk->open_mutex across all callers. This single key conflates locking
hierarchies between independent block streams. For example, if a stacked
driver like loop flushes its internal workqueues inside lo_release() while
holding its own open_mutex, lockdep views this as a potential ABBA deadlock
against the underlying storage stack, leading to numerous circular
dependency splats.

To structurally reduce false positives, this patch splits the global
monolithic lock class into distinct, per-caller instances during disk
allocation. This is done by replacing "struct lock_class_key" with
"struct gendisk_lkclass", which contains two instances of
"struct lock_class_key" for the legacy "(bio completion)" map and
disk->open_mutex respectively.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Changes in v2:
- Replaced a two-element array with a struct with two named members, suggested by Bart Van Assche
  ( https://lore.kernel.org/all/4cf7ecc7-932c-4589-9d0f-3e025e83e27c@acm.org/ ).
- Added changes needed by Rust block layer bindings and rnull module, pointed out by sashiko
  ( https://sashiko.dev/#/patchset/147ed056-03d9-4214-b925-0f10fc00cf27%40I-love.SAKURA.ne.jp ).

Testing result of v1:
- I kept v1 patch in linux-next for several more days, but result was that
  some of circular dependency splats which I thought this change succeeded to
  eliminate are still getting reported. That is, we need to determine whether
  we should make this change without example syzbot reports that demonstrates
  difference. But in general, keeping locking chains simpler and shorter
  should be a good change.

Acknowledgment:
  Since I have no experience with Rust, changes needed by Rust block layer
  bindings and rnull module are made based on conversation with the Gemini
  AI collaborator.

 block/blk-mq.c                   |  4 ++--
 block/blk.h                      |  2 +-
 block/genhd.c                    |  8 ++++----
 drivers/block/rnull/rnull.rs     |  2 +-
 drivers/scsi/sd.c                |  2 +-
 drivers/scsi/sr.c                |  2 +-
 include/linux/blk-mq.h           |  6 +++---
 include/linux/blkdev.h           |  9 +++++++--
 rust/kernel/block/mq/gen_disk.rs | 17 +++++++++++++++--
 9 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a24175441380..5203e8cc6a28 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4492,7 +4492,7 @@ EXPORT_SYMBOL(blk_mq_destroy_queue);
 
 struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct request_queue *q;
 	struct gendisk *disk;
@@ -4513,7 +4513,7 @@ struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 EXPORT_SYMBOL(__blk_mq_alloc_disk);
 
 struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct gendisk *disk;
 
diff --git a/block/blk.h b/block/blk.h
index b998a7761faf..611bcd655357 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -614,7 +614,7 @@ void drop_partition(struct block_device *part);
 void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors);
 
 struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id);
 
 int disk_scan_partitions(struct gendisk *disk, blk_mode_t mode);
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e9..8f4a3d8ca15e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1444,7 +1444,7 @@ dev_t part_devt(struct gendisk *disk, u8 partno)
 }
 
 struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct gendisk *disk;
 
@@ -1467,7 +1467,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 		goto out_free_bdi;
 
 	disk->node_id = node_id;
-	mutex_init(&disk->open_mutex);
+	mutex_init_with_key(&disk->open_mutex, &lkclass->open_mutex_lkclass);
 	xa_init(&disk->part_tbl);
 	if (xa_insert(&disk->part_tbl, 0, disk->part0, GFP_KERNEL))
 		goto out_destroy_part_tbl;
@@ -1482,7 +1482,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 	device_initialize(disk_to_dev(disk));
 	inc_diskseq(disk);
 	q->disk = disk;
-	lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
+	lockdep_init_map(&disk->lockdep_map, "(bio completion)", &lkclass->bio_lkclass, 0);
 #ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 	INIT_LIST_HEAD(&disk->slave_bdevs);
 #endif
@@ -1506,7 +1506,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 }
 
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct queue_limits default_lim = { };
 	struct request_queue *q;
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 0ca8715febe8..476a8910c432 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -61,7 +61,7 @@ fn new(
             .logical_block_size(block_size)?
             .physical_block_size(block_size)?
             .rotational(rotational)
-            .build(fmt!("{}", name.to_str()?), tagset, queue_data)
+            .build(fmt!("{}", name.to_str()?), tagset, queue_data, kernel::my_gendisk_lkclass!())
     }
 }
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 599e75f33334..63fe8c86606a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -112,7 +112,7 @@ static DEFINE_MUTEX(sd_mutex_lock);
 static mempool_t *sd_page_pool;
 static mempool_t *sd_large_page_pool;
 static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
-static struct lock_class_key sd_bio_compl_lkclass;
+static struct gendisk_lkclass sd_bio_compl_lkclass;
 
 static const char *sd_cache_types[] = {
 	"write through", "none", "write back",
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index c36c54ecd354..734567ae0e43 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -106,7 +106,7 @@ static struct scsi_driver sr_template = {
 static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG];
 static DEFINE_SPINLOCK(sr_index_lock);
 
-static struct lock_class_key sr_bio_compl_lkclass;
+static struct gendisk_lkclass sr_bio_compl_lkclass;
 
 static int sr_open(struct cdrom_device_info *, int);
 static void sr_release(struct cdrom_device_info *);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..5aa17e82c3ba 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -726,15 +726,15 @@ enum {
 
 struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 #define blk_mq_alloc_disk(set, lim, queuedata)				\
 ({									\
-	static struct lock_class_key __key;				\
+	static struct gendisk_lkclass __key;				\
 									\
 	__blk_mq_alloc_disk(set, lim, queuedata, &__key);		\
 })
 struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 struct request_queue *blk_mq_alloc_queue(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata);
 int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 890128cdea1c..28b0aee6b3ba 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -49,6 +49,11 @@ extern const struct device_type disk_type;
 extern const struct device_type part_type;
 extern const struct class block_class;
 
+struct gendisk_lkclass {
+	struct lock_class_key bio_lkclass;
+	struct lock_class_key open_mutex_lkclass;
+};
+
 /*
  * Maximum number of blkcg policies allowed to be registered concurrently.
  * Defined here to simplify include dependency.
@@ -974,7 +979,7 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate);
 
 void put_disk(struct gendisk *disk);
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 
 /**
  * blk_alloc_disk - allocate a gendisk structure
@@ -990,7 +995,7 @@ struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
  */
 #define blk_alloc_disk(lim, node_id)					\
 ({									\
-	static struct lock_class_key __key;				\
+	static struct gendisk_lkclass __key;				\
 									\
 	__blk_alloc_disk(lim, node_id, &__key);				\
 })
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 912cb805caf5..b4efe4b1b5f6 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -11,7 +11,6 @@
     error::{self, from_err_ptr, Result},
     fmt::{self, Write},
     prelude::*,
-    static_lock_class,
     str::NullTerminatedFormatter,
     sync::Arc,
     types::{ForeignOwnable, ScopeGuard},
@@ -38,6 +37,19 @@ fn default() -> Self {
     }
 }
 
+/// Helper macro to generate a unique caller-local static lock class struct
+#[macro_export]
+macro_rules! my_gendisk_lkclass {
+    () => {{
+        static mut LKCLASS: $crate::bindings::gendisk_lkclass = $crate::bindings::gendisk_lkclass {
+            bio_lkclass: const { unsafe { ::core::mem::zeroed() } },
+            open_mutex_lkclass: const { unsafe { ::core::mem::zeroed() } },
+        };
+
+        unsafe { &raw mut LKCLASS }
+    }};
+}
+
 impl GenDiskBuilder {
     /// Create a new instance.
     pub fn new() -> Self {
@@ -100,6 +112,7 @@ pub fn build<T: Operations>(
         name: fmt::Arguments<'_>,
         tagset: Arc<TagSet<T>>,
         queue_data: T::QueueData,
+        lkclass: *mut bindings::gendisk_lkclass,
     ) -> Result<GenDisk<T>> {
         let data = queue_data.into_foreign();
         let recover_data = ScopeGuard::new(|| {
@@ -121,7 +134,7 @@ pub fn build<T: Operations>(
                 tagset.raw_tag_set(),
                 &mut lim,
                 data,
-                static_lock_class!().as_ptr(),
+                lkclass,
             )
         })?;
 
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-03  6:25     ` [PATCH v2] " Tetsuo Handa
@ 2026-06-03 11:54       ` Tetsuo Handa
  2026-06-04 21:07         ` Miguel Ojeda
  2026-06-05  7:54         ` Andreas Hindborg
  0 siblings, 2 replies; 13+ messages in thread
From: Tetsuo Handa @ 2026-06-03 11:54 UTC (permalink / raw)
  To: Christoph Hellwig, Bart Van Assche, Jens Axboe
  Cc: linux-block, LKML, Andrew Morton, Ming Lei, Damien Le Moal,
	Qu Wenruo, Hillf Danton, Miguel Ojeda

The block core currently allocates a single monolithic lockdep key for
disk->open_mutex across all callers. This single key conflates locking
hierarchies between independent block streams. For example, if a stacked
driver like loop flushes its internal workqueues inside lo_release() while
holding its own open_mutex, lockdep views this as a potential ABBA deadlock
against the underlying storage stack, leading to numerous circular
dependency splats.

To structurally reduce false positives, this patch splits the global
monolithic lock class into distinct, per-caller instances during disk
allocation. This is done by replacing "struct lock_class_key" with
"struct gendisk_lkclass", which contains two instances of
"struct lock_class_key" for the legacy "(bio completion)" map and
disk->open_mutex respectively.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Changes in v3:
- Adjusted Rust part for safe pointer passing, pointed out by sashiko
  ( https://sashiko.dev/#/patchset/420f723a-8168-4f56-b84a-2a36ecd87fea%40I-love.SAKURA.ne.jp ) .

Changes in v2:
- Replaced a two-element array with a struct with two named members, suggested by Bart Van Assche
  ( https://lore.kernel.org/all/4cf7ecc7-932c-4589-9d0f-3e025e83e27c@acm.org/ ).
- Added changes needed by Rust block layer bindings and rnull module, pointed out by sashiko
  ( https://sashiko.dev/#/patchset/147ed056-03d9-4214-b925-0f10fc00cf27%40I-love.SAKURA.ne.jp ).

Testing result of v1:
- I kept v1 patch in linux-next for several more days, but result was that
  some of circular dependency splats which I thought this change succeeded to
  eliminate are still getting reported. That is, we need to determine whether
  we should make this change without example syzbot reports that demonstrates
  difference. But in general, keeping locking chains simpler and shorter
  should be a good change.

Acknowledgment:
  Since I have no experience with Rust, changes needed by Rust block layer
  bindings and rnull module are made based on conversation with the Gemini
  AI collaborator.

 block/blk-mq.c                   |  4 ++--
 block/blk.h                      |  2 +-
 block/genhd.c                    |  8 +++----
 drivers/block/rnull/rnull.rs     |  2 +-
 drivers/scsi/sd.c                |  2 +-
 drivers/scsi/sr.c                |  2 +-
 include/linux/blk-mq.h           |  6 ++---
 include/linux/blkdev.h           |  9 +++++--
 rust/kernel/block/mq.rs          |  2 +-
 rust/kernel/block/mq/gen_disk.rs | 41 ++++++++++++++++++++++++++++++--
 10 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a24175441380..5203e8cc6a28 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4492,7 +4492,7 @@ EXPORT_SYMBOL(blk_mq_destroy_queue);
 
 struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct request_queue *q;
 	struct gendisk *disk;
@@ -4513,7 +4513,7 @@ struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 EXPORT_SYMBOL(__blk_mq_alloc_disk);
 
 struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct gendisk *disk;
 
diff --git a/block/blk.h b/block/blk.h
index b998a7761faf..611bcd655357 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -614,7 +614,7 @@ void drop_partition(struct block_device *part);
 void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors);
 
 struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id);
 
 int disk_scan_partitions(struct gendisk *disk, blk_mode_t mode);
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e9..8f4a3d8ca15e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1444,7 +1444,7 @@ dev_t part_devt(struct gendisk *disk, u8 partno)
 }
 
 struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct gendisk *disk;
 
@@ -1467,7 +1467,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 		goto out_free_bdi;
 
 	disk->node_id = node_id;
-	mutex_init(&disk->open_mutex);
+	mutex_init_with_key(&disk->open_mutex, &lkclass->open_mutex_lkclass);
 	xa_init(&disk->part_tbl);
 	if (xa_insert(&disk->part_tbl, 0, disk->part0, GFP_KERNEL))
 		goto out_destroy_part_tbl;
@@ -1482,7 +1482,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 	device_initialize(disk_to_dev(disk));
 	inc_diskseq(disk);
 	q->disk = disk;
-	lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
+	lockdep_init_map(&disk->lockdep_map, "(bio completion)", &lkclass->bio_lkclass, 0);
 #ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 	INIT_LIST_HEAD(&disk->slave_bdevs);
 #endif
@@ -1506,7 +1506,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 }
 
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
-		struct lock_class_key *lkclass)
+		struct gendisk_lkclass *lkclass)
 {
 	struct queue_limits default_lim = { };
 	struct request_queue *q;
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 0ca8715febe8..476a8910c432 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -61,7 +61,7 @@ fn new(
             .logical_block_size(block_size)?
             .physical_block_size(block_size)?
             .rotational(rotational)
-            .build(fmt!("{}", name.to_str()?), tagset, queue_data)
+            .build(fmt!("{}", name.to_str()?), tagset, queue_data, kernel::my_gendisk_lkclass!())
     }
 }
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 599e75f33334..63fe8c86606a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -112,7 +112,7 @@ static DEFINE_MUTEX(sd_mutex_lock);
 static mempool_t *sd_page_pool;
 static mempool_t *sd_large_page_pool;
 static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
-static struct lock_class_key sd_bio_compl_lkclass;
+static struct gendisk_lkclass sd_bio_compl_lkclass;
 
 static const char *sd_cache_types[] = {
 	"write through", "none", "write back",
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index c36c54ecd354..734567ae0e43 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -106,7 +106,7 @@ static struct scsi_driver sr_template = {
 static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG];
 static DEFINE_SPINLOCK(sr_index_lock);
 
-static struct lock_class_key sr_bio_compl_lkclass;
+static struct gendisk_lkclass sr_bio_compl_lkclass;
 
 static int sr_open(struct cdrom_device_info *, int);
 static void sr_release(struct cdrom_device_info *);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..5aa17e82c3ba 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -726,15 +726,15 @@ enum {
 
 struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 #define blk_mq_alloc_disk(set, lim, queuedata)				\
 ({									\
-	static struct lock_class_key __key;				\
+	static struct gendisk_lkclass __key;				\
 									\
 	__blk_mq_alloc_disk(set, lim, queuedata, &__key);		\
 })
 struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 struct request_queue *blk_mq_alloc_queue(struct blk_mq_tag_set *set,
 		struct queue_limits *lim, void *queuedata);
 int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 890128cdea1c..28b0aee6b3ba 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -49,6 +49,11 @@ extern const struct device_type disk_type;
 extern const struct device_type part_type;
 extern const struct class block_class;
 
+struct gendisk_lkclass {
+	struct lock_class_key bio_lkclass;
+	struct lock_class_key open_mutex_lkclass;
+};
+
 /*
  * Maximum number of blkcg policies allowed to be registered concurrently.
  * Defined here to simplify include dependency.
@@ -974,7 +979,7 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate);
 
 void put_disk(struct gendisk *disk);
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
-		struct lock_class_key *lkclass);
+		struct gendisk_lkclass *lkclass);
 
 /**
  * blk_alloc_disk - allocate a gendisk structure
@@ -990,7 +995,7 @@ struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
  */
 #define blk_alloc_disk(lim, node_id)					\
 ({									\
-	static struct lock_class_key __key;				\
+	static struct gendisk_lkclass __key;				\
 									\
 	__blk_alloc_disk(lim, node_id, &__key);				\
 })
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 1fd0d54dd549..10f22b200567 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -88,7 +88,7 @@
 //!     Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
 //! let mut disk = gen_disk::GenDiskBuilder::new()
 //!     .capacity_sectors(4096)
-//!     .build(fmt!("myblk"), tagset, ())?;
+//!     .build(fmt!("myblk"), tagset, (), kernel::my_gendisk_lkclass!())?;
 //!
 //! # Ok::<(), kernel::error::Error>(())
 //! ```
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 912cb805caf5..7e669ca5c032 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -11,7 +11,6 @@
     error::{self, from_err_ptr, Result},
     fmt::{self, Write},
     prelude::*,
-    static_lock_class,
     str::NullTerminatedFormatter,
     sync::Arc,
     types::{ForeignOwnable, ScopeGuard},
@@ -38,6 +37,43 @@ fn default() -> Self {
     }
 }
 
+/// A wrapper type for safely passing "struct gendisk_lkclass" argument.
+///
+/// This type can only be instantiated via the [`my_gendisk_lkclass!`] macro.
+pub struct GenDiskLockClass(pub(crate) *mut bindings::gendisk_lkclass);
+
+impl GenDiskLockClass {
+    /// Retrieve the underlying raw pointer.
+    pub(crate) fn as_ptr(&self) -> *mut bindings::gendisk_lkclass {
+        self.0
+    }
+}
+
+#[doc(hidden)]
+pub mod __internal {
+    use super::*;
+
+    /// Internal constructor used ONLY by the `my_gendisk_lkclass!` macro.
+    ///
+    /// SAFETY: `ptr` must point to a valid static `gendisk_lkclass` instance.
+    pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {
+        GenDiskLockClass(ptr)
+    }
+}
+
+/// Helper macro to generate a unique caller-local static lock class struct
+#[macro_export]
+macro_rules! my_gendisk_lkclass {
+    () => {{
+        static mut LKCLASS: $crate::bindings::gendisk_lkclass = $crate::bindings::gendisk_lkclass {
+            bio_lkclass: const { unsafe { ::core::mem::zeroed() } },
+            open_mutex_lkclass: const { unsafe { ::core::mem::zeroed() } },
+        };
+
+        unsafe { $crate::block::mq::gen_disk::__internal::new_lock_class(&raw mut LKCLASS) }
+    }};
+}
+
 impl GenDiskBuilder {
     /// Create a new instance.
     pub fn new() -> Self {
@@ -100,6 +136,7 @@ pub fn build<T: Operations>(
         name: fmt::Arguments<'_>,
         tagset: Arc<TagSet<T>>,
         queue_data: T::QueueData,
+        lkclass: GenDiskLockClass,
     ) -> Result<GenDisk<T>> {
         let data = queue_data.into_foreign();
         let recover_data = ScopeGuard::new(|| {
@@ -121,7 +158,7 @@ pub fn build<T: Operations>(
                 tagset.raw_tag_set(),
                 &mut lim,
                 data,
-                static_lock_class!().as_ptr(),
+                lkclass.as_ptr(),
             )
         })?;
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-03 11:54       ` [PATCH v3] " Tetsuo Handa
@ 2026-06-04 21:07         ` Miguel Ojeda
  2026-06-05  7:36           ` Andreas Hindborg
  2026-06-05 10:08           ` Tetsuo Handa
  2026-06-05  7:54         ` Andreas Hindborg
  1 sibling, 2 replies; 13+ messages in thread
From: Miguel Ojeda @ 2026-06-04 21:07 UTC (permalink / raw)
  To: penguin-kernel
  Cc: akpm, axboe, bvanassche, dlemoal, hch, hdanton, linux-block,
	linux-kernel, ojeda, tom.leiming, wqu, Andreas Hindborg,
	Boqun Feng, rust-for-linux, Mark Brown

On Wed, 03 Jun 2026 20:54:05 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>
> Acknowledgment:
>   Since I have no experience with Rust, changes needed by Rust block layer
>   bindings and rnull module are made based on conversation with the Gemini
>   AI collaborator.

Then please do Cc the right people as `MAINTAINERS` mentions, including
"BLOCK LAYER DEVICE DRIVER API [RUST]" and "RUST"...

I am quite confused. Why was this added to linux-next?

It doesn't go through block, nor has an Ack or review and breaks
the `rustdoc` build in linux-next (and thus rust.docs.kernel.org):

    error: unresolved link to `my_gendisk_lkclass`
      --> rust/kernel/block/mq/gen_disk.rs:42:50
       |
    42 | /// This type can only be instantiated via the [`my_gendisk_lkclass!`] macro.

It is also not Clippy-clean -- it doesn't follow our usual conventions
for safety comments and sections:

    error: unsafe function's docs are missing a `# Safety` section
      --> rust/kernel/block/mq/gen_disk.rs:59:5
       |
    59 |     pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {

    error: function has unnecessary safety comment
      --> rust/kernel/block/mq/gen_disk.rs:59:5
       |
    58 |     /// SAFETY: `ptr` must point to a valid static `gendisk_lkclass` instance.
       |         ------- help: consider changing it to a `# Safety` section: `# Safety`
    59 |     pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {

Please see:

  https://rust-for-linux.com/contributing#submit-checklist-addendum

In any case, it is also too late in the cycle to be experimenting in
linux-next.

So what am I missing? What is going on?

(And on top of all that, for some reason I did not receive it even if I
am apparently in Cc, so I have asked the admins about that.)

Cheers,
Miguel

Cc: Andreas Hindborg <a.hindborg@kernel.org>
Cc: Boqun Feng <boqun@kernel.org>
Cc: rust-for-linux@vger.kernel.org

Cc: Mark Brown <broonie@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-04 21:07         ` Miguel Ojeda
@ 2026-06-05  7:36           ` Andreas Hindborg
  2026-06-05 10:08           ` Tetsuo Handa
  1 sibling, 0 replies; 13+ messages in thread
From: Andreas Hindborg @ 2026-06-05  7:36 UTC (permalink / raw)
  To: Miguel Ojeda, penguin-kernel, Kentaro Takeda
  Cc: akpm, axboe, bvanassche, dlemoal, hch, hdanton, linux-block,
	linux-kernel, ojeda, tom.leiming, wqu, Boqun Feng, rust-for-linux,
	Mark Brown

"Miguel Ojeda" <ojeda@kernel.org> writes:

> On Wed, 03 Jun 2026 20:54:05 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>>
>> Acknowledgment:
>>   Since I have no experience with Rust, changes needed by Rust block layer
>>   bindings and rnull module are made based on conversation with the Gemini
>>   AI collaborator.
>
> Then please do Cc the right people as `MAINTAINERS` mentions, including
> "BLOCK LAYER DEVICE DRIVER API [RUST]" and "RUST"...
>
> I am quite confused. Why was this added to linux-next?
>
> It doesn't go through block, nor has an Ack or review and breaks
> the `rustdoc` build in linux-next (and thus rust.docs.kernel.org):
>
>     error: unresolved link to `my_gendisk_lkclass`
>       --> rust/kernel/block/mq/gen_disk.rs:42:50
>        |
>     42 | /// This type can only be instantiated via the [`my_gendisk_lkclass!`] macro.
>
> It is also not Clippy-clean -- it doesn't follow our usual conventions
> for safety comments and sections:
>
>     error: unsafe function's docs are missing a `# Safety` section
>       --> rust/kernel/block/mq/gen_disk.rs:59:5
>        |
>     59 |     pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {
>
>     error: function has unnecessary safety comment
>       --> rust/kernel/block/mq/gen_disk.rs:59:5
>        |
>     58 |     /// SAFETY: `ptr` must point to a valid static `gendisk_lkclass` instance.
>        |         ------- help: consider changing it to a `# Safety` section: `# Safety`
>     59 |     pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {
>
> Please see:
>
>   https://rust-for-linux.com/contributing#submit-checklist-addendum
>
> In any case, it is also too late in the cycle to be experimenting in
> linux-next.
>
> So what am I missing? What is going on?
>
> (And on top of all that, for some reason I did not receive it even if I
> am apparently in Cc, so I have asked the admins about that.)
>
> Cheers,
> Miguel
>
> Cc: Andreas Hindborg <a.hindborg@kernel.org>
> Cc: Boqun Feng <boqun@kernel.org>
> Cc: rust-for-linux@vger.kernel.org
>
> Cc: Mark Brown <broonie@kernel.org>

Looks like this was pulled through the tomoyo tree:

tomoyo		git://git.code.sf.net/p/tomoyo/tomoyo.git#master

M:	Kentaro Takeda <takedakn@nttdata.co.jp>
M:	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>


Best regards,
Andreas Hindborg



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-03 11:54       ` [PATCH v3] " Tetsuo Handa
  2026-06-04 21:07         ` Miguel Ojeda
@ 2026-06-05  7:54         ` Andreas Hindborg
  2026-06-05 10:14           ` Tetsuo Handa
  1 sibling, 1 reply; 13+ messages in thread
From: Andreas Hindborg @ 2026-06-05  7:54 UTC (permalink / raw)
  To: Tetsuo Handa, Christoph Hellwig, Bart Van Assche, Jens Axboe
  Cc: linux-block, LKML, Andrew Morton, Ming Lei, Damien Le Moal,
	Qu Wenruo, Hillf Danton, Miguel Ojeda

Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:

> The block core currently allocates a single monolithic lockdep key for
> disk->open_mutex across all callers. This single key conflates locking
> hierarchies between independent block streams. For example, if a stacked
> driver like loop flushes its internal workqueues inside lo_release() while
> holding its own open_mutex, lockdep views this as a potential ABBA deadlock
> against the underlying storage stack, leading to numerous circular
> dependency splats.
>
> To structurally reduce false positives, this patch splits the global
> monolithic lock class into distinct, per-caller instances during disk
> allocation. This is done by replacing "struct lock_class_key" with
> "struct gendisk_lkclass", which contains two instances of
> "struct lock_class_key" for the legacy "(bio completion)" map and
> disk->open_mutex respectively.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

For the Rust part, we have existing infrastructure for lock class keys
[1]. Please take a look how we generate lock class keys elsewhere [2].

Best regards,
Andreas Hindborg

[1] https://rust.docs.kernel.org/kernel/sync/struct.LockClassKey.html
[2] https://github.com/torvalds/linux/blob/ddd664bbff63e09e7a7f9acae9c43605d4cf185f/rust/kernel/sync/lock/mutex.rs#L12


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-04 21:07         ` Miguel Ojeda
  2026-06-05  7:36           ` Andreas Hindborg
@ 2026-06-05 10:08           ` Tetsuo Handa
  2026-06-05 11:02             ` Miguel Ojeda
  1 sibling, 1 reply; 13+ messages in thread
From: Tetsuo Handa @ 2026-06-05 10:08 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: akpm, axboe, bvanassche, dlemoal, hch, hdanton, linux-block,
	linux-kernel, tom.leiming, wqu, Andreas Hindborg, Boqun Feng,
	rust-for-linux, Mark Brown

On 2026/06/05 6:07, Miguel Ojeda wrote:
> On Wed, 03 Jun 2026 20:54:05 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>>
>> Acknowledgment:
>>   Since I have no experience with Rust, changes needed by Rust block layer
>>   bindings and rnull module are made based on conversation with the Gemini
>>   AI collaborator.
> 
> Then please do Cc the right people as `MAINTAINERS` mentions, including
> "BLOCK LAYER DEVICE DRIVER API [RUST]" and "RUST"...

Oops, it seems that I overlooked the rust-for-linux@vger.kernel.org line.

> 
> I am quite confused. Why was this added to linux-next?

Since sashiko did not find issues on this v3 patch
( https://sashiko.dev/#/patchset/226152a3-1e4c-4eec-9a17-1d40426a7b18%40I-love.SAKURA.ne.jp ),
I started pre-testing by syzbot while waiting for responses from maintainers.

I am using my tree for allowing syzbot to test debug/experimental patches in linux-next tree
(or to apply workaround patches for bugs that prevent syzbot from testing linux-next tree),
and therefore my tree is subjected to "git reset --hard" changes.

> 
> It doesn't go through block, nor has an Ack or review and breaks
> the `rustdoc` build in linux-next (and thus rust.docs.kernel.org):
> 
>     error: unresolved link to `my_gendisk_lkclass`
>       --> rust/kernel/block/mq/gen_disk.rs:42:50
>        |
>     42 | /// This type can only be instantiated via the [`my_gendisk_lkclass!`] macro.
> 
> It is also not Clippy-clean -- it doesn't follow our usual conventions
> for safety comments and sections:
> 
>     error: unsafe function's docs are missing a `# Safety` section
>       --> rust/kernel/block/mq/gen_disk.rs:59:5
>        |
>     59 |     pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {
> 
>     error: function has unnecessary safety comment
>       --> rust/kernel/block/mq/gen_disk.rs:59:5
>        |
>     58 |     /// SAFETY: `ptr` must point to a valid static `gendisk_lkclass` instance.
>        |         ------- help: consider changing it to a `# Safety` section: `# Safety`
>     59 |     pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {

Hmm, I'm not familiar with comment styles for Rust...

Do you see any technical problems except comment style problem?

> 
> Please see:
> 
>   https://rust-for-linux.com/contributing#submit-checklist-addendum
> 
> In any case, it is also too late in the cycle to be experimenting in
> linux-next.
> 
> So what am I missing? What is going on?

Nothing bad is going on. The final patch will be sent to linux-next tree via
appropriate tree after getting acks from maintainers, and the current patch
in my tree will be dropped when maintainers accepted the final patch.

> 
> (And on top of all that, for some reason I did not receive it even if I
> am apparently in Cc, so I have asked the admins about that.)

I was enabling only SPF, but it seems that gmail started rejecting such mails.
Therefore, last night I also enabled DKIM/ARC and DMARC. I hope this mail is
delivered to gmail users.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-05  7:54         ` Andreas Hindborg
@ 2026-06-05 10:14           ` Tetsuo Handa
  0 siblings, 0 replies; 13+ messages in thread
From: Tetsuo Handa @ 2026-06-05 10:14 UTC (permalink / raw)
  To: Andreas Hindborg, Christoph Hellwig, Bart Van Assche, Jens Axboe
  Cc: linux-block, LKML, Andrew Morton, Ming Lei, Damien Le Moal,
	Qu Wenruo, Hillf Danton, Miguel Ojeda

On 2026/06/05 16:54, Andreas Hindborg wrote:
> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
> 
>> The block core currently allocates a single monolithic lockdep key for
>> disk->open_mutex across all callers. This single key conflates locking
>> hierarchies between independent block streams. For example, if a stacked
>> driver like loop flushes its internal workqueues inside lo_release() while
>> holding its own open_mutex, lockdep views this as a potential ABBA deadlock
>> against the underlying storage stack, leading to numerous circular
>> dependency splats.
>>
>> To structurally reduce false positives, this patch splits the global
>> monolithic lock class into distinct, per-caller instances during disk
>> allocation. This is done by replacing "struct lock_class_key" with
>> "struct gendisk_lkclass", which contains two instances of
>> "struct lock_class_key" for the legacy "(bio completion)" map and
>> disk->open_mutex respectively.
>>
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> 
> For the Rust part, we have existing infrastructure for lock class keys
> [1]. Please take a look how we generate lock class keys elsewhere [2].

My understanding is that we don't have infrastructure for lock class keys
that can be applied to

  +struct gendisk_lkclass {
  +	struct lock_class_key bio_lkclass;
  +	struct lock_class_key open_mutex_lkclass;
  +};

  -	static struct lock_class_key __key;
  +	static struct gendisk_lkclass __key;

change. Alternative approach is welcomed if you have one.

> 
> Best regards,
> Andreas Hindborg
> 
> [1] https://rust.docs.kernel.org/kernel/sync/struct.LockClassKey.html
> [2] https://github.com/torvalds/linux/blob/ddd664bbff63e09e7a7f9acae9c43605d4cf185f/rust/kernel/sync/lock/mutex.rs#L12
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-05 10:08           ` Tetsuo Handa
@ 2026-06-05 11:02             ` Miguel Ojeda
  2026-06-05 12:04               ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Miguel Ojeda @ 2026-06-05 11:02 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Miguel Ojeda, akpm, axboe, bvanassche, dlemoal, hch, hdanton,
	linux-block, linux-kernel, tom.leiming, wqu, Andreas Hindborg,
	Boqun Feng, rust-for-linux, Mark Brown

On Fri, Jun 5, 2026 at 12:15 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> Oops, it seems that I overlooked the rust-for-linux@vger.kernel.org line.

I am not sure what you mean -- it is not just the mailing list, the
maintainers and the reviewers of both entries were not Cc'd either.

So like 9 Ccs are missing...

> I am using my tree for allowing syzbot to test debug/experimental patches in linux-next tree

linux-next is only meant for patches that are already reviewed, tested
and destined for the merge window.

It is not meant for experimental patches, and adding those, especially
this late in the kernel cycle, can break other people's CIs and
systems at the worst possible time. If every maintainers put random
patches in linux-next, then it would be unusable...

(There are exceptions, of course, but only if people is aware and in
agreement...)

> Do you see any technical problems except comment style problem?

The above errors are already a technical problem -- they break the
Clippy and docs build!

I assume Andreas or Boqun will eventually take a look at the patch now
that they know about its existence (is there a particular reason to
rush?).

> Nothing bad is going on. The final patch will be sent to linux-next tree via
> appropriate tree after getting acks from maintainers, and the current patch
> in my tree will be dropped when maintainers accepted the final patch.

No, please drop it now -- the patch shouldn't have been applied to
linux-next yet, since the maintainers didn't ack it (they just
realized it exists...), it is not reviewed (apart from Sashiko), it is
not tested enough (given the errors above) and not destined for the
next merge window (for all the reasons before).

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
  2026-06-05 11:02             ` Miguel Ojeda
@ 2026-06-05 12:04               ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2026-06-05 12:04 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Tetsuo Handa, Miguel Ojeda, akpm, axboe, bvanassche, dlemoal, hch,
	hdanton, linux-block, linux-kernel, tom.leiming, wqu,
	Andreas Hindborg, Boqun Feng, rust-for-linux

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

On Fri, Jun 05, 2026 at 01:02:59PM +0200, Miguel Ojeda wrote:
> On Fri, Jun 5, 2026 at 12:15 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:

> > I am using my tree for allowing syzbot to test debug/experimental patches in linux-next tree

> linux-next is only meant for patches that are already reviewed, tested
> and destined for the merge window.

> It is not meant for experimental patches, and adding those, especially
> this late in the kernel cycle, can break other people's CIs and
> systems at the worst possible time. If every maintainers put random
> patches in linux-next, then it would be unusable...

> (There are exceptions, of course, but only if people is aware and in
> agreement...)

In general the sort of testing that it's good for is "this needs more
exposure" sorts of things - things that look good locally but where
there's a wide variety of users or affected systems that might be
affected and where it's hard to judge the impact.

> > Do you see any technical problems except comment style problem?

> The above errors are already a technical problem -- they break the
> Clippy and docs build!

Oh, bah - something turned off RUST in allmodconfig again so we lost
coverage in -next, sorry about that.  I'll need to work something out to
make sure I notice that happening and can do something about it.  It's
kind of worrying that this keeps happening TBH, otherwise rust conflicts
are likely to result in broken builds.  The dependencies feel really
fraigle here.

> No, please drop it now -- the patch shouldn't have been applied to
> linux-next yet, since the maintainers didn't ack it (they just
> realized it exists...), it is not reviewed (apart from Sashiko), it is
> not tested enough (given the errors above) and not destined for the
> next merge window (for all the reasons before).

Had the rust builds been enabled for allmodconfig as I had expected the
tree would have been kept out of -next as a result of this.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-06-05 12:04 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-30 13:45 [PATCH] block: assign caller-specific lockdep class to disk->open_mutex Tetsuo Handa
2026-05-30 21:15 ` Bart Van Assche
2026-06-01  7:11   ` Christoph Hellwig
2026-06-03  6:25     ` [PATCH v2] " Tetsuo Handa
2026-06-03 11:54       ` [PATCH v3] " Tetsuo Handa
2026-06-04 21:07         ` Miguel Ojeda
2026-06-05  7:36           ` Andreas Hindborg
2026-06-05 10:08           ` Tetsuo Handa
2026-06-05 11:02             ` Miguel Ojeda
2026-06-05 12:04               ` Mark Brown
2026-06-05  7:54         ` Andreas Hindborg
2026-06-05 10:14           ` Tetsuo Handa
2026-05-30 22:50 ` [PATCH] " Hillf Danton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox