f2fs: fix to truncate inline data in inode page when setattr

PATCH In review Chao Yu 2 participants 5 messages Apr 29, 2014

Subsystems (2)

VFS F2FS file system

lore

Share & tools

lore

Chao Yu

author

2014-04-29 06:03 UTC #0 lore raw

PATCH v3 3/3 f2fs: fix to truncate inline data in inode page when setattr

Previous we do not truncate inline data in inode page when setattr, so following case could still read the inline data which has already truncated:

1.write inline data
2.ftruncate size to 0
3.ftruncate size to max inline data size
4.read from offset 0

This patch introduces truncate_inline_data() to fix this problem.

change log from v1:
o fix a bug and do not truncate first page data after truncate inline data.
change log from v2:
o wait to writeback for inode page if it was already submitted.

Signed-off-by Chao Yu <[email protected]>

---
 fs/f2fs/f2fs.h   |    1 +
 fs/f2fs/file.c   |    3 +++
 fs/f2fs/inline.c |   19 +++++++++++++++++++
 3 files changed, 23 insertions(+)

fs/f2fs/f2fs.h +1 -0

index 2b67679..676a2c6 100644
@@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_data(struct inode *, pgoff_t);
 int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
+void truncate_inline_data(struct inode *, u64);
 int recover_inline_data(struct inode *, struct page *);
 #endif

fs/f2fs/file.c +3 -0

index b9f4fbf..d97e5c4 100644
@@ -369,6 +369,9 @@ static void truncate_partial_data_page(struct inode *inode, u64 from)
 	unsigned offset = from & (PAGE_CACHE_SIZE - 1);
 	struct page *page;
 
+	if (f2fs_has_inline_data(inode))
+		return truncate_inline_data(inode, from);
+
 	if (!offset)
 		return;

fs/f2fs/inline.c +19 -0

index 3258c7c..6a13997 100644
@@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
 	return 0;
 }
 
+void truncate_inline_data(struct inode *inode, u64 from)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+	struct page *ipage;
+
+	if (from >= MAX_INLINE_DATA)
+		return;
+
+	ipage = get_node_page(sbi, inode->i_ino);
+	if (IS_ERR(ipage))
+		return;
+
+	f2fs_wait_on_page_writeback(ipage, NODE);
+	zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
+				INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	set_page_dirty(ipage);
+	f2fs_put_page(ipage, 1);
+}
+
 int recover_inline_data(struct inode *inode, struct page *npage)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);

Signature · 10 lines

1.7.9.5



------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.  Get 
unparalleled scalability from the best Selenium testing platform available.
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs

Jaegeuk Kim

2014-04-29 06:16 UTC #1 lore raw

Hi,

2014-04-29 (화), 14:03 +0800, Chao Yu:

Quoted 58 lines

Previous we do not truncate inline data in inode page when setattr, so following case could still read the inline data which has already truncated:
1.write inline data
2.ftruncate size to 0
3.ftruncate size to max inline data size
4.read from offset 0
This patch introduces truncate_inline_data() to fix this problem.
change log from v1:
o fix a bug and do not truncate first page data after truncate inline data.
change log from v2:
o wait to writeback for inode page if it was already submitted.
Signed-off-by Chao Yu <[email protected]>
---
 fs/f2fs/f2fs.h   |    1 +
 fs/f2fs/file.c   |    3 +++
 fs/f2fs/inline.c |   19 +++++++++++++++++++
 3 files changed, 23 insertions(+)
fs/f2fs/f2fs.h +1 -0
index 2b67679..676a2c6 100644
@@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_data(struct inode *, pgoff_t);
 int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
+void truncate_inline_data(struct inode *, u64);
 int recover_inline_data(struct inode *, struct page *);
 #endif
fs/f2fs/file.c +3 -0
index b9f4fbf..d97e5c4 100644
@@ -369,6 +369,9 @@ static void truncate_partial_data_page(struct inode *inode, u64 from)
 	unsigned offset = from & (PAGE_CACHE_SIZE - 1);
 	struct page *page;
 
+	if (f2fs_has_inline_data(inode))
+		return truncate_inline_data(inode, from);
+
 	if (!offset)
 		return;
 
fs/f2fs/inline.c +3 -0
index 3258c7c..6a13997 100644
@@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
 	return 0;
 }
 
+void truncate_inline_data(struct inode *inode, u64 from)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);

fs/f2fs/inline.c int f2fs_write_inline_data(struct inode *inode, · line 191

Quoted 10 lines

fs/f2fs/inline.c +10 -0

+	struct page *ipage;
+
+	if (from >= MAX_INLINE_DATA)
+		return;
+
+	ipage = get_node_page(sbi, inode->i_ino);
+	if (IS_ERR(ipage))
+		return;
+
+	f2fs_wait_on_page_writeback(ipage, NODE);

The get_node_page() triggers,
grab_cache_page_write_begin()
-> wait_for_stable_page()

     -> wait_on_page_writeback().

Any bugs in there?

Quoted 9 lines

fs/f2fs/inline.c +6 -0

+	zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
+				INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	set_page_dirty(ipage);
+	f2fs_put_page(ipage, 1);
+}
+
 int recover_inline_data(struct inode *inode, struct page *npage)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);

Signature · 2 lines

Jaegeuk Kim
Samsung

Chao Yu

author

2014-04-29 07:53 UTC #2 lore raw

Hi Kim,

Quoted 83 lines

-----Original Message-----
From: Jaegeuk Kim [mailto:[email protected]]
Sent: Tuesday, April 29, 2014 2:16 PM
To: Chao Yu
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
Hi,
2014-04-29 (화), 14:03 +0800, Chao Yu:
Quoted 58 lines
Previous we do not truncate inline data in inode page when setattr, so following case could still read the inline data which has already truncated:
1.write inline data
2.ftruncate size to 0
3.ftruncate size to max inline data size
4.read from offset 0
This patch introduces truncate_inline_data() to fix this problem.
change log from v1:
o fix a bug and do not truncate first page data after truncate inline data.
change log from v2:
o wait to writeback for inode page if it was already submitted.
Signed-off-by Chao Yu <[email protected]>
---
 fs/f2fs/f2fs.h   |    1 +
 fs/f2fs/file.c   |    3 +++
 fs/f2fs/inline.c |   19 +++++++++++++++++++
 3 files changed, 23 insertions(+)
fs/f2fs/f2fs.h +1 -0
index 2b67679..676a2c6 100644
@@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_data(struct inode *, pgoff_t);
 int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
+void truncate_inline_data(struct inode *, u64);
 int recover_inline_data(struct inode *, struct page *);
 #endif
fs/f2fs/file.c +3 -0
index b9f4fbf..d97e5c4 100644
@@ -369,6 +369,9 @@ static void truncate_partial_data_page(struct inode *inode, u64 from)
 	unsigned offset = from & (PAGE_CACHE_SIZE - 1);
 	struct page *page;
 
+	if (f2fs_has_inline_data(inode))
+		return truncate_inline_data(inode, from);
+
 	if (!offset)
 		return;
 
fs/f2fs/inline.c +3 -0
index 3258c7c..6a13997 100644
@@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
 	return 0;
 }
 
+void truncate_inline_data(struct inode *inode, u64 from)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
fs/f2fs/inline.c int f2fs_write_inline_data(struct inode *inode, · line 191
Quoted 10 lines
fs/f2fs/inline.c +10 -0
+	struct page *ipage;
+
+	if (from >= MAX_INLINE_DATA)
+		return;
+
+	ipage = get_node_page(sbi, inode->i_ino);
+	if (IS_ERR(ipage))
+		return;
+
+	f2fs_wait_on_page_writeback(ipage, NODE);
The get_node_page() triggers,
grab_cache_page_write_begin()
-> wait_for_stable_page()

Quoted 1 line

     -> wait_on_page_writeback().

Oh, it's my mistake for this missing, thanks for the mention.:)

Quoted 2 lines

Any bugs in there?

If we wait in grab_cache_page_write_begin(), maybe we will face long time delay or potential deadlock caused by f2fs bio cache. So how about use grab_cache_page & f2fs_wait_on_page_writeback to avoid this?

Quoted 14 lines

fs/f2fs/inline.c int f2fs_write_inline_data(struct inode *inode, · line 200

Quoted 10 lines

fs/f2fs/inline.c +6 -0

+	zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
+				INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	set_page_dirty(ipage);
+	f2fs_put_page(ipage, 1);
+}
+
 int recover_inline_data(struct inode *inode, struct page *npage)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);

--
Jaegeuk Kim
Samsung

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Jaegeuk Kim

2014-04-29 08:02 UTC #3 lore raw

Hi,

2014-04-29 (화), 15:53 +0800, Chao Yu:

Quoted 87 lines

Hi Kim,
Quoted 83 lines
-----Original Message-----
From: Jaegeuk Kim [mailto:[email protected]]
Sent: Tuesday, April 29, 2014 2:16 PM
To: Chao Yu
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
Hi,
2014-04-29 (화), 14:03 +0800, Chao Yu:
Quoted 58 lines
Previous we do not truncate inline data in inode page when setattr, so following case could still read the inline data which has already truncated:
1.write inline data
2.ftruncate size to 0
3.ftruncate size to max inline data size
4.read from offset 0
This patch introduces truncate_inline_data() to fix this problem.
change log from v1:
o fix a bug and do not truncate first page data after truncate inline data.
change log from v2:
o wait to writeback for inode page if it was already submitted.
Signed-off-by Chao Yu <[email protected]>
---
 fs/f2fs/f2fs.h   |    1 +
 fs/f2fs/file.c   |    3 +++
 fs/f2fs/inline.c |   19 +++++++++++++++++++
 3 files changed, 23 insertions(+)
fs/f2fs/f2fs.h +1 -0
index 2b67679..676a2c6 100644
@@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_data(struct inode *, pgoff_t);
 int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
+void truncate_inline_data(struct inode *, u64);
 int recover_inline_data(struct inode *, struct page *);
 #endif
fs/f2fs/file.c +3 -0
index b9f4fbf..d97e5c4 100644
@@ -369,6 +369,9 @@ static void truncate_partial_data_page(struct inode *inode, u64 from)
 	unsigned offset = from & (PAGE_CACHE_SIZE - 1);
 	struct page *page;
 
+	if (f2fs_has_inline_data(inode))
+		return truncate_inline_data(inode, from);
+
 	if (!offset)
 		return;
 
fs/f2fs/inline.c +3 -0
index 3258c7c..6a13997 100644
@@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
 	return 0;
 }
 
+void truncate_inline_data(struct inode *inode, u64 from)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
fs/f2fs/inline.c int f2fs_write_inline_data(struct inode *inode, · line 191
Quoted 10 lines
fs/f2fs/inline.c +10 -0
+	struct page *ipage;
+
+	if (from >= MAX_INLINE_DATA)
+		return;
+
+	ipage = get_node_page(sbi, inode->i_ino);
+	if (IS_ERR(ipage))
+		return;
+
+	f2fs_wait_on_page_writeback(ipage, NODE);
The get_node_page() triggers,
grab_cache_page_write_begin()
-> wait_for_stable_page()
Quoted 1 line
     -> wait_on_page_writeback().

Quoted 8 lines

Oh, it's my mistake for this missing, thanks for the mention.:)
Quoted 2 lines
Any bugs in there?
If we wait in grab_cache_page_write_begin(), maybe we will face long time delay or potential deadlock caused by f2fs bio cache. So how about use grab_cache_page & f2fs_wait_on_page_writeback to avoid this?

Nice catch. :)
IMO, it'd better merge your v2, and let me investigate all the paths to avoid that.
Is it okay?

Quoted 21 lines

Quoted 15 lines
fs/f2fs/inline.c int f2fs_write_inline_data(struct inode *inode, · line 200
Quoted 11 lines
fs/f2fs/inline.c +6 -0
+	zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
+				INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	set_page_dirty(ipage);
+	f2fs_put_page(ipage, 1);
+}
+
 int recover_inline_data(struct inode *inode, struct page *npage)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
--
Jaegeuk Kim
Samsung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 15 lines

Jaegeuk Kim
Samsung



------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.  Get 
unparalleled scalability from the best Selenium testing platform available.
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Chao Yu

author

2014-04-29 08:12 UTC #4 lore raw

Hi Kim,

Quoted 104 lines

-----Original Message-----
From: Jaegeuk Kim [mailto:[email protected]]
Sent: Tuesday, April 29, 2014 4:02 PM
To: Chao Yu
Cc: [email protected]; [email protected]; [email protected]
Subject: RE: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
Hi,
2014-04-29 (화), 15:53 +0800, Chao Yu:
Quoted 9 lines
Hi Kim,
Quoted 7 lines
-----Original Message-----
From: Jaegeuk Kim [mailto:[email protected]]
Sent: Tuesday, April 29, 2014 2:16 PM
To: Chao Yu
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when
setattr
Quoted 83 lines
Quoted 76 lines
Hi,
2014-04-29 (화), 14:03 +0800, Chao Yu:
Quoted 58 lines
Previous we do not truncate inline data in inode page when setattr, so following case could still read the inline data which has already truncated:
1.write inline data
2.ftruncate size to 0
3.ftruncate size to max inline data size
4.read from offset 0
This patch introduces truncate_inline_data() to fix this problem.
change log from v1:
o fix a bug and do not truncate first page data after truncate inline data.
change log from v2:
o wait to writeback for inode page if it was already submitted.
Signed-off-by Chao Yu <[email protected]>
---
 fs/f2fs/f2fs.h   |    1 +
 fs/f2fs/file.c   |    3 +++
 fs/f2fs/inline.c |   19 +++++++++++++++++++
 3 files changed, 23 insertions(+)
fs/f2fs/f2fs.h +1 -0
index 2b67679..676a2c6 100644
@@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_data(struct inode *, pgoff_t);
 int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
+void truncate_inline_data(struct inode *, u64);
 int recover_inline_data(struct inode *, struct page *);
 #endif
fs/f2fs/file.c +3 -0
index b9f4fbf..d97e5c4 100644
@@ -369,6 +369,9 @@ static void truncate_partial_data_page(struct inode *inode, u64 from)
 	unsigned offset = from & (PAGE_CACHE_SIZE - 1);
 	struct page *page;
 
+	if (f2fs_has_inline_data(inode))
+		return truncate_inline_data(inode, from);
+
 	if (!offset)
 		return;
 
fs/f2fs/inline.c +3 -0
index 3258c7c..6a13997 100644
@@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
 	return 0;
 }
 
+void truncate_inline_data(struct inode *inode, u64 from)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
fs/f2fs/inline.c int f2fs_write_inline_data(struct inode *inode, · line 191
Quoted 10 lines
fs/f2fs/inline.c +10 -0
+	struct page *ipage;
+
+	if (from >= MAX_INLINE_DATA)
+		return;
+
+	ipage = get_node_page(sbi, inode->i_ino);
+	if (IS_ERR(ipage))
+		return;
+
+	f2fs_wait_on_page_writeback(ipage, NODE);
The get_node_page() triggers,
grab_cache_page_write_begin()
-> wait_for_stable_page()
Quoted 1 line
     -> wait_on_page_writeback().
Oh, it's my mistake for this missing, thanks for the mention.:)
Quoted 2 lines
Any bugs in there?

Quoted 8 lines

Quoted 3 lines
If we wait in grab_cache_page_write_begin(), maybe we will face long time delay or potential deadlock caused by f2fs bio cache. So how about use grab_cache_page & f2fs_wait_on_page_writeback to avoid this?
Nice catch. :)
IMO, it'd better merge your v2, and let me investigate all the paths to avoid that.
Is it okay?

It's okay, and thanks for your review. :)
Regards.

Quoted 23 lines

Quoted 17 lines
Quoted 16 lines
fs/f2fs/inline.c int f2fs_write_inline_data(struct inode *inode, · line 200
Quoted 12 lines
fs/f2fs/inline.c +6 -0
+	zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
+				INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+	set_page_dirty(ipage);
+	f2fs_put_page(ipage, 1);
+}
+
 int recover_inline_data(struct inode *inode, struct page *npage)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
--
Jaegeuk Kim
Samsung
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 3 lines

--
Jaegeuk Kim
Samsung