| Message ID | 20220602225113.10218-1-tyler.erickson@seagate.com |
|---|---|
| Headers | show |
| Series | ata,sd: Fix reading concurrent positioning ranges | expand |
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@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); 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; > };
On Thu, 2 Jun 2022 16:51:10 -0600, Tyler Erickson wrote: > This patch series fixes reading the concurrent positioning ranges. > > The first patch fixes reading this in libata, where it was reading > more data than a drive necessarily supports, resulting in a > command abort. > > The second patch fixes the SCSI translated data to put the VPD page > length in the correct starting byte. > > [...] Applied to 5.19/scsi-fixes, thanks! [3/3] scsi: sd: Fix interpretation of VPD B9h length https://git.kernel.org/mkp/scsi/c/f92de9d11042
On 6/3/22 07:51, Tyler Erickson wrote: > This patch series fixes reading the concurrent positioning ranges. > > The first patch fixes reading this in libata, where it was reading > more data than a drive necessarily supports, resulting in a > command abort. > > The second patch fixes the SCSI translated data to put the VPD page > length in the correct starting byte. > > The third patch in sd, the fix is adding 4 instead of 3 for the header > length. Using 3 will always result in an error and was likely used > incorrectly as T10 specifications list all tables starting at byte 0, > and byte 3 is the page length, which would mean there are 4 total > bytes before the remaining data that contains the ranges and other > information. > > Tyler Erickson (3): > libata: fix reading concurrent positioning ranges log > libata: fix translation of concurrent positioning ranges > scsi: sd: Fix interpretation of VPD B9h length Applied 1-2 to for-5.19-fixes. Thanks ! > > drivers/ata/libata-core.c | 21 +++++++++++++-------- > drivers/ata/libata-scsi.c | 2 +- > drivers/scsi/sd.c | 2 +- > 3 files changed, 15 insertions(+), 10 deletions(-) > > > base-commit: 700170bf6b4d773e328fa54ebb70ba444007c702