fsfreeze: fix sb vs bdev freeze/thaw b0rkage
Subsystems (4)
cover fsfreeze: fix sb vs bdev freeze/thaw b0rkage
The following series is for to address bugs in the emergency thawing code, as well as mismatcheѕ with freezing at the block layer and the superblock that break the freeze/thaw nesting order.
The first two patches fix the emergency thaw infinite loop reported by Jeff Merkey and the deadlock on sb->s_umount that the infinite loop hid. These may be stable kernel candidates.
The remainder of the patches address the bdev/sb mismatch and the fact that sb level freezing does not nest correctly. For all the places that the bdev interfaces are used, we need a superblock anyway so we may as well make freeze/thaw work only at the sb level. As such, this series moves all the nesting code to the sb from the bdev level and removes the freeze_bdev/thaw_bdev interfaces completely. It also converts the emergency thaw to work at the superblock level such that it will now thaw manually frozen filesystems.
A *big* outstanding problem still exists - freezing takes an active reference to the superblock, so unmounting an frozen filesystem has some nasty and unexpected side effects. The existing code results in an unmountable block device:
# mount /dev/vda /mnt/test # xfs_freeze -f /mnt/test # umount /mnt/test # grep test /proc/mounts # mkfs.xfs -f -l size=128m /dev/vda mkfs.xfs: /dev/vda contains a mounted filesystem Usage: mkfs.xfs .... # mount /dev/vda /mnt/test mount: /dev/vda already mounted or /mnt/test busy #
At this point I can't get access to /dev/vda and needs a reboot to get it and /mnt/test back.
This patch series results in the block device being mountable, but remains frozen across unmount/mount:
# mount /dev/vda /mnt/test
# xfs_freeze -f /mnt/test
# umount /mnt/test
# grep test /proc/mounts
# mkfs.xfs -f -l size=128m /dev/vda
mkfs.xfs: /dev/vda contains a mounted filesystem
Usage: mkfs.xfs
....
# mount /dev/vda /mnt/test
# touch /mnt/test/foo &
[1] 2647
#
# xfs_freeze -u /mnt/test
[1]+ Done sudo touch /mnt/test/foo
# umount /mnt/test
# mkfs.xfs -f -l size=128m /dev/vda
meta-data=/dev/vda isize=256 agcount=4, agsize=262144 blks
= sectsz=512 attr=2
data = bsize=4096 blocks=1048576, imaxpct=25
= sunit=0 swidth=0 blks
naming =version 2 bsize=4096 ascii-ci=0
log =internal log bsize=4096 blocks=32768, version=2
= sectsz=512 sunit=0 blks, lazy-count=1
realtime =none extsz=4096 blocks=0, rtextents=0
#This behaviour is only marginally better than the existing behaviour (at least you can release the references). However, I don't really like either option - we used to disallow umount on a frozen filesystems to avoid this problem.
So What is really supposed to happen when we unmount a frozen superblock? Should unmount return EBUSY? Should it be automatically thawed so it doesn't affect block device behaviour after unmount? Something else?
Cheers,
Dave.
PATCH 1/5 fsfreeze: Prevent emergency thaw from looping infinitely
From: Dave Chinner <[email protected]>
The thawing of a filesystem through sysrq-j loops infinitely as it incorrectly detects a thawed filesytsem as frozen and tries to unfreeze repeatedly. This is a regression caused by 4504230a71566785a05d3e6b53fa1ee071b864eb ("freeze_bdev: grab active reference to frozen superblocks") in that it no longer returned -EINVAL for superblocks that were not frozen.
Return -EINVAL when the filesystem is already unfrozen to avoid this problem.
--- fs/block_dev.c | 13 +++++-------- 1 files changed, 5 insertions(+), 8 deletions(-)
fs/block_dev.c +5 -8
@@ -276,22 +276,19 @@ int thaw_bdev(struct block_device *bdev, struct super_block *sb)if (!bdev->bd_fsfreeze_count) goto out; - error =0 ; -if (--bdev->bd_fsfreeze_count >0 ) +if (!sb) goto out; -if (!sb) + error =0 ; +if (--bdev->bd_fsfreeze_count >0 ) goto out; error =thaw_super (sb); -if (error) { +if (error) bdev->bd_fsfreeze_count++; -mutex_unlock (&bdev->bd_fsfreeze_mutex); -return error; - } out:mutex_unlock (&bdev->bd_fsfreeze_mutex); -return 0 ; +return error; }EXPORT_SYMBOL (thaw_bdev);
Signature · 1 line
1.7.1
Re: [PATCH 1/5] fsfreeze: Prevent emergency thaw from looping infinitely
Quoted 2 lines · Dave Chinner
Return
-EINVALwhen the filesystem is already unfrozen to avoid this problem.
This includes some additional changes in addition to the description, and at least one of them seems incorrect.
fs/block_dev.c int thaw_bdev(struct block_device *bdev, struct super_block *sb) · line 284
Quoted 9 lines
fs/block_dev.c +3
-3- error =0 ; -if (--bdev->bd_fsfreeze_count >0 ) +if (!sb) goto out; -if (!sb) + error =0 ; +if (--bdev->bd_fsfreeze_count >0 ) goto out;
Here you reorder the sb check to be before the counter decrement. But we do support calling freeze_bdev on a device without a superblock, and you would leak bd_fsfreeze_count for that case and wrongly return -EINVAL on unthaw for these now.
fs/block_dev.c int thaw_bdev(struct block_device *bdev, struct super_block *sb) · line 291
Quoted 7 lines
fs/block_dev.c +1
-4error =thaw_super (sb); -if (error) { +if (error) bdev->bd_fsfreeze_count++; -mutex_unlock (&bdev->bd_fsfreeze_mutex); -return error; - }
Ok, useful cleanup.
fs/block_dev.c int thaw_bdev(struct block_device *bdev, struct super_block *sb) · line 291
Quoted 4 lines
out:
fs/block_dev.c +1
-1mutex_unlock (&bdev->bd_fsfreeze_mutex); -return 0 ; +return error;
And this is the actual fix of course, also looks good.
Re: [PATCH 1/5] fsfreeze: Prevent emergency thaw from looping infinitely
Quoted 13 lines · Christoph Hellwig
Quoted 2 lines · Dave Chinner
Return
-EINVALwhen the filesystem is already unfrozen to avoid this problem.This includes some additional changes in addition to the description, and at least one of them seems incorrect.
Quoted 5 lines
fs/block_dev.c +1
-2- error =0 ; -if (--bdev->bd_fsfreeze_count >0 ) +if (!sb) goto out;
Quoted 9 lines
fs/block_dev.c int thaw_bdev(struct block_device *bdev, struct super_block *sb) · line 284
Quoted 4 lines
fs/block_dev.c +2
-1-if (!sb) + error =0 ; +if (--bdev->bd_fsfreeze_count >0 ) goto out;Here you reorder the sb check to be before the counter decrement. But we do support calling
freeze_bdevon a device without a superblock, and you would leakbd_fsfreeze_countfor that case and wrongly return-EINVALon unthaw for these now.
Ok, will fix it.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
PATCH 2/5 fsfreeze: emergency thaw will deadlock on s_umount
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
The emergency thaw process uses iterate_super() which holds the sb->s_umount lock in read mode. The current thaw_super() code takes the sb->s_umount lock in write mode, hence leading to an instant deadlock.
Pass the emergency state into the thaw_bdev/thaw_super code to avoid taking the s_umount lock in this case. We are running under the bdev freeze mutex, so this is still serialised against freeze despite only having a read lock on the sb->s_umount. Hence it should be safe to execute in this manner, especially given that emergency thaw is a rarely executed "get-out-of-jail" feature.
--- fs/block_dev.c | 26 ++++++++++++++++++++-- fs/buffer.c | 2 +- fs/super.c | 58 +++++++++++++++++++++++++++++++++++++++++++--------- include/linux/fs.h | 9 ++++++++ 4 files changed, 81 insertions(+), 14 deletions(-)
fs/block_dev.c +23 -3
@@ -262,13 +262,14 @@ struct super_block *freeze_bdev(struct block_device *bdev)EXPORT_SYMBOL (freeze_bdev);/** -* thaw_bdev -- unlock filesystem +* __thaw_bdev -- unlock filesystem * @bdev: blockdevice to unlock * @sb: associated superblock +* @emergency: emergency thaw * * Unlocks the filesystem and marks it writeable again after freeze_bdev(). */ -int thaw_bdev (struct block_device *bdev,struct super_block *sb) +static int __thaw_bdev (struct block_device *bdev,struct super_block *sb,int emergency) {int error = -EINVAL ;
@@ -283,15 +284,34 @@ int thaw_bdev(struct block_device *bdev, struct super_block *sb)if (--bdev->bd_fsfreeze_count >0 ) goto out; - error =thaw_super (sb); +if (emergency) + error =thaw_super_emergency (sb); +else + error =thaw_super (sb);if (error) bdev->bd_fsfreeze_count++; out:mutex_unlock (&bdev->bd_fsfreeze_mutex);return error; } +/** +* 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) +{ +return __thaw_bdev (bdev, sb,0 ); +}EXPORT_SYMBOL (thaw_bdev); +int thaw_bdev_emergency (struct block_device *bdev,struct super_block *sb) +{ +return __thaw_bdev (bdev, sb,1 ); +} +static int blkdev_writepage (struct page *page,struct writeback_control *wbc) {return block_write_full_page (page, blkdev_get_block, wbc);
fs/buffer.c +1 -1
@@ -564,7 +564,7 @@ repeat:static void do_thaw_one (struct super_block *sb,void *unused) {char b[BDEVNAME_SIZE ]; -while (sb->s_bdev && !thaw_bdev (sb->s_bdev, sb)) +while (sb->s_bdev && !thaw_bdev_emergency (sb->s_bdev, sb))printk (KERN_WARNING "Emergency Thaw on %s\n" ,bdevname (sb->s_bdev, b)); }
fs/super.c +48 -10
87 lines not on this page · raw message
include/linux/fs.h +9 -0
@@ -1803,6 +1803,7 @@ extern int iterate_mounts(int (*)(struct vfsmount *, void *), void *,extern int vfs_statfs (struct dentry *,struct kstatfs *);extern int freeze_super (struct super_block *super);extern int thaw_super (struct super_block *super); +extern int thaw_super_emergency (struct super_block *super);extern int current_umask (void );
@@ -1953,6 +1954,8 @@ extern int sync_blockdev(struct block_device *bdev);extern struct super_block *freeze_bdev (struct block_device *);extern void emergency_thaw_all (void );extern int thaw_bdev (struct block_device *bdev,struct super_block *sb); +extern int thaw_bdev_emergency (struct block_device *bdev, +struct super_block *sb);extern int fsync_bdev (struct block_device *);#else static inline void bd_forget (struct inode *inode) {}
@@ -1968,6 +1971,12 @@ static inline int thaw_bdev(struct block_device *bdev, struct super_block *sb) {return 0 ; } + +static inline int thaw_bdev_emergency (struct block_device *bdev, +struct super_block *sb) +{ +return 0 ; +}#endif extern int sync_filesystem (struct super_block *);extern const struct file_operations def_blk_fops;
Signature · 1 line
1.7.1
Re: [PATCH 2/5] fsfreeze: emergency thaw will deadlock on s_umount
Quoted 11 lines · Dave Chinner
The emergency thaw process uses
iterate_super()which holds thesb->s_umountlock in read mode. The currentthaw_super()code takes thesb->s_umountlock in write mode, hence leading to an instant deadlock.Pass the emergency state into the thaw_bdev/thaw_super code to avoid taking the
s_umountlock in this case. We are running under the bdev freeze mutex, so this is still serialised against freeze despite only having a read lock on thesb->s_umount. Hence it should be safe to execute in this manner, especially given that emergency thaw is a rarely executed "get-out-of-jail" feature.
This is correct as long as no one calls thaw_super directly, which is not the case currently.
Re: [PATCH 2/5] fsfreeze: emergency thaw will deadlock on s_umount
Quoted 13 lines · Christoph Hellwig
Quoted 11 lines · Dave Chinner
The emergency thaw process uses
iterate_super()which holds thesb->s_umountlock in read mode. The currentthaw_super()code takes thesb->s_umountlock in write mode, hence leading to an instant deadlock.Pass the emergency state into the thaw_bdev/thaw_super code to avoid taking the
s_umountlock in this case. We are running under the bdev freeze mutex, so this is still serialised against freeze despite only having a read lock on thesb->s_umount. Hence it should be safe to execute in this manner, especially given that emergency thaw is a rarely executed "get-out-of-jail" feature.
Quoted 2 lines
This is correct as long as no one calls
thaw_superdirectly, which is not the case currently.
Yeah, the idea of the first two patches is that they can be applied to a current tree and backported and prevent the infinite loop or deadlock. The problem of thaw_bdev/thaw_super is what the rest of the patches are supposed to address.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 2/5] fsfreeze: emergency thaw will deadlock on s_umount
In reply to Christoph HellwigQuoted 13 lines · Christoph Hellwig
Quoted 11 lines · Dave Chinner
The emergency thaw process uses
iterate_super()which holds thesb->s_umountlock in read mode. The currentthaw_super()code takes thesb->s_umountlock in write mode, hence leading to an instant deadlock.Pass the emergency state into the thaw_bdev/thaw_super code to avoid taking the
s_umountlock in this case. We are running under the bdev freeze mutex, so this is still serialised against freeze despite only having a read lock on thesb->s_umount. Hence it should be safe to execute in this manner, especially given that emergency thaw is a rarely executed "get-out-of-jail" feature.
Quoted 2 lines
This is correct as long as no one calls
thaw_superdirectly, which is not the case currently.
This patch doesn't try to deal with the bdev/super mismatches; all it does is prevent an obvious deadlock. Calling freeze/thaw_super directly will serialise on the s_umount lock, calling freeze/thaw_bdev() will serialise on the bdev freeze mutex, and if we mix the two they'll serialise on the s_umount lock. So I think with this patch serialisation will still occur correctly but avoid the current deadlock.
I'll change the commit message to explain this better.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 2/5] fsfreeze: emergency thaw will deadlock on s_umount
Quoted 9 lines · Dave Chinner
This patch doesn't try to deal with the bdev/super mismatches; all it does is prevent an obvious deadlock. Calling freeze/thaw_super directly will serialise on the
s_umountlock, calling freeze/thaw_bdev() will serialise on the bdev freeze mutex, and if we mix the two they'll serialise on thes_umountlock. So I think with this patch serialisation will still occur correctly but avoid the current deadlock.I'll change the commit message to explain this better.
I don;t think the explanation alone is enough.
Right now thaw_super itself is only serialized by exclusive shareds_umount. thaw_bdev it also serialized by bd_fsfreeze_mutex, but
there are callers of thaw_super that do not go through thaw_bdev, so our locking is not enough here.
PATCH 3/5 fsfreeze: freeze_super and thaw_bdev don't play well together
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
thaw_bdev() has re-enterency guards to allow freezes to nest together. That is, it ensures that the filesystem is not thawed until the last thaw command is issued. This is needed to prevent the filesystem from being unfrozen while an existing freezer is still operating on the filesystem in a frozen state (e.g. dm-snapshot).
Currently, freeze_super() and thaw_super() bypasses these guards, and as a result manual freezing and unfreezing via the ioctl methods do not nest correctly. hence mixing userspace directed freezes with block device level freezes result in inconsistency due to premature thawing of the filesystem.
Move the re-enterency guards to the superblock and into freeze_super and thaw_super() so that userspace directed freezes nest correctly again.
--- fs/block_dev.c | 59 ++++---------------------------------- fs/gfs2/ops_fstype.c | 12 -------- fs/super.c | 77 ++++++++++++++++++++++++++++++------------------- include/linux/fs.h | 8 ++--- 4 files changed, 56 insertions(+), 100 deletions(-)
fs/block_dev.c +6 -53
@@ -230,71 +230,22 @@ struct super_block *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); -drop_super (sb); -mutex_unlock (&bdev->bd_fsfreeze_mutex); -return sb; - } - sb =get_active_super (bdev);if (!sb) goto out; error =freeze_super (sb);if (error) {deactivate_super (sb); - bdev->bd_fsfreeze_count--; -mutex_unlock (&bdev->bd_fsfreeze_mutex);return ERR_PTR (error); }deactivate_super (sb); out:sync_blockdev (bdev); -mutex_unlock (&bdev->bd_fsfreeze_mutex);return sb;/* thaw_bdev releases s->s_umount */ }EXPORT_SYMBOL (freeze_bdev);/** -* __thaw_bdev -- unlock filesystem -* @bdev: blockdevice to unlock -* @sb: associated superblock -* @emergency: emergency thaw -* -* Unlocks the filesystem and marks it writeable again after freeze_bdev(). -*/ -static int __thaw_bdev (struct block_device *bdev,struct super_block *sb,int emergency) -{ -int error = -EINVAL ; - -mutex_lock (&bdev->bd_fsfreeze_mutex); -if (!bdev->bd_fsfreeze_count) - goto out; - -if (!sb) - goto out; - - error =0 ; -if (--bdev->bd_fsfreeze_count >0 ) - goto out; - -if (emergency) - error =thaw_super_emergency (sb); -else - error =thaw_super (sb); -if (error) - bdev->bd_fsfreeze_count++; -out: -mutex_unlock (&bdev->bd_fsfreeze_mutex); -return error; -} -/** * thaw_bdev -- unlock filesystem * @bdev: blockdevice to unlock * @sb: associated superblock
@@ -303,13 +254,17 @@ out:*/ int thaw_bdev (struct block_device *bdev,struct super_block *sb) { -return __thaw_bdev (bdev, sb,0 ); +if (!sb) +return -EINVAL ; +return thaw_super (sb); }EXPORT_SYMBOL (thaw_bdev);int thaw_bdev_emergency (struct block_device *bdev,struct super_block *sb) { -return __thaw_bdev (bdev, sb,1 ); +if (!sb) +return -EINVAL ; +return thaw_super_emergency (sb); }static int blkdev_writepage (struct page *page,struct writeback_control *wbc)
@@ -434,8 +389,6 @@ static void init_once(void *foo)INIT_LIST_HEAD (&bdev->bd_holder_list);#endif inode_init_once (&ei->vfs_inode); -/* Initialize mutex for freeze. */ -mutex_init (&bdev->bd_fsfreeze_mutex); }static inline void __bd_forget (struct inode *inode)
fs/gfs2/ops_fstype.c +0 -12
@@ -1304,19 +1304,7 @@ static int gfs2_get_sb(struct file_system_type *fs_type, int flags,if (IS_ERR (bdev))return PTR_ERR (bdev); -/* -* once the super is inserted into the list by sget, s_umount -* will protect the lockfs code from trying to start a snapshot -* while we are mounting -*/ -mutex_lock (&bdev->bd_fsfreeze_mutex); -if (bdev->bd_fsfreeze_count >0 ) { -mutex_unlock (&bdev->bd_fsfreeze_mutex); - error = -EBUSY ; - goto error_bdev; - } s =sget (fs_type, test_gfs2_super, set_gfs2_super, bdev); -mutex_unlock (&bdev->bd_fsfreeze_mutex); error =PTR_ERR (s);if (IS_ERR (s)) goto error_bdev;
fs/super.c +47 -30
147 lines not on this page · raw message
include/linux/fs.h +3 -5
@@ -672,11 +672,6 @@ struct block_device {* care to not mess up bd_private for that case. */ unsigned long bd_private; - -/* The counter of freeze processes */ -int bd_fsfreeze_count; -/* Mutex for freeze */ -struct mutex bd_fsfreeze_mutex; };/*
@@ -1382,6 +1377,9 @@ struct super_block {* generic_show_options() */ char *s_options; + +int s_freeze_count;/* nr of nested freezes */ +struct mutex s_freeze_mutex;/* nesting lock */ };extern struct timespec current_fs_time (struct super_block *sb);
Signature · 1 line
1.7.1
Re: [PATCH 3/5] fsfreeze: freeze_super and thaw_bdev don't play well together
This breaks the "feature" that we can freeze a block device that doesn't have a filesystem mounted yet. For filesystems using get_sb_bdev that prevents a new filesystem to be mounted on them.
I'm not sure it's a particularly useful feature, but it's been there since day 1 of the freeze support. The easiest way to not break it would be to keep the per-sb freeze count only for that case and only check it during mount.
Re: [PATCH 3/5] fsfreeze: freeze_super and thaw_bdev don't play well together
Quoted 8 lines · Christoph Hellwig
This breaks the "feature" that we can freeze a block device that doesn't have a filesystem mounted yet. For filesystems using
get_sb_bdevthat prevents a new filesystem to be mounted on them.I'm not sure it's a particularly useful feature, but it's been there since day 1 of the freeze support. The easiest way to not break it would be to keep the per-sb freeze count only for that case and only check it during mount.
You mean the per-bdev freeze count, right? So freeze/thaw_bdev would have to remain?
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 3/5] fsfreeze: freeze_super and thaw_bdev don't play well together
Quoted 5 lines · Dave Chinner
Quoted 4 lines · Christoph Hellwig
This breaks the "feature" that we can freeze a block device that doesn't have a filesystem mounted yet. For filesystems using
get_sb_bdevthat prevents a new filesystem to be mounted on them.
Quoted 7 lines
Quoted 4 lines
I'm not sure it's a particularly useful feature, but it's been there since day 1 of the freeze support. The easiest way to not break it would be to keep the per-sb freeze count only for that case and only check it during mount.
You mean the per-bdev freeze count, right? So freeze/thaw_bdev would have to remain?
Yes.
PATCH 4/5 fsfreeze: switch to using super methods everywhere
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
freeze/thaw_bdev are now just trivial wrappers around freeze/thaw_super(). Convert all users of the bdev interfaces to use the superblock interfaces instead, and remove the bdev interfaces.
--- drivers/md/dm.c | 13 +++++++---- fs/block_dev.c | 54 ---------------------------------------------------- fs/buffer.c | 2 +- fs/super.c | 1 + fs/xfs/xfs_fsops.c | 10 ++------ include/linux/fs.h | 19 ------------------ 6 files changed, 13 insertions(+), 86 deletions(-)
drivers/md/dm.c +8 -5
@@ -2427,15 +2427,18 @@ static int lock_fs(struct mapped_device *md)WARN_ON (md->frozen_sb); - md->frozen_sb =freeze_bdev (md->bdev); -if (IS_ERR (md->frozen_sb)) { - r =PTR_ERR (md->frozen_sb); + md->frozen_sb =get_active_super (md->bdev); +if (!md->frozen_sb) +return -EIO ; + r =freeze_super (md->frozen_sb); +if (r) { +deactivate_super (md->frozen_sb); md->frozen_sb =NULL ;return r; } +deactivate_super (md->frozen_sb);set_bit (DMF_FROZEN , &md->flags); -return 0 ; }
@@ -2444,7 +2447,7 @@ static void unlock_fs(struct mapped_device *md)if (!test_bit (DMF_FROZEN , &md->flags))return ; -thaw_bdev (md->bdev, md->frozen_sb); +thaw_super (md->frozen_sb); md->frozen_sb =NULL ;clear_bit (DMF_FROZEN , &md->flags); }
fs/block_dev.c +0 -54
60 lines not on this page · raw message
fs/buffer.c +1 -1
@@ -564,7 +564,7 @@ repeat:static void do_thaw_one (struct super_block *sb,void *unused) {char b[BDEVNAME_SIZE ]; -while (sb->s_bdev && !thaw_bdev_emergency (sb->s_bdev, sb)) +while (sb->s_bdev && !thaw_super_emergency (sb))printk (KERN_WARNING "Emergency Thaw on %s\n" ,bdevname (sb->s_bdev, b)); }
fs/super.c +1 -0
@@ -976,6 +976,7 @@ int freeze_super(struct super_block *sb) } } out_active: +sync_blockdev (sb->s_bdev);up_write (&sb->s_umount); out_unlock:mutex_unlock (&sb->s_freeze_mutex);
fs/xfs/xfs_fsops.c +3 -7
@@ -642,16 +642,12 @@ xfs_fs_goingdown( __uint32_t inflags) {switch (inflags) { -case XFS_FSOP_GOING_FLAGS_DEFAULT : { -struct super_block *sb =freeze_bdev (mp->m_super->s_bdev); - -if (sb && !IS_ERR (sb)) { +case XFS_FSOP_GOING_FLAGS_DEFAULT : +if (!freeze_super (mp->m_super)) {xfs_force_shutdown (mp,SHUTDOWN_FORCE_UMOUNT ); -thaw_bdev (sb->s_bdev, sb); +thaw_super (mp->m_super); } -break ; - }case XFS_FSOP_GOING_FLAGS_LOGFLUSH :xfs_force_shutdown (mp,SHUTDOWN_FORCE_UMOUNT );break ;
include/linux/fs.h +0 -19
@@ -1951,30 +1951,11 @@ extern void invalidate_bdev(struct block_device *);extern int sync_blockdev (struct block_device *bdev);extern struct super_block *freeze_bdev (struct block_device *);extern void emergency_thaw_all (void ); -extern int thaw_bdev (struct block_device *bdev,struct super_block *sb); -extern int thaw_bdev_emergency (struct block_device *bdev, -struct super_block *sb);extern int fsync_bdev (struct block_device *);#else static inline void bd_forget (struct inode *inode) {}static inline int sync_blockdev (struct block_device *bdev) {return 0 ; }static inline void invalidate_bdev (struct block_device *bdev) {} - -static inline struct super_block *freeze_bdev (struct block_device *sb) -{ -return NULL ; -} - -static inline int thaw_bdev (struct block_device *bdev,struct super_block *sb) -{ -return 0 ; -} - -static inline int thaw_bdev_emergency (struct block_device *bdev, -struct super_block *sb) -{ -return 0 ; -}#endif extern int sync_filesystem (struct super_block *);extern const struct file_operations def_blk_fops;
Signature · 1 line
1.7.1
Re: [PATCH 4/5] fsfreeze: switch to using super methods everywhere
Quoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
freeze/thaw_bdev are now just trivial wrappers around freeze/thaw_super(). Convert all users of the bdev interfaces to use the superblock interfaces instead, and remove the bdev interfaces.
This requires exporting get_active_super. Buit if we want to keep suppoting freezing of unmounted block devices we need to keep the helpers anyway.
PATCH 5/5 fsfreeze: move emergency thaw code to fs/super.c
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
It makes no sense having the emergency thaw code in fs/buffer.c when all of it's operations are one superblocks and the code it executes is all in fs/super.c. Move the code there and clean it up.
--- fs/buffer.c | 31 ------------------------------- fs/super.c | 37 +++++++++++++++++++++++++++++++++---- include/linux/fs.h | 1 - 3 files changed, 33 insertions(+), 36 deletions(-)
fs/buffer.c +0 -31
@@ -561,37 +561,6 @@ repeat:return err; } -static void do_thaw_one (struct super_block *sb,void *unused) -{ -char b[BDEVNAME_SIZE ]; -while (sb->s_bdev && !thaw_super_emergency (sb)) -printk (KERN_WARNING "Emergency Thaw on %s\n" , -bdevname (sb->s_bdev, b)); -} - -static void do_thaw_all (struct work_struct *work) -{ -iterate_supers (do_thaw_one,NULL ); -kfree (work); -printk (KERN_WARNING "Emergency Thaw complete\n" ); -} - -/** -* emergency_thaw_all -- forcibly thaw every frozen filesystem -* -* Used for emergency unfreeze of all filesystems via SysRq -*/ -void emergency_thaw_all (void ) -{ -struct work_struct *work; - - work =kmalloc (sizeof (*work),GFP_ATOMIC ); -if (work) { -INIT_WORK (work, do_thaw_all); -schedule_work (work); - } -} -/** * sync_mapping_buffers - write out & wait upon a mapping's "associated" buffers * @mapping: the mapping which wants those buffers written
fs/super.c +33 -4
@@ -1059,7 +1059,7 @@ out_unlock: }/** -* thaw_super -- unlock filesystem +* __thaw_super -- unlock filesystem * @sb: the super to thaw * * Unlocks the filesystem and marks it writeable again after freeze_super().
@@ -1075,11 +1075,40 @@ EXPORT_SYMBOL(thaw_super);* @sb: the super to thaw * * Unlocks the filesystem and marks it writeable again after freeze_super(). -* This avoids taking the s_umount lock if it is already held. +* This avoids taking the s_umount lock because it is already held. +*/ +static void thaw_super_emergency (struct super_block *sb,void *unused) +{ + +if (sb->s_bdev) { +char b[BDEVNAME_SIZE ]; +printk (KERN_WARNING "Emergency Thaw on %s.\n" , +bdevname (sb->s_bdev, b)); + } +while (!__thaw_super (sb,1 )); +} + +static void do_thaw_all (struct work_struct *work) +{ +iterate_supers (thaw_super_emergency,NULL ); +kfree (work); +printk (KERN_WARNING "Emergency Thaw complete\n" ); +} + +/** +* emergency_thaw_all -- forcibly thaw every frozen filesystem +* +* Used for emergency unfreeze of all filesystems via SysRq */ -int thaw_super_emergency (struct super_block *sb) +void emergency_thaw_all (void ) { -return __thaw_super (sb,1 ); +struct work_struct *work; + + work =kmalloc (sizeof (*work),GFP_ATOMIC ); +if (work) { +INIT_WORK (work, do_thaw_all); +schedule_work (work); + } }static struct vfsmount *fs_set_subtype (struct vfsmount *mnt,const char *fstype)
include/linux/fs.h +0 -1
@@ -1801,7 +1801,6 @@ extern int iterate_mounts(int (*)(struct vfsmount *, void *), void *,extern int vfs_statfs (struct dentry *,struct kstatfs *);extern int freeze_super (struct super_block *super);extern int thaw_super (struct super_block *super); -extern int thaw_super_emergency (struct super_block *super);extern int current_umask (void );
Signature · 1 line
1.7.1
Re: [PATCH 5/5] fsfreeze: move emergency thaw code to fs/super.c
Quoted 6 lines · Dave Chinner
From: Dave Chinner <[email protected]>
It makes no sense having the emergency thaw code in
fs/buffer.cwhen all of it's operations are one superblocks and the code it executes is all infs/super.c. Move the code there and clean it up.
fs/super.c EXPORT_SYMBOL(thaw_super); · line 1088
Quoted 9 lines
fs/super.c +9
-0+static void thaw_super_emergency (struct super_block *sb,void *unused) +{ + +if (sb->s_bdev) { +char b[BDEVNAME_SIZE ]; +printk (KERN_WARNING "Emergency Thaw on %s.\n" , +bdevname (sb->s_bdev, b)); + } +while (!__thaw_super (sb,1 ));
Please move the ; to a separate line to make the loop better redable.
Quoted 71 lines · Dave Chinner
The following series is for to address bugs in the emergency thawing code, as well as mismatcheѕ with freezing at the block layer and the superblock that break the freeze/thaw nesting order.
The first two patches fix the emergency thaw infinite loop reported by Jeff Merkey and the deadlock on
sb->s_umountthat the infinite loop hid. These may be stable kernel candidates.The remainder of the patches address the bdev/sb mismatch and the fact that sb level freezing does not nest correctly. For all the places that the bdev interfaces are used, we need a superblock anyway so we may as well make freeze/thaw work only at the sb level. As such, this series moves all the nesting code to the sb from the bdev level and removes the freeze_bdev/thaw_bdev interfaces completely. It also converts the emergency thaw to work at the superblock level such that it will now thaw manually frozen filesystems.
A *big* outstanding problem still exists - freezing takes an active reference to the superblock, so unmounting an frozen filesystem has some nasty and unexpected side effects. The existing code results in an unmountable block device:
# mount /dev/vda /mnt/test # xfs_freeze -f /mnt/test # umount /mnt/test # grep test /proc/mounts # mkfs.xfs -f -l size=128m /dev/vda mkfs.xfs: /dev/vda contains a mounted filesystem Usage: mkfs.xfs .... # mount /dev/vda /mnt/test mount: /dev/vda already mounted or /mnt/test busy #At this point I can't get access to
/dev/vdaand needs a reboot to get it and/mnt/testback.This patch series results in the block device being mountable, but remains frozen across unmount/mount:
# mount /dev/vda /mnt/test # xfs_freeze -f /mnt/test # umount /mnt/test # grep test /proc/mounts # mkfs.xfs -f -l size=128m /dev/vda mkfs.xfs: /dev/vda contains a mounted filesystem Usage: mkfs.xfs .... # mount /dev/vda /mnt/test # touch /mnt/test/foo & [1] 2647 # # xfs_freeze -u /mnt/test [1]+ Done sudo touch /mnt/test/foo # umount /mnt/test # mkfs.xfs -f -l size=128m /dev/vda meta-data=/dev/vda isize=256 agcount=4, agsize=262144 blks = sectsz=512 attr=2 data = bsize=4096 blocks=1048576, imaxpct=25 = sunit=0 swidth=0 blks naming =version 2 bsize=4096 ascii-ci=0 log =internal log bsize=4096 blocks=32768, version=2 = sectsz=512 sunit=0 blks, lazy-count=1 realtime =none extsz=4096 blocks=0, rtextents=0 #This behaviour is only marginally better than the existing behaviour (at least you can release the references). However, I don't really like either option - we used to disallow umount on a frozen filesystems to avoid this problem.
Quoted 5 lines
So What is really supposed to happen when we unmount a frozen superblock? Should unmount return EBUSY? Should it be automatically thawed so it doesn't affect block device behaviour after unmount? Something else?
My vote is for EBUSY, that way we don't get this weird unexpected behavior thing. Plus if dm is doing a snapshot or whatever, unfreezing the fs to let it unmount in the middle of it doing its thing could be unfun. Thanks,
Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html