Re: [PATCH RESEND v2 08/10] md: Implement ->corrupted_range()
On 2021/2/2 上午11:17, Darrick J. Wong wrote:
On Fri, Jan 29, 2021 at 02:27:55PM +0800, Shiyang Ruan wrote:
With the support of ->rmap(), it is possible to obtain the superblock on
a mapped device.
If a pmem device is used as one target of mapped device, we cannot
obtain its superblock directly. With the help of SYSFS, the mapped
device can be found on the target devices. So, we iterate the
bdev->bd_holder_disks to obtain its mapped device.
Signed-off-by: Shiyang Ruan
---
drivers/md/dm.c | 61 +++
drivers/nvdimm/pmem.c | 11 +++-
fs/block_dev.c| 42 -
I feel like this ^^^ part that implements the generic ability for a block
device with a bad sector to notify whatever's holding onto it (fs, other
block device) should be in patch 2. That's generic block layer code,
and it's hard to tell (when you're looking at patch 2) what the bare
function declaration in it is really supposed to do.
Also, this patch is still difficult to review because it mixes device
mapper, nvdimm, and block layer changes!
OK. I'll split this to make it looks simple.
include/linux/genhd.h | 2 ++
4 files changed, 107 insertions(+), 9 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 7bac564f3faa..31b0c340b695 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -507,6 +507,66 @@ static int dm_blk_report_zones(struct gendisk *disk,
sector_t sector,
#define dm_blk_report_zones NULL
#endif /* CONFIG_BLK_DEV_ZONED */
+struct corrupted_hit_info {
+ struct block_device *bdev;
+ sector_t offset;
+};
+
+static int dm_blk_corrupted_hit(struct dm_target *ti, struct dm_dev *dev,
+ sector_t start, sector_t count, void *data)
+{
+ struct corrupted_hit_info *bc = data;
+
+ return bc->bdev == (void *)dev->bdev &&
+ (start <= bc->offset && bc->offset < start + count);
+
+}
+
+struct corrupted_do_info {
+ size_t length;
+ void *data;
+};
+
+static int dm_blk_corrupted_do(struct dm_target *ti, struct block_device *bdev,
+ sector_t disk_sect, void *data)
+{
+ struct corrupted_do_info *bc = data;
+ loff_t disk_off = to_bytes(disk_sect);
+ loff_t bdev_off = to_bytes(disk_sect - get_start_sect(bdev));
+
+ return bd_corrupted_range(bdev, disk_off, bdev_off, bc->length,
bc->data);
+}
+
+static int dm_blk_corrupted_range(struct gendisk *disk,
+ struct block_device *target_bdev,
+ loff_t target_offset, size_t len, void *data)
+{
+ struct mapped_device *md = disk->private_data;
+ struct dm_table *map;
+ struct dm_target *ti;
+ sector_t target_sect = to_sector(target_offset);
+ struct corrupted_hit_info hi = {target_bdev, target_sect};
+ struct corrupted_do_info di = {len, data};
+ int srcu_idx, i, rc = -ENODEV;
+
+ map = dm_get_live_table(md, _idx);
+ if (!map)
+ return rc;
+
+ for (i = 0; i < dm_table_get_num_targets(map); i++) {
+ ti = dm_table_get_target(map, i);
+ if (!(ti->type->iterate_devices && ti->type->rmap))
+ continue;
+ if (!ti->type->iterate_devices(ti, dm_blk_corrupted_hit, ))
+ continue;
+
+ rc = ti->type->rmap(ti, target_sect, dm_blk_corrupted_do, );
Why is it necessary to call ->iterate_devices here?
->iterate_devices() here is to find out which target is the pmem device
which is corrupted now. Then call ->rmap() on this target. Other
targets will be ignored.
If you pass the target_bdev, offset, and length to the dm-target's
->rmap function, it should be able to work backwards through its mapping
logic to come up with all the LBA ranges of the mapped_device that
are affected, and then it can call bd_corrupted_range on each of those
reverse mappings.
It would be helpful to have the changes to dm-linear.c in this patch
too, since that's the only real implementation at this point.
+ break;
+ }
+
+ dm_put_live_table(md, srcu_idx);
+ return rc;
+}
+
static int dm_prepare_ioctl(struct mapped_device *md, int *srcu_idx,
struct block_device **bdev)
{
@@ -3062,6 +3122,7 @@ static const struct block_device_operations dm_blk_dops =
{
.getgeo = dm_blk_getgeo,
.report_zones = dm_blk_report_zones,
.pr_ops = _pr_ops,
+ .corrupted_range = dm_blk_corrupted_range,
.owner = THIS_MODULE
};
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 501959947d48..3d9f4ccbbd9e 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -256,21 +256,16 @@ static int pmem_rw_page(struct block_device *bdev,
sector_t sector,
static int pmem_corrupted_range(struct gendisk *disk, struct block_device
*bdev,
Re: [PATCH RESEND v2 08/10] md: Implement ->corrupted_range()
On Fri, Jan 29, 2021 at 02:27:55PM +0800, Shiyang Ruan wrote:
> With the support of ->rmap(), it is possible to obtain the superblock on
> a mapped device.
>
> If a pmem device is used as one target of mapped device, we cannot
> obtain its superblock directly. With the help of SYSFS, the mapped
> device can be found on the target devices. So, we iterate the
> bdev->bd_holder_disks to obtain its mapped device.
>
> Signed-off-by: Shiyang Ruan
> ---
> drivers/md/dm.c | 61 +++
> drivers/nvdimm/pmem.c | 11 +++-
> fs/block_dev.c| 42 -
I feel like this ^^^ part that implements the generic ability for a block
device with a bad sector to notify whatever's holding onto it (fs, other
block device) should be in patch 2. That's generic block layer code,
and it's hard to tell (when you're looking at patch 2) what the bare
function declaration in it is really supposed to do.
Also, this patch is still difficult to review because it mixes device
mapper, nvdimm, and block layer changes!
> include/linux/genhd.h | 2 ++
> 4 files changed, 107 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 7bac564f3faa..31b0c340b695 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -507,6 +507,66 @@ static int dm_blk_report_zones(struct gendisk *disk,
> sector_t sector,
> #define dm_blk_report_zones NULL
> #endif /* CONFIG_BLK_DEV_ZONED */
>
> +struct corrupted_hit_info {
> + struct block_device *bdev;
> + sector_t offset;
> +};
> +
> +static int dm_blk_corrupted_hit(struct dm_target *ti, struct dm_dev *dev,
> + sector_t start, sector_t count, void *data)
> +{
> + struct corrupted_hit_info *bc = data;
> +
> + return bc->bdev == (void *)dev->bdev &&
> + (start <= bc->offset && bc->offset < start + count);
> +
> +}
> +
> +struct corrupted_do_info {
> + size_t length;
> + void *data;
> +};
> +
> +static int dm_blk_corrupted_do(struct dm_target *ti, struct block_device
> *bdev,
> +sector_t disk_sect, void *data)
> +{
> + struct corrupted_do_info *bc = data;
> + loff_t disk_off = to_bytes(disk_sect);
> + loff_t bdev_off = to_bytes(disk_sect - get_start_sect(bdev));
> +
> + return bd_corrupted_range(bdev, disk_off, bdev_off, bc->length,
> bc->data);
> +}
> +
> +static int dm_blk_corrupted_range(struct gendisk *disk,
> + struct block_device *target_bdev,
> + loff_t target_offset, size_t len, void *data)
> +{
> + struct mapped_device *md = disk->private_data;
> + struct dm_table *map;
> + struct dm_target *ti;
> + sector_t target_sect = to_sector(target_offset);
> + struct corrupted_hit_info hi = {target_bdev, target_sect};
> + struct corrupted_do_info di = {len, data};
> + int srcu_idx, i, rc = -ENODEV;
> +
> + map = dm_get_live_table(md, _idx);
> + if (!map)
> + return rc;
> +
> + for (i = 0; i < dm_table_get_num_targets(map); i++) {
> + ti = dm_table_get_target(map, i);
> + if (!(ti->type->iterate_devices && ti->type->rmap))
> + continue;
> + if (!ti->type->iterate_devices(ti, dm_blk_corrupted_hit, ))
> + continue;
> +
> + rc = ti->type->rmap(ti, target_sect, dm_blk_corrupted_do, );
Why is it necessary to call ->iterate_devices here?
If you pass the target_bdev, offset, and length to the dm-target's
->rmap function, it should be able to work backwards through its mapping
logic to come up with all the LBA ranges of the mapped_device that
are affected, and then it can call bd_corrupted_range on each of those
reverse mappings.
It would be helpful to have the changes to dm-linear.c in this patch
too, since that's the only real implementation at this point.
> + break;
> + }
> +
> + dm_put_live_table(md, srcu_idx);
> + return rc;
> +}
> +
> static int dm_prepare_ioctl(struct mapped_device *md, int *srcu_idx,
> struct block_device **bdev)
> {
> @@ -3062,6 +3122,7 @@ static const struct block_device_operations dm_blk_dops
> = {
> .getgeo = dm_blk_getgeo,
> .report_zones = dm_blk_report_zones,
> .pr_ops = _pr_ops,
> + .corrupted_range = dm_blk_corrupted_range,
> .owner = THIS_MODULE
> };
>
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 501959947d48..3d9f4ccbbd9e 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -256,21 +256,16 @@ static int pmem_rw_page(struct block_device *bdev,
> sector_t sector,
> static int pmem_corrupted_range(struct gendisk *disk, struct block_device
> *bdev,
> loff_t disk_offset, size_t len, void *data)
> {
> - struct super_block *sb;
> loff_t bdev_offset;
> sector_t disk_sector
[PATCH RESEND v2 08/10] md: Implement ->corrupted_range()
With the support of ->rmap(), it is possible to obtain the superblock on
a mapped device.
If a pmem device is used as one target of mapped device, we cannot
obtain its superblock directly. With the help of SYSFS, the mapped
device can be found on the target devices. So, we iterate the
bdev->bd_holder_disks to obtain its mapped device.
Signed-off-by: Shiyang Ruan
---
drivers/md/dm.c | 61 +++
drivers/nvdimm/pmem.c | 11 +++-
fs/block_dev.c| 42 -
include/linux/genhd.h | 2 ++
4 files changed, 107 insertions(+), 9 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 7bac564f3faa..31b0c340b695 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -507,6 +507,66 @@ static int dm_blk_report_zones(struct gendisk *disk,
sector_t sector,
#define dm_blk_report_zonesNULL
#endif /* CONFIG_BLK_DEV_ZONED */
+struct corrupted_hit_info {
+ struct block_device *bdev;
+ sector_t offset;
+};
+
+static int dm_blk_corrupted_hit(struct dm_target *ti, struct dm_dev *dev,
+ sector_t start, sector_t count, void *data)
+{
+ struct corrupted_hit_info *bc = data;
+
+ return bc->bdev == (void *)dev->bdev &&
+ (start <= bc->offset && bc->offset < start + count);
+}
+
+struct corrupted_do_info {
+ size_t length;
+ void *data;
+};
+
+static int dm_blk_corrupted_do(struct dm_target *ti, struct block_device *bdev,
+ sector_t disk_sect, void *data)
+{
+ struct corrupted_do_info *bc = data;
+ loff_t disk_off = to_bytes(disk_sect);
+ loff_t bdev_off = to_bytes(disk_sect - get_start_sect(bdev));
+
+ return bd_corrupted_range(bdev, disk_off, bdev_off, bc->length,
bc->data);
+}
+
+static int dm_blk_corrupted_range(struct gendisk *disk,
+ struct block_device *target_bdev,
+ loff_t target_offset, size_t len, void *data)
+{
+ struct mapped_device *md = disk->private_data;
+ struct dm_table *map;
+ struct dm_target *ti;
+ sector_t target_sect = to_sector(target_offset);
+ struct corrupted_hit_info hi = {target_bdev, target_sect};
+ struct corrupted_do_info di = {len, data};
+ int srcu_idx, i, rc = -ENODEV;
+
+ map = dm_get_live_table(md, _idx);
+ if (!map)
+ return rc;
+
+ for (i = 0; i < dm_table_get_num_targets(map); i++) {
+ ti = dm_table_get_target(map, i);
+ if (!(ti->type->iterate_devices && ti->type->rmap))
+ continue;
+ if (!ti->type->iterate_devices(ti, dm_blk_corrupted_hit, ))
+ continue;
+
+ rc = ti->type->rmap(ti, target_sect, dm_blk_corrupted_do, );
+ break;
+ }
+
+ dm_put_live_table(md, srcu_idx);
+ return rc;
+}
+
static int dm_prepare_ioctl(struct mapped_device *md, int *srcu_idx,
struct block_device **bdev)
{
@@ -3062,6 +3122,7 @@ static const struct block_device_operations dm_blk_dops =
{
.getgeo = dm_blk_getgeo,
.report_zones = dm_blk_report_zones,
.pr_ops = _pr_ops,
+ .corrupted_range = dm_blk_corrupted_range,
.owner = THIS_MODULE
};
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 501959947d48..3d9f4ccbbd9e 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -256,21 +256,16 @@ static int pmem_rw_page(struct block_device *bdev,
sector_t sector,
static int pmem_corrupted_range(struct gendisk *disk, struct block_device
*bdev,
loff_t disk_offset, size_t len, void *data)
{
- struct super_block *sb;
loff_t bdev_offset;
sector_t disk_sector = disk_offset >> SECTOR_SHIFT;
- int rc = 0;
+ int rc = -ENODEV;
bdev = bdget_disk_sector(disk, disk_sector);
if (!bdev)
- return -ENODEV;
+ return rc;
bdev_offset = (disk_sector - get_start_sect(bdev)) << SECTOR_SHIFT;
- sb = get_super(bdev);
- if (sb && sb->s_op->corrupted_range) {
- rc = sb->s_op->corrupted_range(sb, bdev, bdev_offset, len,
data);
- drop_super(sb);
- }
+ rc = bd_corrupted_range(bdev, bdev_offset, bdev_offset, len, data);
bdput(bdev);
return rc;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 3b8963e228a1..3cc2b2911e3a 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1079,6 +1079,27 @@ struct bd_holder_disk {
int refcnt;
};
+static int bd_disk_holder_corrupted_range(struct block_device *bdev, loff_t
off,
+ size_t len, void *data)
+{
+ struct bd_holder_disk *holder;
+ struct gendisk *disk;
+ int rc = 0;
+
+ if (list_empty(&(bdev->bd_holder_disks)))
+
