| Message ID | 20230505175132.2236632-2-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/9] block: consolidate the shutdown logic in blk_mark_disk_dead and del_gendisk | expand |
On Fri 05-05-23 13:51:24, Christoph Hellwig wrote: > blk_mark_disk_dead does very similar work a a section of del_gendisk: > > - set the GD_DEAD flag > - set the capacity to zero > - start a queue drain > > but del_gendisk also sets QUEUE_FLAG_DYING on the queue if it is owned by > the disk, sets the capacity to zero before starting the drain, and both > with sending a uevent and kernel message for this fake capacity change. > > Move the exact logic from the more heavily used del_gendisk into > blk_mark_disk_dead and then call blk_mark_disk_dead from del_gendisk. > > Signed-off-by: Christoph Hellwig <hch@lst.de> I'm somewhat wondering about the lost notification from blk_mark_disk_dead(). E.g. DM uses blk_mark_disk_dead() so if some udev script depends on the event when DM device gets destroyed, we would break it? Honza > --- > block/genhd.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/block/genhd.c b/block/genhd.c > index 90c402771bb570..461999e9489937 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -583,13 +583,22 @@ EXPORT_SYMBOL(device_add_disk); > */ > void blk_mark_disk_dead(struct gendisk *disk) > { > + /* > + * Fail any new I/O. > + */ > set_bit(GD_DEAD, &disk->state); > - blk_queue_start_drain(disk->queue); > + if (test_bit(GD_OWNS_QUEUE, &disk->state)) > + blk_queue_flag_set(QUEUE_FLAG_DYING, disk->queue); > > /* > * Stop buffered writers from dirtying pages that can't be written out. > */ > - set_capacity_and_notify(disk, 0); > + set_capacity(disk, 0); > + > + /* > + * Prevent new I/O from crossing bio_queue_enter(). > + */ > + blk_queue_start_drain(disk->queue); > } > EXPORT_SYMBOL_GPL(blk_mark_disk_dead); > > @@ -632,18 +641,7 @@ void del_gendisk(struct gendisk *disk) > fsync_bdev(disk->part0); > __invalidate_device(disk->part0, true); > > - /* > - * Fail any new I/O. > - */ > - set_bit(GD_DEAD, &disk->state); > - if (test_bit(GD_OWNS_QUEUE, &disk->state)) > - blk_queue_flag_set(QUEUE_FLAG_DYING, q); > - set_capacity(disk, 0); > - > - /* > - * Prevent new I/O from crossing bio_queue_enter(). > - */ > - blk_queue_start_drain(q); > + blk_mark_disk_dead(disk); > > if (!(disk->flags & GENHD_FL_HIDDEN)) { > sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi"); > -- > 2.39.2 >
On Sun, May 07, 2023 at 09:08:11PM +0200, Jan Kara wrote: > I'm somewhat wondering about the lost notification from > blk_mark_disk_dead(). E.g. DM uses blk_mark_disk_dead() so if some udev > script depends on the event when DM device gets destroyed, we would break > it? Maybe. We'll probably find out..
diff --git a/block/genhd.c b/block/genhd.c index 90c402771bb570..461999e9489937 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -583,13 +583,22 @@ EXPORT_SYMBOL(device_add_disk); */ void blk_mark_disk_dead(struct gendisk *disk) { + /* + * Fail any new I/O. + */ set_bit(GD_DEAD, &disk->state); - blk_queue_start_drain(disk->queue); + if (test_bit(GD_OWNS_QUEUE, &disk->state)) + blk_queue_flag_set(QUEUE_FLAG_DYING, disk->queue); /* * Stop buffered writers from dirtying pages that can't be written out. */ - set_capacity_and_notify(disk, 0); + set_capacity(disk, 0); + + /* + * Prevent new I/O from crossing bio_queue_enter(). + */ + blk_queue_start_drain(disk->queue); } EXPORT_SYMBOL_GPL(blk_mark_disk_dead); @@ -632,18 +641,7 @@ void del_gendisk(struct gendisk *disk) fsync_bdev(disk->part0); __invalidate_device(disk->part0, true); - /* - * Fail any new I/O. - */ - set_bit(GD_DEAD, &disk->state); - if (test_bit(GD_OWNS_QUEUE, &disk->state)) - blk_queue_flag_set(QUEUE_FLAG_DYING, q); - set_capacity(disk, 0); - - /* - * Prevent new I/O from crossing bio_queue_enter(). - */ - blk_queue_start_drain(q); + blk_mark_disk_dead(disk); if (!(disk->flags & GENHD_FL_HIDDEN)) { sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
blk_mark_disk_dead does very similar work a a section of del_gendisk: - set the GD_DEAD flag - set the capacity to zero - start a queue drain but del_gendisk also sets QUEUE_FLAG_DYING on the queue if it is owned by the disk, sets the capacity to zero before starting the drain, and both with sending a uevent and kernel message for this fake capacity change. Move the exact logic from the more heavily used del_gendisk into blk_mark_disk_dead and then call blk_mark_disk_dead from del_gendisk. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/genhd.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)