Skip to contentSkip to conversation

fsfreeze: manage kill signal when sb_start_pagefault is called

PATCH Superseded Marco Stornelli 2 participants 3 messages Apr 6, 2013

Subsystems (9)
Memory management VFS Btrfs file system ext4 file system +5 more subsystems

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Marco Stornelli

author
#0 lore raw

PATCH 3/4 fsfreeze: manage kill signal when sb_start_pagefault is called

In every place where sb_start_pagefault was called now we must manage the error code and return VM_FAULT_RETRY.

Signed-off-by Marco Stornelli <[email protected]>
---
 fs/btrfs/inode.c   |    4 +++-
 fs/buffer.c        |    4 +++-
 fs/ext4/inode.c    |    4 +++-
 fs/f2fs/file.c     |    4 +++-
 fs/gfs2/file.c     |    4 +++-
 fs/nilfs2/file.c   |    4 +++-
 fs/ocfs2/mmap.c    |    4 +++-
 include/linux/fs.h |    6 ++++--
 mm/filemap.c       |    7 +++++--
 9 files changed, 30 insertions(+), 11 deletions(-)
fs/btrfs/inode.c +3 -1
index 09c58a3..a6166f4 100644
@@ -7559,7 +7559,9 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	u64 page_start;
 	u64 page_end;
 
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 	ret  = btrfs_delalloc_reserve_space(inode, PAGE_CACHE_SIZE);
 	if (!ret) {
 		ret = file_update_time(vma->vm_file);
fs/buffer.c +3 -1
index b4dcb34..6d3d2cc 100644
@@ -2383,7 +2383,9 @@ 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;
 
-	sb_start_pagefault(sb);
+	ret = sb_start_pagefault(sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 
 	/*
 	 * Update file times before taking page lock. We may end up failing the
fs/ext4/inode.c +3 -1
index b3a5213..efc47f6 100644
@@ -5023,7 +5023,9 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	get_block_t *get_block;
 	int retries = 0;
 
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 	file_update_time(vma->vm_file);
 	/* Delalloc case is easy... */
 	if (test_opt(inode->i_sb, DELALLOC) &&
fs/f2fs/file.c +3 -1
index 958a46d..cce4147 100644
@@ -37,7 +37,9 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 
 	f2fs_balance_fs(sbi);
 
-	sb_start_pagefault(inode->i_sb);
+	err = sb_start_pagefault(inode->i_sb);
+	if (err)
+		return VM_FAULT_RETRY;
 
 	mutex_lock_op(sbi, DATA_NEW);
fs/gfs2/file.c +3 -1
index d79c2da..071e777 100644
@@ -396,7 +396,9 @@ static int gfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	loff_t size;
 	int ret;
 
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 
 	/* Update file times before taking page lock */
 	file_update_time(vma->vm_file);
fs/nilfs2/file.c +3 -1
index 08fdb77..1c7678a 100644
@@ -74,7 +74,9 @@ 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 */
 
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 	lock_page(page);
 	if (page->mapping != inode->i_mapping ||
 	    page_offset(page) >= i_size_read(inode) || !PageUptodate(page)) {
fs/ocfs2/mmap.c +3 -1
index 10d66c7..f0973ae 100644
@@ -136,7 +136,9 @@ static int ocfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	sigset_t oldset;
 	int ret;
 
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 	ocfs2_block_signals(&oldset);
 
 	/*
include/linux/fs.h +4 -2
index 03921d6..550574e 100644
@@ -1407,6 +1407,8 @@ static inline int sb_start_write_trylock(struct super_block *sb)
  * sb_start_pagefault - get write access to a superblock from a page fault
  * @sb: the super we write to
  *
+ * It returns zero when no error occured, the error code otherwise.
+ *
  * When a process starts handling write page fault, it should embed the
  * operation into sb_start_pagefault() - sb_end_pagefault() pair to get
  * exclusion against file system freezing. This is needed since the page fault

@@ -1422,9 +1424,9 @@ static inline int sb_start_write_trylock(struct super_block *sb)
  * mmap_sem
  *   -> sb_start_pagefault
  */
-static inline void sb_start_pagefault(struct super_block *sb)
+static inline int sb_start_pagefault(struct super_block *sb)
 {
-	__sb_start_write_wait(sb, SB_FREEZE_PAGEFAULT, false);
+	__sb_start_write_wait(sb, SB_FREEZE_PAGEFAULT, true);
 }
 
 /*
mm/filemap.c +5 -2
index b238671..acf8d97 100644
@@ -1712,9 +1712,11 @@ int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct page *page = vmf->page;
 	struct inode *inode = file_inode(vma->vm_file);
-	int ret = VM_FAULT_LOCKED;
+	int ret = 0;
 
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 	file_update_time(vma->vm_file);
 	lock_page(page);
 	if (page->mapping != inode->i_mapping) {

@@ -1727,6 +1729,7 @@ int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * progress, we are guaranteed that writeback during freezing will
 	 * see the dirty page and writeprotect it again.
 	 */
+	ret = VM_FAULT_LOCKED;
 	set_page_dirty(page);
 	wait_for_stable_page(page);
 out:
Signature · 1 line
1.7.3.4
#1 lore raw
Quoted 2 lines · Marco Stornelli

In every place where sb_start_pagefault was called now we must manage the error code and return VM_FAULT_RETRY.

Erm ... in patch 1/4:

static inline void sb_start_pagefault(struct super_block *sb) {
- __sb_start_write(sb, SB_FREEZE_PAGEFAULT, true);
+ __sb_start_write_wait(sb, SB_FREEZE_PAGEFAULT, false);
}

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

Quoted 6 lines
fs/btrfs/inode.c +3 -1
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
 	ret  = btrfs_delalloc_reserve_space(inode, PAGE_CACHE_SIZE);

Does the compiler not warn that you're assigning void to 'ret'? Or was there some other SNAFU sending these patches?

Signature · 4 lines
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
author
#2 lore raw

Il 06/04/2013 15:20, Matthew Wilcox ha scritto:

Quoted 13 lines · Marco Stornelli
Quoted 2 lines

In every place where sb_start_pagefault was called now we must manage the error code and return VM_FAULT_RETRY.

Erm ... in patch 1/4:

  static inline void sb_start_pagefault(struct super_block *sb)
  {
(quoted diff) +1 -1
-       __sb_start_write(sb, SB_FREEZE_PAGEFAULT, true);
+       __sb_start_write_wait(sb, SB_FREEZE_PAGEFAULT, false);
  }
Quoted 2 lines
Quoted 9 lines

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

Quoted 5 lines
fs/btrfs/inode.c +3 -1
-	sb_start_pagefault(inode->i_sb);
+	ret = sb_start_pagefault(inode->i_sb);
+	if (ret)
+		return VM_FAULT_RETRY;
  	ret  = btrfs_delalloc_reserve_space(inode, PAGE_CACHE_SIZE);

Does the compiler not warn that you're assigning void to 'ret'? Or was there some other SNAFU sending these patches?

I'm sorry, my fault :) As I said in 00 these patches are completely *not* tested, it was only a "quick coding & review" to understand if someone can see any problem to this kind of implementation, since I touched several points in the kernel. So there is still on-going work and I need to do several tests. Maybe I had to add the RFC tag, sorry again.

Marco