All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 1/2] f2fs: rename f2fs_update_time
@ 2016-10-12 16:14 ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2016-10-12 16:14 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

From: Chao Yu <yuchao0@huawei.com>

For readability, no functionality change.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c |  2 +-
 fs/f2fs/data.c       |  2 +-
 fs/f2fs/dir.c        |  6 +++---
 fs/f2fs/f2fs.h       |  2 +-
 fs/f2fs/file.c       | 24 ++++++++++++------------
 fs/f2fs/super.c      |  4 ++--
 fs/f2fs/xattr.c      |  2 +-
 7 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 654f5d7..d95fada 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1275,7 +1275,7 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 			"checkpoint: version = %llx", ckpt_ver);
 
 	/* do checkpoint periodically */
-	f2fs_update_time(sbi, CP_TIME);
+	f2fs_update_last_optime(sbi, CP_TIME);
 	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "finish checkpoint");
 out:
 	mutex_unlock(&sbi->cp_mutex);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 983aa54e..165b69c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1705,7 +1705,7 @@ static int f2fs_write_end(struct file *file,
 		f2fs_i_size_write(inode, pos + copied);
 unlock_out:
 	f2fs_put_page(page, 1);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return copied;
 }
 
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index a46079a..70e7c39 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -631,7 +631,7 @@ int __f2fs_do_add_link(struct inode *dir, struct fscrypt_name *fname,
 		err = f2fs_add_regular_entry(dir, &new_name, fname->usr_fname,
 							inode, ino, mode);
 
-	f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(dir), REQ_TIME);
 	return err;
 }
 
@@ -671,7 +671,7 @@ int f2fs_do_tmpfile(struct inode *inode, struct inode *dir)
 	clear_inode_flag(inode, FI_NEW_INODE);
 fail:
 	up_write(&F2FS_I(inode)->i_sem);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return err;
 }
 
@@ -710,7 +710,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
 	int slots = GET_DENTRY_SLOTS(le16_to_cpu(dentry->name_len));
 	int i;
 
-	f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(dir), REQ_TIME);
 
 	if (f2fs_has_inline_dentry(dir))
 		return f2fs_delete_inline_entry(dentry, page, dir, inode);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eaac7b4..5c19136 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -936,7 +936,7 @@ static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
 (((u64)part_stat_read(s->sb->s_bdev->bd_part, sectors[1]) -		 \
 		s->sectors_written_start) >> 1)
 
-static inline void f2fs_update_time(struct f2fs_sb_info *sbi, int type)
+static inline void f2fs_update_last_optime(struct f2fs_sb_info *sbi, int type)
 {
 	sbi->last_time[type] = jiffies;
 }
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 0081c79..b46f6e1 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -100,7 +100,7 @@ mapped:
 	clear_cold_data(page);
 out:
 	sb_end_pagefault(inode->i_sb);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	return block_page_mkwrite_return(err);
 }
 
@@ -282,7 +282,7 @@ flush_out:
 	remove_ino_entry(sbi, ino, UPDATE_INO);
 	clear_inode_flag(inode, FI_UPDATE_WRITE);
 	ret = f2fs_issue_flush(sbi);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 out:
 	trace_f2fs_sync_file_exit(inode, need_cp, datasync, ret);
 	f2fs_trace_ios(NULL, 1);
@@ -539,7 +539,7 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
 	}
 	dn->ofs_in_node = ofs;
 
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	trace_f2fs_truncate_data_blocks_range(dn->inode, dn->nid,
 					 dn->ofs_in_node, nr_free);
 	return nr_free;
@@ -1439,7 +1439,7 @@ static long f2fs_fallocate(struct file *file, int mode,
 	if (!ret) {
 		inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 		f2fs_mark_inode_dirty_sync(inode);
-		f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+		f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	}
 
 out:
@@ -1565,7 +1565,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 		goto out;
 
 	set_inode_flag(inode, FI_ATOMIC_FILE);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 
 	if (!get_dirty_pages(inode))
 		goto out;
@@ -1637,7 +1637,7 @@ static int f2fs_ioc_start_volatile_write(struct file *filp)
 		goto out;
 
 	set_inode_flag(inode, FI_VOLATILE_FILE);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 out:
 	inode_unlock(inode);
 	mnt_drop_write_file(filp);
@@ -1697,7 +1697,7 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 	inode_unlock(inode);
 
 	mnt_drop_write_file(filp);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return ret;
 }
 
@@ -1743,7 +1743,7 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		ret = -EINVAL;
 		goto out;
 	}
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 out:
 	mnt_drop_write_file(filp);
 	return ret;
@@ -1781,7 +1781,7 @@ static int f2fs_ioc_fitrim(struct file *filp, unsigned long arg)
 	if (copy_to_user((struct fstrim_range __user *)arg, &range,
 				sizeof(range)))
 		return -EFAULT;
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return 0;
 }
 
@@ -1823,7 +1823,7 @@ static int f2fs_ioc_keyctl(struct file *filp, unsigned long arg)
 		type = XATTR_REPLACE;
 	}
 
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	ret = f2fs_setxattr(inode, F2FS_XATTR_INDEX_KEY,
 				F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
 				value, F2FS_KEY_SIZE, NULL, type);
@@ -1841,7 +1841,7 @@ static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
 							sizeof(policy)))
 		return -EFAULT;
 
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 
 	return fscrypt_process_policy(filp, &policy);
 }
@@ -2129,7 +2129,7 @@ static int f2fs_ioc_defragment(struct file *filp, unsigned long arg)
 	}
 
 	err = f2fs_defragment_range(sbi, filp, &range);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	if (err < 0)
 		goto out;
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bfa4414..e38c9d2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1868,8 +1868,8 @@ skip_recovery:
 			sbi->valid_super_block ? 1 : 2, err);
 	}
 
-	f2fs_update_time(sbi, CP_TIME);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, CP_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	return 0;
 
 free_kobj:
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 83234dd..4b1983e 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -589,6 +589,6 @@ int f2fs_setxattr(struct inode *inode, int index, const char *name,
 	up_write(&F2FS_I(inode)->i_sem);
 	f2fs_unlock_op(sbi);
 
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	return err;
 }
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 1/2] f2fs: rename f2fs_update_time
@ 2016-10-12 16:14 ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2016-10-12 16:14 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

From: Chao Yu <yuchao0@huawei.com>

For readability, no functionality change.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c |  2 +-
 fs/f2fs/data.c       |  2 +-
 fs/f2fs/dir.c        |  6 +++---
 fs/f2fs/f2fs.h       |  2 +-
 fs/f2fs/file.c       | 24 ++++++++++++------------
 fs/f2fs/super.c      |  4 ++--
 fs/f2fs/xattr.c      |  2 +-
 7 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 654f5d7..d95fada 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1275,7 +1275,7 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 			"checkpoint: version = %llx", ckpt_ver);
 
 	/* do checkpoint periodically */
-	f2fs_update_time(sbi, CP_TIME);
+	f2fs_update_last_optime(sbi, CP_TIME);
 	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "finish checkpoint");
 out:
 	mutex_unlock(&sbi->cp_mutex);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 983aa54e..165b69c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1705,7 +1705,7 @@ static int f2fs_write_end(struct file *file,
 		f2fs_i_size_write(inode, pos + copied);
 unlock_out:
 	f2fs_put_page(page, 1);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return copied;
 }
 
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index a46079a..70e7c39 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -631,7 +631,7 @@ int __f2fs_do_add_link(struct inode *dir, struct fscrypt_name *fname,
 		err = f2fs_add_regular_entry(dir, &new_name, fname->usr_fname,
 							inode, ino, mode);
 
-	f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(dir), REQ_TIME);
 	return err;
 }
 
@@ -671,7 +671,7 @@ int f2fs_do_tmpfile(struct inode *inode, struct inode *dir)
 	clear_inode_flag(inode, FI_NEW_INODE);
 fail:
 	up_write(&F2FS_I(inode)->i_sem);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return err;
 }
 
@@ -710,7 +710,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
 	int slots = GET_DENTRY_SLOTS(le16_to_cpu(dentry->name_len));
 	int i;
 
-	f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(dir), REQ_TIME);
 
 	if (f2fs_has_inline_dentry(dir))
 		return f2fs_delete_inline_entry(dentry, page, dir, inode);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eaac7b4..5c19136 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -936,7 +936,7 @@ static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
 (((u64)part_stat_read(s->sb->s_bdev->bd_part, sectors[1]) -		 \
 		s->sectors_written_start) >> 1)
 
-static inline void f2fs_update_time(struct f2fs_sb_info *sbi, int type)
+static inline void f2fs_update_last_optime(struct f2fs_sb_info *sbi, int type)
 {
 	sbi->last_time[type] = jiffies;
 }
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 0081c79..b46f6e1 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -100,7 +100,7 @@ mapped:
 	clear_cold_data(page);
 out:
 	sb_end_pagefault(inode->i_sb);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	return block_page_mkwrite_return(err);
 }
 
@@ -282,7 +282,7 @@ flush_out:
 	remove_ino_entry(sbi, ino, UPDATE_INO);
 	clear_inode_flag(inode, FI_UPDATE_WRITE);
 	ret = f2fs_issue_flush(sbi);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 out:
 	trace_f2fs_sync_file_exit(inode, need_cp, datasync, ret);
 	f2fs_trace_ios(NULL, 1);
@@ -539,7 +539,7 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
 	}
 	dn->ofs_in_node = ofs;
 
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	trace_f2fs_truncate_data_blocks_range(dn->inode, dn->nid,
 					 dn->ofs_in_node, nr_free);
 	return nr_free;
@@ -1439,7 +1439,7 @@ static long f2fs_fallocate(struct file *file, int mode,
 	if (!ret) {
 		inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 		f2fs_mark_inode_dirty_sync(inode);
-		f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+		f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	}
 
 out:
@@ -1565,7 +1565,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 		goto out;
 
 	set_inode_flag(inode, FI_ATOMIC_FILE);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 
 	if (!get_dirty_pages(inode))
 		goto out;
@@ -1637,7 +1637,7 @@ static int f2fs_ioc_start_volatile_write(struct file *filp)
 		goto out;
 
 	set_inode_flag(inode, FI_VOLATILE_FILE);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 out:
 	inode_unlock(inode);
 	mnt_drop_write_file(filp);
@@ -1697,7 +1697,7 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 	inode_unlock(inode);
 
 	mnt_drop_write_file(filp);
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return ret;
 }
 
@@ -1743,7 +1743,7 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		ret = -EINVAL;
 		goto out;
 	}
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 out:
 	mnt_drop_write_file(filp);
 	return ret;
@@ -1781,7 +1781,7 @@ static int f2fs_ioc_fitrim(struct file *filp, unsigned long arg)
 	if (copy_to_user((struct fstrim_range __user *)arg, &range,
 				sizeof(range)))
 		return -EFAULT;
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	return 0;
 }
 
@@ -1823,7 +1823,7 @@ static int f2fs_ioc_keyctl(struct file *filp, unsigned long arg)
 		type = XATTR_REPLACE;
 	}
 
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 	ret = f2fs_setxattr(inode, F2FS_XATTR_INDEX_KEY,
 				F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
 				value, F2FS_KEY_SIZE, NULL, type);
@@ -1841,7 +1841,7 @@ static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
 							sizeof(policy)))
 		return -EFAULT;
 
-	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	f2fs_update_last_optime(F2FS_I_SB(inode), REQ_TIME);
 
 	return fscrypt_process_policy(filp, &policy);
 }
@@ -2129,7 +2129,7 @@ static int f2fs_ioc_defragment(struct file *filp, unsigned long arg)
 	}
 
 	err = f2fs_defragment_range(sbi, filp, &range);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	if (err < 0)
 		goto out;
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bfa4414..e38c9d2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1868,8 +1868,8 @@ skip_recovery:
 			sbi->valid_super_block ? 1 : 2, err);
 	}
 
-	f2fs_update_time(sbi, CP_TIME);
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, CP_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	return 0;
 
 free_kobj:
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 83234dd..4b1983e 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -589,6 +589,6 @@ int f2fs_setxattr(struct inode *inode, int index, const char *name,
 	up_write(&F2FS_I(inode)->i_sem);
 	f2fs_unlock_op(sbi);
 
-	f2fs_update_time(sbi, REQ_TIME);
+	f2fs_update_last_optime(sbi, REQ_TIME);
 	return err;
 }
-- 
2.10.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 2/2] f2fs: fix allocation failure
  2016-10-12 16:14 ` Chao Yu
@ 2016-10-12 16:14   ` Chao Yu
  -1 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2016-10-12 16:14 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

From: Chao Yu <yuchao0@huawei.com>

tests/generic/251 of fstest reports a f2fs bug in below message:

------------[ cut here ]------------
invalid opcode: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 109 Comm: kworker/u8:2 Tainted: G        W  O    4.8.0-rc4+ #22
Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
Workqueue: writeback wb_workfn (flush-251:1)
task: f33c8000 task.stack: f33c6000
EIP: 0060:[<f8992139>] EFLAGS: 00010246 CPU: 1
EIP is at new_curseg+0x2c9/0x2d0 [f2fs]
EAX: 000003f3 EBX: ee3e5000 ECX: 00000400 EDX: 000003f3
ESI: 00000000 EDI: 00000008 EBP: f33c78c4 ESP: f33c7890
 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
CR0: 80050033 CR2: b6706000 CR3: 2e8d70c0 CR4: 000406f0
Stack:
 eb29480c 00000004 f27a0290 000003f3 00000008 f33c78c0 00000001 eb294800
 00000001 00000000 ee3e5000 f899dbb4 00000290 f33c7924 f89926d6 c10b42b6
 00000246 00000000 eed513e8 00000246 f33c78e8 c10b7b4b f33c7924 c178c304
Call Trace:
 [<f89926d6>] allocate_segment_by_default+0x3c6/0x3d0 [f2fs]
 [<f8992b3a>] allocate_data_block+0x13a/0x3c0 [f2fs]
 [<f8992e4b>] do_write_page+0x8b/0x230 [f2fs]
 [<f8993070>] write_node_page+0x20/0x30 [f2fs]
 [<f898a156>] f2fs_write_node_page+0x1a6/0x340 [f2fs]
 [<f898ca45>] sync_node_pages+0x4a5/0x590 [f2fs]
 [<f897ea48>] write_checkpoint+0x218/0x720 [f2fs]
 [<f898143d>] f2fs_gc+0x4cd/0x6b0 [f2fs]
 [<f8990ebe>] f2fs_balance_fs+0x18e/0x1b0 [f2fs]
 [<f8988017>] f2fs_write_data_page+0x197/0x6f0 [f2fs]
 [<f89830fe>] f2fs_write_data_pages+0x28e/0x7e0 [f2fs]
 [<c118b1cd>] do_writepages+0x1d/0x40
 [<c1228cb5>] __writeback_single_inode+0x55/0x7e0
 [<c1229b6b>] writeback_sb_inodes+0x21b/0x490
 [<c1229f6c>] wb_writeback+0xdc/0x590
 [<c122ae18>] wb_workfn+0xf8/0x690
 [<c107c231>] process_one_work+0x1a1/0x580
 [<c107c712>] worker_thread+0x102/0x440
 [<c1082021>] kthread+0xa1/0xc0
 [<c178f862>] ret_from_kernel_thread+0xe/0x24
EIP: [<f8992139>] new_curseg+0x2c9/0x2d0 [f2fs] SS:ESP 0068:f33c7890

The reason is after f2fs enabled lazytime by default, when inode time is
changed, we do not set this inode dirty through ->f2fs_dirty_inode, so
itime updating will be delayed.

Finally it needs to update the dirty time of inode into inode page,
and writeback the page, however, before that, we didn't count the inode
as imeta data. So f2fs won't be aware of dirty metadata page count is
exceeded watermark of GC, result in encountering panic when allocating
free segment.

There is an easy way to produce this bug:
1. mount with lazytime option
2. fragment space
3. touch all files in the image
4. umount

Introduce itime data type and related flow to trace & flush delayed
updating inode to fix this issue.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c | 37 ++++++++++++++++++++++++++++++++
 fs/f2fs/f2fs.h       |  5 +++++
 fs/f2fs/file.c       |  1 +
 fs/f2fs/namei.c      | 38 +++++++++++++++++++++++++++++++++
 fs/f2fs/segment.h    | 11 ++++++----
 fs/f2fs/super.c      | 59 +++++++++++++++++++++++++++++++++++++++++++++-------
 6 files changed, 139 insertions(+), 12 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index d95fada..e27c64f 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -928,6 +928,43 @@ int f2fs_sync_inode_meta(struct f2fs_sb_info *sbi)
 	return 0;
 }
 
+int f2fs_sync_dirty_itime(struct f2fs_sb_info *sbi)
+{
+	struct list_head *head = &sbi->inode_list[DIRTY_ITIME];
+	struct inode *inode;
+	struct f2fs_inode_info *fi;
+	s64 total = get_pages(sbi, F2FS_DIRTY_ITIME);
+
+	while (total--) {
+		if (unlikely(f2fs_cp_error(sbi)))
+			return -EIO;
+
+		spin_lock(&sbi->inode_lock[DIRTY_META]);
+		if (list_empty(head)) {
+			spin_unlock(&sbi->inode_lock[DIRTY_META]);
+			return 0;
+		}
+		fi = list_entry(head->next, struct f2fs_inode_info,
+							gdirty_list);
+		list_move_tail(&fi->gdirty_list, head);
+		inode = igrab(&fi->vfs_inode);
+		spin_unlock(&sbi->inode_lock[DIRTY_META]);
+		if (inode) {
+			spin_lock(&inode->i_lock);
+			if (inode->i_nlink && (inode->i_state & I_DIRTY_TIME)) {
+				inode->i_state &= ~I_DIRTY_TIME;
+				spin_unlock(&inode->i_lock);
+				mark_inode_dirty_sync(inode);
+			} else {
+				spin_unlock(&inode->i_lock);
+			}
+
+			iput(inode);
+		}
+	}
+	return 0;
+}
+
 /*
  * Freeze all the FS-operations for checkpoint.
  */
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5c19136..1f302ff 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -682,6 +682,7 @@ enum count_type {
 	F2FS_DIRTY_META,
 	F2FS_INMEM_PAGES,
 	F2FS_DIRTY_IMETA,
+	F2FS_DIRTY_ITIME,
 	NR_COUNT_TYPE,
 };
 
@@ -734,6 +735,7 @@ enum inode_type {
 	DIR_INODE,			/* for dirty dir inode */
 	FILE_INODE,			/* for dirty regular/symlink inode */
 	DIRTY_META,			/* for all dirtied inode metadata */
+	DIRTY_ITIME,			/* for all I_DIRTY_TIME taged inode */
 	NR_INODE_TYPE,
 };
 
@@ -1613,6 +1615,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
 enum {
 	FI_NEW_INODE,		/* indicate newly allocated inode */
 	FI_DIRTY_INODE,		/* indicate inode is dirty or not */
+	FI_DIRTY_ITIME,		/* inode is dirtied due to time updating */
 	FI_AUTO_RECOVER,	/* indicate inode is recoverable */
 	FI_DIRTY_DIR,		/* indicate directory has dirty pages */
 	FI_INC_LINK,		/* need to increment i_nlink */
@@ -1968,6 +1971,7 @@ int update_inode_page(struct inode *);
 int f2fs_write_inode(struct inode *, struct writeback_control *);
 void f2fs_evict_inode(struct inode *);
 void handle_failed_inode(struct inode *);
+int f2fs_update_time(struct inode *, struct timespec *, int);
 
 /*
  * namei.c
@@ -2135,6 +2139,7 @@ void remove_ino_entry(struct f2fs_sb_info *, nid_t, int type);
 void release_ino_entry(struct f2fs_sb_info *, bool);
 bool exist_written_data(struct f2fs_sb_info *, nid_t, int);
 int f2fs_sync_inode_meta(struct f2fs_sb_info *);
+int f2fs_sync_dirty_itime(struct f2fs_sb_info *);
 int acquire_orphan_inode(struct f2fs_sb_info *);
 void release_orphan_inode(struct f2fs_sb_info *);
 void add_orphan_inode(struct inode *);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b46f6e1..88c8aeb 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -777,6 +777,7 @@ const struct inode_operations f2fs_file_inode_operations = {
 	.removexattr	= generic_removexattr,
 #endif
 	.fiemap		= f2fs_fiemap,
+	.update_time	= f2fs_update_time,
 };
 
 static int fill_zero(struct inode *inode, pgoff_t index,
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 300aef8..a219e93 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -18,6 +18,7 @@
 
 #include "f2fs.h"
 #include "node.h"
+#include "segment.h"
 #include "xattr.h"
 #include "acl.h"
 #include <trace/events/f2fs.h>
@@ -1074,6 +1075,39 @@ errout:
 	return ERR_PTR(res);
 }
 
+int f2fs_update_time(struct inode *inode, struct timespec *time, int flags)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+	int iflags = I_DIRTY_TIME;
+
+	if (flags & S_ATIME)
+		inode->i_atime = *time;
+	if (flags & S_VERSION)
+		inode_inc_iversion(inode);
+	if (flags & S_CTIME)
+		inode->i_ctime = *time;
+	if (flags & S_MTIME)
+		inode->i_mtime = *time;
+
+	if (!(inode->i_sb->s_flags & MS_LAZYTIME) || (flags & S_VERSION))
+		iflags |= I_DIRTY_SYNC;
+
+	if (iflags == I_DIRTY_TIME) {
+		int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
+
+		if (itime_secs >= 16 || (itime_secs >= 8 &&
+				has_not_enough_free_secs(sbi, 0, 0))) {
+			f2fs_sync_dirty_itime(sbi);
+			mutex_lock(&sbi->gc_mutex);
+			f2fs_gc(sbi, false);
+			iflags |= I_DIRTY_SYNC;
+		}
+	}
+
+	__mark_inode_dirty(inode, iflags);
+	return 0;
+}
+
 const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
 	.readlink       = generic_readlink,
 	.get_link       = f2fs_encrypted_get_link,
@@ -1085,6 +1119,7 @@ const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr	= generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
 
 const struct inode_operations f2fs_dir_inode_operations = {
@@ -1108,6 +1143,7 @@ const struct inode_operations f2fs_dir_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr	= generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
 
 const struct inode_operations f2fs_symlink_inode_operations = {
@@ -1121,6 +1157,7 @@ const struct inode_operations f2fs_symlink_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr	= generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
 
 const struct inode_operations f2fs_special_inode_operations = {
@@ -1134,4 +1171,5 @@ const struct inode_operations f2fs_special_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr    = generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index fecb856..116577e 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -471,12 +471,14 @@ static inline bool need_SSR(struct f2fs_sb_info *sbi)
 {
 	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
 	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
+	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
 
 	if (test_opt(sbi, LFS))
 		return false;
 
 	return free_sections(sbi) <= (node_secs + 2 * dent_secs +
-						reserved_sections(sbi) + 1);
+		imeta_secs + itime_secs + reserved_sections(sbi) + 1);
 }
 
 static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
@@ -484,14 +486,15 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 {
 	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
 	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-
-	node_secs += get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
 
 	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
 		return false;
 
 	return (free_sections(sbi) + freed) <=
-		(node_secs + 2 * dent_secs + reserved_sections(sbi) + needed);
+		(node_secs + 2 * dent_secs + imeta_secs + itime_secs +
+		reserved_sections(sbi) + needed);
 }
 
 static inline bool excess_prefree_segs(struct f2fs_sb_info *sbi)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e38c9d2..93e3b07 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -620,16 +620,49 @@ static int f2fs_drop_inode(struct inode *inode)
 	return generic_drop_inode(inode);
 }
 
+int f2fs_set_inode_dirty_time(struct inode *inode)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+
+	spin_lock(&sbi->inode_lock[DIRTY_META]);
+	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
+		spin_unlock(&sbi->inode_lock[DIRTY_META]);
+		return 0;
+	}
+
+	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
+		spin_unlock(&sbi->inode_lock[DIRTY_META]);
+		return 0;
+	}
+
+	set_inode_flag(inode, FI_DIRTY_ITIME);
+	list_add_tail(&F2FS_I(inode)->gdirty_list,
+				&sbi->inode_list[DIRTY_ITIME]);
+	inc_page_count(sbi, F2FS_DIRTY_ITIME);
+	stat_inc_dirty_inode(sbi, DIRTY_ITIME);
+	spin_unlock(&sbi->inode_lock[DIRTY_META]);
+
+	return 1;
+}
+
 int f2fs_inode_dirtied(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 
 	spin_lock(&sbi->inode_lock[DIRTY_META]);
+	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
+		clear_inode_flag(inode, FI_DIRTY_ITIME);
+		list_del_init(&F2FS_I(inode)->gdirty_list);
+		dec_page_count(sbi, F2FS_DIRTY_ITIME);
+		stat_dec_dirty_inode(sbi, DIRTY_ITIME);
+		goto mark_dirty;
+	}
+
 	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
 		spin_unlock(&sbi->inode_lock[DIRTY_META]);
 		return 1;
 	}
-
+mark_dirty:
 	set_inode_flag(inode, FI_DIRTY_INODE);
 	list_add_tail(&F2FS_I(inode)->gdirty_list,
 				&sbi->inode_list[DIRTY_META]);
@@ -645,15 +678,23 @@ void f2fs_inode_synced(struct inode *inode)
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 
 	spin_lock(&sbi->inode_lock[DIRTY_META]);
-	if (!is_inode_flag_set(inode, FI_DIRTY_INODE)) {
+	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
+		clear_inode_flag(inode, FI_DIRTY_ITIME);
+		list_del_init(&F2FS_I(inode)->gdirty_list);
+		dec_page_count(sbi, F2FS_DIRTY_ITIME);
+		stat_dec_dirty_inode(sbi, DIRTY_ITIME);
 		spin_unlock(&sbi->inode_lock[DIRTY_META]);
 		return;
 	}
-	list_del_init(&F2FS_I(inode)->gdirty_list);
-	clear_inode_flag(inode, FI_DIRTY_INODE);
-	clear_inode_flag(inode, FI_AUTO_RECOVER);
-	dec_page_count(sbi, F2FS_DIRTY_IMETA);
-	stat_dec_dirty_inode(F2FS_I_SB(inode), DIRTY_META);
+
+	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
+		clear_inode_flag(inode, FI_DIRTY_INODE);
+		clear_inode_flag(inode, FI_AUTO_RECOVER);
+		list_del_init(&F2FS_I(inode)->gdirty_list);
+		dec_page_count(sbi, F2FS_DIRTY_IMETA);
+		stat_dec_dirty_inode(sbi, DIRTY_META);
+	}
+
 	spin_unlock(&sbi->inode_lock[DIRTY_META]);
 }
 
@@ -670,8 +711,10 @@ static void f2fs_dirty_inode(struct inode *inode, int flags)
 			inode->i_ino == F2FS_META_INO(sbi))
 		return;
 
-	if (flags == I_DIRTY_TIME)
+	if (flags == I_DIRTY_TIME) {
+		f2fs_set_inode_dirty_time(inode);
 		return;
+	}
 
 	if (is_inode_flag_set(inode, FI_AUTO_RECOVER))
 		clear_inode_flag(inode, FI_AUTO_RECOVER);
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 2/2] f2fs: fix allocation failure
@ 2016-10-12 16:14   ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2016-10-12 16:14 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

From: Chao Yu <yuchao0@huawei.com>

tests/generic/251 of fstest reports a f2fs bug in below message:

------------[ cut here ]------------
invalid opcode: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 109 Comm: kworker/u8:2 Tainted: G        W  O    4.8.0-rc4+ #22
Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
Workqueue: writeback wb_workfn (flush-251:1)
task: f33c8000 task.stack: f33c6000
EIP: 0060:[<f8992139>] EFLAGS: 00010246 CPU: 1
EIP is at new_curseg+0x2c9/0x2d0 [f2fs]
EAX: 000003f3 EBX: ee3e5000 ECX: 00000400 EDX: 000003f3
ESI: 00000000 EDI: 00000008 EBP: f33c78c4 ESP: f33c7890
 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
CR0: 80050033 CR2: b6706000 CR3: 2e8d70c0 CR4: 000406f0
Stack:
 eb29480c 00000004 f27a0290 000003f3 00000008 f33c78c0 00000001 eb294800
 00000001 00000000 ee3e5000 f899dbb4 00000290 f33c7924 f89926d6 c10b42b6
 00000246 00000000 eed513e8 00000246 f33c78e8 c10b7b4b f33c7924 c178c304
Call Trace:
 [<f89926d6>] allocate_segment_by_default+0x3c6/0x3d0 [f2fs]
 [<f8992b3a>] allocate_data_block+0x13a/0x3c0 [f2fs]
 [<f8992e4b>] do_write_page+0x8b/0x230 [f2fs]
 [<f8993070>] write_node_page+0x20/0x30 [f2fs]
 [<f898a156>] f2fs_write_node_page+0x1a6/0x340 [f2fs]
 [<f898ca45>] sync_node_pages+0x4a5/0x590 [f2fs]
 [<f897ea48>] write_checkpoint+0x218/0x720 [f2fs]
 [<f898143d>] f2fs_gc+0x4cd/0x6b0 [f2fs]
 [<f8990ebe>] f2fs_balance_fs+0x18e/0x1b0 [f2fs]
 [<f8988017>] f2fs_write_data_page+0x197/0x6f0 [f2fs]
 [<f89830fe>] f2fs_write_data_pages+0x28e/0x7e0 [f2fs]
 [<c118b1cd>] do_writepages+0x1d/0x40
 [<c1228cb5>] __writeback_single_inode+0x55/0x7e0
 [<c1229b6b>] writeback_sb_inodes+0x21b/0x490
 [<c1229f6c>] wb_writeback+0xdc/0x590
 [<c122ae18>] wb_workfn+0xf8/0x690
 [<c107c231>] process_one_work+0x1a1/0x580
 [<c107c712>] worker_thread+0x102/0x440
 [<c1082021>] kthread+0xa1/0xc0
 [<c178f862>] ret_from_kernel_thread+0xe/0x24
EIP: [<f8992139>] new_curseg+0x2c9/0x2d0 [f2fs] SS:ESP 0068:f33c7890

The reason is after f2fs enabled lazytime by default, when inode time is
changed, we do not set this inode dirty through ->f2fs_dirty_inode, so
itime updating will be delayed.

Finally it needs to update the dirty time of inode into inode page,
and writeback the page, however, before that, we didn't count the inode
as imeta data. So f2fs won't be aware of dirty metadata page count is
exceeded watermark of GC, result in encountering panic when allocating
free segment.

There is an easy way to produce this bug:
1. mount with lazytime option
2. fragment space
3. touch all files in the image
4. umount

Introduce itime data type and related flow to trace & flush delayed
updating inode to fix this issue.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c | 37 ++++++++++++++++++++++++++++++++
 fs/f2fs/f2fs.h       |  5 +++++
 fs/f2fs/file.c       |  1 +
 fs/f2fs/namei.c      | 38 +++++++++++++++++++++++++++++++++
 fs/f2fs/segment.h    | 11 ++++++----
 fs/f2fs/super.c      | 59 +++++++++++++++++++++++++++++++++++++++++++++-------
 6 files changed, 139 insertions(+), 12 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index d95fada..e27c64f 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -928,6 +928,43 @@ int f2fs_sync_inode_meta(struct f2fs_sb_info *sbi)
 	return 0;
 }
 
+int f2fs_sync_dirty_itime(struct f2fs_sb_info *sbi)
+{
+	struct list_head *head = &sbi->inode_list[DIRTY_ITIME];
+	struct inode *inode;
+	struct f2fs_inode_info *fi;
+	s64 total = get_pages(sbi, F2FS_DIRTY_ITIME);
+
+	while (total--) {
+		if (unlikely(f2fs_cp_error(sbi)))
+			return -EIO;
+
+		spin_lock(&sbi->inode_lock[DIRTY_META]);
+		if (list_empty(head)) {
+			spin_unlock(&sbi->inode_lock[DIRTY_META]);
+			return 0;
+		}
+		fi = list_entry(head->next, struct f2fs_inode_info,
+							gdirty_list);
+		list_move_tail(&fi->gdirty_list, head);
+		inode = igrab(&fi->vfs_inode);
+		spin_unlock(&sbi->inode_lock[DIRTY_META]);
+		if (inode) {
+			spin_lock(&inode->i_lock);
+			if (inode->i_nlink && (inode->i_state & I_DIRTY_TIME)) {
+				inode->i_state &= ~I_DIRTY_TIME;
+				spin_unlock(&inode->i_lock);
+				mark_inode_dirty_sync(inode);
+			} else {
+				spin_unlock(&inode->i_lock);
+			}
+
+			iput(inode);
+		}
+	}
+	return 0;
+}
+
 /*
  * Freeze all the FS-operations for checkpoint.
  */
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5c19136..1f302ff 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -682,6 +682,7 @@ enum count_type {
 	F2FS_DIRTY_META,
 	F2FS_INMEM_PAGES,
 	F2FS_DIRTY_IMETA,
+	F2FS_DIRTY_ITIME,
 	NR_COUNT_TYPE,
 };
 
@@ -734,6 +735,7 @@ enum inode_type {
 	DIR_INODE,			/* for dirty dir inode */
 	FILE_INODE,			/* for dirty regular/symlink inode */
 	DIRTY_META,			/* for all dirtied inode metadata */
+	DIRTY_ITIME,			/* for all I_DIRTY_TIME taged inode */
 	NR_INODE_TYPE,
 };
 
@@ -1613,6 +1615,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
 enum {
 	FI_NEW_INODE,		/* indicate newly allocated inode */
 	FI_DIRTY_INODE,		/* indicate inode is dirty or not */
+	FI_DIRTY_ITIME,		/* inode is dirtied due to time updating */
 	FI_AUTO_RECOVER,	/* indicate inode is recoverable */
 	FI_DIRTY_DIR,		/* indicate directory has dirty pages */
 	FI_INC_LINK,		/* need to increment i_nlink */
@@ -1968,6 +1971,7 @@ int update_inode_page(struct inode *);
 int f2fs_write_inode(struct inode *, struct writeback_control *);
 void f2fs_evict_inode(struct inode *);
 void handle_failed_inode(struct inode *);
+int f2fs_update_time(struct inode *, struct timespec *, int);
 
 /*
  * namei.c
@@ -2135,6 +2139,7 @@ void remove_ino_entry(struct f2fs_sb_info *, nid_t, int type);
 void release_ino_entry(struct f2fs_sb_info *, bool);
 bool exist_written_data(struct f2fs_sb_info *, nid_t, int);
 int f2fs_sync_inode_meta(struct f2fs_sb_info *);
+int f2fs_sync_dirty_itime(struct f2fs_sb_info *);
 int acquire_orphan_inode(struct f2fs_sb_info *);
 void release_orphan_inode(struct f2fs_sb_info *);
 void add_orphan_inode(struct inode *);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b46f6e1..88c8aeb 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -777,6 +777,7 @@ const struct inode_operations f2fs_file_inode_operations = {
 	.removexattr	= generic_removexattr,
 #endif
 	.fiemap		= f2fs_fiemap,
+	.update_time	= f2fs_update_time,
 };
 
 static int fill_zero(struct inode *inode, pgoff_t index,
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 300aef8..a219e93 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -18,6 +18,7 @@
 
 #include "f2fs.h"
 #include "node.h"
+#include "segment.h"
 #include "xattr.h"
 #include "acl.h"
 #include <trace/events/f2fs.h>
@@ -1074,6 +1075,39 @@ errout:
 	return ERR_PTR(res);
 }
 
+int f2fs_update_time(struct inode *inode, struct timespec *time, int flags)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+	int iflags = I_DIRTY_TIME;
+
+	if (flags & S_ATIME)
+		inode->i_atime = *time;
+	if (flags & S_VERSION)
+		inode_inc_iversion(inode);
+	if (flags & S_CTIME)
+		inode->i_ctime = *time;
+	if (flags & S_MTIME)
+		inode->i_mtime = *time;
+
+	if (!(inode->i_sb->s_flags & MS_LAZYTIME) || (flags & S_VERSION))
+		iflags |= I_DIRTY_SYNC;
+
+	if (iflags == I_DIRTY_TIME) {
+		int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
+
+		if (itime_secs >= 16 || (itime_secs >= 8 &&
+				has_not_enough_free_secs(sbi, 0, 0))) {
+			f2fs_sync_dirty_itime(sbi);
+			mutex_lock(&sbi->gc_mutex);
+			f2fs_gc(sbi, false);
+			iflags |= I_DIRTY_SYNC;
+		}
+	}
+
+	__mark_inode_dirty(inode, iflags);
+	return 0;
+}
+
 const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
 	.readlink       = generic_readlink,
 	.get_link       = f2fs_encrypted_get_link,
@@ -1085,6 +1119,7 @@ const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr	= generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
 
 const struct inode_operations f2fs_dir_inode_operations = {
@@ -1108,6 +1143,7 @@ const struct inode_operations f2fs_dir_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr	= generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
 
 const struct inode_operations f2fs_symlink_inode_operations = {
@@ -1121,6 +1157,7 @@ const struct inode_operations f2fs_symlink_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr	= generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
 
 const struct inode_operations f2fs_special_inode_operations = {
@@ -1134,4 +1171,5 @@ const struct inode_operations f2fs_special_inode_operations = {
 	.listxattr	= f2fs_listxattr,
 	.removexattr    = generic_removexattr,
 #endif
+	.update_time	= f2fs_update_time,
 };
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index fecb856..116577e 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -471,12 +471,14 @@ static inline bool need_SSR(struct f2fs_sb_info *sbi)
 {
 	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
 	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
+	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
 
 	if (test_opt(sbi, LFS))
 		return false;
 
 	return free_sections(sbi) <= (node_secs + 2 * dent_secs +
-						reserved_sections(sbi) + 1);
+		imeta_secs + itime_secs + reserved_sections(sbi) + 1);
 }
 
 static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
@@ -484,14 +486,15 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 {
 	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
 	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-
-	node_secs += get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
 
 	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
 		return false;
 
 	return (free_sections(sbi) + freed) <=
-		(node_secs + 2 * dent_secs + reserved_sections(sbi) + needed);
+		(node_secs + 2 * dent_secs + imeta_secs + itime_secs +
+		reserved_sections(sbi) + needed);
 }
 
 static inline bool excess_prefree_segs(struct f2fs_sb_info *sbi)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e38c9d2..93e3b07 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -620,16 +620,49 @@ static int f2fs_drop_inode(struct inode *inode)
 	return generic_drop_inode(inode);
 }
 
+int f2fs_set_inode_dirty_time(struct inode *inode)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+
+	spin_lock(&sbi->inode_lock[DIRTY_META]);
+	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
+		spin_unlock(&sbi->inode_lock[DIRTY_META]);
+		return 0;
+	}
+
+	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
+		spin_unlock(&sbi->inode_lock[DIRTY_META]);
+		return 0;
+	}
+
+	set_inode_flag(inode, FI_DIRTY_ITIME);
+	list_add_tail(&F2FS_I(inode)->gdirty_list,
+				&sbi->inode_list[DIRTY_ITIME]);
+	inc_page_count(sbi, F2FS_DIRTY_ITIME);
+	stat_inc_dirty_inode(sbi, DIRTY_ITIME);
+	spin_unlock(&sbi->inode_lock[DIRTY_META]);
+
+	return 1;
+}
+
 int f2fs_inode_dirtied(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 
 	spin_lock(&sbi->inode_lock[DIRTY_META]);
+	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
+		clear_inode_flag(inode, FI_DIRTY_ITIME);
+		list_del_init(&F2FS_I(inode)->gdirty_list);
+		dec_page_count(sbi, F2FS_DIRTY_ITIME);
+		stat_dec_dirty_inode(sbi, DIRTY_ITIME);
+		goto mark_dirty;
+	}
+
 	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
 		spin_unlock(&sbi->inode_lock[DIRTY_META]);
 		return 1;
 	}
-
+mark_dirty:
 	set_inode_flag(inode, FI_DIRTY_INODE);
 	list_add_tail(&F2FS_I(inode)->gdirty_list,
 				&sbi->inode_list[DIRTY_META]);
@@ -645,15 +678,23 @@ void f2fs_inode_synced(struct inode *inode)
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 
 	spin_lock(&sbi->inode_lock[DIRTY_META]);
-	if (!is_inode_flag_set(inode, FI_DIRTY_INODE)) {
+	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
+		clear_inode_flag(inode, FI_DIRTY_ITIME);
+		list_del_init(&F2FS_I(inode)->gdirty_list);
+		dec_page_count(sbi, F2FS_DIRTY_ITIME);
+		stat_dec_dirty_inode(sbi, DIRTY_ITIME);
 		spin_unlock(&sbi->inode_lock[DIRTY_META]);
 		return;
 	}
-	list_del_init(&F2FS_I(inode)->gdirty_list);
-	clear_inode_flag(inode, FI_DIRTY_INODE);
-	clear_inode_flag(inode, FI_AUTO_RECOVER);
-	dec_page_count(sbi, F2FS_DIRTY_IMETA);
-	stat_dec_dirty_inode(F2FS_I_SB(inode), DIRTY_META);
+
+	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
+		clear_inode_flag(inode, FI_DIRTY_INODE);
+		clear_inode_flag(inode, FI_AUTO_RECOVER);
+		list_del_init(&F2FS_I(inode)->gdirty_list);
+		dec_page_count(sbi, F2FS_DIRTY_IMETA);
+		stat_dec_dirty_inode(sbi, DIRTY_META);
+	}
+
 	spin_unlock(&sbi->inode_lock[DIRTY_META]);
 }
 
@@ -670,8 +711,10 @@ static void f2fs_dirty_inode(struct inode *inode, int flags)
 			inode->i_ino == F2FS_META_INO(sbi))
 		return;
 
-	if (flags == I_DIRTY_TIME)
+	if (flags == I_DIRTY_TIME) {
+		f2fs_set_inode_dirty_time(inode);
 		return;
+	}
 
 	if (is_inode_flag_set(inode, FI_AUTO_RECOVER))
 		clear_inode_flag(inode, FI_AUTO_RECOVER);
-- 
2.10.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 2/2] f2fs: fix allocation failure
  2016-10-12 16:14   ` Chao Yu
  (?)
@ 2016-10-13 20:49   ` Jaegeuk Kim
  2016-10-14 14:09       ` Chao Yu
  -1 siblings, 1 reply; 8+ messages in thread
From: Jaegeuk Kim @ 2016-10-13 20:49 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

Hi Chao,

On Thu, Oct 13, 2016 at 12:14:27AM +0800, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> tests/generic/251 of fstest reports a f2fs bug in below message:
> 
> ------------[ cut here ]------------
> invalid opcode: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 109 Comm: kworker/u8:2 Tainted: G        W  O    4.8.0-rc4+ #22
> Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> Workqueue: writeback wb_workfn (flush-251:1)
> task: f33c8000 task.stack: f33c6000
> EIP: 0060:[<f8992139>] EFLAGS: 00010246 CPU: 1
> EIP is at new_curseg+0x2c9/0x2d0 [f2fs]
> EAX: 000003f3 EBX: ee3e5000 ECX: 00000400 EDX: 000003f3
> ESI: 00000000 EDI: 00000008 EBP: f33c78c4 ESP: f33c7890
>  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> CR0: 80050033 CR2: b6706000 CR3: 2e8d70c0 CR4: 000406f0
> Stack:
>  eb29480c 00000004 f27a0290 000003f3 00000008 f33c78c0 00000001 eb294800
>  00000001 00000000 ee3e5000 f899dbb4 00000290 f33c7924 f89926d6 c10b42b6
>  00000246 00000000 eed513e8 00000246 f33c78e8 c10b7b4b f33c7924 c178c304
> Call Trace:
>  [<f89926d6>] allocate_segment_by_default+0x3c6/0x3d0 [f2fs]
>  [<f8992b3a>] allocate_data_block+0x13a/0x3c0 [f2fs]
>  [<f8992e4b>] do_write_page+0x8b/0x230 [f2fs]
>  [<f8993070>] write_node_page+0x20/0x30 [f2fs]
>  [<f898a156>] f2fs_write_node_page+0x1a6/0x340 [f2fs]
>  [<f898ca45>] sync_node_pages+0x4a5/0x590 [f2fs]
>  [<f897ea48>] write_checkpoint+0x218/0x720 [f2fs]
>  [<f898143d>] f2fs_gc+0x4cd/0x6b0 [f2fs]
>  [<f8990ebe>] f2fs_balance_fs+0x18e/0x1b0 [f2fs]
>  [<f8988017>] f2fs_write_data_page+0x197/0x6f0 [f2fs]
>  [<f89830fe>] f2fs_write_data_pages+0x28e/0x7e0 [f2fs]
>  [<c118b1cd>] do_writepages+0x1d/0x40
>  [<c1228cb5>] __writeback_single_inode+0x55/0x7e0
>  [<c1229b6b>] writeback_sb_inodes+0x21b/0x490
>  [<c1229f6c>] wb_writeback+0xdc/0x590
>  [<c122ae18>] wb_workfn+0xf8/0x690
>  [<c107c231>] process_one_work+0x1a1/0x580
>  [<c107c712>] worker_thread+0x102/0x440
>  [<c1082021>] kthread+0xa1/0xc0
>  [<c178f862>] ret_from_kernel_thread+0xe/0x24
> EIP: [<f8992139>] new_curseg+0x2c9/0x2d0 [f2fs] SS:ESP 0068:f33c7890
> 
> The reason is after f2fs enabled lazytime by default, when inode time is
> changed, we do not set this inode dirty through ->f2fs_dirty_inode, so
> itime updating will be delayed.
> 
> Finally it needs to update the dirty time of inode into inode page,
> and writeback the page, however, before that, we didn't count the inode
> as imeta data. So f2fs won't be aware of dirty metadata page count is
> exceeded watermark of GC, result in encountering panic when allocating
> free segment.
> 
> There is an easy way to produce this bug:
> 1. mount with lazytime option
> 2. fragment space
> 3. touch all files in the image
> 4. umount

I think modifying has_not_enough_secs() is enough like this.

---
 fs/f2fs/segment.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index fecb856..a6efb5c 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -471,11 +471,12 @@ static inline bool need_SSR(struct f2fs_sb_info *sbi)
 {
 	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
 	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
+	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
 
 	if (test_opt(sbi, LFS))
 		return false;
 
-	return free_sections(sbi) <= (node_secs + 2 * dent_secs +
+	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
 						reserved_sections(sbi) + 1);
 }
 
@@ -484,6 +485,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 {
 	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
 	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
+	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
 
 	node_secs += get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
 
@@ -491,7 +493,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 		return false;
 
 	return (free_sections(sbi) + freed) <=
-		(node_secs + 2 * dent_secs + reserved_sections(sbi) + needed);
+		(node_secs + 2 * dent_secs + imeta_secs +
+		reserved_sections(sbi) + needed);
 }
 
 static inline bool excess_prefree_segs(struct f2fs_sb_info *sbi)
-- 
2.8.3

Thanks,

> 
> Introduce itime data type and related flow to trace & flush delayed
> updating inode to fix this issue.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/checkpoint.c | 37 ++++++++++++++++++++++++++++++++
>  fs/f2fs/f2fs.h       |  5 +++++
>  fs/f2fs/file.c       |  1 +
>  fs/f2fs/namei.c      | 38 +++++++++++++++++++++++++++++++++
>  fs/f2fs/segment.h    | 11 ++++++----
>  fs/f2fs/super.c      | 59 +++++++++++++++++++++++++++++++++++++++++++++-------
>  6 files changed, 139 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index d95fada..e27c64f 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -928,6 +928,43 @@ int f2fs_sync_inode_meta(struct f2fs_sb_info *sbi)
>  	return 0;
>  }
>  
> +int f2fs_sync_dirty_itime(struct f2fs_sb_info *sbi)
> +{
> +	struct list_head *head = &sbi->inode_list[DIRTY_ITIME];
> +	struct inode *inode;
> +	struct f2fs_inode_info *fi;
> +	s64 total = get_pages(sbi, F2FS_DIRTY_ITIME);
> +
> +	while (total--) {
> +		if (unlikely(f2fs_cp_error(sbi)))
> +			return -EIO;
> +
> +		spin_lock(&sbi->inode_lock[DIRTY_META]);
> +		if (list_empty(head)) {
> +			spin_unlock(&sbi->inode_lock[DIRTY_META]);
> +			return 0;
> +		}
> +		fi = list_entry(head->next, struct f2fs_inode_info,
> +							gdirty_list);
> +		list_move_tail(&fi->gdirty_list, head);
> +		inode = igrab(&fi->vfs_inode);
> +		spin_unlock(&sbi->inode_lock[DIRTY_META]);
> +		if (inode) {
> +			spin_lock(&inode->i_lock);
> +			if (inode->i_nlink && (inode->i_state & I_DIRTY_TIME)) {
> +				inode->i_state &= ~I_DIRTY_TIME;
> +				spin_unlock(&inode->i_lock);
> +				mark_inode_dirty_sync(inode);
> +			} else {
> +				spin_unlock(&inode->i_lock);
> +			}
> +
> +			iput(inode);
> +		}
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Freeze all the FS-operations for checkpoint.
>   */
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 5c19136..1f302ff 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -682,6 +682,7 @@ enum count_type {
>  	F2FS_DIRTY_META,
>  	F2FS_INMEM_PAGES,
>  	F2FS_DIRTY_IMETA,
> +	F2FS_DIRTY_ITIME,
>  	NR_COUNT_TYPE,
>  };
>  
> @@ -734,6 +735,7 @@ enum inode_type {
>  	DIR_INODE,			/* for dirty dir inode */
>  	FILE_INODE,			/* for dirty regular/symlink inode */
>  	DIRTY_META,			/* for all dirtied inode metadata */
> +	DIRTY_ITIME,			/* for all I_DIRTY_TIME taged inode */
>  	NR_INODE_TYPE,
>  };
>  
> @@ -1613,6 +1615,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
>  enum {
>  	FI_NEW_INODE,		/* indicate newly allocated inode */
>  	FI_DIRTY_INODE,		/* indicate inode is dirty or not */
> +	FI_DIRTY_ITIME,		/* inode is dirtied due to time updating */
>  	FI_AUTO_RECOVER,	/* indicate inode is recoverable */
>  	FI_DIRTY_DIR,		/* indicate directory has dirty pages */
>  	FI_INC_LINK,		/* need to increment i_nlink */
> @@ -1968,6 +1971,7 @@ int update_inode_page(struct inode *);
>  int f2fs_write_inode(struct inode *, struct writeback_control *);
>  void f2fs_evict_inode(struct inode *);
>  void handle_failed_inode(struct inode *);
> +int f2fs_update_time(struct inode *, struct timespec *, int);
>  
>  /*
>   * namei.c
> @@ -2135,6 +2139,7 @@ void remove_ino_entry(struct f2fs_sb_info *, nid_t, int type);
>  void release_ino_entry(struct f2fs_sb_info *, bool);
>  bool exist_written_data(struct f2fs_sb_info *, nid_t, int);
>  int f2fs_sync_inode_meta(struct f2fs_sb_info *);
> +int f2fs_sync_dirty_itime(struct f2fs_sb_info *);
>  int acquire_orphan_inode(struct f2fs_sb_info *);
>  void release_orphan_inode(struct f2fs_sb_info *);
>  void add_orphan_inode(struct inode *);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index b46f6e1..88c8aeb 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -777,6 +777,7 @@ const struct inode_operations f2fs_file_inode_operations = {
>  	.removexattr	= generic_removexattr,
>  #endif
>  	.fiemap		= f2fs_fiemap,
> +	.update_time	= f2fs_update_time,
>  };
>  
>  static int fill_zero(struct inode *inode, pgoff_t index,
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 300aef8..a219e93 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -18,6 +18,7 @@
>  
>  #include "f2fs.h"
>  #include "node.h"
> +#include "segment.h"
>  #include "xattr.h"
>  #include "acl.h"
>  #include <trace/events/f2fs.h>
> @@ -1074,6 +1075,39 @@ errout:
>  	return ERR_PTR(res);
>  }
>  
> +int f2fs_update_time(struct inode *inode, struct timespec *time, int flags)
> +{
> +	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> +	int iflags = I_DIRTY_TIME;
> +
> +	if (flags & S_ATIME)
> +		inode->i_atime = *time;
> +	if (flags & S_VERSION)
> +		inode_inc_iversion(inode);
> +	if (flags & S_CTIME)
> +		inode->i_ctime = *time;
> +	if (flags & S_MTIME)
> +		inode->i_mtime = *time;
> +
> +	if (!(inode->i_sb->s_flags & MS_LAZYTIME) || (flags & S_VERSION))
> +		iflags |= I_DIRTY_SYNC;
> +
> +	if (iflags == I_DIRTY_TIME) {
> +		int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
> +
> +		if (itime_secs >= 16 || (itime_secs >= 8 &&
> +				has_not_enough_free_secs(sbi, 0, 0))) {
> +			f2fs_sync_dirty_itime(sbi);
> +			mutex_lock(&sbi->gc_mutex);
> +			f2fs_gc(sbi, false);
> +			iflags |= I_DIRTY_SYNC;
> +		}
> +	}
> +
> +	__mark_inode_dirty(inode, iflags);
> +	return 0;
> +}
> +
>  const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
>  	.readlink       = generic_readlink,
>  	.get_link       = f2fs_encrypted_get_link,
> @@ -1085,6 +1119,7 @@ const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
>  	.listxattr	= f2fs_listxattr,
>  	.removexattr	= generic_removexattr,
>  #endif
> +	.update_time	= f2fs_update_time,
>  };
>  
>  const struct inode_operations f2fs_dir_inode_operations = {
> @@ -1108,6 +1143,7 @@ const struct inode_operations f2fs_dir_inode_operations = {
>  	.listxattr	= f2fs_listxattr,
>  	.removexattr	= generic_removexattr,
>  #endif
> +	.update_time	= f2fs_update_time,
>  };
>  
>  const struct inode_operations f2fs_symlink_inode_operations = {
> @@ -1121,6 +1157,7 @@ const struct inode_operations f2fs_symlink_inode_operations = {
>  	.listxattr	= f2fs_listxattr,
>  	.removexattr	= generic_removexattr,
>  #endif
> +	.update_time	= f2fs_update_time,
>  };
>  
>  const struct inode_operations f2fs_special_inode_operations = {
> @@ -1134,4 +1171,5 @@ const struct inode_operations f2fs_special_inode_operations = {
>  	.listxattr	= f2fs_listxattr,
>  	.removexattr    = generic_removexattr,
>  #endif
> +	.update_time	= f2fs_update_time,
>  };
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index fecb856..116577e 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -471,12 +471,14 @@ static inline bool need_SSR(struct f2fs_sb_info *sbi)
>  {
>  	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>  	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> +	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> +	int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
>  
>  	if (test_opt(sbi, LFS))
>  		return false;
>  
>  	return free_sections(sbi) <= (node_secs + 2 * dent_secs +
> -						reserved_sections(sbi) + 1);
> +		imeta_secs + itime_secs + reserved_sections(sbi) + 1);
>  }
>  
>  static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
> @@ -484,14 +486,15 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  {
>  	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>  	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> -
> -	node_secs += get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> +	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> +	int itime_secs = get_blocktype_secs(sbi, F2FS_DIRTY_ITIME);
>  
>  	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>  		return false;
>  
>  	return (free_sections(sbi) + freed) <=
> -		(node_secs + 2 * dent_secs + reserved_sections(sbi) + needed);
> +		(node_secs + 2 * dent_secs + imeta_secs + itime_secs +
> +		reserved_sections(sbi) + needed);
>  }
>  
>  static inline bool excess_prefree_segs(struct f2fs_sb_info *sbi)
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index e38c9d2..93e3b07 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -620,16 +620,49 @@ static int f2fs_drop_inode(struct inode *inode)
>  	return generic_drop_inode(inode);
>  }
>  
> +int f2fs_set_inode_dirty_time(struct inode *inode)
> +{
> +	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> +
> +	spin_lock(&sbi->inode_lock[DIRTY_META]);
> +	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
> +		spin_unlock(&sbi->inode_lock[DIRTY_META]);
> +		return 0;
> +	}
> +
> +	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
> +		spin_unlock(&sbi->inode_lock[DIRTY_META]);
> +		return 0;
> +	}
> +
> +	set_inode_flag(inode, FI_DIRTY_ITIME);
> +	list_add_tail(&F2FS_I(inode)->gdirty_list,
> +				&sbi->inode_list[DIRTY_ITIME]);
> +	inc_page_count(sbi, F2FS_DIRTY_ITIME);
> +	stat_inc_dirty_inode(sbi, DIRTY_ITIME);
> +	spin_unlock(&sbi->inode_lock[DIRTY_META]);
> +
> +	return 1;
> +}
> +
>  int f2fs_inode_dirtied(struct inode *inode)
>  {
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  
>  	spin_lock(&sbi->inode_lock[DIRTY_META]);
> +	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
> +		clear_inode_flag(inode, FI_DIRTY_ITIME);
> +		list_del_init(&F2FS_I(inode)->gdirty_list);
> +		dec_page_count(sbi, F2FS_DIRTY_ITIME);
> +		stat_dec_dirty_inode(sbi, DIRTY_ITIME);
> +		goto mark_dirty;
> +	}
> +
>  	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
>  		spin_unlock(&sbi->inode_lock[DIRTY_META]);
>  		return 1;
>  	}
> -
> +mark_dirty:
>  	set_inode_flag(inode, FI_DIRTY_INODE);
>  	list_add_tail(&F2FS_I(inode)->gdirty_list,
>  				&sbi->inode_list[DIRTY_META]);
> @@ -645,15 +678,23 @@ void f2fs_inode_synced(struct inode *inode)
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  
>  	spin_lock(&sbi->inode_lock[DIRTY_META]);
> -	if (!is_inode_flag_set(inode, FI_DIRTY_INODE)) {
> +	if (is_inode_flag_set(inode, FI_DIRTY_ITIME)) {
> +		clear_inode_flag(inode, FI_DIRTY_ITIME);
> +		list_del_init(&F2FS_I(inode)->gdirty_list);
> +		dec_page_count(sbi, F2FS_DIRTY_ITIME);
> +		stat_dec_dirty_inode(sbi, DIRTY_ITIME);
>  		spin_unlock(&sbi->inode_lock[DIRTY_META]);
>  		return;
>  	}
> -	list_del_init(&F2FS_I(inode)->gdirty_list);
> -	clear_inode_flag(inode, FI_DIRTY_INODE);
> -	clear_inode_flag(inode, FI_AUTO_RECOVER);
> -	dec_page_count(sbi, F2FS_DIRTY_IMETA);
> -	stat_dec_dirty_inode(F2FS_I_SB(inode), DIRTY_META);
> +
> +	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
> +		clear_inode_flag(inode, FI_DIRTY_INODE);
> +		clear_inode_flag(inode, FI_AUTO_RECOVER);
> +		list_del_init(&F2FS_I(inode)->gdirty_list);
> +		dec_page_count(sbi, F2FS_DIRTY_IMETA);
> +		stat_dec_dirty_inode(sbi, DIRTY_META);
> +	}
> +
>  	spin_unlock(&sbi->inode_lock[DIRTY_META]);
>  }
>  
> @@ -670,8 +711,10 @@ static void f2fs_dirty_inode(struct inode *inode, int flags)
>  			inode->i_ino == F2FS_META_INO(sbi))
>  		return;
>  
> -	if (flags == I_DIRTY_TIME)
> +	if (flags == I_DIRTY_TIME) {
> +		f2fs_set_inode_dirty_time(inode);
>  		return;
> +	}
>  
>  	if (is_inode_flag_set(inode, FI_AUTO_RECOVER))
>  		clear_inode_flag(inode, FI_AUTO_RECOVER);
> -- 
> 2.10.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 2/2] f2fs: fix allocation failure
  2016-10-13 20:49   ` Jaegeuk Kim
@ 2016-10-14 14:09       ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2016-10-14 14:09 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

Hi Jaegeuk,

On 2016/10/14 4:49, Jaegeuk Kim wrote:
> Hi Chao,
> 
> On Thu, Oct 13, 2016 at 12:14:27AM +0800, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> tests/generic/251 of fstest reports a f2fs bug in below message:
>>
>> ------------[ cut here ]------------
>> invalid opcode: 0000 [#1] PREEMPT SMP
>> CPU: 1 PID: 109 Comm: kworker/u8:2 Tainted: G        W  O    4.8.0-rc4+ #22
>> Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>> Workqueue: writeback wb_workfn (flush-251:1)
>> task: f33c8000 task.stack: f33c6000
>> EIP: 0060:[<f8992139>] EFLAGS: 00010246 CPU: 1
>> EIP is at new_curseg+0x2c9/0x2d0 [f2fs]
>> EAX: 000003f3 EBX: ee3e5000 ECX: 00000400 EDX: 000003f3
>> ESI: 00000000 EDI: 00000008 EBP: f33c78c4 ESP: f33c7890
>>  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
>> CR0: 80050033 CR2: b6706000 CR3: 2e8d70c0 CR4: 000406f0
>> Stack:
>>  eb29480c 00000004 f27a0290 000003f3 00000008 f33c78c0 00000001 eb294800
>>  00000001 00000000 ee3e5000 f899dbb4 00000290 f33c7924 f89926d6 c10b42b6
>>  00000246 00000000 eed513e8 00000246 f33c78e8 c10b7b4b f33c7924 c178c304
>> Call Trace:
>>  [<f89926d6>] allocate_segment_by_default+0x3c6/0x3d0 [f2fs]
>>  [<f8992b3a>] allocate_data_block+0x13a/0x3c0 [f2fs]
>>  [<f8992e4b>] do_write_page+0x8b/0x230 [f2fs]
>>  [<f8993070>] write_node_page+0x20/0x30 [f2fs]
>>  [<f898a156>] f2fs_write_node_page+0x1a6/0x340 [f2fs]
>>  [<f898ca45>] sync_node_pages+0x4a5/0x590 [f2fs]
>>  [<f897ea48>] write_checkpoint+0x218/0x720 [f2fs]
>>  [<f898143d>] f2fs_gc+0x4cd/0x6b0 [f2fs]
>>  [<f8990ebe>] f2fs_balance_fs+0x18e/0x1b0 [f2fs]
>>  [<f8988017>] f2fs_write_data_page+0x197/0x6f0 [f2fs]
>>  [<f89830fe>] f2fs_write_data_pages+0x28e/0x7e0 [f2fs]
>>  [<c118b1cd>] do_writepages+0x1d/0x40
>>  [<c1228cb5>] __writeback_single_inode+0x55/0x7e0
>>  [<c1229b6b>] writeback_sb_inodes+0x21b/0x490
>>  [<c1229f6c>] wb_writeback+0xdc/0x590
>>  [<c122ae18>] wb_workfn+0xf8/0x690
>>  [<c107c231>] process_one_work+0x1a1/0x580
>>  [<c107c712>] worker_thread+0x102/0x440
>>  [<c1082021>] kthread+0xa1/0xc0
>>  [<c178f862>] ret_from_kernel_thread+0xe/0x24
>> EIP: [<f8992139>] new_curseg+0x2c9/0x2d0 [f2fs] SS:ESP 0068:f33c7890
>>
>> The reason is after f2fs enabled lazytime by default, when inode time is
>> changed, we do not set this inode dirty through ->f2fs_dirty_inode, so
>> itime updating will be delayed.
>>
>> Finally it needs to update the dirty time of inode into inode page,
>> and writeback the page, however, before that, we didn't count the inode
>> as imeta data. So f2fs won't be aware of dirty metadata page count is
>> exceeded watermark of GC, result in encountering panic when allocating
>> free segment.
>>
>> There is an easy way to produce this bug:
>> 1. mount with lazytime option
>> 2. fragment space
>> 3. touch all files in the image
>> 4. umount
> 
> I think modifying has_not_enough_secs() is enough like this.

Seems it won't solve this problem as I tested, the root cause here is that if
huge number of inode updates due to time changes, actually inodes won't be set
dirty as we return directly if flags is I_DIRTY_TIME in f2fs_dirty_inode, then
once inode cache is been shrunk, inodes in lru list will be set dirty in iput:

In iput()
		if (inode->i_nlink && (inode->i_state & I_DIRTY_TIME)) {
			atomic_inc(&inode->i_count);
			inode->i_state &= ~I_DIRTY_TIME;
			spin_unlock(&inode->i_lock);
			trace_writeback_lazytime_iput(inode);
			mark_inode_dirty_sync(inode);
			goto retry;

After that once someone calls write_checkpoint(), if number of dirty imeta data
is exceeded remain blocks in free segments, we will encounter this bug.

In order to fix this bug, I try to account these delayed dirtied inodes to
detect actual dirty metadata number, by this way we can set delayed dirtied
inode dirty and flush them in advance to avoid the dirty metadata number
exceeding blocks number in free segments, finally allocation failure issue can
be solved.

Thanks,

> 
> ---
>  fs/f2fs/segment.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index fecb856..a6efb5c 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -471,11 +471,12 @@ static inline bool need_SSR(struct f2fs_sb_info *sbi)
>  {
>  	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>  	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> +	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>  
>  	if (test_opt(sbi, LFS))
>  		return false;
>  
> -	return free_sections(sbi) <= (node_secs + 2 * dent_secs +
> +	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
>  						reserved_sections(sbi) + 1);
>  }
>  
> @@ -484,6 +485,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  {
>  	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>  	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> +	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>  
>  	node_secs += get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>  
> @@ -491,7 +493,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  		return false;
>  
>  	return (free_sections(sbi) + freed) <=
> -		(node_secs + 2 * dent_secs + reserved_sections(sbi) + needed);
> +		(node_secs + 2 * dent_secs + imeta_secs +
> +		reserved_sections(sbi) + needed);
>  }
>  
>  static inline bool excess_prefree_segs(struct f2fs_sb_info *sbi)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 2/2] f2fs: fix allocation failure
@ 2016-10-14 14:09       ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2016-10-14 14:09 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

Hi Jaegeuk,

On 2016/10/14 4:49, Jaegeuk Kim wrote:
> Hi Chao,
> 
> On Thu, Oct 13, 2016 at 12:14:27AM +0800, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> tests/generic/251 of fstest reports a f2fs bug in below message:
>>
>> ------------[ cut here ]------------
>> invalid opcode: 0000 [#1] PREEMPT SMP
>> CPU: 1 PID: 109 Comm: kworker/u8:2 Tainted: G        W  O    4.8.0-rc4+ #22
>> Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>> Workqueue: writeback wb_workfn (flush-251:1)
>> task: f33c8000 task.stack: f33c6000
>> EIP: 0060:[<f8992139>] EFLAGS: 00010246 CPU: 1
>> EIP is at new_curseg+0x2c9/0x2d0 [f2fs]
>> EAX: 000003f3 EBX: ee3e5000 ECX: 00000400 EDX: 000003f3
>> ESI: 00000000 EDI: 00000008 EBP: f33c78c4 ESP: f33c7890
>>  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
>> CR0: 80050033 CR2: b6706000 CR3: 2e8d70c0 CR4: 000406f0
>> Stack:
>>  eb29480c 00000004 f27a0290 000003f3 00000008 f33c78c0 00000001 eb294800
>>  00000001 00000000 ee3e5000 f899dbb4 00000290 f33c7924 f89926d6 c10b42b6
>>  00000246 00000000 eed513e8 00000246 f33c78e8 c10b7b4b f33c7924 c178c304
>> Call Trace:
>>  [<f89926d6>] allocate_segment_by_default+0x3c6/0x3d0 [f2fs]
>>  [<f8992b3a>] allocate_data_block+0x13a/0x3c0 [f2fs]
>>  [<f8992e4b>] do_write_page+0x8b/0x230 [f2fs]
>>  [<f8993070>] write_node_page+0x20/0x30 [f2fs]
>>  [<f898a156>] f2fs_write_node_page+0x1a6/0x340 [f2fs]
>>  [<f898ca45>] sync_node_pages+0x4a5/0x590 [f2fs]
>>  [<f897ea48>] write_checkpoint+0x218/0x720 [f2fs]
>>  [<f898143d>] f2fs_gc+0x4cd/0x6b0 [f2fs]
>>  [<f8990ebe>] f2fs_balance_fs+0x18e/0x1b0 [f2fs]
>>  [<f8988017>] f2fs_write_data_page+0x197/0x6f0 [f2fs]
>>  [<f89830fe>] f2fs_write_data_pages+0x28e/0x7e0 [f2fs]
>>  [<c118b1cd>] do_writepages+0x1d/0x40
>>  [<c1228cb5>] __writeback_single_inode+0x55/0x7e0
>>  [<c1229b6b>] writeback_sb_inodes+0x21b/0x490
>>  [<c1229f6c>] wb_writeback+0xdc/0x590
>>  [<c122ae18>] wb_workfn+0xf8/0x690
>>  [<c107c231>] process_one_work+0x1a1/0x580
>>  [<c107c712>] worker_thread+0x102/0x440
>>  [<c1082021>] kthread+0xa1/0xc0
>>  [<c178f862>] ret_from_kernel_thread+0xe/0x24
>> EIP: [<f8992139>] new_curseg+0x2c9/0x2d0 [f2fs] SS:ESP 0068:f33c7890
>>
>> The reason is after f2fs enabled lazytime by default, when inode time is
>> changed, we do not set this inode dirty through ->f2fs_dirty_inode, so
>> itime updating will be delayed.
>>
>> Finally it needs to update the dirty time of inode into inode page,
>> and writeback the page, however, before that, we didn't count the inode
>> as imeta data. So f2fs won't be aware of dirty metadata page count is
>> exceeded watermark of GC, result in encountering panic when allocating
>> free segment.
>>
>> There is an easy way to produce this bug:
>> 1. mount with lazytime option
>> 2. fragment space
>> 3. touch all files in the image
>> 4. umount
> 
> I think modifying has_not_enough_secs() is enough like this.

Seems it won't solve this problem as I tested, the root cause here is that if
huge number of inode updates due to time changes, actually inodes won't be set
dirty as we return directly if flags is I_DIRTY_TIME in f2fs_dirty_inode, then
once inode cache is been shrunk, inodes in lru list will be set dirty in iput:

In iput()
		if (inode->i_nlink && (inode->i_state & I_DIRTY_TIME)) {
			atomic_inc(&inode->i_count);
			inode->i_state &= ~I_DIRTY_TIME;
			spin_unlock(&inode->i_lock);
			trace_writeback_lazytime_iput(inode);
			mark_inode_dirty_sync(inode);
			goto retry;

After that once someone calls write_checkpoint(), if number of dirty imeta data
is exceeded remain blocks in free segments, we will encounter this bug.

In order to fix this bug, I try to account these delayed dirtied inodes to
detect actual dirty metadata number, by this way we can set delayed dirtied
inode dirty and flush them in advance to avoid the dirty metadata number
exceeding blocks number in free segments, finally allocation failure issue can
be solved.

Thanks,

> 
> ---
>  fs/f2fs/segment.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index fecb856..a6efb5c 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -471,11 +471,12 @@ static inline bool need_SSR(struct f2fs_sb_info *sbi)
>  {
>  	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>  	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> +	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>  
>  	if (test_opt(sbi, LFS))
>  		return false;
>  
> -	return free_sections(sbi) <= (node_secs + 2 * dent_secs +
> +	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
>  						reserved_sections(sbi) + 1);
>  }
>  
> @@ -484,6 +485,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  {
>  	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>  	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> +	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>  
>  	node_secs += get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>  
> @@ -491,7 +493,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  		return false;
>  
>  	return (free_sections(sbi) + freed) <=
> -		(node_secs + 2 * dent_secs + reserved_sections(sbi) + needed);
> +		(node_secs + 2 * dent_secs + imeta_secs +
> +		reserved_sections(sbi) + needed);
>  }
>  
>  static inline bool excess_prefree_segs(struct f2fs_sb_info *sbi)
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 2/2] f2fs: fix allocation failure
  2016-10-14 14:09       ` Chao Yu
  (?)
@ 2016-10-14 20:37       ` Jaegeuk Kim
  -1 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2016-10-14 20:37 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

On Fri, Oct 14, 2016 at 10:09:29PM +0800, Chao Yu wrote:
...

> >> Finally it needs to update the dirty time of inode into inode page,
> >> and writeback the page, however, before that, we didn't count the inode
> >> as imeta data. So f2fs won't be aware of dirty metadata page count is
> >> exceeded watermark of GC, result in encountering panic when allocating
> >> free segment.
> >>
> >> There is an easy way to produce this bug:
> >> 1. mount with lazytime option
> >> 2. fragment space
> >> 3. touch all files in the image
> >> 4. umount
> > 
> > I think modifying has_not_enough_secs() is enough like this.
> 
> Seems it won't solve this problem as I tested, the root cause here is that if
> huge number of inode updates due to time changes, actually inodes won't be set
> dirty as we return directly if flags is I_DIRTY_TIME in f2fs_dirty_inode, then
> once inode cache is been shrunk, inodes in lru list will be set dirty in iput:
> 
> In iput()
> 		if (inode->i_nlink && (inode->i_state & I_DIRTY_TIME)) {
> 			atomic_inc(&inode->i_count);
> 			inode->i_state &= ~I_DIRTY_TIME;
> 			spin_unlock(&inode->i_lock);
> 			trace_writeback_lazytime_iput(inode);
> 			mark_inode_dirty_sync(inode);
> 			goto retry;
> 
> After that once someone calls write_checkpoint(), if number of dirty imeta data
> is exceeded remain blocks in free segments, we will encounter this bug.
> 
> In order to fix this bug, I try to account these delayed dirtied inodes to
> detect actual dirty metadata number, by this way we can set delayed dirtied
> inode dirty and flush them in advance to avoid the dirty metadata number
> exceeding blocks number in free segments, finally allocation failure issue can
> be solved.

Yup, I did understand like that before. But actually, I tested this patch and
unfortunately I got a deadlock when running fsstress quickly. So, this makes me
rethink the whole design in terms of how to reuse the existing codes.

I think we're able to consider inode cache apart from FS consistency. The
IMETA list was actually keeping all the dirty inodes for FS consistency, but
it seems we don't need to store all of them which will consume free segments
dramatically during checkpoint.

I wrote a patch to add dirty inodes into IMETA list selectively.
The goal is to avoid adding dirty inodes by mark_inode_dirty_sync() called by
vfs, not by f2fs. Instead, those changes will be done by f2fs_write_inode().

Could you take a look at this?

>From bc165410f30274dce76e053b922d39a61b012e43 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Fri, 14 Oct 2016 11:51:23 -0700
Subject: [PATCH] f2fs: keep dirty inodes selectively for checkpoint

This is to avoid no free segment bug during checkpoint caused by a number of
dirty inodes.

The case was reported by Chao like this.

1. mount with lazytime option
2. fragment space
3. touch all files in the image
4. umount

In this case, we actually don't need to flush dirty inode to inode page during
checkpoint.

Reported-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/acl.c          |  2 +-
 fs/f2fs/dir.c          |  6 +++---
 fs/f2fs/extent_cache.c |  2 +-
 fs/f2fs/f2fs.h         | 25 +++++++++++++------------
 fs/f2fs/file.c         |  8 ++++----
 fs/f2fs/inline.c       |  2 +-
 fs/f2fs/inode.c        | 10 +++++-----
 fs/f2fs/namei.c        |  6 +++---
 fs/f2fs/super.c        |  3 +++
 fs/f2fs/xattr.c        |  4 ++--
 10 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
index 1e29630..6266fba 100644
--- a/fs/f2fs/acl.c
+++ b/fs/f2fs/acl.c
@@ -386,7 +386,7 @@ int f2fs_init_acl(struct inode *inode, struct inode *dir, struct page *ipage,
 	if (error)
 		return error;
 
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 
 	if (default_acl) {
 		error = __f2fs_set_acl(inode, ACL_TYPE_DEFAULT, default_acl,
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index a46079a..e71793c 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -313,7 +313,7 @@ void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
 	set_page_dirty(page);
 
 	dir->i_mtime = dir->i_ctime = CURRENT_TIME;
-	f2fs_mark_inode_dirty_sync(dir);
+	f2fs_mark_inode_dirty_sync(dir, false);
 	f2fs_put_page(page, 1);
 }
 
@@ -466,7 +466,7 @@ void update_parent_metadata(struct inode *dir, struct inode *inode,
 		clear_inode_flag(inode, FI_NEW_INODE);
 	}
 	dir->i_mtime = dir->i_ctime = CURRENT_TIME;
-	f2fs_mark_inode_dirty_sync(dir);
+	f2fs_mark_inode_dirty_sync(dir, false);
 
 	if (F2FS_I(dir)->i_current_depth != current_depth)
 		f2fs_i_depth_write(dir, current_depth);
@@ -731,7 +731,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
 	set_page_dirty(page);
 
 	dir->i_ctime = dir->i_mtime = CURRENT_TIME;
-	f2fs_mark_inode_dirty_sync(dir);
+	f2fs_mark_inode_dirty_sync(dir, false);
 
 	if (inode)
 		f2fs_drop_nlink(dir, inode);
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 2b06d4f..4db44da 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -172,7 +172,7 @@ static void __drop_largest_extent(struct inode *inode,
 
 	if (fofs < largest->fofs + largest->len && fofs + len > largest->fofs) {
 		largest->len = 0;
-		f2fs_mark_inode_dirty_sync(inode);
+		f2fs_mark_inode_dirty_sync(inode, true);
 	}
 }
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 6dc004e..921f44d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -513,13 +513,13 @@ static inline bool __is_front_mergeable(struct extent_info *cur,
 	return __is_extent_mergeable(cur, front);
 }
 
-extern void f2fs_mark_inode_dirty_sync(struct inode *);
+extern void f2fs_mark_inode_dirty_sync(struct inode *, bool);
 static inline void __try_update_largest_extent(struct inode *inode,
 			struct extent_tree *et, struct extent_node *en)
 {
 	if (en->ei.len > et->largest.len) {
 		et->largest = en->ei;
-		f2fs_mark_inode_dirty_sync(inode);
+		f2fs_mark_inode_dirty_sync(inode, true);
 	}
 }
 
@@ -1625,6 +1625,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
 enum {
 	FI_NEW_INODE,		/* indicate newly allocated inode */
 	FI_DIRTY_INODE,		/* indicate inode is dirty or not */
+	FI_DIRTY_INODE_SYNC,	/* indicate inode needs to be synced or not */
 	FI_AUTO_RECOVER,	/* indicate inode is recoverable */
 	FI_DIRTY_DIR,		/* indicate directory has dirty pages */
 	FI_INC_LINK,		/* need to increment i_nlink */
@@ -1659,7 +1660,7 @@ static inline void __mark_inode_dirty_flag(struct inode *inode,
 			return;
 	case FI_DATA_EXIST:
 	case FI_INLINE_DOTS:
-		f2fs_mark_inode_dirty_sync(inode);
+		f2fs_mark_inode_dirty_sync(inode, true);
 	}
 }
 
@@ -1686,7 +1687,7 @@ static inline void set_acl_inode(struct inode *inode, umode_t mode)
 {
 	F2FS_I(inode)->i_acl_mode = mode;
 	set_inode_flag(inode, FI_ACL_MODE);
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, false);
 }
 
 static inline void f2fs_i_links_write(struct inode *inode, bool inc)
@@ -1695,7 +1696,7 @@ static inline void f2fs_i_links_write(struct inode *inode, bool inc)
 		inc_nlink(inode);
 	else
 		drop_nlink(inode);
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 static inline void f2fs_i_blocks_write(struct inode *inode,
@@ -1706,7 +1707,7 @@ static inline void f2fs_i_blocks_write(struct inode *inode,
 
 	inode->i_blocks = add ? inode->i_blocks + diff :
 				inode->i_blocks - diff;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 	if (clean || recover)
 		set_inode_flag(inode, FI_AUTO_RECOVER);
 }
@@ -1720,7 +1721,7 @@ static inline void f2fs_i_size_write(struct inode *inode, loff_t i_size)
 		return;
 
 	i_size_write(inode, i_size);
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 	if (clean || recover)
 		set_inode_flag(inode, FI_AUTO_RECOVER);
 }
@@ -1735,19 +1736,19 @@ static inline bool f2fs_skip_inode_update(struct inode *inode)
 static inline void f2fs_i_depth_write(struct inode *inode, unsigned int depth)
 {
 	F2FS_I(inode)->i_current_depth = depth;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 static inline void f2fs_i_xnid_write(struct inode *inode, nid_t xnid)
 {
 	F2FS_I(inode)->i_xattr_nid = xnid;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 static inline void f2fs_i_pino_write(struct inode *inode, nid_t pino)
 {
 	F2FS_I(inode)->i_pino = pino;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 static inline void get_inline_info(struct inode *inode, struct f2fs_inode *ri)
@@ -1875,13 +1876,13 @@ static inline int is_file(struct inode *inode, int type)
 static inline void set_file(struct inode *inode, int type)
 {
 	F2FS_I(inode)->i_advise |= type;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 static inline void clear_file(struct inode *inode, int type)
 {
 	F2FS_I(inode)->i_advise &= ~type;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 static inline int f2fs_readonly(struct super_block *sb)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index bfce575..aa778c6 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -266,7 +266,6 @@ sync_nodes:
 	}
 
 	if (need_inode_block_update(sbi, ino)) {
-		f2fs_mark_inode_dirty_sync(inode);
 		f2fs_write_inode(inode, NULL);
 		goto sync_nodes;
 	}
@@ -671,7 +670,7 @@ int f2fs_truncate(struct inode *inode)
 		return err;
 
 	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, false);
 	return 0;
 }
 
@@ -760,7 +759,8 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 		}
 	}
 
-	f2fs_mark_inode_dirty_sync(inode);
+	/* update attributes only */
+	f2fs_mark_inode_dirty_sync(inode, false);
 
 	/* inode change will produce dirty node pages flushed by checkpoint */
 	f2fs_balance_fs(F2FS_I_SB(inode), true);
@@ -1441,7 +1441,7 @@ static long f2fs_fallocate(struct file *file, int mode,
 
 	if (!ret) {
 		inode->i_mtime = inode->i_ctime = CURRENT_TIME;
-		f2fs_mark_inode_dirty_sync(inode);
+		f2fs_mark_inode_dirty_sync(inode, false);
 		f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
 	}
 
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 8d71d64..a05be5d 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -575,7 +575,7 @@ void f2fs_delete_inline_entry(struct f2fs_dir_entry *dentry, struct page *page,
 	f2fs_put_page(page, 1);
 
 	dir->i_ctime = dir->i_mtime = CURRENT_TIME;
-	f2fs_mark_inode_dirty_sync(dir);
+	f2fs_mark_inode_dirty_sync(dir, false);
 
 	if (inode)
 		f2fs_drop_nlink(dir, inode);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index d32fd03..4368ee7 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -19,10 +19,13 @@
 
 #include <trace/events/f2fs.h>
 
-void f2fs_mark_inode_dirty_sync(struct inode *inode)
+void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync)
 {
+	if (sync)
+		set_inode_flag(inode, FI_DIRTY_INODE_SYNC);
 	if (f2fs_inode_dirtied(inode))
 		return;
+
 	mark_inode_dirty_sync(inode);
 }
 
@@ -43,7 +46,7 @@ void f2fs_set_inode_flags(struct inode *inode)
 		new_fl |= S_DIRSYNC;
 	inode_set_flags(inode, new_fl,
 			S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC);
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, false);
 }
 
 static void __get_inode_rdev(struct inode *inode, struct f2fs_inode *ri)
@@ -331,9 +334,6 @@ int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)
 			inode->i_ino == F2FS_META_INO(sbi))
 		return 0;
 
-	if (!is_inode_flag_set(inode, FI_DIRTY_INODE))
-		return 0;
-
 	/*
 	 * We need to balance fs here to prevent from producing dirty node pages
 	 * during the urgent cleaning time when runing out of free sections.
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 300aef8..21b336a 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -778,7 +778,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	up_write(&F2FS_I(old_inode)->i_sem);
 
 	old_inode->i_ctime = CURRENT_TIME;
-	f2fs_mark_inode_dirty_sync(old_inode);
+	f2fs_mark_inode_dirty_sync(old_inode, false);
 
 	f2fs_delete_entry(old_entry, old_page, old_dir, NULL);
 
@@ -938,7 +938,7 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 		f2fs_i_links_write(old_dir, old_nlink > 0);
 		up_write(&F2FS_I(old_dir)->i_sem);
 	}
-	f2fs_mark_inode_dirty_sync(old_dir);
+	f2fs_mark_inode_dirty_sync(old_dir, false);
 
 	/* update directory entry info of new dir inode */
 	f2fs_set_link(new_dir, new_entry, new_page, old_inode);
@@ -953,7 +953,7 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 		f2fs_i_links_write(new_dir, new_nlink > 0);
 		up_write(&F2FS_I(new_dir)->i_sem);
 	}
-	f2fs_mark_inode_dirty_sync(new_dir);
+	f2fs_mark_inode_dirty_sync(new_dir, false);
 
 	f2fs_unlock_op(sbi);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 0559cd6..d22a4d2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -651,6 +651,7 @@ void f2fs_inode_synced(struct inode *inode)
 	}
 	list_del_init(&F2FS_I(inode)->gdirty_list);
 	clear_inode_flag(inode, FI_DIRTY_INODE);
+	clear_inode_flag(inode, FI_DIRTY_INODE_SYNC);
 	clear_inode_flag(inode, FI_AUTO_RECOVER);
 	dec_page_count(sbi, F2FS_DIRTY_IMETA);
 	stat_dec_dirty_inode(F2FS_I_SB(inode), DIRTY_META);
@@ -672,6 +673,8 @@ static void f2fs_dirty_inode(struct inode *inode, int flags)
 
 	if (flags == I_DIRTY_TIME)
 		return;
+	if (!is_inode_flag_set(inode, FI_DIRTY_INODE_SYNC))
+		return;
 
 	if (is_inode_flag_set(inode, FI_AUTO_RECOVER))
 		clear_inode_flag(inode, FI_AUTO_RECOVER);
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 83234dd..4effb96 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -106,7 +106,7 @@ static int f2fs_xattr_advise_set(const struct xattr_handler *handler,
 		return -EINVAL;
 
 	F2FS_I(inode)->i_advise |= *(char *)value;
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 	return 0;
 }
 
@@ -561,7 +561,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
 		else
 			file_clear_key(inode);
 	}
-	f2fs_mark_inode_dirty_sync(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
 	if (!error && S_ISDIR(inode->i_mode))
 		set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP);
 exit:
-- 
2.8.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-10-14 20:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-12 16:14 [RFC PATCH 1/2] f2fs: rename f2fs_update_time Chao Yu
2016-10-12 16:14 ` Chao Yu
2016-10-12 16:14 ` [RFC PATCH 2/2] f2fs: fix allocation failure Chao Yu
2016-10-12 16:14   ` Chao Yu
2016-10-13 20:49   ` Jaegeuk Kim
2016-10-14 14:09     ` Chao Yu
2016-10-14 14:09       ` Chao Yu
2016-10-14 20:37       ` Jaegeuk Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.