Re: [PATCH] block: simplify bdev_del_partition()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon 16-10-23 17:27:18, Christian Brauner wrote:
> 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.
> 
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

BTW, now when there's only one delete_partition() caller, we could just
opencode it in its callsite...

								Honza

> ---
> Hey Jens,
> 
> This came out of an ongoing locking design discussion and is related
> to what we currently have in vfs.super. So if everyone still agrees my
> reasoning is right and you don't have big objections I'd take it through
> there.
> 
> As usual, thanks to Jan and Christoph for good discussions here.
> 
> Thanks!
> Christian
> ---
>  block/partitions/core.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/block/partitions/core.c b/block/partitions/core.c
> index e137a87f4db0..b0585536b407 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);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux