Re: [PATCH 1/3] Implement generic freeze feature

Christoph Hellwig
2008-07-01T08:07:55Z

> {
> struct super_block *sb;
>
> + if (test_and_set_bit(BD_FREEZE_OP, &bdev->bd_state))
> + return ERR_PTR(-EBUSY);
> +
> + sb = get_super(bdev);
> +
> + /* If super_block has been already frozen, return. */
> + if (sb && sb->s_frozen != SB_UNFROZEN) {
> + drop_super(sb);
> + clear_bit(BD_FREEZE_OP, &bdev->bd_state);
> + return sb;
> + }
> +
> + if (sb)
> + drop_super(sb);
> +
> down(&bdev->bd_mount_sem);
> sb = get_super(bdev);
> if (sb && !(sb->s_flags & MS_RDONLY)) {
> @@ -219,6 +234,8 @@ struct super_block *freeze_bdev(struct b
> }
>
> sync_blockdev(bdev);
> + clear_bit(BD_FREEZE_OP, &bdev->bd_state);
> +

Please only clear BD_FREEZE_OP in thaw_bdev, that way you can also get
rid of the frozen check above, and the double-get_super. Also
bd_mount_sem could be removed that way by checking for BD_FREEZE_OP
in the unmount path.

> /*
> + * ioctl_freeze - Freeze the filesystem.
> + *
> + * @filp: target file
> + *
> + * Call freeze_bdev() to freeze the filesystem.
> + */

This is not a kerneldoc comment. But I think it can be simply removed
anyway, as it's a quite trivial function with static scope.

> +/*
> + * ioctl_thaw - Thaw the filesystem.
> + *
> + * @filp: target file
> + *
> + * Call thaw_bdev() to thaw the filesystem.
> + */

Same here.