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

[Jaegeuk Kim <jaegeuk.kim@samsung.com>]

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


------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk