Re: [PATCH v2 0/3] ata,sd: Fix reading concurrent positioning ranges

From: Christoph Hellwig <hch@infradead.org>
To: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Cc: Tyler Erickson <tyler.erickson@seagate.com>,
	jejb@linux.ibm.com, martin.petersen@oracle.com,
	linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org,
	muhammad.ahmad@seagate.com
Subject: Re: [PATCH v2 0/3] ata,sd: Fix reading concurrent positioning ranges
Date: Thu, 2 Jun 2022 22:23:39 -0700	[thread overview]
Message-ID: <Ypma2xNMaHYJflNx@infradead.org> (raw)
In-Reply-To: <d3cc3cff-b483-b2dd-b6eb-131500b97d54@opensource.wdc.com>

On Fri, Jun 03, 2022 at 10:30:04AM +0900, Damien Le Moal wrote:
> Looks all good to me. I tested this and really wonder how I did not catch
> these mistakes earlier :)
> 
> Using a tcmu emulator for various concurrent positioning range configs to
> test, I got a lockdep splat when unplugging the drive:

You probably want something like this:

---
From 4340b85be3532149310326b5f0caf329e1f4c748 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Mon, 23 May 2022 09:18:44 +0200
Subject: block: don't take sysfs_lock in blk_ia_range_sysfs_show

sysfs already synchronizes internally against kobject removal, so remove
the extra lock.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-ia-ranges.c  | 8 +-------
 include/linux/blkdev.h | 1 -
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/block/blk-ia-ranges.c b/block/blk-ia-ranges.c
index 853be76b9808b..e9e7ebf02737f 100644
--- a/block/blk-ia-ranges.c
+++ b/block/blk-ia-ranges.c
@@ -54,13 +54,8 @@ static ssize_t blk_ia_range_sysfs_show(struct kobject *kobj,
 		container_of(attr, struct blk_ia_range_sysfs_entry, attr);
 	struct blk_independent_access_range *iar =
 		container_of(kobj, struct blk_independent_access_range, kobj);
-	ssize_t ret;
 
-	mutex_lock(&iar->queue->sysfs_lock);
-	ret = entry->show(iar, buf);
-	mutex_unlock(&iar->queue->sysfs_lock);
-
-	return ret;
+	return entry->show(iar, buf);
 }
 
 static const struct sysfs_ops blk_ia_range_sysfs_ops = {
@@ -149,7 +144,6 @@ int disk_register_independent_access_ranges(struct gendisk *disk,
 	}
 
 	for (i = 0; i < iars->nr_ia_ranges; i++) {
-		iars->ia_range[i].queue = q;
 		ret = kobject_init_and_add(&iars->ia_range[i].kobj,
 					   &blk_ia_range_ktype, &iars->kobj,
 					   "%d", i);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index a72203ed25454..0ceb85ca52af4 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -348,7 +348,6 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
  */
 struct blk_independent_access_range {
 	struct kobject		kobj;
-	struct request_queue	*queue;
 	sector_t		sector;
 	sector_t		nr_sectors;
 };
-- 
2.30.2

