On Tue 24-11-20 14:27:10, Christoph Hellwig wrote:
> Store the frozen superblock in struct block_device to avoid the awkward
> interface that can return a sb only used a cookie, an ERR_PTR or NULL.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Some comments below...
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index d8664f5c1ff669..60492620d51866 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -548,55 +548,47 @@ EXPORT_SYMBOL(fsync_bdev);
> * count down in thaw_bdev(). When it becomes 0, thaw_bdev() will unfreeze
> * actually.
> */
> -struct super_block *freeze_bdev(struct block_device *bdev)
> +int freeze_bdev(struct block_device *bdev)
> {
> struct super_block *sb;
> int error = 0;
>
> mutex_lock(&bdev->bd_fsfreeze_mutex);
> - if (++bdev->bd_fsfreeze_count > 1) {
> - /*
> - * We don't even need to grab a reference - the first call
> - * to freeze_bdev grab an active reference and only the last
> - * thaw_bdev drops it.
> - */
> - sb = get_super(bdev);
> - if (sb)
> - drop_super(sb);
> - mutex_unlock(&bdev->bd_fsfreeze_mutex);
> - return sb;
> - }
> + if (++bdev->bd_fsfreeze_count > 1)
> + goto done;
>
> sb = get_active_super(bdev);
> if (!sb)
> - goto out;
> + goto sync;
> if (sb->s_op->freeze_super)
> error = sb->s_op->freeze_super(sb);
> else
> error = freeze_super(sb);
> + deactivate_super(sb);
> +
> if (error) {
> - deactivate_super(sb);
> bdev->bd_fsfreeze_count--;
> - mutex_unlock(&bdev->bd_fsfreeze_mutex);
> - return ERR_PTR(error);
> + goto done;
> }
> - deactivate_super(sb);
> - out:
> + bdev->bd_fsfreeze_sb = sb;
> +
> +sync:
> sync_blockdev(bdev);
> +done:
> mutex_unlock(&bdev->bd_fsfreeze_mutex);
> - return sb; /* thaw_bdev releases s->s_umount */
> + return error; /* thaw_bdev releases s->s_umount */
The comment about thaw_bdev() seems to be stale? At least I don't see what
it's speaking about...
> }
> EXPORT_SYMBOL(freeze_bdev);
>
> /**
> * thaw_bdev -- unlock filesystem
> * @bdev: blockdevice to unlock
> - * @sb: associated superblock
> *
> * Unlocks the filesystem and marks it writeable again after freeze_bdev().
> */
> -int thaw_bdev(struct block_device *bdev, struct super_block *sb)
> +int thaw_bdev(struct block_device *bdev)
> {
> + struct super_block *sb;
> int error = -EINVAL;
>
> mutex_lock(&bdev->bd_fsfreeze_mutex);
> @@ -607,6 +599,7 @@ int thaw_bdev(struct block_device *bdev, struct super_block *sb)
> if (--bdev->bd_fsfreeze_count > 0)
> goto out;
>
> + sb = bdev->bd_fsfreeze_sb;
> if (!sb)
> goto out;
>
> @@ -618,7 +611,7 @@ int thaw_bdev(struct block_device *bdev, struct super_block *sb)
> bdev->bd_fsfreeze_count++;
> out:
> mutex_unlock(&bdev->bd_fsfreeze_mutex);
> - return error;
> + return 0;
But we now won't return -EINVAL if this gets called e.g. with
bd_fsfreeze_count == 0, right?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
