Re: [PATCH] btrfs: Don't call btrfs_start_transaction() on frozen fs to avoid deadlock.

[David Sterba <dsterba@suse.cz>]

On Mon, Jan 19, 2015 at 03:42:41PM +0800, Qu Wenruo wrote:
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1000,6 +1000,14 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
>  			 */
>  			if (fs_info->pending_changes == 0)
>  				return 0;
> +			/*
> +			 * Test if the fs is frozen, or start_trasaction
> +			 * will deadlock on itself.
> +			 */
> +			if (__sb_start_write(sb, SB_FREEZE_FS, false))
> +				__sb_end_write(sb, SB_FREEZE_FS);
> +			else
> +				return 0;

The more I look into that the more I think that the first fix is the
right one.

Has been pointed out in this thread, it is ok to skip processing the
pending changes if the filesystem is frozen.

The pending changes have to flushed from sync (by design), we cannot use
mnt_want_write or the sb_start* protections that.

The btrfs_freeze callback can safely do the last commit, that's under
s_umount held by vfs::freeze_super. Then any other new transaction would
block. Any other call to btrfs_sync_fs will not find any active
transaction and with this patch will not start one. Sounds safe to me.

I think the right level to check is SB_FREEZE_WRITE though, to stop any
potential writes as soon as possible and when the s_umount lock is still
held in vfs::freeze_super.

I'll collect the relevant patches and will send it for review.


>  			trans = btrfs_start_transaction(root, 0);
>  		} else {
>  			return PTR_ERR(trans);