[1/5] block: simplify bdev_del_partition()

Commit Message

Christoph Hellwig Oct. 17, 2023, 6:48 p.m. UTC

From: Christian Brauner <[e-mail]>

BLKPG_DEL_PARTITION refuses to delete partitions that still have
openers, i.e., that has an elevated @bdev->bd_openers count. If a device
is claimed by setting @bdev->bd_holder and @bdev->bd_holder_ops
@bdev->bd_openers and @bdev->bd_holders are incremented.
@bdev->bd_openers is effectively guaranteed to be >= @bdev->bd_holders.
So as long as @bdev->bd_openers isn't zero we know that this partition
is still in active use and that there might still be @bdev->bd_holder
and @bdev->bd_holder_ops set.

The only current example is @fs_holder_ops for filesystems. But that
means bdev_mark_dead() which calls into
bdev->bd_holder_ops->mark_dead::fs_bdev_mark_dead() is a nop. As long as
there's an elevated @bdev->bd_openers count we can't delete the
partition and if there isn't an elevated @bdev->bd_openers count then
there's no @bdev->bd_holder or @bdev->bd_holder_ops.

So simply open-code what we need to do. This gets rid of one more
instance where we acquire s_umount under @disk->open_mutex.

Link: https://lore.kernel.org/r/20231016-fototermin-umriss-59f1ea6c1fe6@brauner
Reviewed-by: Christoph Hellwig <[e-mail]>
Reviewed-by: Jan Kara <[e-mail]>
Reviewed-by: Jens Axboe <[e-mail]>
Signed-off-by: Christian Brauner <[e-mail]>
Signed-off-by: Christoph Hellwig <[e-mail]>
---
 block/partitions/core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Ming Lei Oct. 18, 2023, 2:33 a.m. UTC | #1

On Tue, Oct 17, 2023 at 08:48:19PM +0200, Christoph Hellwig wrote:
> From: Christian Brauner <[e-mail]>
> 
> BLKPG_DEL_PARTITION refuses to delete partitions that still have
> openers, i.e., that has an elevated @bdev->bd_openers count. If a device
> is claimed by setting @bdev->bd_holder and @bdev->bd_holder_ops
> @bdev->bd_openers and @bdev->bd_holders are incremented.
> @bdev->bd_openers is effectively guaranteed to be >= @bdev->bd_holders.
> So as long as @bdev->bd_openers isn't zero we know that this partition
> is still in active use and that there might still be @bdev->bd_holder
> and @bdev->bd_holder_ops set.
> 
> The only current example is @fs_holder_ops for filesystems. But that
> means bdev_mark_dead() which calls into
> bdev->bd_holder_ops->mark_dead::fs_bdev_mark_dead() is a nop. As long as
> there's an elevated @bdev->bd_openers count we can't delete the
> partition and if there isn't an elevated @bdev->bd_openers count then
> there's no @bdev->bd_holder or @bdev->bd_holder_ops.
> 
> So simply open-code what we need to do. This gets rid of one more
> instance where we acquire s_umount under @disk->open_mutex.
> 
> Link: https://lore.kernel.org/r/20231016-fototermin-umriss-59f1ea6c1fe6@brauner
> Reviewed-by: Christoph Hellwig <[e-mail]>
> Reviewed-by: Jan Kara <[e-mail]>
> Reviewed-by: Jens Axboe <[e-mail]>
> Signed-off-by: Christian Brauner <[e-mail]>
> Signed-off-by: Christoph Hellwig <[e-mail]>

inc/dec(part->bd_openers) is always done with ->open_mutex held, so this
change is correct.

Reviewed-by: Ming Lei <[e-mail]>

Thanks,
Ming

Patch

diff --git a/block/partitions/core.c b/block/partitions/core.c
index e137a87f4db0d3..b0585536b407a5 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -485,7 +485,18 @@  int bdev_del_partition(struct gendisk *disk, int partno)
 	if (atomic_read(&part->bd_openers))
 		goto out_unlock;
 
-	delete_partition(part);
+	/*
+	 * We verified that @part->bd_openers is zero above and so
+	 * @part->bd_holder{_ops} can't be set. And since we hold
+	 * @disk->open_mutex the device can't be claimed by anyone.
+	 *
+	 * So no need to call @part->bd_holder_ops->mark_dead() here.
+	 * Just delete the partition and invalidate it.
+	 */
+
+	remove_inode_hash(part->bd_inode);
+	invalidate_bdev(part);
+	drop_partition(part);
 	ret = 0;
 out_unlock:
 	mutex_unlock(&disk->open_mutex);