| Message ID | 20230505175132.2236632-7-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] block: consolidate the shutdown logic in blk_mark_disk_dead and del_gendisk | expand |
On Fri, May 05, 2023 at 01:51:29PM -0400, Christoph Hellwig wrote: > Add a mark_dead method to blk_holder_ops that is called from blk_mark_disk_dead > to notify the holder that the block device it is using has been marked dead. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/genhd.c | 24 ++++++++++++++++++++++++ > include/linux/blkdev.h | 1 + > 2 files changed, 25 insertions(+) > > diff --git a/block/genhd.c b/block/genhd.c > index d1c673b967c254..af97a4ef1e926c 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -575,6 +575,28 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, > } > EXPORT_SYMBOL(device_add_disk); > > +static void blk_report_disk_dead(struct gendisk *disk) > +{ > + struct block_device *bdev; > + unsigned long idx; > + > + rcu_read_lock(); > + xa_for_each(&disk->part_tbl, idx, bdev) { > + if (!kobject_get_unless_zero(&bdev->bd_device.kobj)) > + continue; > + rcu_read_unlock(); > + > + mutex_lock(&bdev->bd_holder_lock); > + if (bdev->bd_holder_ops && bdev->bd_holder_ops->mark_dead) > + bdev->bd_holder_ops->mark_dead(bdev); > + mutex_unlock(&bdev->bd_holder_lock); > + > + put_device(&bdev->bd_device); > + rcu_read_lock(); > + } > + rcu_read_unlock(); > +} > + > /** > * blk_mark_disk_dead - mark a disk as dead > * @disk: disk to mark as dead > @@ -602,6 +624,8 @@ void blk_mark_disk_dead(struct gendisk *disk) > * Prevent new I/O from crossing bio_queue_enter(). > */ > blk_queue_start_drain(disk->queue); > + > + blk_report_disk_dead(disk); > } > EXPORT_SYMBOL_GPL(blk_mark_disk_dead); > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 3f41f8c9b103cf..9706bec2be16a5 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -1469,6 +1469,7 @@ void blkdev_show(struct seq_file *seqf, off_t offset); > #endif > > struct blk_holder_ops { > + void (*mark_dead)(struct block_device *bdev); Now that we have blockdev callbacks, can we add a pair for freeze_bdev and thaw_bdev so that LVM snapshotting an xfs log device can actually quiesce the filesystem? This patch itself looks ok, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > }; > > struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder, > -- > 2.39.2 >
On Fri 05-05-23 13:51:29, Christoph Hellwig wrote: > Add a mark_dead method to blk_holder_ops that is called from blk_mark_disk_dead > to notify the holder that the block device it is using has been marked dead. > > Signed-off-by: Christoph Hellwig <hch@lst.de> One question below: > --- > block/genhd.c | 24 ++++++++++++++++++++++++ > include/linux/blkdev.h | 1 + > 2 files changed, 25 insertions(+) > > diff --git a/block/genhd.c b/block/genhd.c > index d1c673b967c254..af97a4ef1e926c 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -575,6 +575,28 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, > } > EXPORT_SYMBOL(device_add_disk); > > +static void blk_report_disk_dead(struct gendisk *disk) > +{ > + struct block_device *bdev; > + unsigned long idx; > + > + rcu_read_lock(); > + xa_for_each(&disk->part_tbl, idx, bdev) { > + if (!kobject_get_unless_zero(&bdev->bd_device.kobj)) > + continue; > + rcu_read_unlock(); > + > + mutex_lock(&bdev->bd_holder_lock); > + if (bdev->bd_holder_ops && bdev->bd_holder_ops->mark_dead) > + bdev->bd_holder_ops->mark_dead(bdev); > + mutex_unlock(&bdev->bd_holder_lock); > + > + put_device(&bdev->bd_device); > + rcu_read_lock(); > + } > + rcu_read_unlock(); > +} > + > /** > * blk_mark_disk_dead - mark a disk as dead > * @disk: disk to mark as dead > @@ -602,6 +624,8 @@ void blk_mark_disk_dead(struct gendisk *disk) > * Prevent new I/O from crossing bio_queue_enter(). > */ > blk_queue_start_drain(disk->queue); > + > + blk_report_disk_dead(disk); Hum, but this gets called from del_gendisk() after blk_drop_partitions() happens. So how is this going to be able to iterate anything? Honza
On Fri, May 05, 2023 at 11:37:49AM -0700, Darrick J. Wong wrote: > Now that we have blockdev callbacks, can we add a pair for freeze_bdev > and thaw_bdev so that LVM snapshotting an xfs log device can actually > quiesce the filesystem? Probably. Assuming we're not creating any really weird locking dependency that way.
On Sun, May 07, 2023 at 09:19:46PM +0200, Jan Kara wrote: > > @@ -602,6 +624,8 @@ void blk_mark_disk_dead(struct gendisk *disk) > > * Prevent new I/O from crossing bio_queue_enter(). > > */ > > blk_queue_start_drain(disk->queue); > > + > > + blk_report_disk_dead(disk); > > Hum, but this gets called from del_gendisk() after blk_drop_partitions() > happens. So how is this going to be able to iterate anything? It isn't, and doesn't work for partitions right now. I guess del_gendisk needs a bit of refacoring that we do two pases over the inode and/or move the ->mark_deal call for partitions into blk_drop_partitions.
On Tue, May 09, 2023 at 03:32:09PM +0200, Christoph Hellwig wrote: > On Sun, May 07, 2023 at 09:19:46PM +0200, Jan Kara wrote: > > > @@ -602,6 +624,8 @@ void blk_mark_disk_dead(struct gendisk *disk) > > > * Prevent new I/O from crossing bio_queue_enter(). > > > */ > > > blk_queue_start_drain(disk->queue); > > > + > > > + blk_report_disk_dead(disk); > > > > Hum, but this gets called from del_gendisk() after blk_drop_partitions() > > happens. So how is this going to be able to iterate anything? > > It isn't, and doesn't work for partitions right now. I guess del_gendisk > needs a bit of refacoring that we do two pases over the inode and/or > move the ->mark_deal call for partitions into blk_drop_partitions. Might be worth a comment. Otherwise looks good to me, Acked-by: Christian Brauner <brauner@kernel.org>
diff --git a/block/genhd.c b/block/genhd.c index d1c673b967c254..af97a4ef1e926c 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -575,6 +575,28 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, } EXPORT_SYMBOL(device_add_disk); +static void blk_report_disk_dead(struct gendisk *disk) +{ + struct block_device *bdev; + unsigned long idx; + + rcu_read_lock(); + xa_for_each(&disk->part_tbl, idx, bdev) { + if (!kobject_get_unless_zero(&bdev->bd_device.kobj)) + continue; + rcu_read_unlock(); + + mutex_lock(&bdev->bd_holder_lock); + if (bdev->bd_holder_ops && bdev->bd_holder_ops->mark_dead) + bdev->bd_holder_ops->mark_dead(bdev); + mutex_unlock(&bdev->bd_holder_lock); + + put_device(&bdev->bd_device); + rcu_read_lock(); + } + rcu_read_unlock(); +} + /** * blk_mark_disk_dead - mark a disk as dead * @disk: disk to mark as dead @@ -602,6 +624,8 @@ void blk_mark_disk_dead(struct gendisk *disk) * Prevent new I/O from crossing bio_queue_enter(). */ blk_queue_start_drain(disk->queue); + + blk_report_disk_dead(disk); } EXPORT_SYMBOL_GPL(blk_mark_disk_dead); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 3f41f8c9b103cf..9706bec2be16a5 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1469,6 +1469,7 @@ void blkdev_show(struct seq_file *seqf, off_t offset); #endif struct blk_holder_ops { + void (*mark_dead)(struct block_device *bdev); }; struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder,
Add a mark_dead method to blk_holder_ops that is called from blk_mark_disk_dead to notify the holder that the block device it is using has been marked dead. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/genhd.c | 24 ++++++++++++++++++++++++ include/linux/blkdev.h | 1 + 2 files changed, 25 insertions(+)