On Thu, Oct 04, 2007 at 12:15:39PM -0700, akpm@xxxxxxxxxxxxxxxxxxxx wrote:
>
> The patch titled
> update sb->s_frozen when freezing read-only mounted device, too
> has been added to the -mm tree. Its filename is
> update-sb-s_frozen-when-freezing-read-only-mounted-device-too.patch
>
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>
> See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
> out what to do about this
>
> ------------------------------------------------------
> Subject: update sb->s_frozen when freezing read-only mounted device, too
> From: Akinobu Mita <akinobu.mita@xxxxxxxxx>
>
> freeze_bdev() with the device which is mounted as read only does not change
> sb->s_frozen from SB_UNFROZEN to SB_FREEZE_TRANS.
>
> Because of this behavior, xfs_freeze can break read-only XFS filesystem.
>
> Because xfs_thaw does nothing for the filesystem whose sb->s_frozen is
> SB_UNFROZEN. So freezed readonly XFS filesystem will never be unfreezed.
> Then we cannot do any unmount/remount operations for that filesystem.
>
> This patch updates sb->s_frozen when freeze_bdev() is called for read-only
> mounted device, too.
IMO, this is wrong.
By definition, a read only filesystem is the equivalent of a frozen
filesystem. i.e. freezing a rw filesystem temporarily puts it into
ro state until it is unfrozen. IOWs, freeze_bdev() should not return
with the bd_mount_sem if it's a read only filesystem because
it is a no-op and there's nothing for a thaw to do...
However, IMO the freeze/thaw stuff needs a rewrite - the way it uses
the bd_mount_sem makes it impossible to prevent racing XFS
freeze/thaw threads from getting screwed up as above. For every
freeze we must have a matching thaw due to the locking requirement,
but we have no reliable way of detecting whether it is safe to
call thaw_bdev() or not....
Perhaps freeze_bdev() could become a generic method (e.g.
generic_freeze_sb()) for a ->freeze superblock operation and
filesystems that care can implement there own, non-spagetti freeze
code.
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
|