Re: [PATCH 3/7] bdev: implement freeze and thaw holder operations

[Christoph Hellwig <hch@lst.de>]

>  
> +static struct super_block *get_bdev_super(const struct block_device *bdev)
> +{
> +	struct super_block *sb_bdev = bdev->bd_holder, *sb = NULL;
> +
> +	if (!sb_bdev)
> +		return NULL;
> +	if (super_lock_excl(sb_bdev) && atomic_inc_not_zero(&sb_bdev->s_active))
> +		sb = sb_bdev;
> +	super_unlock_excl(sb_bdev);
> +	return sb;

I find the flow here a bit confusing, because to me sb_bdev implies
the super_block of the bdev fs, and because the super_lock_excl calling
convention that always locks no matter of the return value is very
confusing.  Maybe at least rename sb_bdev to holder_bdev?

> +static int fs_bdev_freeze(struct block_device *bdev)
> +	__releases(&bdev->bd_holder_lock)
> +{
> +	struct super_block *sb;
> +	int error = 0;
> +
> +	lockdep_assert_held(&bdev->bd_holder_lock);
> +
> +	sb = get_bdev_super(bdev);
> +	if (sb) {

We always have a sb in bdev->bd_holder.  So the only way get_bdev_super
could return NULL is if we can't get an active reference or the fs is
marked as SB_DYING.  I think the best is to just give up and not even
bother with the sync, which is going to cause more problems than it
could help.  I think we're better off just straight returning an
error here, and don't bother with the sync_blockdev.