On 6/3/22 14:23, Christoph Hellwig wrote:
> 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@xxxxxx>
> 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@xxxxxx>
> ---
> 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);
I sent a patch that Jens applied doing exactly that. But forgot to add the
removal of the queue pointer you have below. Sending an incremental patch
for that.
> }
>
> 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;
> };
--
Damien Le Moal
Western Digital Research
