fs: file freeze support

We introduce per-file freeze feature for unifying defrag ext4 and xfs as first ingredient. We get the idea courtesy of Dave Chinner
(https://lkml.org/lkml/2014/7/14/759)
per-file freeze will be used to avoid that file is not modified while userspace is doing the defrag.

This patch tries to implement file write freeze functionality. Introduce new inode state, I_WRITE_FREEZED.
When this state is set, any process which tries to modify the file's address space, either by pagefault mmap writes or using write(2), will block until the this state is cleared. I_WRITE_FREEZED is set by calling FS_IOC_FWFREEZE ioctl and clear by FS_IOC_FWTHAW ioctl.

File write freeze functionality, when used in conjunction with inode's immutable flag can be used for creating truly stable file snapshots wherein write freeze will prevent any modification to the file from already open file descriptors and immutable flag will prevent any new modification to the file. One of the intended uses for stable file snapshots would be in the defragmentation applications which defrags single file.

For implementation purpose, initially we tried to keep percpu usage counters inside struct inode just like there is struct sb_writers in super_block. But considering that it will significantly bloat up struct inode when actually the usage of file write freeze will be infrequent, we dropped this idea. Instead we have tried to use already present filesystem freezing infrastructure. Current approach makes it possible for implementing file write freeze without bloating any of struct super_block/inode.
In FS_IOC_FWFREEZE, we wait for complete fs to be frozen, set I_WRITE_FREEZED to inode's state and unfreeze the fs.

TODO : prevent inode eviction when I_WRITE_FREEZED state is set. TODO : xfstests testcase for file freeze.

Signed-off-by Namjae Jeon <[email protected]>

Signed-off-by Ashish Sangwan <[email protected]>

---
 fs/btrfs/inode.c        |  1 +
 fs/buffer.c             |  1 +
 fs/ext4/inode.c         |  1 +
 fs/f2fs/file.c          |  1 +
 fs/gfs2/file.c          |  1 +
 fs/inode.c              | 19 ++++++++++++++++++
 fs/ioctl.c              | 30 +++++++++++++++++++++++++++++
 fs/nilfs2/file.c        |  1 +
 fs/ocfs2/mmap.c         |  1 +
 fs/super.c              | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h      | 13 +++++++++++++
 include/uapi/linux/fs.h |  2 ++
 mm/filemap.c            |  1 +
 13 files changed, 123 insertions(+)

fs/btrfs/inode.c +1 -0

index e687bb0..357c749 100644
@@ -8262,6 +8262,7 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	u64 page_start;
 	u64 page_end;
 
+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);
 	ret  = btrfs_delalloc_reserve_space(inode, PAGE_CACHE_SIZE);
 	if (!ret) {

fs/buffer.c +1 -0

index 20805db..966d50b 100644
@@ -2444,6 +2444,7 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
 	int ret;
 	struct super_block *sb = file_inode(vma->vm_file)->i_sb;
 
+	inode_start_write(file_inode(vma->vm_file));
 	sb_start_pagefault(sb);
 
 	/*

fs/ext4/inode.c +1 -0

index 5653fa4..7e1e41e 100644
@@ -4981,6 +4981,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	get_block_t *get_block;
 	int retries = 0;
 
+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);
 	file_update_time(vma->vm_file);
 	/* Delalloc case is easy... */

fs/f2fs/file.c +0 -0

index f172ddc4..9e23f8d 100644

fs/f2fs/file.c +0 -0

 	f2fs_bug_on(sbi, f2fs_has_inline_data(inode));

fs/gfs2/file.c +1 -0

index 6e600ab..bddf66b 100644
@@ -393,6 +393,7 @@ static int gfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	loff_t size;
 	int ret;
 
+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);
 
 	/* Update file times before taking page lock */

fs/inode.c +0 -0

index aa149e7..82a96d0 100644

fs/inode.c +12 -0

+	if (inode->i_state & I_WRITE_FREEZED) {
+		DEFINE_WAIT(wait);
+
+		prepare_to_wait(&sb->s_writers.wait_unfrozen, &wait,
+				TASK_UNINTERRUPTIBLE);
+		spin_unlock(&inode->i_lock);
+		schedule();
+		finish_wait(&sb->s_writers.wait_unfrozen, &wait);
+		goto retry;
+	}
+	spin_unlock(&inode->i_lock);
+}

fs/ioctl.c +22 -0

index 214c3c1..c8e9ae3 100644
@@ -540,6 +540,28 @@ static int ioctl_fsthaw(struct file *filp)
 	return thaw_super(sb);
 }
 
+static int ioctl_filefreeze(struct file *filp)
+{
+	struct inode *inode = file_inode(filp);
+
+	if (!inode_owner_or_capable(inode))
+		return -EPERM;
+
+	/* Freeze */
+	return file_write_freeze(inode);
+}
 
+
+static int ioctl_filethaw(struct file *filp)
+{
+	struct inode *inode = file_inode(filp);
+
+	if (!inode_owner_or_capable(inode))
+		return -EPERM;
+
+	/* Thaw */
+	return file_write_unfreeze(inode);
+}
+
 /*
  * When you add any new common ioctls to the switches above and below

* please update compat_sys_ioctl() too.

fs/ioctl.c +8 -0

@@ -589,6 +611,14 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
 		error = ioctl_fsthaw(filp);
 		break;
 
+	case FS_IOC_FWFREEZE:
+		error = ioctl_filefreeze(filp);
+		break;
+
+	case FS_IOC_FWTHAW:
+		error = ioctl_filethaw(filp);
+		break;
+
 	case FS_IOC_FIEMAP:
 		return ioctl_fiemap(filp, arg);
 

fs/nilfs2/file.c +1 -0

index 3a03e0a..5110d9d 100644
@@ -66,6 +66,7 @@ static int nilfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (unlikely(nilfs_near_disk_full(inode->i_sb->s_fs_info)))
 		return VM_FAULT_SIGBUS; /* -ENOSPC */
 
+	inode_start_write(file_inode(vma->vm_file));
 	sb_start_pagefault(inode->i_sb);
 	lock_page(page);
 	if (page->mapping != inode->i_mapping ||

fs/ocfs2/mmap.c +1 -0

index 10d66c7..d073fc2 100644
@@ -136,6 +136,7 @@ static int ocfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	sigset_t oldset;
 	int ret;
 
+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);
 	ocfs2_block_signals(&oldset);
 

fs/super.c +0 -0

index eae088f..5e44e42 100644

fs/super.c +50 -0

+int file_write_freeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	int ret = 0;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (inode->i_state & I_WRITE_FREEZED)
+		ret = -EBUSY;
+	spin_unlock(&inode->i_lock);
+	if (ret)
+		return ret;
+
+	ret = freeze_super(sb);
+	if (ret)
+		return ret;
+
+	spin_lock(&inode->i_lock);
+	inode->i_state |= I_WRITE_FREEZED;
+	smp_wmb();
+	spin_unlock(&inode->i_lock);
+
+	return thaw_super(sb);
+}
+EXPORT_SYMBOL(file_write_freeze);
+
+int file_write_unfreeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (!(inode->i_state & I_WRITE_FREEZED)) {
+		spin_unlock(&inode->i_lock);
+		return -EINVAL;
+	}
+
+	inode->i_state &= ~I_WRITE_FREEZED;
+	smp_wmb();
+	wake_up(&sb->s_writers.wait_unfrozen);
+	spin_unlock(&inode->i_lock);
+	return 0;
+}
+EXPORT_SYMBOL(file_write_unfreeze);

include/linux/fs.h +13 -0

index 8639770..13eba85 100644
@@ -1313,8 +1313,12 @@ extern struct timespec current_fs_time(struct super_block *sb);
  * Snapshotting support.
  */
 
+int file_write_freeze(struct inode *inode);
+int file_write_unfreeze(struct inode *inode);
+
 void __sb_end_write(struct super_block *sb, int level);
 int __sb_start_write(struct super_block *sb, int level, bool wait);
+void inode_start_write(struct inode *inode);
 
 /**
  * sb_end_write - drop write access to a superblock

@@ -1730,6 +1734,12 @@ struct super_operations {
  *
  * I_DIO_WAKEUP		Never set.  Only used as a key for wait_on_bit().
  *
+ * I_WRITE_FREEZED	This bit is set by calling file_write_freeze ioctl and
+ *			unset by file_write_thaw ioctl. If this is set, the
+ *			processes which have already open the file and try to
+ *			modify the file address space will block until the bit
+ *			is cleared.
+ *
  * Q: What is the difference between I_WILL_FREE and I_FREEING?
  */
 #define I_DIRTY_SYNC		(1 << 0)

@@ -1746,6 +1756,7 @@ struct super_operations {
 #define __I_DIO_WAKEUP		9
 #define I_DIO_WAKEUP		(1 << I_DIO_WAKEUP)
 #define I_LINKABLE		(1 << 10)
+#define I_WRITE_FREEZED	(1 << 11)
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
 

@@ -2321,6 +2332,7 @@ static inline void file_start_write(struct file *file)
 {
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return;
+	inode_start_write(file_inode(file));
 	__sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, true);
 }
 

@@ -2328,6 +2340,7 @@ static inline bool file_start_write_trylock(struct file *file)
 {
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return true;
+	inode_start_write(file_inode(file));
 	return __sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, false);
 }
 

include/uapi/linux/fs.h +2 -0

index 3735fa0..3f529a7 100644
@@ -167,6 +167,8 @@ struct inodes_stat_t {
 #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
 #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
 #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
+#define FS_IOC_FWFREEZE		_IOWR('X', 122, int) /* File Freeze */
+#define FS_IOC_FWTHAW			_IOWR('X', 123, int) /* File Thaw */
 
 /*
  * Inode flags (FS_IOC_GETFLAGS / FS_IOC_SETFLAGS)

mm/filemap.c +1 -0

index bd8543c..b07a873 100644
@@ -2066,6 +2066,7 @@ int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	struct inode *inode = file_inode(vma->vm_file);
 	int ret = VM_FAULT_LOCKED;
 
+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);
 	file_update_time(vma->vm_file);
 	lock_page(page);

Signature · 1 line

1.8.5.5

Quoted 10 lines

For implementation purpose, initially we tried to keep percpu usage counters inside struct inode just like there is struct sb_writers in super_block. But considering that it will significantly bloat up struct inode when actually the usage of file write freeze will be infrequent, we dropped this idea. Instead we have tried to use already present filesystem freezing infrastructure. Current approach makes it possible for implementing file write freeze without bloating any of struct super_block/inode.
In FS_IOC_FWFREEZE, we wait for complete fs to be frozen, set I_WRITE_FREEZED to inode's state and unfreeze the fs.

Looks interesting. I have added some comments below.

fs/f2fs/file.c static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma, · line 44

Quoted 8 lines

fs/f2fs/file.c +1 -0

@@ -40,6 +40,7 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 
 	f2fs_balance_fs(sbi);
 
+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);

IMHO it is reasonable to fold sb_start_{write,pagefault}, to inode_start_{write,pagefault}

fs/inode.c void inode_set_flags(struct inode *inode, unsigned int flags, · line 1938

Quoted 7 lines

fs/inode.c +6 -0

+void inode_start_write(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+retry:
+	spin_lock(&inode->i_lock);

This means that i_lock will be acquired on each mkpage_write for all users who do not care about fsfreeze which result smp performance drawback It is reasonable to add lockless test first because flag is set while whole fs is frozen so we can not enter this routine.

Quoted 91 lines

fs/inode.c +12 -0

+	if (inode->i_state & I_WRITE_FREEZED) {
+		DEFINE_WAIT(wait);
+
+		prepare_to_wait(&sb->s_writers.wait_unfrozen, &wait,
+				TASK_UNINTERRUPTIBLE);
+		spin_unlock(&inode->i_lock);
+		schedule();
+		finish_wait(&sb->s_writers.wait_unfrozen, &wait);
+		goto retry;
+	}
+	spin_unlock(&inode->i_lock);
+}

fs/ioctl.c +22 -0

index 214c3c1..c8e9ae3 100644
@@ -540,6 +540,28 @@ static int ioctl_fsthaw(struct file *filp)
 	return thaw_super(sb);
 }
 
+static int ioctl_filefreeze(struct file *filp)
+{
+	struct inode *inode = file_inode(filp);
+
+	if (!inode_owner_or_capable(inode))
+		return -EPERM;
+
+	/* Freeze */
+	return file_write_freeze(inode);
+}
 
+
+static int ioctl_filethaw(struct file *filp)
+{
+	struct inode *inode = file_inode(filp);
+
+	if (!inode_owner_or_capable(inode))
+		return -EPERM;
+
+	/* Thaw */
+	return file_write_unfreeze(inode);
+}
+
 /*
  * When you add any new common ioctls to the switches above and below

* please update compat_sys_ioctl() too.

fs/ioctl.c +8 -0

@@ -589,6 +611,14 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
 		error = ioctl_fsthaw(filp);
 		break;
 
+	case FS_IOC_FWFREEZE:
+		error = ioctl_filefreeze(filp);
+		break;
+
+	case FS_IOC_FWTHAW:
+		error = ioctl_filethaw(filp);
+		break;
+
 	case FS_IOC_FIEMAP:
 		return ioctl_fiemap(filp, arg);
 

fs/nilfs2/file.c +1 -0

index 3a03e0a..5110d9d 100644
@@ -66,6 +66,7 @@ static int nilfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (unlikely(nilfs_near_disk_full(inode->i_sb->s_fs_info)))
 		return VM_FAULT_SIGBUS; /* -ENOSPC */
 
+	inode_start_write(file_inode(vma->vm_file));
 	sb_start_pagefault(inode->i_sb);
 	lock_page(page);
 	if (page->mapping != inode->i_mapping ||

fs/ocfs2/mmap.c +1 -0

index 10d66c7..d073fc2 100644
@@ -136,6 +136,7 @@ static int ocfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	sigset_t oldset;
 	int ret;
 
+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);
 	ocfs2_block_signals(&oldset);
 

fs/super.c +0 -0

index eae088f..5e44e42 100644

fs/super.c out: · line 1396

Quoted 4 lines

fs/super.c +1 -0

@@ -1393,3 +1393,54 @@ out:
 	return 0;
 }
 EXPORT_SYMBOL(thaw_super);
+

IMHO it is reasonable to open code this procedure so user is responsible for calling freeze_super(), thaw_super() . This allow to call for several inodes in a row like follows:

ioctl(sb,FIFREEZE)
while (f = pop(files_list))
ioctl(f,FS_IOC_FWFREEZE)
ioctl(sb,FITHAW)

This required for directory defragmentation(small files compacting)

We introduce per-file freeze feature for unifying defrag ext4 and xfs as first ingredient. We get the idea courtesy of Dave Chinner
(https://lkml.org/lkml/2014/7/14/759)
per-file freeze will be used to avoid that file is not modified while userspace is doing the defrag.

This patch tries to implement file write freeze functionality. Introduce new inode state, I_WRITE_FREEZED.
When this state is set, any process which tries to modify the file's address space, either by pagefault mmap writes or using write(2), will block until the this state is cleared. I_WRITE_FREEZED is set by calling FS_IOC_FWFREEZE ioctl and clear by FS_IOC_FWTHAW ioctl.

File write freeze functionality, when used in conjunction with inode's immutable flag can be used for creating truly stable file snapshots wherein write freeze will prevent any modification to the file from already open file descriptors and immutable flag will prevent any new modification to the file. One of the intended uses for stable file snapshots would be in the defragmentation applications which defrags single file.

For implementation purpose, initially we tried to keep percpu usage counters inside struct inode just like there is struct sb_writers in super_block. But considering that it will significantly bloat up struct inode when actually the usage of file write freeze will be infrequent, we dropped this idea. Instead we have tried to use already present filesystem freezing infrastructure. Current approach makes it possible for implementing file write freeze without bloating any of struct super_block/inode.
In FS_IOC_FWFREEZE, we wait for complete fs to be frozen, set I_WRITE_FREEZED to inode's state and unfreeze the fs.

TODO : prevent inode eviction when I_WRITE_FREEZED state is set. TODO : xfstests testcase for file freeze.

Signed-off-by Namjae Jeon <[email protected]>

Signed-off-by Ashish Sangwan <[email protected]>

---
 fs/btrfs/inode.c        |  1 +
 fs/buffer.c             |  1 +
 fs/ext4/inode.c         |  1 +
 fs/f2fs/file.c          |  1 +
 fs/gfs2/file.c          |  1 +
 fs/inode.c              | 19 ++++++++++++++++++
 fs/ioctl.c              | 30 +++++++++++++++++++++++++++++
 fs/nilfs2/file.c        |  1 +
 fs/ocfs2/mmap.c         |  1 +
 fs/super.c              | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h      | 13 +++++++++++++
 include/uapi/linux/fs.h |  2 ++
 mm/filemap.c            |  1 +
 13 files changed, 123 insertions(+)

fs/inode.c +19 -0

index aa149e7..82a96d0 100644
@@ -1929,3 +1929,22 @@ void inode_set_flags(struct inode *inode, unsigned int flags,
 				  new_flags) != old_flags));
 }
 EXPORT_SYMBOL(inode_set_flags);
+
+void inode_start_write(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+retry:
+	spin_lock(&inode->i_lock);
+	if (inode->i_state & I_WRITE_FREEZED) {
+		DEFINE_WAIT(wait);
+
+		prepare_to_wait(&sb->s_writers.wait_unfrozen, &wait,
+				TASK_UNINTERRUPTIBLE);
+		spin_unlock(&inode->i_lock);
+		schedule();
+		finish_wait(&sb->s_writers.wait_unfrozen, &wait);
+		goto retry;
+	}
+	spin_unlock(&inode->i_lock);
+}

fs/super.c +41 -0

index eae088f..5e44e42 100644
@@ -1393,3 +1393,54 @@ out:
 	return 0;
 }
 EXPORT_SYMBOL(thaw_super);
+
+int file_write_freeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	int ret = 0;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (inode->i_state & I_WRITE_FREEZED)
+		ret = -EBUSY;
+	spin_unlock(&inode->i_lock);
+	if (ret)
+		return ret;
+
+	ret = freeze_super(sb);
+	if (ret)
+		return ret;
+
+	spin_lock(&inode->i_lock);
+	inode->i_state |= I_WRITE_FREEZED;
+	smp_wmb();
+	spin_unlock(&inode->i_lock);
+
+	return thaw_super(sb);
+}
+EXPORT_SYMBOL(file_write_freeze);
+
+int file_write_unfreeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (!(inode->i_state & I_WRITE_FREEZED)) {
+		spin_unlock(&inode->i_lock);

Hello,

Quoted 22 lines

fs/super.c +20 -0

+
+int file_write_unfreeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (!(inode->i_state & I_WRITE_FREEZED)) {
+		spin_unlock(&inode->i_lock);
+		return -EINVAL;
+	}
+
+	inode->i_state &= ~I_WRITE_FREEZED;
+	smp_wmb();
+	wake_up(&sb->s_writers.wait_unfrozen);
+	spin_unlock(&inode->i_lock);
+	return 0;

Review

Quoted 2 lines

(quoted diff) +2 -0

+}
+EXPORT_SYMBOL(file_write_unfreeze);

So I was looking at the implementation and I have a few comments:
1) The trick with freezing superblock looks nice but I'm somewhat worried that if we wanted to heavily use per-inode freezing to defrag the whole filesystem it may be too slow to freeze the whole fs, mark one inode as frozen and then unfreeze the fs. But I guess we'll see that once have some reasonably working implementation.

2) The tests you are currently doing are racy. If things happen as:
CPU1 CPU2
inode_start_write()
file_write_freeze()
sb_start_pagefault()
Do modifications.

Then you have a CPU modifying a file while file_write_freeze() has succeeded so it should be frozen.

If you swap inode_start_write() with sb_start_pagefault() the above race doesn't happen but userspace program has to be really careful not to hit a deadlock. E.g. if you tried to freeze two inodes the following could happen:
CPU1 CPU2
file_write_freeze(inode1)
fault on inode1:
sb_start_pagefault()
inode_start_write() -> blocks
file_write_freeze(inode2)
blocks in freeze_super()

So I don't think this is a good scheme for inode freezing...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

Quoted 19 lines

Quoted 19 lines

fs/super.c +19 -0

+int file_write_unfreeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (!(inode->i_state & I_WRITE_FREEZED)) {
+		spin_unlock(&inode->i_lock);
+		return -EINVAL;
+	}
+
+	inode->i_state &= ~I_WRITE_FREEZED;
+	smp_wmb();
+	wake_up(&sb->s_writers.wait_unfrozen);
+	spin_unlock(&inode->i_lock);
+	return 0;

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+}
+EXPORT_SYMBOL(file_write_unfreeze);

So I was looking at the implementation and I have a few comments:
1) The trick with freezing superblock looks nice but I'm somewhat worried that if we wanted to heavily use per-inode freezing to defrag the whole filesystem it may be too slow to freeze the whole fs, mark one inode as frozen and then unfreeze the fs. But I guess we'll see that once have some reasonably working implementation.

Dmitry has given a good idea to avoid multiple freeze fs and unfreeze fs calls.

ioctl(sb,FIFREEZE)
while (f = pop(files_list))
ioctl(f,FS_IOC_FWFREEZE)
ioctl(sb,FITHAW)

In file_write_freeze, we could first check if the fs is already frozen, if yes than we can directly set inode write freeze state after taking relevant lock to prevent fs_thaw while the inode state is being set.

Quoted 22 lines

2) The tests you are currently doing are racy. If things happen as:
CPU1 CPU2
inode_start_write()
file_write_freeze()
sb_start_pagefault()
Do modifications.

Then you have a CPU modifying a file while file_write_freeze() has succeeded so it should be frozen.

If you swap inode_start_write() with sb_start_pagefault() the above race doesn't happen but userspace program has to be really careful not to hit a deadlock. E.g. if you tried to freeze two inodes the following could happen:
CPU1 CPU2
file_write_freeze(inode1)
fault on inode1:
sb_start_pagefault()
inode_start_write() -> blocks
file_write_freeze(inode2)
blocks in freeze_super()

Quoted 1 line

So I don't think this is a good scheme for inode freezing...

To solve this race, we can fold inode_start_write with sb_start_write and use similar appraoch of __sb_start_write.
How about the below scheme ?

void inode_start_write(struct inode *inode)

{
	struct super_block *sb = inode->i_sb;

retry:

	if (unlikely(inode->i_state & I_WRITE_FREEZED)) {
		DEFINE_WAIT(wait);

		prepare_to_wait(&sb->s_writers.wait_unfrozen, &wait,
			TASK_UNINTERRUPTIBLE);
		schedule();
		finish_wait(&sb->s_writers.wait_unfrozen, &wait);

		goto retry;
	}

	sb_start_write(sb);

	/* check if file_write_freeze race with us */
	if (unlikely(inode->i_state & I_WRITE_FREEZED) {
		sb_end_write(sb);
		goto retry;
	}
}

Hello,

Quoted 30 lines

Quoted 19 lines

Quoted 19 lines

fs/super.c +19 -0

+int file_write_unfreeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (!(inode->i_state & I_WRITE_FREEZED)) {
+		spin_unlock(&inode->i_lock);
+		return -EINVAL;
+	}
+
+	inode->i_state &= ~I_WRITE_FREEZED;
+	smp_wmb();
+	wake_up(&sb->s_writers.wait_unfrozen);
+	spin_unlock(&inode->i_lock);
+	return 0;

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+}
+EXPORT_SYMBOL(file_write_unfreeze);

So I was looking at the implementation and I have a few comments:
1) The trick with freezing superblock looks nice but I'm somewhat worried that if we wanted to heavily use per-inode freezing to defrag the whole filesystem it may be too slow to freeze the whole fs, mark one inode as frozen and then unfreeze the fs. But I guess we'll see that once have some reasonably working implementation.

Dmitry has given a good idea to avoid multiple freeze fs and unfreeze fs calls.

Quoted 8 lines

ioctl(sb,FIFREEZE)
while (f = pop(files_list))
ioctl(f,FS_IOC_FWFREEZE)
ioctl(sb,FITHAW)

In file_write_freeze, we could first check if the fs is already frozen, if yes than we can directly set inode write freeze state after taking relevant lock to prevent fs_thaw while the inode state is being set.

Well, doing fs-wide freezing from userspace makes sense as Dmitry pointed
out. We can then just fail FS_IOC_FWFREEZE with error when the whole fs isn't frozen. I'm just somewhat worried whether the fs-wide freezing won't be too fragile. E.g. consider a situation when you are running a defrag program which is freezing and unfreezing the filesystem and then some background work kicks which will want to snapshot the filesystem so it will freeze & unfreeze the fs as well. Now depending on how exactly defrag and snapshot race one of the FIFREEZE ioctls will return EBUSY and the process (hopefully gracefully) fails.

This isn't a new situation - if you ran two snapshots at once, you'd see the same failure. But the more fs-wide freezing gets used in different places the stranger and less expected failure you'll see...

Quoted 47 lines

Quoted 23 lines

2) The tests you are currently doing are racy. If things happen as:
CPU1 CPU2
inode_start_write()
file_write_freeze()
sb_start_pagefault()
Do modifications.

Then you have a CPU modifying a file while file_write_freeze() has succeeded so it should be frozen.

If you swap inode_start_write() with sb_start_pagefault() the above race doesn't happen but userspace program has to be really careful not to hit a deadlock. E.g. if you tried to freeze two inodes the following could happen:
CPU1 CPU2
file_write_freeze(inode1)
fault on inode1:
sb_start_pagefault()
inode_start_write() -> blocks
file_write_freeze(inode2)
blocks in freeze_super()

Quoted 1 line

So I don't think this is a good scheme for inode freezing...

To solve this race, we can fold inode_start_write with sb_start_write and use similar appraoch of __sb_start_write.
How about the below scheme ?

void inode_start_write(struct inode *inode)

{
	struct super_block *sb = inode->i_sb;

retry:

	if (unlikely(inode->i_state & I_WRITE_FREEZED)) {
		DEFINE_WAIT(wait);

		prepare_to_wait(&sb->s_writers.wait_unfrozen, &wait,
			TASK_UNINTERRUPTIBLE);
		schedule();
		finish_wait(&sb->s_writers.wait_unfrozen, &wait);

		goto retry;
	}

	sb_start_write(sb);

Quoted 6 lines

	/* check if file_write_freeze race with us */
	if (unlikely(inode->i_state & I_WRITE_FREEZED) {
		sb_end_write(sb);
		goto retry;
	}
}

Yes, this should work. You'll need a similar wrapper for page faults but
that's easy enough.

Honza

--
Jan Kara <[email protected]>
SUSE Labs, CR

We introduce per-file freeze feature for unifying defrag ext4 and xfs as first ingredient. We get the idea courtesy of Dave Chinner
(https://lkml.org/lkml/2014/7/14/759)

per-file freeze will be used to avoid that file is not modified while userspace is doing the defrag.

This patch tries to implement file write freeze functionality. Introduce new inode state, I_WRITE_FREEZED.

When this state is set, any process which tries to modify the file's address space, either by pagefault mmap writes or using write(2), will block until the this state is cleared. I_WRITE_FREEZED is set by calling FS_IOC_FWFREEZE ioctl and clear by FS_IOC_FWTHAW ioctl.

File write freeze functionality, when used in conjunction with inode's immutable flag can be used for creating truly stable file snapshots wherein write freeze will prevent any modification to the file from already open file descriptors and immutable flag will prevent any new modification to the file. One of the intended uses for stable file snapshots would be in the defragmentation applications which defrags single file.

For implementation purpose, initially we tried to keep percpu usage counters inside struct inode just like there is struct sb_writers in super_block. But considering that it will significantly bloat up struct inode when actually the usage of file write freeze will be infrequent, we dropped this idea. Instead we have tried to use already present filesystem freezing infrastructure. Current approach makes it possible for implementing file write freeze without bloating any of struct super_block/inode.
In FS_IOC_FWFREEZE, we wait for complete fs to be frozen, set I_WRITE_FREEZED to inode's state and unfreeze the fs.

TODO : prevent inode eviction when I_WRITE_FREEZED state is set.

TODO : xfstests testcase for file freeze.

Signed-off-by Namjae Jeon <[email protected]>

Signed-off-by Ashish Sangwan <[email protected]>

---
 fs/btrfs/inode.c        |  1 +
 fs/buffer.c             |  1 +
 fs/ext4/inode.c         |  1 +
 fs/f2fs/file.c          |  1 +
 fs/gfs2/file.c          |  1 +
 fs/inode.c              | 19 ++++++++++++++++++
 fs/ioctl.c              | 30 +++++++++++++++++++++++++++++
 fs/nilfs2/file.c        |  1 +
 fs/ocfs2/mmap.c         |  1 +
 fs/super.c              | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h      | 13 +++++++++++++
 include/uapi/linux/fs.h |  2 ++
 mm/filemap.c            |  1 +
 13 files changed, 123 insertions(+)

fs/btrfs/inode.c +0 -0

index e687bb0..357c749 100644

fs/btrfs/inode.c +7 -0

+void inode_start_write(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+retry:
+	spin_lock(&inode->i_lock);
+	if (inode->i_state & I_WRITE_FREEZED) {

fs/btrfs/inode.c +41 -0

+
+int file_write_freeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	int ret = 0;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (inode->i_state & I_WRITE_FREEZED)
+		ret = -EBUSY;
+	spin_unlock(&inode->i_lock);
+	if (ret)
+		return ret;
+
+	ret = freeze_super(sb);
+	if (ret)
+		return ret;
+
+	spin_lock(&inode->i_lock);
+	inode->i_state |= I_WRITE_FREEZED;
+	smp_wmb();
+	spin_unlock(&inode->i_lock);
+
+	return thaw_super(sb);
+}
+EXPORT_SYMBOL(file_write_freeze);
+
+int file_write_unfreeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (!(inode->i_state & I_WRITE_FREEZED)) {
+		spin_unlock(&inode->i_lock);

Quoted 3 lines

We introduce per-file freeze feature for unifying defrag ext4 and xfs as first ingredient. We get the idea courtesy of Dave Chinner
(https://lkml.org/lkml/2014/7/14/759)

/me pages the context back in

Quoted 5 lines

per-file freeze will be used to avoid that file is not modified while userspace is doing the defrag.

This patch tries to implement file write freeze functionality. Introduce new inode state, I_WRITE_FREEZED.

I_WRITE_FROZEN.

Quoted 6 lines

When this state is set, any process which tries to modify the file's address space, either by pagefault mmap writes or using write(2), will block until the this state is cleared. I_WRITE_FREEZED is set by calling FS_IOC_FWFREEZE ioctl and clear by FS_IOC_FWTHAW ioctl.

Quoted 6 lines

File write freeze functionality, when used in conjunction with inode's immutable flag can be used for creating truly stable file snapshots wherein write freeze will prevent any modification to the file from already open file descriptors and immutable flag will prevent any new modification to the file. One of the intended uses for stable file snapshots would be in the defragmentation applications which defrags single file.

I don't quite understand why the full filesystem freeze is necessary? The thaw occurs immediately after I_WRITE_FREEZED is set,

which means there's nothing that prevent the file from being truncated or otherwise modified by fallocate, etc while it is frozen....

AFAICT, fsync will bring the file down to a consistent state and we've already got freeze hooks for all inode modification operations. We also have IO barriers for truncate operations so that we can wait for all outstanding IO to complete, so I would have thought this covers all bases for an inode freeze. i.e.:

i_mutex -> I_FROZEN -> fsync -> inode_dio_wait

Should give us a clean inode where there are not ongoing operations by the time that inode_dio_wait() completes. All new modification operations need to check I_FROZEN in addition to the superblock freeze checks...

Quoted 11 lines

For implementation purpose, initially we tried to keep percpu usage counters inside struct inode just like there is struct sb_writers in super_block. But considering that it will significantly bloat up struct inode when actually the usage of file write freeze will be infrequent, we dropped this idea. Instead we have tried to use already present filesystem freezing infrastructure. Current approach makes it possible for implementing file write freeze without bloating any of struct super_block/inode.
In FS_IOC_FWFREEZE, we wait for complete fs to be frozen, set I_WRITE_FREEZED to inode's state and unfreeze the fs.

Quoted 1 line

TODO : prevent inode eviction when I_WRITE_FREEZED state is set.

The app needs to retain a reference to the inode (i.e. open file descriptor) for the length of time that the inode is to be frozen. That will prevent reclaim of the inode.

If the app fails to thaw the inode before it drops all references to it, then all bets are off - we do not want to give userspace a mechanism to pin arbitrarily large amounts of memory....

Quoted 29 lines

TODO : xfstests testcase for file freeze.

Signed-off-by Namjae Jeon <[email protected]>

Signed-off-by Ashish Sangwan <[email protected]>

---
 fs/btrfs/inode.c        |  1 +
 fs/buffer.c             |  1 +
 fs/ext4/inode.c         |  1 +
 fs/f2fs/file.c          |  1 +
 fs/gfs2/file.c          |  1 +
 fs/inode.c              | 19 ++++++++++++++++++
 fs/ioctl.c              | 30 +++++++++++++++++++++++++++++
 fs/nilfs2/file.c        |  1 +
 fs/ocfs2/mmap.c         |  1 +
 fs/super.c              | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h      | 13 +++++++++++++
 include/uapi/linux/fs.h |  2 ++
 mm/filemap.c            |  1 +
 13 files changed, 123 insertions(+)

fs/btrfs/inode.c +0 -0

index e687bb0..357c749 100644
@@ -8262,6 +8262,7 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	u64 page_start;
 	u64 page_end;
 

fs/btrfs/inode.c int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) · line 8266

Quoted 2 lines

fs/btrfs/inode.c +1 -0

+	inode_start_write(inode);
 	sb_start_pagefault(inode->i_sb);

Wouldn't it be better to have:

	inode_start_pagefault(inode);

and then have it call sb_start_pagefault() internally?

Quoted 12 lines

fs/btrfs/inode.c +11 -0

+void inode_start_write(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+retry:
+	spin_lock(&inode->i_lock);
+	if (inode->i_state & I_WRITE_FREEZED) {
+		DEFINE_WAIT(wait);
+
+		prepare_to_wait(&sb->s_writers.wait_unfrozen, &wait,
+				TASK_UNINTERRUPTIBLE);

fs/btrfs/inode.c

Quoted 6 lines

fs/btrfs/inode.c +6 -0

+		spin_unlock(&inode->i_lock);
+		schedule();
+		finish_wait(&sb->s_writers.wait_unfrozen, &wait);
+		goto retry;
+	}
+	spin_unlock(&inode->i_lock);

Please use a while() loop, not goto.

	spin_lock(&inode->i_lock);
	while(inode->i_state & I_WRITE_FROZEN) {
		DEFINE_WAIT(wait);

		prepare_to_wait(&sb->s_writers.wait_unfrozen, &wait,
				TASK_UNINTERRUPTIBLE);
		spin_unlock(&inode->i_lock);
		schedule();
		finish_wait(&sb->s_writers.wait_unfrozen, &wait);
		spin_lock(&inode->i_lock);
	}
	spin_unlock(&inode->i_lock);

Quoted 41 lines

fs/btrfs/inode.c +41 -0

+
+int file_write_freeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	int ret = 0;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (inode->i_state & I_WRITE_FREEZED)
+		ret = -EBUSY;
+	spin_unlock(&inode->i_lock);
+	if (ret)
+		return ret;
+
+	ret = freeze_super(sb);
+	if (ret)
+		return ret;
+
+	spin_lock(&inode->i_lock);
+	inode->i_state |= I_WRITE_FREEZED;
+	smp_wmb();
+	spin_unlock(&inode->i_lock);
+
+	return thaw_super(sb);
+}
+EXPORT_SYMBOL(file_write_freeze);
+
+int file_write_unfreeze(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	spin_lock(&inode->i_lock);
+
+	if (!(inode->i_state & I_WRITE_FREEZED)) {
+		spin_unlock(&inode->i_lock);

fs/btrfs/inode.c

Quoted 10 lines

fs/btrfs/inode.c +10 -0

+		return -EINVAL;
+	}
+
+	inode->i_state &= ~I_WRITE_FREEZED;
+	smp_wmb();
+	wake_up(&sb->s_writers.wait_unfrozen);
+	spin_unlock(&inode->i_lock);
+	return 0;
+}
+EXPORT_SYMBOL(file_write_unfreeze);

These look terrible racy. Freeze can race with other filesystem freeze and thaw operations, so if thaw_super() fails you could have frozen the inode but you still get -EINVAL for the ioctl. Or you could have multiple applications doing file freeze/thaw, and so an unfreeze occurs while a freeze if waiting on a thaw, and so the app waiting on a freeze might return with an unfrozen inode, even though there were no errors....

fs/btrfs/inode.c static inline bool file_start_write_trylock(struct file *file) · line 2345

Quoted 9 lines

fs/btrfs/inode.c +1 -0

@@ -2328,6 +2340,7 @@ static inline bool file_start_write_trylock(struct file *file)
 {
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return true;
+	inode_start_write(file_inode(file));
 	return __sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, false);
 }

This needs "trylock" semantics, not blocking semantics.

Cheers,

Dave.
--
Dave Chinner
[email protected]

Quoted 12 lines

Quoted 11 lines

When this state is set, any process which tries to modify the file's address space, either by pagefault mmap writes or using write(2), will block until the this state is cleared. I_WRITE_FREEZED is set by calling FS_IOC_FWFREEZE ioctl and clear by FS_IOC_FWTHAW ioctl.

File write freeze functionality, when used in conjunction with inode's immutable flag can be used for creating truly stable file snapshots wherein write freeze will prevent any modification to the file from already open file descriptors and immutable flag will prevent any new modification to the file. One of the intended uses for stable file snapshots would be in the defragmentation applications which defrags single file.

Quoted 2 lines

I don't quite understand why the full filesystem freeze is necessary? The thaw occurs immediately after I_WRITE_FREEZED is set,

We started by looking at fs freeze for file freeze implementation, So got biased for using fs freeze or similar approach. Thanks for suggesting a better way.

Quoted 3 lines

which means there's nothing that prevent the file from being truncated or otherwise modified by fallocate, etc while it is frozen....

Right, So, After that, we had also thought of setting immutable flag of inode. Immutable flag + I_WRITE_FROZEN => truly frozen file.

Quoted 6 lines

AFAICT, fsync will bring the file down to a consistent state and we've already got freeze hooks for all inode modification operations. We also have IO barriers for truncate operations so that we can wait for all outstanding IO to complete, so I would have thought this covers all bases for an inode freeze. i.e.:

Right.

Quoted 7 lines

i_mutex -> I_FROZEN -> fsync -> inode_dio_wait

Should give us a clean inode where there are not ongoing operations by the time that inode_dio_wait() completes. All new modification operations need to check I_FROZEN in addition to the superblock freeze checks...

I checked the routines where checks for I_FROZEN would be required. Most of them are Ok but do_unlinkat() confuses me a little. vfs_unlink is called under parent inode's i_mutex, so we cannot sleep keeping parent's i_mutex held.
i.e while freezing file, all file in directory are blocked by parent i_mutex. Is it ok to release parnets->mutex before checking for I_FROZEN or there is some idea?

Quoted 4 lines · Namjae Jeon

Quoted 4 lines

Quoted 4 lines

When this state is set, any process which tries to modify the file's address space, either by pagefault mmap writes or using write(2), will block until the this state is cleared. I_WRITE_FREEZED is set by calling FS_IOC_FWFREEZE ioctl and clear by FS_IOC_FWTHAW ioctl.

Quoted 7 lines

I checked the routines where checks for I_FROZEN would be required. Most of them are Ok but do_unlinkat() confuses me a little. vfs_unlink is called under parent inode's i_mutex, so we cannot sleep keeping parent's i_mutex held.
i.e while freezing file, all file in directory are blocked by parent i_mutex. Is it ok to release parnets->mutex before checking for I_FROZEN or there is some idea?

So I believe Dave thought that you'd just reuse places we currently use
to call sb_start_write() / mnt_want_write(). You'd probably have to come up with a function like path_want_write() (takes struct path as an argument) and which will call mnt_want_write(), sb_start_write(), and do appropriate inode freeze handling. Then you replace all calls to mnt_want_write() with calls to path_want_write()... Possibly you can also provide a trivial wrapper for path_want_write() which takes struct file instead.

This should also deal with the locking problems you describe above as mnt_want_write() is always called before taking i_mutex.

Quoted 39 lines

Quoted 12 lines

Quoted 11 lines

When this state is set, any process which tries to modify the file's address space, either by pagefault mmap writes or using write(2), will block until the this state is cleared. I_WRITE_FREEZED is set by calling FS_IOC_FWFREEZE ioctl and clear by FS_IOC_FWTHAW ioctl.

File write freeze functionality, when used in conjunction with inode's immutable flag can be used for creating truly stable file snapshots wherein write freeze will prevent any modification to the file from already open file descriptors and immutable flag will prevent any new modification to the file. One of the intended uses for stable file snapshots would be in the defragmentation applications which defrags single file.

Quoted 2 lines

I don't quite understand why the full filesystem freeze is necessary? The thaw occurs immediately after I_WRITE_FREEZED is set,

We started by looking at fs freeze for file freeze implementation, So got biased for using fs freeze or similar approach. Thanks for suggesting a better way.

Quoted 3 lines

which means there's nothing that prevent the file from being truncated or otherwise modified by fallocate, etc while it is frozen....

Right, So, After that, we had also thought of setting immutable flag of inode. Immutable flag + I_WRITE_FROZEN => truly frozen file.

Quoted 6 lines

AFAICT, fsync will bring the file down to a consistent state and we've already got freeze hooks for all inode modification operations. We also have IO barriers for truncate operations so that we can wait for all outstanding IO to complete, so I would have thought this covers all bases for an inode freeze. i.e.:

Right.

Quoted 7 lines

i_mutex -> I_FROZEN -> fsync -> inode_dio_wait

Should give us a clean inode where there are not ongoing operations by the time that inode_dio_wait() completes. All new modification operations need to check I_FROZEN in addition to the superblock freeze checks...

Quoted 7 lines

I checked the routines where checks for I_FROZEN would be required. Most of them are Ok but do_unlinkat() confuses me a little. vfs_unlink is called under parent inode's i_mutex, so we cannot sleep keeping parent's i_mutex held.
i.e while freezing file, all file in directory are blocked by parent i_mutex. Is it ok to release parnets->mutex before checking for I_FROZEN or there is some idea?

So I believe Dave thought that you'd just reuse places we currently use
to call sb_start_write() / mnt_want_write(). You'd probably have to come up with a function like path_want_write() (takes struct path as an argument) and which will call mnt_want_write(), sb_start_write(), and do appropriate inode freeze handling. Then you replace all calls to mnt_want_write() with calls to path_want_write()... Possibly you can also provide a trivial wrapper for path_want_write() which takes struct file instead.

This should also deal with the locking problems you describe above as mnt_want_write() is always called before taking i_mutex.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR