[f2fs-dev][PATCH V2 0/6] f2fs: Enable f2fs support inline data

[f2fs-dev][PATCH V2 0/6] f2fs: Enable f2fs support inline data

[Huajun Li]

From: Huajun Li <huajun.li@intel.com>

f2fs inode is so large, so small files can be stored directly in the inode,
rather than just storing a single block address and storing the data elsewhere.

This patch set makes files less than ~3.4K store directly in inode block.
a) space saving
   Test with kernel src(without repo data), it can save about 10% space
   with this patch set;
b) performance
   Test this patch set with iozone, there is no obvious performance difference
   with the results of disabling this feature.


V2: - Update f2fs_reserve_block() according to comments on V1
    - Add function f2fs_may_inline() to check whether the file meet 
      inline requirements
    - Try to write inline data to normal data block before clearing it
      from inode block 
    - Change lock scope while converting inline data
    - Add inline_data description to f2fs documentation

Huajun Li (6):
  f2fs: Add flags and helpers to support inline data
  f2fs: Add a new mount option: inline_data
  f2fs: Add a new function: f2fs_reserve_block()
  f2fs: Key functions to handle inline data
  f2fs: Handle inline data operations
  f2fs: update f2fs Documentation

 Documentation/filesystems/f2fs.txt |    2 +
 fs/f2fs/Makefile                   |    2 +-
 fs/f2fs/data.c                     |   97 +++++++++++++-------
 fs/f2fs/f2fs.h                     |   23 +++++
 fs/f2fs/file.c                     |   81 +++++++++--------
 fs/f2fs/inline.c                   |  172 ++++++++++++++++++++++++++++++++++++
 fs/f2fs/super.c                    |    8 +-
 include/linux/f2fs_fs.h            |    8 ++
 8 files changed, 325 insertions(+), 68 deletions(-)
 create mode 100644 fs/f2fs/inline.c

-- 
1.7.9.5

[f2fs-dev][PATCH V2 1/6] f2fs: Add flags and helpers to support inline data

[Huajun Li]

From: Huajun Li <huajun.li@intel.com>

Add new inode flags F2FS_INLINE_DATA and FI_INLINE_DATA to indicate
whether the inode has inline data.

Inline data makes use of inode block's data indices region to save small
file. Currently there are 923 data indices in an inode block. Since
inline xattr has made use of the last 50 indices to save its data, there
are 873 indices left which can be used for inline data. When
FI_INLINE_DATA is set, the layout of inode block's indices region is
like below:

+-----------------+
|                 | Reserved. reserve_new_block() will make use of
| i_addr[0]       | i_addr[0] when we need to reserve a new data block
|                 | to convert inline data into regular one's.
|-----------------|
|                 | Used by inline data. A file whose size is less than
| i_addr[1~872]   | 3488 bytes(~3.4k) and doesn't reserve extra
|                 | blocks by fallocate() can be saved here.
|-----------------|
|                 |
| i_addr[873~922] | Reserved for inline xattr
|                 |
+-----------------+

Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>
Signed-off-by: Huajun Li <huajun.li@intel.com>
Signed-off-by: Weihong Xu <weihong.xu@intel.com>
---
 fs/f2fs/f2fs.h          |   14 ++++++++++++++
 include/linux/f2fs_fs.h |    8 ++++++++
 2 files changed, 22 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 89dc750..cd7d2f9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -37,6 +37,7 @@
 #define F2FS_MOUNT_POSIX_ACL		0x00000020
 #define F2FS_MOUNT_DISABLE_EXT_IDENTIFY	0x00000040
 #define F2FS_MOUNT_INLINE_XATTR		0x00000080
+#define F2FS_MOUNT_INLINE_DATA		0x00000100
 
 #define clear_opt(sbi, option)	(sbi->mount_opt.opt &= ~F2FS_MOUNT_##option)
 #define set_opt(sbi, option)	(sbi->mount_opt.opt |= F2FS_MOUNT_##option)
@@ -877,6 +878,7 @@ enum {
 	FI_UPDATE_DIR,		/* should update inode block for consistency */
 	FI_DELAY_IPUT,		/* used for the recovery */
 	FI_INLINE_XATTR,	/* used for inline xattr */
+	FI_INLINE_DATA,		/* used for inline data*/
 };
 
 static inline void set_inode_flag(struct f2fs_inode_info *fi, int flag)
@@ -914,6 +916,8 @@ static inline void get_inline_info(struct f2fs_inode_info *fi,
 {
 	if (ri->i_inline & F2FS_INLINE_XATTR)
 		set_inode_flag(fi, FI_INLINE_XATTR);
+	if (ri->i_inline & F2FS_INLINE_DATA)
+		set_inode_flag(fi, FI_INLINE_DATA);
 }
 
 static inline void set_raw_inline(struct f2fs_inode_info *fi,
@@ -923,6 +927,8 @@ static inline void set_raw_inline(struct f2fs_inode_info *fi,
 
 	if (is_inode_flag_set(fi, FI_INLINE_XATTR))
 		ri->i_inline |= F2FS_INLINE_XATTR;
+	if (is_inode_flag_set(fi, FI_INLINE_DATA))
+		ri->i_inline |= F2FS_INLINE_DATA;
 }
 
 static inline unsigned int addrs_per_inode(struct f2fs_inode_info *fi)
@@ -948,6 +954,13 @@ static inline int inline_xattr_size(struct inode *inode)
 		return 0;
 }
 
+static inline void *inline_data_addr(struct page *page)
+{
+	struct f2fs_inode *ri;
+	ri = (struct f2fs_inode *)page_address(page);
+	return (void *)&(ri->i_addr[1]);
+}
+
 static inline int f2fs_readonly(struct super_block *sb)
 {
 	return sb->s_flags & MS_RDONLY;
@@ -1238,4 +1251,5 @@ extern const struct address_space_operations f2fs_meta_aops;
 extern const struct inode_operations f2fs_dir_inode_operations;
 extern const struct inode_operations f2fs_symlink_inode_operations;
 extern const struct inode_operations f2fs_special_inode_operations;
+
 #endif
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index bb942f6..aea5eed 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -153,6 +153,14 @@ struct f2fs_extent {
 #define	NODE_DIND_BLOCK		(DEF_ADDRS_PER_INODE + 5)
 
 #define F2FS_INLINE_XATTR	0x01	/* file inline xattr flag */
+#define F2FS_INLINE_DATA	0x02	/* file inline data flag */
+
+
+#define MAX_INLINE_DATA		(sizeof(__le32) * (DEF_ADDRS_PER_INODE - \
+						F2FS_INLINE_XATTR_ADDRS - 1))
+
+#define INLINE_DATA_OFFSET	(PAGE_CACHE_SIZE - sizeof(struct node_footer) \
+				- sizeof(__le32)*(DEF_ADDRS_PER_INODE + 5 - 1))
 
 struct f2fs_inode {
 	__le16 i_mode;			/* file mode */
-- 
1.7.9.5

[f2fs-dev][PATCH V2 2/6] f2fs: Add a new mount option: inline_data

[Huajun Li]

From: Huajun Li <huajun.li@intel.com>

Add a mount option: inline_data. If the mount option is set,
data of New created small files can be stored in their inode.

Signed-off-by: Huajun Li <huajun.li@intel.com>
Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>
Signed-off-by: Weihong Xu <weihong.xu@intel.com>
---
 fs/f2fs/super.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bafff72..12e89ac 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -50,6 +50,7 @@ enum {
 	Opt_active_logs,
 	Opt_disable_ext_identify,
 	Opt_inline_xattr,
+	Opt_inline_data,
 	Opt_err,
 };
 
@@ -65,6 +66,7 @@ static match_table_t f2fs_tokens = {
 	{Opt_active_logs, "active_logs=%u"},
 	{Opt_disable_ext_identify, "disable_ext_identify"},
 	{Opt_inline_xattr, "inline_xattr"},
+	{Opt_inline_data, "inline_data"},
 	{Opt_err, NULL},
 };
 
@@ -311,6 +313,9 @@ static int parse_options(struct super_block *sb, char *options)
 		case Opt_disable_ext_identify:
 			set_opt(sbi, DISABLE_EXT_IDENTIFY);
 			break;
+		case Opt_inline_data:
+			set_opt(sbi, INLINE_DATA);
+			break;
 		default:
 			f2fs_msg(sb, KERN_ERR,
 				"Unrecognized mount option \"%s\" or missing value",
@@ -508,7 +513,8 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
 #endif
 	if (test_opt(sbi, DISABLE_EXT_IDENTIFY))
 		seq_puts(seq, ",disable_ext_identify");
-
+	if (test_opt(sbi, INLINE_DATA))
+		seq_puts(seq, ",inline_data");
 	seq_printf(seq, ",active_logs=%u", sbi->active_logs);
 
 	return 0;
-- 
1.7.9.5

[f2fs-dev][PATCH V2 3/6] f2fs: Add a new function: f2fs_reserve_block()

[Huajun Li]

From: Huajun Li <huajun.li@intel.com>

Add the function f2fs_reserve_block() to easily reserve new blocks, and
use it to clean up more codes.

Signed-off-by: Huajun Li <huajun.li@intel.com>
Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>
Signed-off-by: Weihong Xu <weihong.xu@intel.com>
---
 fs/f2fs/data.c |   50 +++++++++++++++++++++++---------------------------
 fs/f2fs/f2fs.h |    1 +
 fs/f2fs/file.c |   38 ++++++--------------------------------
 3 files changed, 30 insertions(+), 59 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index aa3438c..92d0724 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -64,6 +64,22 @@ int reserve_new_block(struct dnode_of_data *dn)
 	return 0;
 }
 
+int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
+{
+	bool need_put = dn->inode_page ? false : true;
+	int err;
+
+	err = get_dnode_of_data(dn, index, ALLOC_NODE);
+	if (err)
+		return err;
+	if (dn->data_blkaddr == NULL_ADDR)
+		err = reserve_new_block(dn);
+
+	if (need_put)
+		f2fs_put_dnode(dn);
+	return err;
+}
+
 static int check_extent_cache(struct inode *inode, pgoff_t pgofs,
 					struct buffer_head *bh_result)
 {
@@ -300,19 +316,10 @@ struct page *get_new_data_page(struct inode *inode,
 	int err;
 
 	set_new_dnode(&dn, inode, npage, npage, 0);
-	err = get_dnode_of_data(&dn, index, ALLOC_NODE);
+	err = f2fs_reserve_block(&dn, index);
 	if (err)
 		return ERR_PTR(err);
 
-	if (dn.data_blkaddr == NULL_ADDR) {
-		if (reserve_new_block(&dn)) {
-			if (!npage)
-				f2fs_put_dnode(&dn);
-			return ERR_PTR(-ENOSPC);
-		}
-	}
-	if (!npage)
-		f2fs_put_dnode(&dn);
 repeat:
 	page = grab_cache_page(mapping, index);
 	if (!page)
@@ -644,21 +651,15 @@ repeat:
 	*pagep = page;
 
 	f2fs_lock_op(sbi);
-
 	set_new_dnode(&dn, inode, NULL, NULL, 0);
-	err = get_dnode_of_data(&dn, index, ALLOC_NODE);
-	if (err)
-		goto err;
-
-	if (dn.data_blkaddr == NULL_ADDR)
-		err = reserve_new_block(&dn);
-
-	f2fs_put_dnode(&dn);
-	if (err)
-		goto err;
-
+	err = f2fs_reserve_block(&dn, index);
 	f2fs_unlock_op(sbi);
 
+	if (err) {
+		f2fs_put_page(page, 1);
+		return err;
+	}
+
 	if ((len == PAGE_CACHE_SIZE) || PageUptodate(page))
 		return 0;
 
@@ -691,11 +692,6 @@ out:
 	SetPageUptodate(page);
 	clear_cold_data(page);
 	return 0;
-
-err:
-	f2fs_unlock_op(sbi);
-	f2fs_put_page(page, 1);
-	return err;
 }
 
 static int f2fs_write_end(struct file *file,
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index cd7d2f9..de84f52 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1119,6 +1119,7 @@ void destroy_checkpoint_caches(void);
  * data.c
  */
 int reserve_new_block(struct dnode_of_data *);
+int f2fs_reserve_block(struct dnode_of_data *, pgoff_t);
 void update_extent_cache(block_t, struct dnode_of_data *);
 struct page *find_data_page(struct inode *, pgoff_t, bool);
 struct page *get_lock_data_page(struct inode *, pgoff_t);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 7d714f4..1cd8e44 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -33,7 +33,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	struct page *page = vmf->page;
 	struct inode *inode = file_inode(vma->vm_file);
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
-	block_t old_blk_addr;
 	struct dnode_of_data dn;
 	int err;
 
@@ -44,24 +43,10 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	/* block allocation */
 	f2fs_lock_op(sbi);
 	set_new_dnode(&dn, inode, NULL, NULL, 0);
-	err = get_dnode_of_data(&dn, page->index, ALLOC_NODE);
-	if (err) {
-		f2fs_unlock_op(sbi);
-		goto out;
-	}
-
-	old_blk_addr = dn.data_blkaddr;
-
-	if (old_blk_addr == NULL_ADDR) {
-		err = reserve_new_block(&dn);
-		if (err) {
-			f2fs_put_dnode(&dn);
-			f2fs_unlock_op(sbi);
-			goto out;
-		}
-	}
-	f2fs_put_dnode(&dn);
+	err = f2fs_reserve_block(&dn, page->index);
 	f2fs_unlock_op(sbi);
+	if (err)
+		goto out;
 
 	file_update_time(vma->vm_file);
 	lock_page(page);
@@ -532,22 +517,11 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 
 		f2fs_lock_op(sbi);
 		set_new_dnode(&dn, inode, NULL, NULL, 0);
-		ret = get_dnode_of_data(&dn, index, ALLOC_NODE);
-		if (ret) {
-			f2fs_unlock_op(sbi);
+		ret = f2fs_reserve_block(&dn, index);
+		f2fs_unlock_op(sbi);
+		if (ret)
 			break;
-		}
 
-		if (dn.data_blkaddr == NULL_ADDR) {
-			ret = reserve_new_block(&dn);
-			if (ret) {
-				f2fs_put_dnode(&dn);
-				f2fs_unlock_op(sbi);
-				break;
-			}
-		}
-		f2fs_put_dnode(&dn);
-		f2fs_unlock_op(sbi);
 
 		if (pg_start == pg_end)
 			new_size = offset + len;
-- 
1.7.9.5

[f2fs-dev][PATCH V2 4/6] f2fs: Key functions to handle inline data

[Huajun Li]

From: Huajun Li <huajun.li@intel.com>

Functions to implement inline data read/write, and move inline data to
normal data block when file size exceeds inline data limitation.

Signed-off-by: Huajun Li <huajun.li@intel.com>
Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>
Signed-off-by: Weihong Xu <weihong.xu@intel.com>
---
 fs/f2fs/Makefile |    2 +-
 fs/f2fs/f2fs.h   |    8 +++
 fs/f2fs/inline.c |  172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 181 insertions(+), 1 deletion(-)
 create mode 100644 fs/f2fs/inline.c

diff --git a/fs/f2fs/Makefile b/fs/f2fs/Makefile
index 27a0820..2e35da1 100644
--- a/fs/f2fs/Makefile
+++ b/fs/f2fs/Makefile
@@ -1,6 +1,6 @@
 obj-$(CONFIG_F2FS_FS) += f2fs.o
 
-f2fs-y		:= dir.o file.o inode.o namei.o hash.o super.o
+f2fs-y		:= dir.o file.o inode.o namei.o hash.o super.o inline.o
 f2fs-y		+= checkpoint.o gc.o data.o node.o segment.o recovery.o
 f2fs-$(CONFIG_F2FS_STAT_FS) += debug.o
 f2fs-$(CONFIG_F2FS_FS_XATTR) += xattr.o
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index de84f52..5f78c9c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1253,4 +1253,12 @@ extern const struct inode_operations f2fs_dir_inode_operations;
 extern const struct inode_operations f2fs_symlink_inode_operations;
 extern const struct inode_operations f2fs_special_inode_operations;
 
+/*
+ * inline.c
+ */
+inline int f2fs_has_inline_data(struct inode *);
+bool f2fs_may_inline(struct inode *);
+int f2fs_read_inline_data(struct inode *, struct page *);
+int f2fs_convert_inline_data(struct inode *, struct page *, unsigned);
+int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
 #endif
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
new file mode 100644
index 0000000..e9b33d7
--- /dev/null
+++ b/fs/f2fs/inline.c
@@ -0,0 +1,172 @@
+/*
+ * fs/f2fs/inline.c
+ * Copyright (c) 2013, Intel Corporation
+ * Authors: Huajun Li <huajun.li@intel.com>
+ *          Haicheng Li <haicheng.li@intel.com>
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/fs.h>
+#include <linux/f2fs_fs.h>
+
+#include "f2fs.h"
+
+inline int f2fs_has_inline_data(struct inode *inode)
+{
+	return is_inode_flag_set(F2FS_I(inode), FI_INLINE_DATA);
+}
+
+bool f2fs_may_inline(struct inode *inode)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+	block_t nr_blocks;
+	loff_t i_size;
+
+	if (!test_opt(sbi, INLINE_DATA))
+		return false;
+
+	nr_blocks = F2FS_I(inode)->i_xattr_nid ? 3 : 2;
+	if (inode->i_blocks > nr_blocks)
+		return false;
+
+	i_size = i_size_read(inode);
+	if (i_size > MAX_INLINE_DATA)
+		return false;
+
+	return true;
+}
+
+int f2fs_read_inline_data(struct inode *inode, struct page *page)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+	struct page *ipage;
+	void *src_addr, *dst_addr;
+
+	ipage = get_node_page(sbi, inode->i_ino);
+	if (IS_ERR(ipage))
+		return PTR_ERR(ipage);
+
+	src_addr = inline_data_addr(ipage);
+	dst_addr = page_address(page);
+
+	zero_user_segment(page, INLINE_DATA_OFFSET,
+				INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	/* Copy the whole inline data block */
+	memcpy(dst_addr, src_addr, MAX_INLINE_DATA);
+	f2fs_put_page(ipage, 1);
+
+	SetPageUptodate(page);
+	unlock_page(page);
+
+	return 0;
+}
+
+static int __f2fs_convert_inline_data(struct inode *inode, struct page *page)
+{
+	int err;
+	struct page *ipage;
+	struct dnode_of_data dn;
+	void *src_addr, *dst_addr;
+	block_t old_blk_addr, new_blk_addr;
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+
+	f2fs_lock_op(sbi);
+	ipage = get_node_page(sbi, inode->i_ino);
+	if (IS_ERR(ipage))
+		return PTR_ERR(ipage);
+
+	/*
+	 * i_addr[0] is not used for inline data,
+	 * so reserving new block will not destroy inline data
+	 */
+	set_new_dnode(&dn, inode, ipage, ipage, 0);
+	err = f2fs_reserve_block(&dn, 0);
+	if (err) {
+		f2fs_put_page(ipage, 1);
+		f2fs_unlock_op(sbi);
+		return err;
+	}
+
+	src_addr = inline_data_addr(ipage);
+	dst_addr = page_address(page);
+	zero_user_segment(page, 0, PAGE_CACHE_SIZE);
+
+	/* Copy the whole inline data block */
+	memcpy(dst_addr, src_addr, MAX_INLINE_DATA);
+
+	/* write data page to try to make data consistent */
+	old_blk_addr = dn.data_blkaddr;
+	set_page_writeback(page);
+	write_data_page(inode, page, &dn,
+			old_blk_addr, &new_blk_addr);
+	update_extent_cache(new_blk_addr, &dn);
+	f2fs_wait_on_page_writeback(page, DATA, true);
+
+	/* clear inline data and flag after data writeback */
+	zero_user_segment(ipage, INLINE_DATA_OFFSET,
+				 INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	clear_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
+
+	sync_inode_page(&dn);
+	f2fs_put_page(ipage, 1);
+	f2fs_unlock_op(sbi);
+
+	return err;
+}
+
+int f2fs_convert_inline_data(struct inode *inode,
+			     struct page *p, unsigned flags)
+{
+	int err;
+	struct page *page;
+
+	if (!p || p->index) {
+		page = grab_cache_page_write_begin(inode->i_mapping, 0, flags);
+		if (IS_ERR(page))
+			return PTR_ERR(page);
+	} else {
+		page = p;
+	}
+
+	err = __f2fs_convert_inline_data(inode, page);
+
+	if (!p || p->index)
+		f2fs_put_page(page, 1);
+
+	return err;
+}
+
+int f2fs_write_inline_data(struct inode *inode,
+			   struct page *page, unsigned size)
+{
+	void *src_addr, *dst_addr;
+	struct page *ipage;
+	struct dnode_of_data dn;
+	int err;
+
+	set_new_dnode(&dn, inode, NULL, NULL, 0);
+	err = get_dnode_of_data(&dn, 0, LOOKUP_NODE);
+	if (err)
+		return err;
+	ipage = dn.inode_page;
+
+	src_addr = page_address(page);
+	dst_addr = inline_data_addr(ipage);
+
+	zero_user_segment(ipage, INLINE_DATA_OFFSET,
+				 INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	memcpy(dst_addr, src_addr, size);
+
+	/* Release the first data block if it is allocated */
+	if (!f2fs_has_inline_data(inode)) {
+		truncate_data_blocks_range(&dn, 1);
+		set_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
+	}
+
+	sync_inode_page(&dn);
+	f2fs_put_dnode(&dn);
+
+	return 0;
+}
-- 
1.7.9.5

[f2fs-dev][PATCH V2 5/6] f2fs: Handle inline data operations

[Huajun Li]

From: Huajun Li <huajun.li@intel.com>

Hook inline data read/write, truncate, fallocate, setattr, etc.

Files need meet following 2 requirement to inline:
 1) file size is not greater than MAX_INLINE_DATA;
 2) file doesn't pre-allocate data blocks by fallocate().

FI_INLINE_DATA will not be set while creating a new regular inode because
most of the files are bigger than ~3.4K. Set FI_INLINE_DATA only when
data is submitted to block layer, ranther than set it while creating a new
inode, this also avoids converting data from inline to normal data block
and vice versa.

While writting inline data to inode block, the first data block should be
released if the file has a block indexed by i_addr[0].

On the other hand, when a file operation is appied to a file with inline
data, we need to test if this file can remain inline by doing this
operation, otherwise it should be convert into normal file by reserving
a new data block, copying inline data to this new block and clear
FI_INLINE_DATA flag. Because reserve a new data block here will make use
of i_addr[0], if we save inline data in i_addr[0..872], then the first
4 bytes would be overwriten. This problem can be avoided simply by
not using i_addr[0] for inline data.

Signed-off-by: Huajun Li <huajun.li@intel.com>
Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>
Signed-off-by: Weihong Xu <weihong.xu@intel.com>
---
 fs/f2fs/data.c |   49 ++++++++++++++++++++++++++++++++++++++++++++-----
 fs/f2fs/file.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 84 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 92d0724..2e72260 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -471,13 +471,28 @@ static int get_data_block_ro(struct inode *inode, sector_t iblock,
 
 static int f2fs_read_data_page(struct file *file, struct page *page)
 {
-	return mpage_readpage(page, get_data_block_ro);
+	int ret;
+	struct inode *inode = file->f_mapping->host;
+
+	/* If the file has inline data, try to read it directlly */
+	if (f2fs_has_inline_data(inode))
+		ret = f2fs_read_inline_data(inode, page);
+	else
+		ret = mpage_readpage(page, get_data_block_ro);
+
+	return ret;
 }
 
 static int f2fs_read_data_pages(struct file *file,
 			struct address_space *mapping,
 			struct list_head *pages, unsigned nr_pages)
 {
+	struct inode *inode = file->f_mapping->host;
+
+	/* If the file has inline data, skip readpages */
+	if (f2fs_has_inline_data(inode))
+		return 0;
+
 	return mpage_readpages(mapping, pages, nr_pages, get_data_block_ro);
 }
 
@@ -528,7 +543,7 @@ static int f2fs_write_data_page(struct page *page,
 	loff_t i_size = i_size_read(inode);
 	const pgoff_t end_index = ((unsigned long long) i_size)
 							>> PAGE_CACHE_SHIFT;
-	unsigned offset;
+	unsigned offset = 0;
 	bool need_balance_fs = false;
 	int err = 0;
 
@@ -562,7 +577,14 @@ write:
 		err = do_write_data_page(page);
 	} else {
 		f2fs_lock_op(sbi);
-		err = do_write_data_page(page);
+		if (f2fs_has_inline_data(inode) || f2fs_may_inline(inode)) {
+			err = f2fs_write_inline_data(inode, page, offset);
+			f2fs_unlock_op(sbi);
+			goto out;
+		} else {
+			err = do_write_data_page(page);
+		}
+
 		f2fs_unlock_op(sbi);
 		need_balance_fs = true;
 	}
@@ -650,6 +672,15 @@ repeat:
 		return -ENOMEM;
 	*pagep = page;
 
+	if ((pos + len) < MAX_INLINE_DATA) {
+		if (f2fs_has_inline_data(inode))
+			goto inline_data;
+	} else if (f2fs_has_inline_data(inode)) {
+		err = f2fs_convert_inline_data(inode, page, flags);
+		if (err)
+			return err;
+	}
+
 	f2fs_lock_op(sbi);
 	set_new_dnode(&dn, inode, NULL, NULL, 0);
 	err = f2fs_reserve_block(&dn, index);
@@ -659,7 +690,7 @@ repeat:
 		f2fs_put_page(page, 1);
 		return err;
 	}
-
+inline_data:
 	if ((len == PAGE_CACHE_SIZE) || PageUptodate(page))
 		return 0;
 
@@ -675,7 +706,11 @@ repeat:
 	if (dn.data_blkaddr == NEW_ADDR) {
 		zero_user_segment(page, 0, PAGE_CACHE_SIZE);
 	} else {
-		err = f2fs_readpage(sbi, page, dn.data_blkaddr, READ_SYNC);
+		if (f2fs_has_inline_data(inode))
+			err = f2fs_read_inline_data(inode, page);
+		else
+			err = f2fs_readpage(sbi, page,
+					dn.data_blkaddr, READ_SYNC);
 		if (err)
 			return err;
 		lock_page(page);
@@ -724,6 +759,10 @@ static ssize_t f2fs_direct_IO(int rw, struct kiocb *iocb,
 	if (rw == WRITE)
 		return 0;
 
+	/* Let buffer I/O handle the inline data case. */
+	if (f2fs_has_inline_data(inode))
+		return 0;
+
 	/* Needs synchronization with the cleaner */
 	return blockdev_direct_IO(rw, iocb, inode, iov, offset, nr_segs,
 						  get_data_block_ro);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 1cd8e44..ffafcb9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -291,7 +291,9 @@ static int truncate_blocks(struct inode *inode, u64 from)
 
 	f2fs_put_dnode(&dn);
 free_next:
-	err = truncate_inode_blocks(inode, free_from);
+
+	if (!f2fs_has_inline_data(inode))
+		err = truncate_inode_blocks(inode, free_from);
 	f2fs_unlock_op(sbi);
 
 	/* lastly zero out the first data page */
@@ -367,8 +369,17 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 
 	if ((attr->ia_valid & ATTR_SIZE) &&
 			attr->ia_size != i_size_read(inode)) {
+		if (f2fs_has_inline_data(inode) &&
+				(attr->ia_size > MAX_INLINE_DATA)) {
+			unsigned flags = AOP_FLAG_NOFS;
+			err = f2fs_convert_inline_data(inode, NULL, flags);
+			if (err)
+				return err;
+		}
+
 		truncate_setsize(inode, attr->ia_size);
-		f2fs_truncate(inode);
+		if (!f2fs_has_inline_data(inode))
+			f2fs_truncate(inode);
 		f2fs_balance_fs(F2FS_SB(inode->i_sb));
 	}
 
@@ -450,6 +461,26 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len, int mode)
 	loff_t off_start, off_end;
 	int ret = 0;
 
+	if (f2fs_has_inline_data(inode)) {
+		struct page *page;
+		unsigned flags = AOP_FLAG_NOFS;
+		page = grab_cache_page_write_begin(inode->i_mapping, 0, flags);
+		if (IS_ERR(page))
+			return PTR_ERR(page);
+		if (offset + len > MAX_INLINE_DATA) {
+			ret = f2fs_convert_inline_data(inode, page, flags);
+			f2fs_put_page(page, 1);
+			if (ret)
+				return ret;
+		} else {
+			zero_user_segment(page, offset, offset + len);
+			SetPageUptodate(page);
+			set_page_dirty(page);
+			f2fs_put_page(page, 1);
+			goto out;
+		}
+	}
+
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;
 
@@ -483,7 +514,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len, int mode)
 			f2fs_unlock_op(sbi);
 		}
 	}
-
+out:
 	if (!(mode & FALLOC_FL_KEEP_SIZE) &&
 		i_size_read(inode) <= (offset + len)) {
 		i_size_write(inode, offset);
@@ -502,6 +533,12 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	loff_t off_start, off_end;
 	int ret = 0;
 
+	if (f2fs_has_inline_data(inode) && (offset + len > MAX_INLINE_DATA)) {
+		ret = f2fs_convert_inline_data(inode, NULL, AOP_FLAG_NOFS);
+		if (ret)
+			return ret;
+	}
+
 	ret = inode_newsize_ok(inode, (len + offset));
 	if (ret)
 		return ret;
-- 
1.7.9.5

[f2fs-dev][PATCH V2 6/6] f2fs: update f2fs Documentation

[Huajun Li]

From: Huajun Li <huajun.li@intel.com>

This patch describes the inline_data support in f2fs document.

Signed-off-by: Huajun Li <huajun.li@intel.com>
---
 Documentation/filesystems/f2fs.txt |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
index a3fe811..9849372 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -120,6 +120,8 @@ active_logs=%u         Support configuring the number of active logs. In the
 disable_ext_identify   Disable the extension list configured by mkfs, so f2fs
                        does not aware of cold files such as media files.
 inline_xattr           Enable the inline xattrs feature.
+inline_data            Enable the inline data feature: New created small(<~3.4k)
+                       files can be written into inode block.
 
 ================================================================================
 DEBUGFS ENTRIES
-- 
1.7.9.5

Re: [f2fs-dev][PATCH V2 4/6] f2fs: Key functions to handle inline data

[Jaegeuk Kim]

Hi Huajun,

[snip]

> +static int __f2fs_convert_inline_data(struct inode *inode, struct page *page)
> +{
> +	int err;
> +	struct page *ipage;
> +	struct dnode_of_data dn;
> +	void *src_addr, *dst_addr;
> +	block_t old_blk_addr, new_blk_addr;
> +	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> +
> +	f2fs_lock_op(sbi);
> +	ipage = get_node_page(sbi, inode->i_ino);
> +	if (IS_ERR(ipage))
> +		return PTR_ERR(ipage);
> +
> +	/*
> +	 * i_addr[0] is not used for inline data,
> +	 * so reserving new block will not destroy inline data
> +	 */
> +	set_new_dnode(&dn, inode, ipage, ipage, 0);
> +	err = f2fs_reserve_block(&dn, 0);
> +	if (err) {
> +		f2fs_put_page(ipage, 1);
> +		f2fs_unlock_op(sbi);
> +		return err;
> +	}
> +
> +	src_addr = inline_data_addr(ipage);
> +	dst_addr = page_address(page);
> +	zero_user_segment(page, 0, PAGE_CACHE_SIZE);
> +
> +	/* Copy the whole inline data block */
> +	memcpy(dst_addr, src_addr, MAX_INLINE_DATA);
> +
> +	/* write data page to try to make data consistent */
> +	old_blk_addr = dn.data_blkaddr;
> +	set_page_writeback(page);
> +	write_data_page(inode, page, &dn,
> +			old_blk_addr, &new_blk_addr);
> +	update_extent_cache(new_blk_addr, &dn);
> +	f2fs_wait_on_page_writeback(page, DATA, true);
> +
> +	/* clear inline data and flag after data writeback */
> +	zero_user_segment(ipage, INLINE_DATA_OFFSET,
> +				 INLINE_DATA_OFFSET + MAX_INLINE_DATA);
> +	clear_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
> +
> +	sync_inode_page(&dn);
> +	f2fs_put_page(ipage, 1);

Again, it seems that you missed what I mentioned.
If we write the inlined data block only, we cannot recover the data
block after SPO.
In order to avoid that, we should write its dnode block too by
triggering sync_node_pages(ino) at this point as similar as fsync
routine.

Thanks,

-- 
Jaegeuk Kim
Samsung

Re: [f2fs-dev][PATCH V2 4/6] f2fs: Key functions to handle inline data

[Huajun Li]

On Fri, Nov 15, 2013 at 3:49 PM, Jaegeuk Kim <jaegeuk.kim@samsung.com> wrote:
> Hi Huajun,
>
> [snip]
>
>> +static int __f2fs_convert_inline_data(struct inode *inode, struct page *page)
>> +{
>> +     int err;
>> +     struct page *ipage;
>> +     struct dnode_of_data dn;
>> +     void *src_addr, *dst_addr;
>> +     block_t old_blk_addr, new_blk_addr;
>> +     struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
>> +
>> +     f2fs_lock_op(sbi);
>> +     ipage = get_node_page(sbi, inode->i_ino);
>> +     if (IS_ERR(ipage))
>> +             return PTR_ERR(ipage);
>> +
>> +     /*
>> +      * i_addr[0] is not used for inline data,
>> +      * so reserving new block will not destroy inline data
>> +      */
>> +     set_new_dnode(&dn, inode, ipage, ipage, 0);
>> +     err = f2fs_reserve_block(&dn, 0);
>> +     if (err) {
>> +             f2fs_put_page(ipage, 1);
>> +             f2fs_unlock_op(sbi);
>> +             return err;
>> +     }
>> +
>> +     src_addr = inline_data_addr(ipage);
>> +     dst_addr = page_address(page);
>> +     zero_user_segment(page, 0, PAGE_CACHE_SIZE);
>> +
>> +     /* Copy the whole inline data block */
>> +     memcpy(dst_addr, src_addr, MAX_INLINE_DATA);
>> +
>> +     /* write data page to try to make data consistent */
>> +     old_blk_addr = dn.data_blkaddr;
>> +     set_page_writeback(page);
>> +     write_data_page(inode, page, &dn,
>> +                     old_blk_addr, &new_blk_addr);
>> +     update_extent_cache(new_blk_addr, &dn);
>> +     f2fs_wait_on_page_writeback(page, DATA, true);
>> +
>> +     /* clear inline data and flag after data writeback */
>> +     zero_user_segment(ipage, INLINE_DATA_OFFSET,
>> +                              INLINE_DATA_OFFSET + MAX_INLINE_DATA);
>> +     clear_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
>> +
>> +     sync_inode_page(&dn);
>> +     f2fs_put_page(ipage, 1);
>
> Again, it seems that you missed what I mentioned.
> If we write the inlined data block only, we cannot recover the data
> block after SPO.
> In order to avoid that, we should write its dnode block too by
> triggering sync_node_pages(ino) at this point as similar as fsync
> routine.
>
> Thanks,
>
> --
> Jaegeuk Kim
> Samsung
>

Hi Jaegeuk,
Previously, I refactored f2fs_sync_file() and tried to call it while
converting inline data, but find it is easily dead lock, so just write
data block here.
Then, how about following enhancement to this patch ?  it only copies
some codes to the end of  __f2fs_convert_inline_data(),  and keeps
others same as before.

-----------------------------------------------------------------------------
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index e9b33d7..6ceb2e6 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -10,6 +10,8 @@

 #include <linux/fs.h>
 #include <linux/f2fs_fs.h>
+#include <linux/writeback.h>
+#include <linux/blkdev.h>

 #include "f2fs.h"

@@ -71,6 +73,11 @@ static int __f2fs_convert_inline_data(struct inode
*inode, struct page *page)
     void *src_addr, *dst_addr;
     block_t old_blk_addr, new_blk_addr;
     struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+    struct writeback_control wbc = {
+        .sync_mode = WB_SYNC_ALL,
+        .nr_to_write = LONG_MAX,
+        .for_reclaim = 0,
+    };

     f2fs_lock_op(sbi);
     ipage = get_node_page(sbi, inode->i_ino);
@@ -108,9 +115,20 @@ static int __f2fs_convert_inline_data(struct
inode *inode, struct page *page)
     zero_user_segment(ipage, INLINE_DATA_OFFSET,
                  INLINE_DATA_OFFSET + MAX_INLINE_DATA);
     clear_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
-
     sync_inode_page(&dn);
     f2fs_put_page(ipage, 1);
+
+    while (!sync_node_pages(sbi, inode->i_ino, &wbc)) {
+        mark_inode_dirty_sync(inode);
+        err = f2fs_write_inode(inode, NULL);
+        if (err)
+            goto out;
+    }
+    err = wait_on_node_pages_writeback(sbi, inode->i_ino);
+    if (err)
+        goto out;
+    err = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
+
+out:
     f2fs_unlock_op(sbi);

     return err;

Re: [f2fs-dev][PATCH V2 4/6] f2fs: Key functions to handle inline data

[Jaegeuk Kim]

Hi Huajun,

2013-11-20 (수), 20:51 +0800, Huajun Li:
> On Fri, Nov 15, 2013 at 3:49 PM, Jaegeuk Kim <jaegeuk.kim@samsung.com> wrote:
> > Hi Huajun,
> >
> > [snip]
> >
> >> +static int __f2fs_convert_inline_data(struct inode *inode, struct page *page)
> >> +{
> >> +     int err;
> >> +     struct page *ipage;
> >> +     struct dnode_of_data dn;
> >> +     void *src_addr, *dst_addr;
> >> +     block_t old_blk_addr, new_blk_addr;
> >> +     struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> >> +
> >> +     f2fs_lock_op(sbi);
> >> +     ipage = get_node_page(sbi, inode->i_ino);
> >> +     if (IS_ERR(ipage))
> >> +             return PTR_ERR(ipage);
> >> +
> >> +     /*
> >> +      * i_addr[0] is not used for inline data,
> >> +      * so reserving new block will not destroy inline data
> >> +      */
> >> +     set_new_dnode(&dn, inode, ipage, ipage, 0);
> >> +     err = f2fs_reserve_block(&dn, 0);
> >> +     if (err) {
> >> +             f2fs_put_page(ipage, 1);
> >> +             f2fs_unlock_op(sbi);
> >> +             return err;
> >> +     }
> >> +
> >> +     src_addr = inline_data_addr(ipage);
> >> +     dst_addr = page_address(page);
> >> +     zero_user_segment(page, 0, PAGE_CACHE_SIZE);
> >> +
> >> +     /* Copy the whole inline data block */
> >> +     memcpy(dst_addr, src_addr, MAX_INLINE_DATA);
> >> +
> >> +     /* write data page to try to make data consistent */
> >> +     old_blk_addr = dn.data_blkaddr;
> >> +     set_page_writeback(page);
> >> +     write_data_page(inode, page, &dn,
> >> +                     old_blk_addr, &new_blk_addr);
> >> +     update_extent_cache(new_blk_addr, &dn);
> >> +     f2fs_wait_on_page_writeback(page, DATA, true);
> >> +
> >> +     /* clear inline data and flag after data writeback */
> >> +     zero_user_segment(ipage, INLINE_DATA_OFFSET,
> >> +                              INLINE_DATA_OFFSET + MAX_INLINE_DATA);
> >> +     clear_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
> >> +
> >> +     sync_inode_page(&dn);
> >> +     f2fs_put_page(ipage, 1);
> >
> > Again, it seems that you missed what I mentioned.
> > If we write the inlined data block only, we cannot recover the data
> > block after SPO.
> > In order to avoid that, we should write its dnode block too by
> > triggering sync_node_pages(ino) at this point as similar as fsync
> > routine.
> >
> > Thanks,
> >
> > --
> > Jaegeuk Kim
> > Samsung
> >
> 
> Hi Jaegeuk,
> Previously, I refactored f2fs_sync_file() and tried to call it while
> converting inline data, but find it is easily dead lock, so just write
> data block here.
> Then, how about following enhancement to this patch ?  it only copies
> some codes to the end of  __f2fs_convert_inline_data(),  and keeps
> others same as before.

Sorry for the late response.
It takes some time for me to verify the consistency problem in more
detail.

What I've concerned was the following issues:
 - inlined data was synced before or not,
 - inlined data was fsynced before or not,
 - its parent directory inode was synced before or not,
 - recovery can be safe?
 - ...

Most of these issues are based on the question that "can we recover the
inlined data after sudden-power-off safely?".

And initially what I concerned was from the following scenario.
 1. user writes 3KB data
 2. sync or fsync
 3. user writes 4KB data
   : remove direct pointers in the inode page,
       and cache a converted data page.
 4. do checkpoint
   : write the inode page only

 ** After power-cut, user expect at least the file should have 3KB data,
but there is no data due to the converted inline data.

Lastly, I found that, it'd be ok if we can cover the following lock
coverages.
  - f2fs_lock_op
  |    - lock_page(inode_page)
  |   |   -- convert_inline_data()
  |   |      1. write_data_page()
  |   |      2. update its inode page()
  |    - unlock_page(inode_page)
  - f2fs_unlock_op

This means that, the step #4 can guarantee that the inode has the direct
pointer of 4KB data.
And when considering other cases, I couldn't find any issues.

So, yes, I concluded that your first approach which writes data pages
only was correct.

However I found that it needs to modify some recovery routine integrated
to your patch, [5/6].

In do_recover_data(inode_page),
 1. get the first file offset of the inode_page,
 2. get its previous written inode page,
 3. diff direct pointers between previous inode page and current inode
page
 4. check previous and current direct pointers

Let's suppose that the previous inode page has inline data and current
inode page is a coverted node page or vice versa.

[direct pointers]    [previous inode page]     [current inode page]
       [0]             abcd (inline data)         xxx (block addr)
or,
       [0]             xxx (block addr)           abcd (inline data)

In this case, f2fs will recover errorneous block addresses, so it needs
to avoid mishandling the direct pointers too.

Thanks,

-- 
Jaegeuk Kim
Samsung

Re: [f2fs-dev][PATCH V2 3/6] f2fs: Add a new function: f2fs_reserve_block()

[Jaegeuk Kim]

Hi Huajun,

I think this clean-up is not related to inline_data series, so I merged
this patch earlier.

Thanks,

2013-11-10 (일), 23:13 +0800, Huajun Li:
> From: Huajun Li <huajun.li@intel.com>
> 
> Add the function f2fs_reserve_block() to easily reserve new blocks, and
> use it to clean up more codes.
> 
> Signed-off-by: Huajun Li <huajun.li@intel.com>
> Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>
> Signed-off-by: Weihong Xu <weihong.xu@intel.com>
> ---
>  fs/f2fs/data.c |   50 +++++++++++++++++++++++---------------------------
>  fs/f2fs/f2fs.h |    1 +
>  fs/f2fs/file.c |   38 ++++++--------------------------------
>  3 files changed, 30 insertions(+), 59 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index aa3438c..92d0724 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -64,6 +64,22 @@ int reserve_new_block(struct dnode_of_data *dn)
>  	return 0;
>  }
>  
> +int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
> +{
> +	bool need_put = dn->inode_page ? false : true;
> +	int err;
> +
> +	err = get_dnode_of_data(dn, index, ALLOC_NODE);
> +	if (err)
> +		return err;
> +	if (dn->data_blkaddr == NULL_ADDR)
> +		err = reserve_new_block(dn);
> +
> +	if (need_put)
> +		f2fs_put_dnode(dn);
> +	return err;
> +}
> +
>  static int check_extent_cache(struct inode *inode, pgoff_t pgofs,
>  					struct buffer_head *bh_result)
>  {
> @@ -300,19 +316,10 @@ struct page *get_new_data_page(struct inode *inode,
>  	int err;
>  
>  	set_new_dnode(&dn, inode, npage, npage, 0);
> -	err = get_dnode_of_data(&dn, index, ALLOC_NODE);
> +	err = f2fs_reserve_block(&dn, index);
>  	if (err)
>  		return ERR_PTR(err);
>  
> -	if (dn.data_blkaddr == NULL_ADDR) {
> -		if (reserve_new_block(&dn)) {
> -			if (!npage)
> -				f2fs_put_dnode(&dn);
> -			return ERR_PTR(-ENOSPC);
> -		}
> -	}
> -	if (!npage)
> -		f2fs_put_dnode(&dn);
>  repeat:
>  	page = grab_cache_page(mapping, index);
>  	if (!page)
> @@ -644,21 +651,15 @@ repeat:
>  	*pagep = page;
>  
>  	f2fs_lock_op(sbi);
> -
>  	set_new_dnode(&dn, inode, NULL, NULL, 0);
> -	err = get_dnode_of_data(&dn, index, ALLOC_NODE);
> -	if (err)
> -		goto err;
> -
> -	if (dn.data_blkaddr == NULL_ADDR)
> -		err = reserve_new_block(&dn);
> -
> -	f2fs_put_dnode(&dn);
> -	if (err)
> -		goto err;
> -
> +	err = f2fs_reserve_block(&dn, index);
>  	f2fs_unlock_op(sbi);
>  
> +	if (err) {
> +		f2fs_put_page(page, 1);
> +		return err;
> +	}
> +
>  	if ((len == PAGE_CACHE_SIZE) || PageUptodate(page))
>  		return 0;
>  
> @@ -691,11 +692,6 @@ out:
>  	SetPageUptodate(page);
>  	clear_cold_data(page);
>  	return 0;
> -
> -err:
> -	f2fs_unlock_op(sbi);
> -	f2fs_put_page(page, 1);
> -	return err;
>  }
>  
>  static int f2fs_write_end(struct file *file,
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index cd7d2f9..de84f52 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1119,6 +1119,7 @@ void destroy_checkpoint_caches(void);
>   * data.c
>   */
>  int reserve_new_block(struct dnode_of_data *);
> +int f2fs_reserve_block(struct dnode_of_data *, pgoff_t);
>  void update_extent_cache(block_t, struct dnode_of_data *);
>  struct page *find_data_page(struct inode *, pgoff_t, bool);
>  struct page *get_lock_data_page(struct inode *, pgoff_t);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 7d714f4..1cd8e44 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -33,7 +33,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
>  	struct page *page = vmf->page;
>  	struct inode *inode = file_inode(vma->vm_file);
>  	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> -	block_t old_blk_addr;
>  	struct dnode_of_data dn;
>  	int err;
>  
> @@ -44,24 +43,10 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
>  	/* block allocation */
>  	f2fs_lock_op(sbi);
>  	set_new_dnode(&dn, inode, NULL, NULL, 0);
> -	err = get_dnode_of_data(&dn, page->index, ALLOC_NODE);
> -	if (err) {
> -		f2fs_unlock_op(sbi);
> -		goto out;
> -	}
> -
> -	old_blk_addr = dn.data_blkaddr;
> -
> -	if (old_blk_addr == NULL_ADDR) {
> -		err = reserve_new_block(&dn);
> -		if (err) {
> -			f2fs_put_dnode(&dn);
> -			f2fs_unlock_op(sbi);
> -			goto out;
> -		}
> -	}
> -	f2fs_put_dnode(&dn);
> +	err = f2fs_reserve_block(&dn, page->index);
>  	f2fs_unlock_op(sbi);
> +	if (err)
> +		goto out;
>  
>  	file_update_time(vma->vm_file);
>  	lock_page(page);
> @@ -532,22 +517,11 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>  
>  		f2fs_lock_op(sbi);
>  		set_new_dnode(&dn, inode, NULL, NULL, 0);
> -		ret = get_dnode_of_data(&dn, index, ALLOC_NODE);
> -		if (ret) {
> -			f2fs_unlock_op(sbi);
> +		ret = f2fs_reserve_block(&dn, index);
> +		f2fs_unlock_op(sbi);
> +		if (ret)
>  			break;
> -		}
>  
> -		if (dn.data_blkaddr == NULL_ADDR) {
> -			ret = reserve_new_block(&dn);
> -			if (ret) {
> -				f2fs_put_dnode(&dn);
> -				f2fs_unlock_op(sbi);
> -				break;
> -			}
> -		}
> -		f2fs_put_dnode(&dn);
> -		f2fs_unlock_op(sbi);
>  
>  		if (pg_start == pg_end)
>  			new_size = offset + len;

-- 
Jaegeuk Kim
Samsung

Re: [f2fs-dev][PATCH V2 0/6] f2fs: Enable f2fs support inline data

[Jaegeuk Kim]

Hi Huajun,

Please adjust the following bug fixes in your patches.

---
 fs/f2fs/data.c   |  2 +-
 fs/f2fs/file.c   | 12 +++++++-----
 fs/f2fs/inline.c | 19 +++++++++++--------
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2eed6e3..e44f0ba 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -534,8 +534,8 @@ static int get_data_block_ro(struct inode *inode,
sector_t iblock,
 
 static int f2fs_read_data_page(struct file *file, struct page *page)
 {
+	struct inode *inode = page->mapping->host;
 	int ret;
-	struct inode *inode = file->f_mapping->host;

>> The file pointer is NULL if this page contains file's symlink data,
which induces NULL pointer error.
 
 	/* If the file has inline data, try to read it directlly */
 	if (f2fs_has_inline_data(inode))
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ffafcb9..52bb211 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -257,8 +257,7 @@ static int truncate_blocks(struct inode *inode, u64
from)
 	unsigned int blocksize = inode->i_sb->s_blocksize;
 	struct dnode_of_data dn;
 	pgoff_t free_from;
-	int count = 0;
-	int err;
+	int count = 0, err = 0;
 
 	trace_f2fs_truncate_blocks_enter(inode, from);
 
@@ -266,6 +265,10 @@ static int truncate_blocks(struct inode *inode, u64
from)
 			((from + blocksize - 1) >> (sbi->log_blocksize));
 
 	f2fs_lock_op(sbi);
+
+	if (f2fs_has_inline_data(inode))
+		goto done;
+

>> We don't need to do the below actions. Let's skip that.

 	set_new_dnode(&dn, inode, NULL, NULL, 0);
 	err = get_dnode_of_data(&dn, free_from, LOOKUP_NODE);
 	if (err) {
@@ -291,9 +294,8 @@ static int truncate_blocks(struct inode *inode, u64
from)
 
 	f2fs_put_dnode(&dn);
 free_next:
-
-	if (!f2fs_has_inline_data(inode))
-		err = truncate_inode_blocks(inode, free_from);
+	err = truncate_inode_blocks(inode, free_from);
+done:
 	f2fs_unlock_op(sbi);
 
 	/* lastly zero out the first data page */
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index e9b33d7..6c301cc 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -48,13 +48,14 @@ int f2fs_read_inline_data(struct inode *inode,
struct page *page)
 	if (IS_ERR(ipage))
 		return PTR_ERR(ipage);
 
-	src_addr = inline_data_addr(ipage);
-	dst_addr = page_address(page);
-
 	zero_user_segment(page, INLINE_DATA_OFFSET,
 				INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+
 	/* Copy the whole inline data block */
+	src_addr = inline_data_addr(ipage);
+	dst_addr = kmap(page);
 	memcpy(dst_addr, src_addr, MAX_INLINE_DATA);
+	kunmap(page);

>> We should do kmap/kunmap for normal data pages.

 	f2fs_put_page(ipage, 1);
 
 	SetPageUptodate(page);
@@ -89,12 +90,13 @@ static int __f2fs_convert_inline_data(struct inode
*inode, struct page *page)
 		return err;
 	}
 
-	src_addr = inline_data_addr(ipage);
-	dst_addr = page_address(page);
 	zero_user_segment(page, 0, PAGE_CACHE_SIZE);
 
 	/* Copy the whole inline data block */
+	src_addr = inline_data_addr(ipage);
+	dst_addr = kmap(page);
 	memcpy(dst_addr, src_addr, MAX_INLINE_DATA);
+	kunmap(page);

>> ditto.
 
 	/* write data page to try to make data consistent */
 	old_blk_addr = dn.data_blkaddr;
@@ -152,12 +154,13 @@ int f2fs_write_inline_data(struct inode *inode,
 		return err;
 	ipage = dn.inode_page;
 
-	src_addr = page_address(page);
-	dst_addr = inline_data_addr(ipage);
-
 	zero_user_segment(ipage, INLINE_DATA_OFFSET,
 				 INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+
+	src_addr = kmap(page);
+	dst_addr = inline_data_addr(ipage);
 	memcpy(dst_addr, src_addr, size);
+	kunmap(page);

>> ditto.
 
 	/* Release the first data block if it is allocated */
 	if (!f2fs_has_inline_data(inode)) {
-- 
1.8.4.474.g128a96c



-- 
Jaegeuk Kim
Samsung