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

Christoph Hellwig
2022-06-03T05:23:45Z

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