* [PATCH 1/4] f2fs: don't keep meta pages used for block migration
@ 2018-07-27 10:15 ` Chao Yu
0 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-27 10:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
For migration of encrypted inode's block, we load data of encrypted block
into meta inode's page cache, after checkpoint, those all intermediate
pages should be clean, and no one will read them again, so let's just
release them for more memory.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/checkpoint.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 12bebb8fa13d..67834d0ca422 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1499,6 +1499,14 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
commit_checkpoint(sbi, ckpt, start_blk);
wait_on_all_pages_writeback(sbi);
+ /*
+ * invalidate intermediate page cache borrowed from meta inode
+ * which are used for migration of encrypted inode's blocks.
+ */
+ if (f2fs_sb_has_encrypt(sbi->sb))
+ invalidate_mapping_pages(META_MAPPING(sbi),
+ MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1);
+
f2fs_release_ino_entry(sbi, false);
f2fs_reset_fsync_node_info(sbi);
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/4] f2fs: fix to active page in lru list for read path
2018-07-27 10:15 ` Chao Yu
@ 2018-07-27 10:15 ` Chao Yu
-1 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-27 10:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
If config CONFIG_F2FS_FAULT_INJECTION is on, for both read or write path
we will call find_lock_page() to get the page, but for read path, it
missed to passing FGP_ACCESSED to allocator to active the page in LRU
list, result in being reclaimed in advance incorrectly, fix it.
Reported-by: Xianrong Zhou <zhouxianrong@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/f2fs.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index fa5d0ebf8998..5f9e6a406284 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1974,8 +1974,13 @@ static inline struct page *f2fs_grab_cache_page(struct address_space *mapping,
pgoff_t index, bool for_write)
{
#ifdef CONFIG_F2FS_FAULT_INJECTION
- struct page *page = find_lock_page(mapping, index);
+ struct page *page;
+ if (!for_write)
+ page = find_get_page_flags(mapping, index,
+ FGP_LOCK | FGP_ACCESSED);
+ else
+ page = find_lock_page(mapping, index);
if (page)
return page;
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 2/4] f2fs: fix to active page in lru list for read path
@ 2018-07-27 10:15 ` Chao Yu
0 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-27 10:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
If config CONFIG_F2FS_FAULT_INJECTION is on, for both read or write path
we will call find_lock_page() to get the page, but for read path, it
missed to passing FGP_ACCESSED to allocator to active the page in LRU
list, result in being reclaimed in advance incorrectly, fix it.
Reported-by: Xianrong Zhou <zhouxianrong@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/f2fs.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index fa5d0ebf8998..5f9e6a406284 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1974,8 +1974,13 @@ static inline struct page *f2fs_grab_cache_page(struct address_space *mapping,
pgoff_t index, bool for_write)
{
#ifdef CONFIG_F2FS_FAULT_INJECTION
- struct page *page = find_lock_page(mapping, index);
+ struct page *page;
+ if (!for_write)
+ page = find_get_page_flags(mapping, index,
+ FGP_LOCK | FGP_ACCESSED);
+ else
+ page = find_lock_page(mapping, index);
if (page)
return page;
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/4] f2fs: fix avoid race between truncate and background GC
2018-07-27 10:15 ` Chao Yu
@ 2018-07-27 10:15 ` Chao Yu
-1 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-27 10:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
Thread A Background GC
- f2fs_setattr isize to 0
- truncate_setsize
- gc_data_segment
- f2fs_get_read_data_page page #0
- set_page_dirty
- set_cold_data
- f2fs_truncate
- f2fs_setattr isize to 4k
- read 4k <--- hit data in cached page #0
Above race condition can cause read out invalid data in a truncated
page, fix it by i_gc_rwsem[WRITE] lock.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/data.c | 4 ++++
fs/f2fs/file.c | 33 +++++++++++++++++++--------------
2 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 071224ded5f4..a29f3162b887 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2214,10 +2214,14 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
loff_t i_size = i_size_read(inode);
if (to > i_size) {
+ down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
down_write(&F2FS_I(inode)->i_mmap_sem);
+
truncate_pagecache(inode, i_size);
f2fs_truncate_blocks(inode, i_size, true);
+
up_write(&F2FS_I(inode)->i_mmap_sem);
+ up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
}
}
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 7bd2412a8c37..ed5c9b0e0d0c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -796,22 +796,25 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
}
if (attr->ia_valid & ATTR_SIZE) {
- if (attr->ia_size <= i_size_read(inode)) {
- down_write(&F2FS_I(inode)->i_mmap_sem);
- truncate_setsize(inode, attr->ia_size);
+ bool to_smaller = (attr->ia_size <= i_size_read(inode));
+
+ down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
+ down_write(&F2FS_I(inode)->i_mmap_sem);
+
+ truncate_setsize(inode, attr->ia_size);
+
+ if (to_smaller)
err = f2fs_truncate(inode);
- up_write(&F2FS_I(inode)->i_mmap_sem);
- if (err)
- return err;
- } else {
- /*
- * do not trim all blocks after i_size if target size is
- * larger than i_size.
- */
- down_write(&F2FS_I(inode)->i_mmap_sem);
- truncate_setsize(inode, attr->ia_size);
- up_write(&F2FS_I(inode)->i_mmap_sem);
+ /*
+ * do not trim all blocks after i_size if target size is
+ * larger than i_size.
+ */
+ up_write(&F2FS_I(inode)->i_mmap_sem);
+ up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
+ if (err)
+ return err;
+ if (!to_smaller) {
/* should convert inline inode here */
if (!f2fs_may_inline_data(inode)) {
err = f2fs_convert_inline_inode(inode);
@@ -958,6 +961,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
blk_start = (loff_t)pg_start << PAGE_SHIFT;
blk_end = (loff_t)pg_end << PAGE_SHIFT;
+ down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
down_write(&F2FS_I(inode)->i_mmap_sem);
truncate_inode_pages_range(mapping, blk_start,
blk_end - 1);
@@ -966,6 +970,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
ret = f2fs_truncate_hole(inode, pg_start, pg_end);
f2fs_unlock_op(sbi);
up_write(&F2FS_I(inode)->i_mmap_sem);
+ up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
}
}
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 3/4] f2fs: fix avoid race between truncate and background GC
@ 2018-07-27 10:15 ` Chao Yu
0 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-27 10:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
Thread A Background GC
- f2fs_setattr isize to 0
- truncate_setsize
- gc_data_segment
- f2fs_get_read_data_page page #0
- set_page_dirty
- set_cold_data
- f2fs_truncate
- f2fs_setattr isize to 4k
- read 4k <--- hit data in cached page #0
Above race condition can cause read out invalid data in a truncated
page, fix it by i_gc_rwsem[WRITE] lock.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/data.c | 4 ++++
fs/f2fs/file.c | 33 +++++++++++++++++++--------------
2 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 071224ded5f4..a29f3162b887 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2214,10 +2214,14 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
loff_t i_size = i_size_read(inode);
if (to > i_size) {
+ down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
down_write(&F2FS_I(inode)->i_mmap_sem);
+
truncate_pagecache(inode, i_size);
f2fs_truncate_blocks(inode, i_size, true);
+
up_write(&F2FS_I(inode)->i_mmap_sem);
+ up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
}
}
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 7bd2412a8c37..ed5c9b0e0d0c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -796,22 +796,25 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
}
if (attr->ia_valid & ATTR_SIZE) {
- if (attr->ia_size <= i_size_read(inode)) {
- down_write(&F2FS_I(inode)->i_mmap_sem);
- truncate_setsize(inode, attr->ia_size);
+ bool to_smaller = (attr->ia_size <= i_size_read(inode));
+
+ down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
+ down_write(&F2FS_I(inode)->i_mmap_sem);
+
+ truncate_setsize(inode, attr->ia_size);
+
+ if (to_smaller)
err = f2fs_truncate(inode);
- up_write(&F2FS_I(inode)->i_mmap_sem);
- if (err)
- return err;
- } else {
- /*
- * do not trim all blocks after i_size if target size is
- * larger than i_size.
- */
- down_write(&F2FS_I(inode)->i_mmap_sem);
- truncate_setsize(inode, attr->ia_size);
- up_write(&F2FS_I(inode)->i_mmap_sem);
+ /*
+ * do not trim all blocks after i_size if target size is
+ * larger than i_size.
+ */
+ up_write(&F2FS_I(inode)->i_mmap_sem);
+ up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
+ if (err)
+ return err;
+ if (!to_smaller) {
/* should convert inline inode here */
if (!f2fs_may_inline_data(inode)) {
err = f2fs_convert_inline_inode(inode);
@@ -958,6 +961,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
blk_start = (loff_t)pg_start << PAGE_SHIFT;
blk_end = (loff_t)pg_end << PAGE_SHIFT;
+ down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
down_write(&F2FS_I(inode)->i_mmap_sem);
truncate_inode_pages_range(mapping, blk_start,
blk_end - 1);
@@ -966,6 +970,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
ret = f2fs_truncate_hole(inode, pg_start, pg_end);
f2fs_unlock_op(sbi);
up_write(&F2FS_I(inode)->i_mmap_sem);
+ up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
}
}
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 3/4] f2fs: fix avoid race between truncate and background GC
2018-07-27 10:15 ` Chao Yu
@ 2018-07-29 1:58 ` Jaegeuk Kim
-1 siblings, 0 replies; 21+ messages in thread
From: Jaegeuk Kim @ 2018-07-29 1:58 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 07/27, Chao Yu wrote:
> Thread A Background GC
> - f2fs_setattr isize to 0
> - truncate_setsize
> - gc_data_segment
> - f2fs_get_read_data_page page #0
> - set_page_dirty
> - set_cold_data
> - f2fs_truncate
>
> - f2fs_setattr isize to 4k
> - read 4k <--- hit data in cached page #0
>
> Above race condition can cause read out invalid data in a truncated
> page, fix it by i_gc_rwsem[WRITE] lock.
We can truncate pages again after f2fs_truncate()?
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/data.c | 4 ++++
> fs/f2fs/file.c | 33 +++++++++++++++++++--------------
> 2 files changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 071224ded5f4..a29f3162b887 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2214,10 +2214,14 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
> loff_t i_size = i_size_read(inode);
>
> if (to > i_size) {
> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> down_write(&F2FS_I(inode)->i_mmap_sem);
> +
> truncate_pagecache(inode, i_size);
> f2fs_truncate_blocks(inode, i_size, true);
> +
> up_write(&F2FS_I(inode)->i_mmap_sem);
> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> }
> }
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 7bd2412a8c37..ed5c9b0e0d0c 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -796,22 +796,25 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
> }
>
> if (attr->ia_valid & ATTR_SIZE) {
> - if (attr->ia_size <= i_size_read(inode)) {
> - down_write(&F2FS_I(inode)->i_mmap_sem);
> - truncate_setsize(inode, attr->ia_size);
> + bool to_smaller = (attr->ia_size <= i_size_read(inode));
> +
> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> + down_write(&F2FS_I(inode)->i_mmap_sem);
> +
> + truncate_setsize(inode, attr->ia_size);
> +
> + if (to_smaller)
> err = f2fs_truncate(inode);
> - up_write(&F2FS_I(inode)->i_mmap_sem);
> - if (err)
> - return err;
> - } else {
> - /*
> - * do not trim all blocks after i_size if target size is
> - * larger than i_size.
> - */
> - down_write(&F2FS_I(inode)->i_mmap_sem);
> - truncate_setsize(inode, attr->ia_size);
> - up_write(&F2FS_I(inode)->i_mmap_sem);
> + /*
> + * do not trim all blocks after i_size if target size is
> + * larger than i_size.
> + */
> + up_write(&F2FS_I(inode)->i_mmap_sem);
> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> + if (err)
> + return err;
>
> + if (!to_smaller) {
> /* should convert inline inode here */
> if (!f2fs_may_inline_data(inode)) {
> err = f2fs_convert_inline_inode(inode);
> @@ -958,6 +961,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
>
> blk_start = (loff_t)pg_start << PAGE_SHIFT;
> blk_end = (loff_t)pg_end << PAGE_SHIFT;
> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> down_write(&F2FS_I(inode)->i_mmap_sem);
> truncate_inode_pages_range(mapping, blk_start,
> blk_end - 1);
> @@ -966,6 +970,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
> ret = f2fs_truncate_hole(inode, pg_start, pg_end);
> f2fs_unlock_op(sbi);
> up_write(&F2FS_I(inode)->i_mmap_sem);
> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> }
> }
>
> --
> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/4] f2fs: fix avoid race between truncate and background GC
@ 2018-07-29 1:58 ` Jaegeuk Kim
0 siblings, 0 replies; 21+ messages in thread
From: Jaegeuk Kim @ 2018-07-29 1:58 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 07/27, Chao Yu wrote:
> Thread A Background GC
> - f2fs_setattr isize to 0
> - truncate_setsize
> - gc_data_segment
> - f2fs_get_read_data_page page #0
> - set_page_dirty
> - set_cold_data
> - f2fs_truncate
>
> - f2fs_setattr isize to 4k
> - read 4k <--- hit data in cached page #0
>
> Above race condition can cause read out invalid data in a truncated
> page, fix it by i_gc_rwsem[WRITE] lock.
We can truncate pages again after f2fs_truncate()?
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/data.c | 4 ++++
> fs/f2fs/file.c | 33 +++++++++++++++++++--------------
> 2 files changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 071224ded5f4..a29f3162b887 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2214,10 +2214,14 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
> loff_t i_size = i_size_read(inode);
>
> if (to > i_size) {
> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> down_write(&F2FS_I(inode)->i_mmap_sem);
> +
> truncate_pagecache(inode, i_size);
> f2fs_truncate_blocks(inode, i_size, true);
> +
> up_write(&F2FS_I(inode)->i_mmap_sem);
> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> }
> }
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 7bd2412a8c37..ed5c9b0e0d0c 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -796,22 +796,25 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
> }
>
> if (attr->ia_valid & ATTR_SIZE) {
> - if (attr->ia_size <= i_size_read(inode)) {
> - down_write(&F2FS_I(inode)->i_mmap_sem);
> - truncate_setsize(inode, attr->ia_size);
> + bool to_smaller = (attr->ia_size <= i_size_read(inode));
> +
> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> + down_write(&F2FS_I(inode)->i_mmap_sem);
> +
> + truncate_setsize(inode, attr->ia_size);
> +
> + if (to_smaller)
> err = f2fs_truncate(inode);
> - up_write(&F2FS_I(inode)->i_mmap_sem);
> - if (err)
> - return err;
> - } else {
> - /*
> - * do not trim all blocks after i_size if target size is
> - * larger than i_size.
> - */
> - down_write(&F2FS_I(inode)->i_mmap_sem);
> - truncate_setsize(inode, attr->ia_size);
> - up_write(&F2FS_I(inode)->i_mmap_sem);
> + /*
> + * do not trim all blocks after i_size if target size is
> + * larger than i_size.
> + */
> + up_write(&F2FS_I(inode)->i_mmap_sem);
> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> + if (err)
> + return err;
>
> + if (!to_smaller) {
> /* should convert inline inode here */
> if (!f2fs_may_inline_data(inode)) {
> err = f2fs_convert_inline_inode(inode);
> @@ -958,6 +961,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
>
> blk_start = (loff_t)pg_start << PAGE_SHIFT;
> blk_end = (loff_t)pg_end << PAGE_SHIFT;
> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> down_write(&F2FS_I(inode)->i_mmap_sem);
> truncate_inode_pages_range(mapping, blk_start,
> blk_end - 1);
> @@ -966,6 +970,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
> ret = f2fs_truncate_hole(inode, pg_start, pg_end);
> f2fs_unlock_op(sbi);
> up_write(&F2FS_I(inode)->i_mmap_sem);
> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> }
> }
>
> --
> 2.18.0.rc1
------------------------------------------------------------------------------
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] 21+ messages in thread* Re: [PATCH 3/4] f2fs: fix avoid race between truncate and background GC
2018-07-29 1:58 ` Jaegeuk Kim
(?)
@ 2018-07-29 6:02 ` Chao Yu
2018-07-29 6:13 ` Jaegeuk Kim
-1 siblings, 1 reply; 21+ messages in thread
From: Chao Yu @ 2018-07-29 6:02 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel, linux-kernel
On 2018/7/29 9:58, Jaegeuk Kim wrote:
> On 07/27, Chao Yu wrote:
>> Thread A Background GC
>> - f2fs_setattr isize to 0
>> - truncate_setsize
>> - gc_data_segment
>> - f2fs_get_read_data_page page #0
>> - set_page_dirty
>> - set_cold_data
>> - f2fs_truncate
>>
>> - f2fs_setattr isize to 4k
>> - read 4k <--- hit data in cached page #0
>>
>> Above race condition can cause read out invalid data in a truncated
>> page, fix it by i_gc_rwsem[WRITE] lock.
>
> We can truncate pages again after f2fs_truncate()?
I think it can fix this issue, although it looks a little strange to truncate
page cache twice.
Thanks,
>
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>> fs/f2fs/data.c | 4 ++++
>> fs/f2fs/file.c | 33 +++++++++++++++++++--------------
>> 2 files changed, 23 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 071224ded5f4..a29f3162b887 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -2214,10 +2214,14 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
>> loff_t i_size = i_size_read(inode);
>>
>> if (to > i_size) {
>> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> down_write(&F2FS_I(inode)->i_mmap_sem);
>> +
>> truncate_pagecache(inode, i_size);
>> f2fs_truncate_blocks(inode, i_size, true);
>> +
>> up_write(&F2FS_I(inode)->i_mmap_sem);
>> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> }
>> }
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 7bd2412a8c37..ed5c9b0e0d0c 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -796,22 +796,25 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
>> }
>>
>> if (attr->ia_valid & ATTR_SIZE) {
>> - if (attr->ia_size <= i_size_read(inode)) {
>> - down_write(&F2FS_I(inode)->i_mmap_sem);
>> - truncate_setsize(inode, attr->ia_size);
>> + bool to_smaller = (attr->ia_size <= i_size_read(inode));
>> +
>> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> + down_write(&F2FS_I(inode)->i_mmap_sem);
>> +
>> + truncate_setsize(inode, attr->ia_size);
>> +
>> + if (to_smaller)
>> err = f2fs_truncate(inode);
>> - up_write(&F2FS_I(inode)->i_mmap_sem);
>> - if (err)
>> - return err;
>> - } else {
>> - /*
>> - * do not trim all blocks after i_size if target size is
>> - * larger than i_size.
>> - */
>> - down_write(&F2FS_I(inode)->i_mmap_sem);
>> - truncate_setsize(inode, attr->ia_size);
>> - up_write(&F2FS_I(inode)->i_mmap_sem);
>> + /*
>> + * do not trim all blocks after i_size if target size is
>> + * larger than i_size.
>> + */
>> + up_write(&F2FS_I(inode)->i_mmap_sem);
>> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> + if (err)
>> + return err;
>>
>> + if (!to_smaller) {
>> /* should convert inline inode here */
>> if (!f2fs_may_inline_data(inode)) {
>> err = f2fs_convert_inline_inode(inode);
>> @@ -958,6 +961,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
>>
>> blk_start = (loff_t)pg_start << PAGE_SHIFT;
>> blk_end = (loff_t)pg_end << PAGE_SHIFT;
>> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> down_write(&F2FS_I(inode)->i_mmap_sem);
>> truncate_inode_pages_range(mapping, blk_start,
>> blk_end - 1);
>> @@ -966,6 +970,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
>> ret = f2fs_truncate_hole(inode, pg_start, pg_end);
>> f2fs_unlock_op(sbi);
>> up_write(&F2FS_I(inode)->i_mmap_sem);
>> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> }
>> }
>>
>> --
>> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/4] f2fs: fix avoid race between truncate and background GC
2018-07-29 6:02 ` Chao Yu
@ 2018-07-29 6:13 ` Jaegeuk Kim
2018-07-29 6:16 ` Chao Yu
0 siblings, 1 reply; 21+ messages in thread
From: Jaegeuk Kim @ 2018-07-29 6:13 UTC (permalink / raw)
To: Chao Yu; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel
On 07/29, Chao Yu wrote:
> On 2018/7/29 9:58, Jaegeuk Kim wrote:
> > On 07/27, Chao Yu wrote:
> >> Thread A Background GC
> >> - f2fs_setattr isize to 0
> >> - truncate_setsize
> >> - gc_data_segment
> >> - f2fs_get_read_data_page page #0
> >> - set_page_dirty
> >> - set_cold_data
> >> - f2fs_truncate
> >>
> >> - f2fs_setattr isize to 4k
> >> - read 4k <--- hit data in cached page #0
> >>
> >> Above race condition can cause read out invalid data in a truncated
> >> page, fix it by i_gc_rwsem[WRITE] lock.
> >
> > We can truncate pages again after f2fs_truncate()?
>
> I think it can fix this issue, although it looks a little strange to truncate
> page cache twice.
It'd be fine to do setsize first, and drop the cache later.
>
> Thanks,
>
> >
> >>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >> fs/f2fs/data.c | 4 ++++
> >> fs/f2fs/file.c | 33 +++++++++++++++++++--------------
> >> 2 files changed, 23 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >> index 071224ded5f4..a29f3162b887 100644
> >> --- a/fs/f2fs/data.c
> >> +++ b/fs/f2fs/data.c
> >> @@ -2214,10 +2214,14 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
> >> loff_t i_size = i_size_read(inode);
> >>
> >> if (to > i_size) {
> >> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >> down_write(&F2FS_I(inode)->i_mmap_sem);
> >> +
> >> truncate_pagecache(inode, i_size);
> >> f2fs_truncate_blocks(inode, i_size, true);
> >> +
> >> up_write(&F2FS_I(inode)->i_mmap_sem);
> >> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >> }
> >> }
> >>
> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >> index 7bd2412a8c37..ed5c9b0e0d0c 100644
> >> --- a/fs/f2fs/file.c
> >> +++ b/fs/f2fs/file.c
> >> @@ -796,22 +796,25 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
> >> }
> >>
> >> if (attr->ia_valid & ATTR_SIZE) {
> >> - if (attr->ia_size <= i_size_read(inode)) {
> >> - down_write(&F2FS_I(inode)->i_mmap_sem);
> >> - truncate_setsize(inode, attr->ia_size);
> >> + bool to_smaller = (attr->ia_size <= i_size_read(inode));
> >> +
> >> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >> + down_write(&F2FS_I(inode)->i_mmap_sem);
> >> +
> >> + truncate_setsize(inode, attr->ia_size);
> >> +
> >> + if (to_smaller)
> >> err = f2fs_truncate(inode);
> >> - up_write(&F2FS_I(inode)->i_mmap_sem);
> >> - if (err)
> >> - return err;
> >> - } else {
> >> - /*
> >> - * do not trim all blocks after i_size if target size is
> >> - * larger than i_size.
> >> - */
> >> - down_write(&F2FS_I(inode)->i_mmap_sem);
> >> - truncate_setsize(inode, attr->ia_size);
> >> - up_write(&F2FS_I(inode)->i_mmap_sem);
> >> + /*
> >> + * do not trim all blocks after i_size if target size is
> >> + * larger than i_size.
> >> + */
> >> + up_write(&F2FS_I(inode)->i_mmap_sem);
> >> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >> + if (err)
> >> + return err;
> >>
> >> + if (!to_smaller) {
> >> /* should convert inline inode here */
> >> if (!f2fs_may_inline_data(inode)) {
> >> err = f2fs_convert_inline_inode(inode);
> >> @@ -958,6 +961,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
> >>
> >> blk_start = (loff_t)pg_start << PAGE_SHIFT;
> >> blk_end = (loff_t)pg_end << PAGE_SHIFT;
> >> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >> down_write(&F2FS_I(inode)->i_mmap_sem);
> >> truncate_inode_pages_range(mapping, blk_start,
> >> blk_end - 1);
> >> @@ -966,6 +970,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
> >> ret = f2fs_truncate_hole(inode, pg_start, pg_end);
> >> f2fs_unlock_op(sbi);
> >> up_write(&F2FS_I(inode)->i_mmap_sem);
> >> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >> }
> >> }
> >>
> >> --
> >> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/4] f2fs: fix avoid race between truncate and background GC
2018-07-29 6:13 ` Jaegeuk Kim
@ 2018-07-29 6:16 ` Chao Yu
0 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-29 6:16 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel
On 2018/7/29 14:13, Jaegeuk Kim wrote:
> On 07/29, Chao Yu wrote:
>> On 2018/7/29 9:58, Jaegeuk Kim wrote:
>>> On 07/27, Chao Yu wrote:
>>>> Thread A Background GC
>>>> - f2fs_setattr isize to 0
>>>> - truncate_setsize
>>>> - gc_data_segment
>>>> - f2fs_get_read_data_page page #0
>>>> - set_page_dirty
>>>> - set_cold_data
>>>> - f2fs_truncate
>>>>
>>>> - f2fs_setattr isize to 4k
>>>> - read 4k <--- hit data in cached page #0
>>>>
>>>> Above race condition can cause read out invalid data in a truncated
>>>> page, fix it by i_gc_rwsem[WRITE] lock.
>>>
>>> We can truncate pages again after f2fs_truncate()?
>>
>> I think it can fix this issue, although it looks a little strange to truncate
>> page cache twice.
>
> It'd be fine to do setsize first, and drop the cache later.
Let me rethink about this solution, since we have different case like punch_hole
which may not need setsize.
Thanks,
>
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>> fs/f2fs/data.c | 4 ++++
>>>> fs/f2fs/file.c | 33 +++++++++++++++++++--------------
>>>> 2 files changed, 23 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index 071224ded5f4..a29f3162b887 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -2214,10 +2214,14 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
>>>> loff_t i_size = i_size_read(inode);
>>>>
>>>> if (to > i_size) {
>>>> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>> down_write(&F2FS_I(inode)->i_mmap_sem);
>>>> +
>>>> truncate_pagecache(inode, i_size);
>>>> f2fs_truncate_blocks(inode, i_size, true);
>>>> +
>>>> up_write(&F2FS_I(inode)->i_mmap_sem);
>>>> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>> }
>>>> }
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 7bd2412a8c37..ed5c9b0e0d0c 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -796,22 +796,25 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
>>>> }
>>>>
>>>> if (attr->ia_valid & ATTR_SIZE) {
>>>> - if (attr->ia_size <= i_size_read(inode)) {
>>>> - down_write(&F2FS_I(inode)->i_mmap_sem);
>>>> - truncate_setsize(inode, attr->ia_size);
>>>> + bool to_smaller = (attr->ia_size <= i_size_read(inode));
>>>> +
>>>> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>> + down_write(&F2FS_I(inode)->i_mmap_sem);
>>>> +
>>>> + truncate_setsize(inode, attr->ia_size);
>>>> +
>>>> + if (to_smaller)
>>>> err = f2fs_truncate(inode);
>>>> - up_write(&F2FS_I(inode)->i_mmap_sem);
>>>> - if (err)
>>>> - return err;
>>>> - } else {
>>>> - /*
>>>> - * do not trim all blocks after i_size if target size is
>>>> - * larger than i_size.
>>>> - */
>>>> - down_write(&F2FS_I(inode)->i_mmap_sem);
>>>> - truncate_setsize(inode, attr->ia_size);
>>>> - up_write(&F2FS_I(inode)->i_mmap_sem);
>>>> + /*
>>>> + * do not trim all blocks after i_size if target size is
>>>> + * larger than i_size.
>>>> + */
>>>> + up_write(&F2FS_I(inode)->i_mmap_sem);
>>>> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>> + if (err)
>>>> + return err;
>>>>
>>>> + if (!to_smaller) {
>>>> /* should convert inline inode here */
>>>> if (!f2fs_may_inline_data(inode)) {
>>>> err = f2fs_convert_inline_inode(inode);
>>>> @@ -958,6 +961,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
>>>>
>>>> blk_start = (loff_t)pg_start << PAGE_SHIFT;
>>>> blk_end = (loff_t)pg_end << PAGE_SHIFT;
>>>> + down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>> down_write(&F2FS_I(inode)->i_mmap_sem);
>>>> truncate_inode_pages_range(mapping, blk_start,
>>>> blk_end - 1);
>>>> @@ -966,6 +970,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
>>>> ret = f2fs_truncate_hole(inode, pg_start, pg_end);
>>>> f2fs_unlock_op(sbi);
>>>> up_write(&F2FS_I(inode)->i_mmap_sem);
>>>> + up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>> }
>>>> }
>>>>
>>>> --
>>>> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/4] f2fs: fix to spread clear_cold_data()
2018-07-27 10:15 ` Chao Yu
@ 2018-07-27 10:15 ` Chao Yu
-1 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-27 10:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu, Weichao Guo
We need to drop PG_checked flag on page as well when we clear PG_uptodate
flag, in order to avoid treating the page as GCing one later.
Signed-off-by: Weichao Guo <guoweichao@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/data.c | 8 +++++++-
fs/f2fs/dir.c | 1 +
fs/f2fs/segment.c | 4 +++-
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a29f3162b887..2817e2f4eb17 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
/* This page is already truncated */
if (fio->old_blkaddr == NULL_ADDR) {
ClearPageUptodate(page);
+ clear_cold_data(page);
goto out_writepage;
}
got_it:
@@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
out:
inode_dec_dirty_pages(inode);
- if (err)
+ if (err) {
ClearPageUptodate(page);
+ clear_cold_data(page);
+ }
if (wbc->for_reclaim) {
f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
@@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
}
}
+ clear_cold_data(page);
+
/* This is atomic written page, keep Private */
if (IS_ATOMIC_WRITTEN_PAGE(page))
return f2fs_drop_inmem_page(inode, page);
@@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
if (IS_ATOMIC_WRITTEN_PAGE(page))
return 0;
+ clear_cold_data(page);
set_page_private(page, 0);
ClearPagePrivate(page);
return 1;
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 7f955c4e86a4..1e4a4122eb0c 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
clear_page_dirty_for_io(page);
ClearPagePrivate(page);
ClearPageUptodate(page);
+ clear_cold_data(page);
inode_dec_dirty_pages(dir);
f2fs_remove_dirty_inode(dir);
}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 58abbdc53561..4d83961745e6 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
}
next:
/* we don't need to invalidate this in the sccessful status */
- if (drop || recover)
+ if (drop || recover) {
ClearPageUptodate(page);
+ clear_cold_data(page);
+ }
set_page_private(page, 0);
ClearPagePrivate(page);
f2fs_put_page(page, 1);
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 4/4] f2fs: fix to spread clear_cold_data()
@ 2018-07-27 10:15 ` Chao Yu
0 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-27 10:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu, Weichao Guo
We need to drop PG_checked flag on page as well when we clear PG_uptodate
flag, in order to avoid treating the page as GCing one later.
Signed-off-by: Weichao Guo <guoweichao@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/data.c | 8 +++++++-
fs/f2fs/dir.c | 1 +
fs/f2fs/segment.c | 4 +++-
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a29f3162b887..2817e2f4eb17 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
/* This page is already truncated */
if (fio->old_blkaddr == NULL_ADDR) {
ClearPageUptodate(page);
+ clear_cold_data(page);
goto out_writepage;
}
got_it:
@@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
out:
inode_dec_dirty_pages(inode);
- if (err)
+ if (err) {
ClearPageUptodate(page);
+ clear_cold_data(page);
+ }
if (wbc->for_reclaim) {
f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
@@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
}
}
+ clear_cold_data(page);
+
/* This is atomic written page, keep Private */
if (IS_ATOMIC_WRITTEN_PAGE(page))
return f2fs_drop_inmem_page(inode, page);
@@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
if (IS_ATOMIC_WRITTEN_PAGE(page))
return 0;
+ clear_cold_data(page);
set_page_private(page, 0);
ClearPagePrivate(page);
return 1;
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 7f955c4e86a4..1e4a4122eb0c 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
clear_page_dirty_for_io(page);
ClearPagePrivate(page);
ClearPageUptodate(page);
+ clear_cold_data(page);
inode_dec_dirty_pages(dir);
f2fs_remove_dirty_inode(dir);
}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 58abbdc53561..4d83961745e6 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
}
next:
/* we don't need to invalidate this in the sccessful status */
- if (drop || recover)
+ if (drop || recover) {
ClearPageUptodate(page);
+ clear_cold_data(page);
+ }
set_page_private(page, 0);
ClearPagePrivate(page);
f2fs_put_page(page, 1);
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
2018-07-27 10:15 ` Chao Yu
@ 2018-07-29 2:00 ` Jaegeuk Kim
-1 siblings, 0 replies; 21+ messages in thread
From: Jaegeuk Kim @ 2018-07-29 2:00 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao, Weichao Guo
On 07/27, Chao Yu wrote:
> We need to drop PG_checked flag on page as well when we clear PG_uptodate
> flag, in order to avoid treating the page as GCing one later.
What do you mean "treating the page as GCing one"?
>
> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/data.c | 8 +++++++-
> fs/f2fs/dir.c | 1 +
> fs/f2fs/segment.c | 4 +++-
> 3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index a29f3162b887..2817e2f4eb17 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
> /* This page is already truncated */
> if (fio->old_blkaddr == NULL_ADDR) {
> ClearPageUptodate(page);
> + clear_cold_data(page);
> goto out_writepage;
> }
> got_it:
> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
>
> out:
> inode_dec_dirty_pages(inode);
> - if (err)
> + if (err) {
> ClearPageUptodate(page);
> + clear_cold_data(page);
> + }
>
> if (wbc->for_reclaim) {
> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
> }
> }
>
> + clear_cold_data(page);
> +
> /* This is atomic written page, keep Private */
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> return f2fs_drop_inmem_page(inode, page);
> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> return 0;
>
> + clear_cold_data(page);
> set_page_private(page, 0);
> ClearPagePrivate(page);
> return 1;
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 7f955c4e86a4..1e4a4122eb0c 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
> clear_page_dirty_for_io(page);
> ClearPagePrivate(page);
> ClearPageUptodate(page);
> + clear_cold_data(page);
> inode_dec_dirty_pages(dir);
> f2fs_remove_dirty_inode(dir);
> }
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 58abbdc53561..4d83961745e6 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
> }
> next:
> /* we don't need to invalidate this in the sccessful status */
> - if (drop || recover)
> + if (drop || recover) {
> ClearPageUptodate(page);
> + clear_cold_data(page);
> + }
> set_page_private(page, 0);
> ClearPagePrivate(page);
> f2fs_put_page(page, 1);
> --
> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
@ 2018-07-29 2:00 ` Jaegeuk Kim
0 siblings, 0 replies; 21+ messages in thread
From: Jaegeuk Kim @ 2018-07-29 2:00 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 07/27, Chao Yu wrote:
> We need to drop PG_checked flag on page as well when we clear PG_uptodate
> flag, in order to avoid treating the page as GCing one later.
What do you mean "treating the page as GCing one"?
>
> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/data.c | 8 +++++++-
> fs/f2fs/dir.c | 1 +
> fs/f2fs/segment.c | 4 +++-
> 3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index a29f3162b887..2817e2f4eb17 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
> /* This page is already truncated */
> if (fio->old_blkaddr == NULL_ADDR) {
> ClearPageUptodate(page);
> + clear_cold_data(page);
> goto out_writepage;
> }
> got_it:
> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
>
> out:
> inode_dec_dirty_pages(inode);
> - if (err)
> + if (err) {
> ClearPageUptodate(page);
> + clear_cold_data(page);
> + }
>
> if (wbc->for_reclaim) {
> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
> }
> }
>
> + clear_cold_data(page);
> +
> /* This is atomic written page, keep Private */
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> return f2fs_drop_inmem_page(inode, page);
> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> return 0;
>
> + clear_cold_data(page);
> set_page_private(page, 0);
> ClearPagePrivate(page);
> return 1;
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 7f955c4e86a4..1e4a4122eb0c 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
> clear_page_dirty_for_io(page);
> ClearPagePrivate(page);
> ClearPageUptodate(page);
> + clear_cold_data(page);
> inode_dec_dirty_pages(dir);
> f2fs_remove_dirty_inode(dir);
> }
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 58abbdc53561..4d83961745e6 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
> }
> next:
> /* we don't need to invalidate this in the sccessful status */
> - if (drop || recover)
> + if (drop || recover) {
> ClearPageUptodate(page);
> + clear_cold_data(page);
> + }
> set_page_private(page, 0);
> ClearPagePrivate(page);
> f2fs_put_page(page, 1);
> --
> 2.18.0.rc1
------------------------------------------------------------------------------
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] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
2018-07-29 2:00 ` Jaegeuk Kim
@ 2018-07-29 2:19 ` Chao Yu
-1 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-29 2:19 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Weichao Guo
On 2018/7/29 10:00, Jaegeuk Kim wrote:
> On 07/27, Chao Yu wrote:
>> We need to drop PG_checked flag on page as well when we clear PG_uptodate
>> flag, in order to avoid treating the page as GCing one later.
>
> What do you mean "treating the page as GCing one"?
I mean if PG_checked flag in page is not cleared, allocator will make it goes
into cold area.
static int __get_segment_type_6(struct f2fs_io_info *fio)
...
if (is_cold_data(fio->page) || file_is_cold(inode))
return CURSEG_COLD_DATA;
Thanks,
>
>>
>> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>> fs/f2fs/data.c | 8 +++++++-
>> fs/f2fs/dir.c | 1 +
>> fs/f2fs/segment.c | 4 +++-
>> 3 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index a29f3162b887..2817e2f4eb17 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>> /* This page is already truncated */
>> if (fio->old_blkaddr == NULL_ADDR) {
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> goto out_writepage;
>> }
>> got_it:
>> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
>>
>> out:
>> inode_dec_dirty_pages(inode);
>> - if (err)
>> + if (err) {
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> + }
>>
>> if (wbc->for_reclaim) {
>> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
>> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>> }
>> }
>>
>> + clear_cold_data(page);
>> +
>> /* This is atomic written page, keep Private */
>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>> return f2fs_drop_inmem_page(inode, page);
>> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>> return 0;
>>
>> + clear_cold_data(page);
>> set_page_private(page, 0);
>> ClearPagePrivate(page);
>> return 1;
>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>> index 7f955c4e86a4..1e4a4122eb0c 100644
>> --- a/fs/f2fs/dir.c
>> +++ b/fs/f2fs/dir.c
>> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
>> clear_page_dirty_for_io(page);
>> ClearPagePrivate(page);
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> inode_dec_dirty_pages(dir);
>> f2fs_remove_dirty_inode(dir);
>> }
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 58abbdc53561..4d83961745e6 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
>> }
>> next:
>> /* we don't need to invalidate this in the sccessful status */
>> - if (drop || recover)
>> + if (drop || recover) {
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> + }
>> set_page_private(page, 0);
>> ClearPagePrivate(page);
>> f2fs_put_page(page, 1);
>> --
>> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
@ 2018-07-29 2:19 ` Chao Yu
0 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-29 2:19 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 2018/7/29 10:00, Jaegeuk Kim wrote:
> On 07/27, Chao Yu wrote:
>> We need to drop PG_checked flag on page as well when we clear PG_uptodate
>> flag, in order to avoid treating the page as GCing one later.
>
> What do you mean "treating the page as GCing one"?
I mean if PG_checked flag in page is not cleared, allocator will make it goes
into cold area.
static int __get_segment_type_6(struct f2fs_io_info *fio)
...
if (is_cold_data(fio->page) || file_is_cold(inode))
return CURSEG_COLD_DATA;
Thanks,
>
>>
>> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>> fs/f2fs/data.c | 8 +++++++-
>> fs/f2fs/dir.c | 1 +
>> fs/f2fs/segment.c | 4 +++-
>> 3 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index a29f3162b887..2817e2f4eb17 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>> /* This page is already truncated */
>> if (fio->old_blkaddr == NULL_ADDR) {
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> goto out_writepage;
>> }
>> got_it:
>> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
>>
>> out:
>> inode_dec_dirty_pages(inode);
>> - if (err)
>> + if (err) {
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> + }
>>
>> if (wbc->for_reclaim) {
>> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
>> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>> }
>> }
>>
>> + clear_cold_data(page);
>> +
>> /* This is atomic written page, keep Private */
>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>> return f2fs_drop_inmem_page(inode, page);
>> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>> return 0;
>>
>> + clear_cold_data(page);
>> set_page_private(page, 0);
>> ClearPagePrivate(page);
>> return 1;
>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>> index 7f955c4e86a4..1e4a4122eb0c 100644
>> --- a/fs/f2fs/dir.c
>> +++ b/fs/f2fs/dir.c
>> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
>> clear_page_dirty_for_io(page);
>> ClearPagePrivate(page);
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> inode_dec_dirty_pages(dir);
>> f2fs_remove_dirty_inode(dir);
>> }
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 58abbdc53561..4d83961745e6 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
>> }
>> next:
>> /* we don't need to invalidate this in the sccessful status */
>> - if (drop || recover)
>> + if (drop || recover) {
>> ClearPageUptodate(page);
>> + clear_cold_data(page);
>> + }
>> set_page_private(page, 0);
>> ClearPagePrivate(page);
>> f2fs_put_page(page, 1);
>> --
>> 2.18.0.rc1
------------------------------------------------------------------------------
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] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
2018-07-29 2:19 ` Chao Yu
@ 2018-07-29 2:44 ` Jaegeuk Kim
-1 siblings, 0 replies; 21+ messages in thread
From: Jaegeuk Kim @ 2018-07-29 2:44 UTC (permalink / raw)
To: Chao Yu; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel, Weichao Guo
On 07/29, Chao Yu wrote:
> On 2018/7/29 10:00, Jaegeuk Kim wrote:
> > On 07/27, Chao Yu wrote:
> >> We need to drop PG_checked flag on page as well when we clear PG_uptodate
> >> flag, in order to avoid treating the page as GCing one later.
> >
> > What do you mean "treating the page as GCing one"?
>
> I mean if PG_checked flag in page is not cleared, allocator will make it goes
> into cold area.
>
> static int __get_segment_type_6(struct f2fs_io_info *fio)
> ...
> if (is_cold_data(fio->page) || file_is_cold(inode))
> return CURSEG_COLD_DATA;
This will come only when page is dirty. So, it'd better to clear this in
f2fs_set_data_page_dirty()? set_cold_data() is called after set_page_dirty().
>
> Thanks,
>
> >
> >>
> >> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >> fs/f2fs/data.c | 8 +++++++-
> >> fs/f2fs/dir.c | 1 +
> >> fs/f2fs/segment.c | 4 +++-
> >> 3 files changed, 11 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >> index a29f3162b887..2817e2f4eb17 100644
> >> --- a/fs/f2fs/data.c
> >> +++ b/fs/f2fs/data.c
> >> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
> >> /* This page is already truncated */
> >> if (fio->old_blkaddr == NULL_ADDR) {
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> goto out_writepage;
> >> }
> >> got_it:
> >> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
> >>
> >> out:
> >> inode_dec_dirty_pages(inode);
> >> - if (err)
> >> + if (err) {
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> + }
> >>
> >> if (wbc->for_reclaim) {
> >> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
> >> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
> >> }
> >> }
> >>
> >> + clear_cold_data(page);
> >> +
> >> /* This is atomic written page, keep Private */
> >> if (IS_ATOMIC_WRITTEN_PAGE(page))
> >> return f2fs_drop_inmem_page(inode, page);
> >> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
> >> if (IS_ATOMIC_WRITTEN_PAGE(page))
> >> return 0;
> >>
> >> + clear_cold_data(page);
> >> set_page_private(page, 0);
> >> ClearPagePrivate(page);
> >> return 1;
> >> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> >> index 7f955c4e86a4..1e4a4122eb0c 100644
> >> --- a/fs/f2fs/dir.c
> >> +++ b/fs/f2fs/dir.c
> >> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
> >> clear_page_dirty_for_io(page);
> >> ClearPagePrivate(page);
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> inode_dec_dirty_pages(dir);
> >> f2fs_remove_dirty_inode(dir);
> >> }
> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >> index 58abbdc53561..4d83961745e6 100644
> >> --- a/fs/f2fs/segment.c
> >> +++ b/fs/f2fs/segment.c
> >> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
> >> }
> >> next:
> >> /* we don't need to invalidate this in the sccessful status */
> >> - if (drop || recover)
> >> + if (drop || recover) {
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> + }
> >> set_page_private(page, 0);
> >> ClearPagePrivate(page);
> >> f2fs_put_page(page, 1);
> >> --
> >> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
@ 2018-07-29 2:44 ` Jaegeuk Kim
0 siblings, 0 replies; 21+ messages in thread
From: Jaegeuk Kim @ 2018-07-29 2:44 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 07/29, Chao Yu wrote:
> On 2018/7/29 10:00, Jaegeuk Kim wrote:
> > On 07/27, Chao Yu wrote:
> >> We need to drop PG_checked flag on page as well when we clear PG_uptodate
> >> flag, in order to avoid treating the page as GCing one later.
> >
> > What do you mean "treating the page as GCing one"?
>
> I mean if PG_checked flag in page is not cleared, allocator will make it goes
> into cold area.
>
> static int __get_segment_type_6(struct f2fs_io_info *fio)
> ...
> if (is_cold_data(fio->page) || file_is_cold(inode))
> return CURSEG_COLD_DATA;
This will come only when page is dirty. So, it'd better to clear this in
f2fs_set_data_page_dirty()? set_cold_data() is called after set_page_dirty().
>
> Thanks,
>
> >
> >>
> >> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >> fs/f2fs/data.c | 8 +++++++-
> >> fs/f2fs/dir.c | 1 +
> >> fs/f2fs/segment.c | 4 +++-
> >> 3 files changed, 11 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >> index a29f3162b887..2817e2f4eb17 100644
> >> --- a/fs/f2fs/data.c
> >> +++ b/fs/f2fs/data.c
> >> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
> >> /* This page is already truncated */
> >> if (fio->old_blkaddr == NULL_ADDR) {
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> goto out_writepage;
> >> }
> >> got_it:
> >> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
> >>
> >> out:
> >> inode_dec_dirty_pages(inode);
> >> - if (err)
> >> + if (err) {
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> + }
> >>
> >> if (wbc->for_reclaim) {
> >> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
> >> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
> >> }
> >> }
> >>
> >> + clear_cold_data(page);
> >> +
> >> /* This is atomic written page, keep Private */
> >> if (IS_ATOMIC_WRITTEN_PAGE(page))
> >> return f2fs_drop_inmem_page(inode, page);
> >> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
> >> if (IS_ATOMIC_WRITTEN_PAGE(page))
> >> return 0;
> >>
> >> + clear_cold_data(page);
> >> set_page_private(page, 0);
> >> ClearPagePrivate(page);
> >> return 1;
> >> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> >> index 7f955c4e86a4..1e4a4122eb0c 100644
> >> --- a/fs/f2fs/dir.c
> >> +++ b/fs/f2fs/dir.c
> >> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
> >> clear_page_dirty_for_io(page);
> >> ClearPagePrivate(page);
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> inode_dec_dirty_pages(dir);
> >> f2fs_remove_dirty_inode(dir);
> >> }
> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >> index 58abbdc53561..4d83961745e6 100644
> >> --- a/fs/f2fs/segment.c
> >> +++ b/fs/f2fs/segment.c
> >> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
> >> }
> >> next:
> >> /* we don't need to invalidate this in the sccessful status */
> >> - if (drop || recover)
> >> + if (drop || recover) {
> >> ClearPageUptodate(page);
> >> + clear_cold_data(page);
> >> + }
> >> set_page_private(page, 0);
> >> ClearPagePrivate(page);
> >> f2fs_put_page(page, 1);
> >> --
> >> 2.18.0.rc1
------------------------------------------------------------------------------
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] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
2018-07-29 2:44 ` Jaegeuk Kim
@ 2018-07-29 3:19 ` Chao Yu
-1 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-29 3:19 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel, Weichao Guo
On 2018/7/29 10:44, Jaegeuk Kim wrote:
> On 07/29, Chao Yu wrote:
>> On 2018/7/29 10:00, Jaegeuk Kim wrote:
>>> On 07/27, Chao Yu wrote:
>>>> We need to drop PG_checked flag on page as well when we clear PG_uptodate
>>>> flag, in order to avoid treating the page as GCing one later.
>>>
>>> What do you mean "treating the page as GCing one"?
>>
>> I mean if PG_checked flag in page is not cleared, allocator will make it goes
>> into cold area.
>>
>> static int __get_segment_type_6(struct f2fs_io_info *fio)
>> ...
>> if (is_cold_data(fio->page) || file_is_cold(inode))
>> return CURSEG_COLD_DATA;
>
> This will come only when page is dirty. So, it'd better to clear this in
> f2fs_set_data_page_dirty()? set_cold_data() is called after set_page_dirty().
Looks fine, let me send v2.
Thanks,
>
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>> fs/f2fs/data.c | 8 +++++++-
>>>> fs/f2fs/dir.c | 1 +
>>>> fs/f2fs/segment.c | 4 +++-
>>>> 3 files changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index a29f3162b887..2817e2f4eb17 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>>> /* This page is already truncated */
>>>> if (fio->old_blkaddr == NULL_ADDR) {
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> goto out_writepage;
>>>> }
>>>> got_it:
>>>> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
>>>>
>>>> out:
>>>> inode_dec_dirty_pages(inode);
>>>> - if (err)
>>>> + if (err) {
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> + }
>>>>
>>>> if (wbc->for_reclaim) {
>>>> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
>>>> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>>>> }
>>>> }
>>>>
>>>> + clear_cold_data(page);
>>>> +
>>>> /* This is atomic written page, keep Private */
>>>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>>>> return f2fs_drop_inmem_page(inode, page);
>>>> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
>>>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>>>> return 0;
>>>>
>>>> + clear_cold_data(page);
>>>> set_page_private(page, 0);
>>>> ClearPagePrivate(page);
>>>> return 1;
>>>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>>>> index 7f955c4e86a4..1e4a4122eb0c 100644
>>>> --- a/fs/f2fs/dir.c
>>>> +++ b/fs/f2fs/dir.c
>>>> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
>>>> clear_page_dirty_for_io(page);
>>>> ClearPagePrivate(page);
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> inode_dec_dirty_pages(dir);
>>>> f2fs_remove_dirty_inode(dir);
>>>> }
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 58abbdc53561..4d83961745e6 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
>>>> }
>>>> next:
>>>> /* we don't need to invalidate this in the sccessful status */
>>>> - if (drop || recover)
>>>> + if (drop || recover) {
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> + }
>>>> set_page_private(page, 0);
>>>> ClearPagePrivate(page);
>>>> f2fs_put_page(page, 1);
>>>> --
>>>> 2.18.0.rc1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/4] f2fs: fix to spread clear_cold_data()
@ 2018-07-29 3:19 ` Chao Yu
0 siblings, 0 replies; 21+ messages in thread
From: Chao Yu @ 2018-07-29 3:19 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2018/7/29 10:44, Jaegeuk Kim wrote:
> On 07/29, Chao Yu wrote:
>> On 2018/7/29 10:00, Jaegeuk Kim wrote:
>>> On 07/27, Chao Yu wrote:
>>>> We need to drop PG_checked flag on page as well when we clear PG_uptodate
>>>> flag, in order to avoid treating the page as GCing one later.
>>>
>>> What do you mean "treating the page as GCing one"?
>>
>> I mean if PG_checked flag in page is not cleared, allocator will make it goes
>> into cold area.
>>
>> static int __get_segment_type_6(struct f2fs_io_info *fio)
>> ...
>> if (is_cold_data(fio->page) || file_is_cold(inode))
>> return CURSEG_COLD_DATA;
>
> This will come only when page is dirty. So, it'd better to clear this in
> f2fs_set_data_page_dirty()? set_cold_data() is called after set_page_dirty().
Looks fine, let me send v2.
Thanks,
>
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>> fs/f2fs/data.c | 8 +++++++-
>>>> fs/f2fs/dir.c | 1 +
>>>> fs/f2fs/segment.c | 4 +++-
>>>> 3 files changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index a29f3162b887..2817e2f4eb17 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1768,6 +1768,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>>> /* This page is already truncated */
>>>> if (fio->old_blkaddr == NULL_ADDR) {
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> goto out_writepage;
>>>> }
>>>> got_it:
>>>> @@ -1943,8 +1944,10 @@ static int __write_data_page(struct page *page, bool *submitted,
>>>>
>>>> out:
>>>> inode_dec_dirty_pages(inode);
>>>> - if (err)
>>>> + if (err) {
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> + }
>>>>
>>>> if (wbc->for_reclaim) {
>>>> f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
>>>> @@ -2534,6 +2537,8 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>>>> }
>>>> }
>>>>
>>>> + clear_cold_data(page);
>>>> +
>>>> /* This is atomic written page, keep Private */
>>>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>>>> return f2fs_drop_inmem_page(inode, page);
>>>> @@ -2552,6 +2557,7 @@ int f2fs_release_page(struct page *page, gfp_t wait)
>>>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>>>> return 0;
>>>>
>>>> + clear_cold_data(page);
>>>> set_page_private(page, 0);
>>>> ClearPagePrivate(page);
>>>> return 1;
>>>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>>>> index 7f955c4e86a4..1e4a4122eb0c 100644
>>>> --- a/fs/f2fs/dir.c
>>>> +++ b/fs/f2fs/dir.c
>>>> @@ -734,6 +734,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
>>>> clear_page_dirty_for_io(page);
>>>> ClearPagePrivate(page);
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> inode_dec_dirty_pages(dir);
>>>> f2fs_remove_dirty_inode(dir);
>>>> }
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 58abbdc53561..4d83961745e6 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -267,8 +267,10 @@ static int __revoke_inmem_pages(struct inode *inode,
>>>> }
>>>> next:
>>>> /* we don't need to invalidate this in the sccessful status */
>>>> - if (drop || recover)
>>>> + if (drop || recover) {
>>>> ClearPageUptodate(page);
>>>> + clear_cold_data(page);
>>>> + }
>>>> set_page_private(page, 0);
>>>> ClearPagePrivate(page);
>>>> f2fs_put_page(page, 1);
>>>> --
>>>> 2.18.0.rc1
------------------------------------------------------------------------------
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] 21+ messages in thread