All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block()
@ 2021-05-10  9:30 ` Chao Yu
  0 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-10  9:30 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

f2fs_get_block() and f2fs_put_dnode() should be called as a pair,
add missing f2fs_put_dnode() in prepare_compress_overwrite().

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/compress.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index c208563eac28..d5cb0ba9a0e1 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1088,6 +1088,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 
 		for (i = cc->cluster_size - 1; i > 0; i--) {
 			ret = f2fs_get_block(&dn, start_idx + i);
+			f2fs_put_dnode(&dn);
 			if (ret) {
 				i = cc->cluster_size;
 				break;
-- 
2.29.2


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

* [f2fs-dev] [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block()
@ 2021-05-10  9:30 ` Chao Yu
  0 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-10  9:30 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

f2fs_get_block() and f2fs_put_dnode() should be called as a pair,
add missing f2fs_put_dnode() in prepare_compress_overwrite().

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/compress.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index c208563eac28..d5cb0ba9a0e1 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1088,6 +1088,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 
 		for (i = cc->cluster_size - 1; i > 0; i--) {
 			ret = f2fs_get_block(&dn, start_idx + i);
+			f2fs_put_dnode(&dn);
 			if (ret) {
 				i = cc->cluster_size;
 				break;
-- 
2.29.2



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* [PATCH 2/3] f2fs: compress: fix race condition of overwrite vs truncate
  2021-05-10  9:30 ` [f2fs-dev] " Chao Yu
@ 2021-05-10  9:30   ` Chao Yu
  -1 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-10  9:30 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

pos_fsstress testcase complains a panic as belew:

------------[ cut here ]------------
kernel BUG at fs/f2fs/compress.c:1082!
invalid opcode: 0000 [#1] SMP PTI
CPU: 4 PID: 2753477 Comm: kworker/u16:2 Tainted: G           OE     5.12.0-rc1-custom #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
Workqueue: writeback wb_workfn (flush-252:16)
RIP: 0010:prepare_compress_overwrite+0x4c0/0x760 [f2fs]
Call Trace:
 f2fs_prepare_compress_overwrite+0x5f/0x80 [f2fs]
 f2fs_write_cache_pages+0x468/0x8a0 [f2fs]
 f2fs_write_data_pages+0x2a4/0x2f0 [f2fs]
 do_writepages+0x38/0xc0
 __writeback_single_inode+0x44/0x2a0
 writeback_sb_inodes+0x223/0x4d0
 __writeback_inodes_wb+0x56/0xf0
 wb_writeback+0x1dd/0x290
 wb_workfn+0x309/0x500
 process_one_work+0x220/0x3c0
 worker_thread+0x53/0x420
 kthread+0x12f/0x150
 ret_from_fork+0x22/0x30

The root cause is truncate() may race with overwrite as below,
so that one reference count left in page can not guarantee the
page attaching in mapping tree all the time, after truncation,
later find_lock_page() may return NULL pointer.

- prepare_compress_overwrite
 - f2fs_pagecache_get_page
 - unlock_page
					- f2fs_setattr
					 - truncate_setsize
					  - truncate_inode_page
					   - delete_from_page_cache
 - find_lock_page

Fix this by avoiding referencing updated page.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/compress.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d5cb0ba9a0e1..340815cd0887 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -118,19 +118,6 @@ static void f2fs_unlock_rpages(struct compress_ctx *cc, int len)
 	f2fs_drop_rpages(cc, len, true);
 }
 
-static void f2fs_put_rpages_mapping(struct address_space *mapping,
-				pgoff_t start, int len)
-{
-	int i;
-
-	for (i = 0; i < len; i++) {
-		struct page *page = find_get_page(mapping, start + i);
-
-		put_page(page);
-		put_page(page);
-	}
-}
-
 static void f2fs_put_rpages_wbc(struct compress_ctx *cc,
 		struct writeback_control *wbc, bool redirty, int unlock)
 {
@@ -1040,7 +1027,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 		}
 
 		if (PageUptodate(page))
-			unlock_page(page);
+			f2fs_put_page(page, 1);
 		else
 			f2fs_compress_ctx_add_page(cc, page);
 	}
@@ -1050,32 +1037,34 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 
 		ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size,
 					&last_block_in_bio, false, true);
+		f2fs_put_rpages(cc);
 		f2fs_destroy_compress_ctx(cc);
 		if (ret)
-			goto release_pages;
+			goto out;
 		if (bio)
 			f2fs_submit_bio(sbi, bio, DATA);
 
 		ret = f2fs_init_compress_ctx(cc);
 		if (ret)
-			goto release_pages;
+			goto out;
 	}
 
 	for (i = 0; i < cc->cluster_size; i++) {
 		f2fs_bug_on(sbi, cc->rpages[i]);
 
 		page = find_lock_page(mapping, start_idx + i);
-		f2fs_bug_on(sbi, !page);
+		if (!page) {
+			/* page can be truncated */
+			goto release_and_retry;
+		}
 
 		f2fs_wait_on_page_writeback(page, DATA, true, true);
-
 		f2fs_compress_ctx_add_page(cc, page);
-		f2fs_put_page(page, 0);
 
 		if (!PageUptodate(page)) {
+release_and_retry:
+			f2fs_put_rpages(cc);
 			f2fs_unlock_rpages(cc, i + 1);
-			f2fs_put_rpages_mapping(mapping, start_idx,
-					cc->cluster_size);
 			f2fs_destroy_compress_ctx(cc);
 			goto retry;
 		}
@@ -1108,10 +1097,10 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 	}
 
 unlock_pages:
+	f2fs_put_rpages(cc);
 	f2fs_unlock_rpages(cc, i);
-release_pages:
-	f2fs_put_rpages_mapping(mapping, start_idx, i);
 	f2fs_destroy_compress_ctx(cc);
+out:
 	return ret;
 }
 
-- 
2.29.2


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

* [f2fs-dev] [PATCH 2/3] f2fs: compress: fix race condition of overwrite vs truncate
@ 2021-05-10  9:30   ` Chao Yu
  0 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-10  9:30 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

pos_fsstress testcase complains a panic as belew:

------------[ cut here ]------------
kernel BUG at fs/f2fs/compress.c:1082!
invalid opcode: 0000 [#1] SMP PTI
CPU: 4 PID: 2753477 Comm: kworker/u16:2 Tainted: G           OE     5.12.0-rc1-custom #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
Workqueue: writeback wb_workfn (flush-252:16)
RIP: 0010:prepare_compress_overwrite+0x4c0/0x760 [f2fs]
Call Trace:
 f2fs_prepare_compress_overwrite+0x5f/0x80 [f2fs]
 f2fs_write_cache_pages+0x468/0x8a0 [f2fs]
 f2fs_write_data_pages+0x2a4/0x2f0 [f2fs]
 do_writepages+0x38/0xc0
 __writeback_single_inode+0x44/0x2a0
 writeback_sb_inodes+0x223/0x4d0
 __writeback_inodes_wb+0x56/0xf0
 wb_writeback+0x1dd/0x290
 wb_workfn+0x309/0x500
 process_one_work+0x220/0x3c0
 worker_thread+0x53/0x420
 kthread+0x12f/0x150
 ret_from_fork+0x22/0x30

The root cause is truncate() may race with overwrite as below,
so that one reference count left in page can not guarantee the
page attaching in mapping tree all the time, after truncation,
later find_lock_page() may return NULL pointer.

- prepare_compress_overwrite
 - f2fs_pagecache_get_page
 - unlock_page
					- f2fs_setattr
					 - truncate_setsize
					  - truncate_inode_page
					   - delete_from_page_cache
 - find_lock_page

Fix this by avoiding referencing updated page.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/compress.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d5cb0ba9a0e1..340815cd0887 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -118,19 +118,6 @@ static void f2fs_unlock_rpages(struct compress_ctx *cc, int len)
 	f2fs_drop_rpages(cc, len, true);
 }
 
-static void f2fs_put_rpages_mapping(struct address_space *mapping,
-				pgoff_t start, int len)
-{
-	int i;
-
-	for (i = 0; i < len; i++) {
-		struct page *page = find_get_page(mapping, start + i);
-
-		put_page(page);
-		put_page(page);
-	}
-}
-
 static void f2fs_put_rpages_wbc(struct compress_ctx *cc,
 		struct writeback_control *wbc, bool redirty, int unlock)
 {
@@ -1040,7 +1027,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 		}
 
 		if (PageUptodate(page))
-			unlock_page(page);
+			f2fs_put_page(page, 1);
 		else
 			f2fs_compress_ctx_add_page(cc, page);
 	}
@@ -1050,32 +1037,34 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 
 		ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size,
 					&last_block_in_bio, false, true);
+		f2fs_put_rpages(cc);
 		f2fs_destroy_compress_ctx(cc);
 		if (ret)
-			goto release_pages;
+			goto out;
 		if (bio)
 			f2fs_submit_bio(sbi, bio, DATA);
 
 		ret = f2fs_init_compress_ctx(cc);
 		if (ret)
-			goto release_pages;
+			goto out;
 	}
 
 	for (i = 0; i < cc->cluster_size; i++) {
 		f2fs_bug_on(sbi, cc->rpages[i]);
 
 		page = find_lock_page(mapping, start_idx + i);
-		f2fs_bug_on(sbi, !page);
+		if (!page) {
+			/* page can be truncated */
+			goto release_and_retry;
+		}
 
 		f2fs_wait_on_page_writeback(page, DATA, true, true);
-
 		f2fs_compress_ctx_add_page(cc, page);
-		f2fs_put_page(page, 0);
 
 		if (!PageUptodate(page)) {
+release_and_retry:
+			f2fs_put_rpages(cc);
 			f2fs_unlock_rpages(cc, i + 1);
-			f2fs_put_rpages_mapping(mapping, start_idx,
-					cc->cluster_size);
 			f2fs_destroy_compress_ctx(cc);
 			goto retry;
 		}
@@ -1108,10 +1097,10 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 	}
 
 unlock_pages:
+	f2fs_put_rpages(cc);
 	f2fs_unlock_rpages(cc, i);
-release_pages:
-	f2fs_put_rpages_mapping(mapping, start_idx, i);
 	f2fs_destroy_compress_ctx(cc);
+out:
 	return ret;
 }
 
-- 
2.29.2



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* [PATCH 3/3] f2fs: compress: fix to assign cc.cluster_idx correctly
  2021-05-10  9:30 ` [f2fs-dev] " Chao Yu
@ 2021-05-10  9:30   ` Chao Yu
  -1 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-10  9:30 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
may check wrong cluster metadata, fix it.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/compress.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 340815cd0887..30b003447510 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1066,6 +1066,8 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 			f2fs_put_rpages(cc);
 			f2fs_unlock_rpages(cc, i + 1);
 			f2fs_destroy_compress_ctx(cc);
+			cc->cluster_idx = index >>
+					F2FS_I(cc->inode)->i_log_cluster_size;
 			goto retry;
 		}
 	}
-- 
2.29.2


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

* [f2fs-dev] [PATCH 3/3] f2fs: compress: fix to assign cc.cluster_idx correctly
@ 2021-05-10  9:30   ` Chao Yu
  0 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-10  9:30 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
may check wrong cluster metadata, fix it.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/compress.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 340815cd0887..30b003447510 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1066,6 +1066,8 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 			f2fs_put_rpages(cc);
 			f2fs_unlock_rpages(cc, i + 1);
 			f2fs_destroy_compress_ctx(cc);
+			cc->cluster_idx = index >>
+					F2FS_I(cc->inode)->i_log_cluster_size;
 			goto retry;
 		}
 	}
-- 
2.29.2



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block()
  2021-05-10  9:30 ` [f2fs-dev] " Chao Yu
@ 2021-05-10 15:51   ` Jaegeuk Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-05-10 15:51 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 05/10, Chao Yu wrote:
> f2fs_get_block() and f2fs_put_dnode() should be called as a pair,
> add missing f2fs_put_dnode() in prepare_compress_overwrite().
> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/compress.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index c208563eac28..d5cb0ba9a0e1 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1088,6 +1088,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>  
>  		for (i = cc->cluster_size - 1; i > 0; i--) {
>  			ret = f2fs_get_block(&dn, start_idx + i);
> +			f2fs_put_dnode(&dn);

f2fs_reserve_block()
 -> need_put = true;
  -> f2fs_put_dnode();

>  			if (ret) {
>  				i = cc->cluster_size;
>  				break;
> -- 
> 2.29.2

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

* Re: [f2fs-dev] [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block()
@ 2021-05-10 15:51   ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-05-10 15:51 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 05/10, Chao Yu wrote:
> f2fs_get_block() and f2fs_put_dnode() should be called as a pair,
> add missing f2fs_put_dnode() in prepare_compress_overwrite().
> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/compress.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index c208563eac28..d5cb0ba9a0e1 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1088,6 +1088,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>  
>  		for (i = cc->cluster_size - 1; i > 0; i--) {
>  			ret = f2fs_get_block(&dn, start_idx + i);
> +			f2fs_put_dnode(&dn);

f2fs_reserve_block()
 -> need_put = true;
  -> f2fs_put_dnode();

>  			if (ret) {
>  				i = cc->cluster_size;
>  				break;
> -- 
> 2.29.2


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [PATCH 3/3] f2fs: compress: fix to assign cc.cluster_idx correctly
  2021-05-10  9:30   ` [f2fs-dev] " Chao Yu
@ 2021-05-10 16:09     ` Jaegeuk Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-05-10 16:09 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 05/10, Chao Yu wrote:
> In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
> cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
> may check wrong cluster metadata, fix it.
> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/compress.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 340815cd0887..30b003447510 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1066,6 +1066,8 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>  			f2fs_put_rpages(cc);
>  			f2fs_unlock_rpages(cc, i + 1);
>  			f2fs_destroy_compress_ctx(cc);
> +			cc->cluster_idx = index >>
> +					F2FS_I(cc->inode)->i_log_cluster_size;

I didn't test tho, how about this?

From 904abb77e82ea982f68960148b75d0a12f771c2e Mon Sep 17 00:00:00 2001
From: Chao Yu <yuchao0@huawei.com>
Date: Mon, 10 May 2021 17:30:32 +0800
Subject: [PATCH] f2fs: compress: fix to assign cc.cluster_idx correctly

In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
may check wrong cluster metadata, fix it.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/compress.c | 17 +++++++++--------
 fs/f2fs/data.c     |  6 +++---
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 79348bc56e35..925a5ca3744a 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -145,13 +145,14 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc)
 	return cc->rpages ? 0 : -ENOMEM;
 }
 
-void f2fs_destroy_compress_ctx(struct compress_ctx *cc)
+void f2fs_destroy_compress_ctx(struct compress_ctx *cc, bool reuse)
 {
 	page_array_free(cc->inode, cc->rpages, cc->cluster_size);
 	cc->rpages = NULL;
 	cc->nr_rpages = 0;
 	cc->nr_cpages = 0;
-	cc->cluster_idx = NULL_CLUSTER;
+	if (!reuse)
+		cc->cluster_idx = NULL_CLUSTER;
 }
 
 void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page)
@@ -1034,7 +1035,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 		ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size,
 					&last_block_in_bio, false, true);
 		f2fs_put_rpages(cc);
-		f2fs_destroy_compress_ctx(cc);
+		f2fs_destroy_compress_ctx(cc, true);
 		if (ret)
 			goto out;
 		if (bio)
@@ -1061,7 +1062,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 release_and_retry:
 			f2fs_put_rpages(cc);
 			f2fs_unlock_rpages(cc, i + 1);
-			f2fs_destroy_compress_ctx(cc);
+			f2fs_destroy_compress_ctx(cc, true);
 			goto retry;
 		}
 	}
@@ -1094,7 +1095,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 unlock_pages:
 	f2fs_put_rpages(cc);
 	f2fs_unlock_rpages(cc, i);
-	f2fs_destroy_compress_ctx(cc);
+	f2fs_destroy_compress_ctx(cc, true);
 out:
 	return ret;
 }
@@ -1130,7 +1131,7 @@ bool f2fs_compress_write_end(struct inode *inode, void *fsdata,
 		set_cluster_dirty(&cc);
 
 	f2fs_put_rpages_wbc(&cc, NULL, false, 1);
-	f2fs_destroy_compress_ctx(&cc);
+	f2fs_destroy_compress_ctx(&cc, false);
 
 	return first_index;
 }
@@ -1350,7 +1351,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 	f2fs_put_rpages(cc);
 	page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
 	cc->cpages = NULL;
-	f2fs_destroy_compress_ctx(cc);
+	f2fs_destroy_compress_ctx(cc, false);
 	return 0;
 
 out_destroy_crypt:
@@ -1512,7 +1513,7 @@ int f2fs_write_multi_pages(struct compress_ctx *cc,
 	err = f2fs_write_raw_pages(cc, submitted, wbc, io_type);
 	f2fs_put_rpages_wbc(cc, wbc, false, 0);
 destroy_out:
-	f2fs_destroy_compress_ctx(cc);
+	f2fs_destroy_compress_ctx(cc, false);
 	return err;
 }
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 96f1a354f89f..33e56ae84e35 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2287,7 +2287,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
 							max_nr_pages,
 							&last_block_in_bio,
 							rac != NULL, false);
-				f2fs_destroy_compress_ctx(&cc);
+				f2fs_destroy_compress_ctx(&cc, false);
 				if (ret)
 					goto set_error_page;
 			}
@@ -2332,7 +2332,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
 							max_nr_pages,
 							&last_block_in_bio,
 							rac != NULL, false);
-				f2fs_destroy_compress_ctx(&cc);
+				f2fs_destroy_compress_ctx(&cc, false);
 			}
 		}
 #endif
@@ -3033,7 +3033,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
 		}
 	}
 	if (f2fs_compressed_file(inode))
-		f2fs_destroy_compress_ctx(&cc);
+		f2fs_destroy_compress_ctx(&cc, false);
 #endif
 	if (retry) {
 		index = 0;
-- 
2.31.1.607.g51e8a6a459-goog


>  			goto retry;
>  		}
>  	}
> -- 
> 2.29.2

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

* Re: [f2fs-dev] [PATCH 3/3] f2fs: compress: fix to assign cc.cluster_idx correctly
@ 2021-05-10 16:09     ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-05-10 16:09 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 05/10, Chao Yu wrote:
> In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
> cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
> may check wrong cluster metadata, fix it.
> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/compress.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 340815cd0887..30b003447510 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1066,6 +1066,8 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>  			f2fs_put_rpages(cc);
>  			f2fs_unlock_rpages(cc, i + 1);
>  			f2fs_destroy_compress_ctx(cc);
> +			cc->cluster_idx = index >>
> +					F2FS_I(cc->inode)->i_log_cluster_size;

I didn't test tho, how about this?

From 904abb77e82ea982f68960148b75d0a12f771c2e Mon Sep 17 00:00:00 2001
From: Chao Yu <yuchao0@huawei.com>
Date: Mon, 10 May 2021 17:30:32 +0800
Subject: [PATCH] f2fs: compress: fix to assign cc.cluster_idx correctly

In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
may check wrong cluster metadata, fix it.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/compress.c | 17 +++++++++--------
 fs/f2fs/data.c     |  6 +++---
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 79348bc56e35..925a5ca3744a 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -145,13 +145,14 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc)
 	return cc->rpages ? 0 : -ENOMEM;
 }
 
-void f2fs_destroy_compress_ctx(struct compress_ctx *cc)
+void f2fs_destroy_compress_ctx(struct compress_ctx *cc, bool reuse)
 {
 	page_array_free(cc->inode, cc->rpages, cc->cluster_size);
 	cc->rpages = NULL;
 	cc->nr_rpages = 0;
 	cc->nr_cpages = 0;
-	cc->cluster_idx = NULL_CLUSTER;
+	if (!reuse)
+		cc->cluster_idx = NULL_CLUSTER;
 }
 
 void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page)
@@ -1034,7 +1035,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 		ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size,
 					&last_block_in_bio, false, true);
 		f2fs_put_rpages(cc);
-		f2fs_destroy_compress_ctx(cc);
+		f2fs_destroy_compress_ctx(cc, true);
 		if (ret)
 			goto out;
 		if (bio)
@@ -1061,7 +1062,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 release_and_retry:
 			f2fs_put_rpages(cc);
 			f2fs_unlock_rpages(cc, i + 1);
-			f2fs_destroy_compress_ctx(cc);
+			f2fs_destroy_compress_ctx(cc, true);
 			goto retry;
 		}
 	}
@@ -1094,7 +1095,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 unlock_pages:
 	f2fs_put_rpages(cc);
 	f2fs_unlock_rpages(cc, i);
-	f2fs_destroy_compress_ctx(cc);
+	f2fs_destroy_compress_ctx(cc, true);
 out:
 	return ret;
 }
@@ -1130,7 +1131,7 @@ bool f2fs_compress_write_end(struct inode *inode, void *fsdata,
 		set_cluster_dirty(&cc);
 
 	f2fs_put_rpages_wbc(&cc, NULL, false, 1);
-	f2fs_destroy_compress_ctx(&cc);
+	f2fs_destroy_compress_ctx(&cc, false);
 
 	return first_index;
 }
@@ -1350,7 +1351,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 	f2fs_put_rpages(cc);
 	page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
 	cc->cpages = NULL;
-	f2fs_destroy_compress_ctx(cc);
+	f2fs_destroy_compress_ctx(cc, false);
 	return 0;
 
 out_destroy_crypt:
@@ -1512,7 +1513,7 @@ int f2fs_write_multi_pages(struct compress_ctx *cc,
 	err = f2fs_write_raw_pages(cc, submitted, wbc, io_type);
 	f2fs_put_rpages_wbc(cc, wbc, false, 0);
 destroy_out:
-	f2fs_destroy_compress_ctx(cc);
+	f2fs_destroy_compress_ctx(cc, false);
 	return err;
 }
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 96f1a354f89f..33e56ae84e35 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2287,7 +2287,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
 							max_nr_pages,
 							&last_block_in_bio,
 							rac != NULL, false);
-				f2fs_destroy_compress_ctx(&cc);
+				f2fs_destroy_compress_ctx(&cc, false);
 				if (ret)
 					goto set_error_page;
 			}
@@ -2332,7 +2332,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
 							max_nr_pages,
 							&last_block_in_bio,
 							rac != NULL, false);
-				f2fs_destroy_compress_ctx(&cc);
+				f2fs_destroy_compress_ctx(&cc, false);
 			}
 		}
 #endif
@@ -3033,7 +3033,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
 		}
 	}
 	if (f2fs_compressed_file(inode))
-		f2fs_destroy_compress_ctx(&cc);
+		f2fs_destroy_compress_ctx(&cc, false);
 #endif
 	if (retry) {
 		index = 0;
-- 
2.31.1.607.g51e8a6a459-goog


>  			goto retry;
>  		}
>  	}
> -- 
> 2.29.2


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block()
  2021-05-10 15:51   ` [f2fs-dev] " Jaegeuk Kim
@ 2021-05-11  1:37     ` Chao Yu
  -1 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-11  1:37 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2021/5/10 23:51, Jaegeuk Kim wrote:
> On 05/10, Chao Yu wrote:
>> f2fs_get_block() and f2fs_put_dnode() should be called as a pair,
>> add missing f2fs_put_dnode() in prepare_compress_overwrite().
>>
>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>   fs/f2fs/compress.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index c208563eac28..d5cb0ba9a0e1 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -1088,6 +1088,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>>   
>>   		for (i = cc->cluster_size - 1; i > 0; i--) {
>>   			ret = f2fs_get_block(&dn, start_idx + i);
>> +			f2fs_put_dnode(&dn);
> 
> f2fs_reserve_block()
>   -> need_put = true;
>    -> f2fs_put_dnode();

Correct, it looks f2fs_vm_page_mkwrite() doesn't need to call
f2fs_put_dnode() as well.

Thanks,

> 
>>   			if (ret) {
>>   				i = cc->cluster_size;
>>   				break;
>> -- 
>> 2.29.2
> .
> 

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

* Re: [f2fs-dev] [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block()
@ 2021-05-11  1:37     ` Chao Yu
  0 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-11  1:37 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2021/5/10 23:51, Jaegeuk Kim wrote:
> On 05/10, Chao Yu wrote:
>> f2fs_get_block() and f2fs_put_dnode() should be called as a pair,
>> add missing f2fs_put_dnode() in prepare_compress_overwrite().
>>
>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>   fs/f2fs/compress.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index c208563eac28..d5cb0ba9a0e1 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -1088,6 +1088,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>>   
>>   		for (i = cc->cluster_size - 1; i > 0; i--) {
>>   			ret = f2fs_get_block(&dn, start_idx + i);
>> +			f2fs_put_dnode(&dn);
> 
> f2fs_reserve_block()
>   -> need_put = true;
>    -> f2fs_put_dnode();

Correct, it looks f2fs_vm_page_mkwrite() doesn't need to call
f2fs_put_dnode() as well.

Thanks,

> 
>>   			if (ret) {
>>   				i = cc->cluster_size;
>>   				break;
>> -- 
>> 2.29.2
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [PATCH 3/3] f2fs: compress: fix to assign cc.cluster_idx correctly
  2021-05-10 16:09     ` [f2fs-dev] " Jaegeuk Kim
@ 2021-05-11  1:38       ` Chao Yu
  -1 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-11  1:38 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2021/5/11 0:09, Jaegeuk Kim wrote:
> On 05/10, Chao Yu wrote:
>> In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
>> cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
>> may check wrong cluster metadata, fix it.
>>
>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>   fs/f2fs/compress.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index 340815cd0887..30b003447510 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -1066,6 +1066,8 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>>   			f2fs_put_rpages(cc);
>>   			f2fs_unlock_rpages(cc, i + 1);
>>   			f2fs_destroy_compress_ctx(cc);
>> +			cc->cluster_idx = index >>
>> +					F2FS_I(cc->inode)->i_log_cluster_size;
> 
> I didn't test tho, how about this?

Looks more clean. :)

Thanks,

> 
>>From 904abb77e82ea982f68960148b75d0a12f771c2e Mon Sep 17 00:00:00 2001
> From: Chao Yu <yuchao0@huawei.com>
> Date: Mon, 10 May 2021 17:30:32 +0800
> Subject: [PATCH] f2fs: compress: fix to assign cc.cluster_idx correctly
> 
> In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
> cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
> may check wrong cluster metadata, fix it.
> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/compress.c | 17 +++++++++--------
>   fs/f2fs/data.c     |  6 +++---
>   2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 79348bc56e35..925a5ca3744a 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -145,13 +145,14 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc)
>   	return cc->rpages ? 0 : -ENOMEM;
>   }
>   
> -void f2fs_destroy_compress_ctx(struct compress_ctx *cc)
> +void f2fs_destroy_compress_ctx(struct compress_ctx *cc, bool reuse)
>   {
>   	page_array_free(cc->inode, cc->rpages, cc->cluster_size);
>   	cc->rpages = NULL;
>   	cc->nr_rpages = 0;
>   	cc->nr_cpages = 0;
> -	cc->cluster_idx = NULL_CLUSTER;
> +	if (!reuse)
> +		cc->cluster_idx = NULL_CLUSTER;
>   }
>   
>   void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page)
> @@ -1034,7 +1035,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>   		ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size,
>   					&last_block_in_bio, false, true);
>   		f2fs_put_rpages(cc);
> -		f2fs_destroy_compress_ctx(cc);
> +		f2fs_destroy_compress_ctx(cc, true);
>   		if (ret)
>   			goto out;
>   		if (bio)
> @@ -1061,7 +1062,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>   release_and_retry:
>   			f2fs_put_rpages(cc);
>   			f2fs_unlock_rpages(cc, i + 1);
> -			f2fs_destroy_compress_ctx(cc);
> +			f2fs_destroy_compress_ctx(cc, true);
>   			goto retry;
>   		}
>   	}
> @@ -1094,7 +1095,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>   unlock_pages:
>   	f2fs_put_rpages(cc);
>   	f2fs_unlock_rpages(cc, i);
> -	f2fs_destroy_compress_ctx(cc);
> +	f2fs_destroy_compress_ctx(cc, true);
>   out:
>   	return ret;
>   }
> @@ -1130,7 +1131,7 @@ bool f2fs_compress_write_end(struct inode *inode, void *fsdata,
>   		set_cluster_dirty(&cc);
>   
>   	f2fs_put_rpages_wbc(&cc, NULL, false, 1);
> -	f2fs_destroy_compress_ctx(&cc);
> +	f2fs_destroy_compress_ctx(&cc, false);
>   
>   	return first_index;
>   }
> @@ -1350,7 +1351,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>   	f2fs_put_rpages(cc);
>   	page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
>   	cc->cpages = NULL;
> -	f2fs_destroy_compress_ctx(cc);
> +	f2fs_destroy_compress_ctx(cc, false);
>   	return 0;
>   
>   out_destroy_crypt:
> @@ -1512,7 +1513,7 @@ int f2fs_write_multi_pages(struct compress_ctx *cc,
>   	err = f2fs_write_raw_pages(cc, submitted, wbc, io_type);
>   	f2fs_put_rpages_wbc(cc, wbc, false, 0);
>   destroy_out:
> -	f2fs_destroy_compress_ctx(cc);
> +	f2fs_destroy_compress_ctx(cc, false);
>   	return err;
>   }
>   
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 96f1a354f89f..33e56ae84e35 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2287,7 +2287,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
>   							max_nr_pages,
>   							&last_block_in_bio,
>   							rac != NULL, false);
> -				f2fs_destroy_compress_ctx(&cc);
> +				f2fs_destroy_compress_ctx(&cc, false);
>   				if (ret)
>   					goto set_error_page;
>   			}
> @@ -2332,7 +2332,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
>   							max_nr_pages,
>   							&last_block_in_bio,
>   							rac != NULL, false);
> -				f2fs_destroy_compress_ctx(&cc);
> +				f2fs_destroy_compress_ctx(&cc, false);
>   			}
>   		}
>   #endif
> @@ -3033,7 +3033,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>   		}
>   	}
>   	if (f2fs_compressed_file(inode))
> -		f2fs_destroy_compress_ctx(&cc);
> +		f2fs_destroy_compress_ctx(&cc, false);
>   #endif
>   	if (retry) {
>   		index = 0;
> 

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

* Re: [f2fs-dev] [PATCH 3/3] f2fs: compress: fix to assign cc.cluster_idx correctly
@ 2021-05-11  1:38       ` Chao Yu
  0 siblings, 0 replies; 14+ messages in thread
From: Chao Yu @ 2021-05-11  1:38 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2021/5/11 0:09, Jaegeuk Kim wrote:
> On 05/10, Chao Yu wrote:
>> In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
>> cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
>> may check wrong cluster metadata, fix it.
>>
>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>   fs/f2fs/compress.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index 340815cd0887..30b003447510 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -1066,6 +1066,8 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>>   			f2fs_put_rpages(cc);
>>   			f2fs_unlock_rpages(cc, i + 1);
>>   			f2fs_destroy_compress_ctx(cc);
>> +			cc->cluster_idx = index >>
>> +					F2FS_I(cc->inode)->i_log_cluster_size;
> 
> I didn't test tho, how about this?

Looks more clean. :)

Thanks,

> 
>>From 904abb77e82ea982f68960148b75d0a12f771c2e Mon Sep 17 00:00:00 2001
> From: Chao Yu <yuchao0@huawei.com>
> Date: Mon, 10 May 2021 17:30:32 +0800
> Subject: [PATCH] f2fs: compress: fix to assign cc.cluster_idx correctly
> 
> In f2fs_destroy_compress_ctx(), after f2fs_destroy_compress_ctx(),
> cc.cluster_idx will be cleared w/ NULL_CLUSTER, f2fs_cluster_blocks()
> may check wrong cluster metadata, fix it.
> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/compress.c | 17 +++++++++--------
>   fs/f2fs/data.c     |  6 +++---
>   2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 79348bc56e35..925a5ca3744a 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -145,13 +145,14 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc)
>   	return cc->rpages ? 0 : -ENOMEM;
>   }
>   
> -void f2fs_destroy_compress_ctx(struct compress_ctx *cc)
> +void f2fs_destroy_compress_ctx(struct compress_ctx *cc, bool reuse)
>   {
>   	page_array_free(cc->inode, cc->rpages, cc->cluster_size);
>   	cc->rpages = NULL;
>   	cc->nr_rpages = 0;
>   	cc->nr_cpages = 0;
> -	cc->cluster_idx = NULL_CLUSTER;
> +	if (!reuse)
> +		cc->cluster_idx = NULL_CLUSTER;
>   }
>   
>   void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page)
> @@ -1034,7 +1035,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>   		ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size,
>   					&last_block_in_bio, false, true);
>   		f2fs_put_rpages(cc);
> -		f2fs_destroy_compress_ctx(cc);
> +		f2fs_destroy_compress_ctx(cc, true);
>   		if (ret)
>   			goto out;
>   		if (bio)
> @@ -1061,7 +1062,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>   release_and_retry:
>   			f2fs_put_rpages(cc);
>   			f2fs_unlock_rpages(cc, i + 1);
> -			f2fs_destroy_compress_ctx(cc);
> +			f2fs_destroy_compress_ctx(cc, true);
>   			goto retry;
>   		}
>   	}
> @@ -1094,7 +1095,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>   unlock_pages:
>   	f2fs_put_rpages(cc);
>   	f2fs_unlock_rpages(cc, i);
> -	f2fs_destroy_compress_ctx(cc);
> +	f2fs_destroy_compress_ctx(cc, true);
>   out:
>   	return ret;
>   }
> @@ -1130,7 +1131,7 @@ bool f2fs_compress_write_end(struct inode *inode, void *fsdata,
>   		set_cluster_dirty(&cc);
>   
>   	f2fs_put_rpages_wbc(&cc, NULL, false, 1);
> -	f2fs_destroy_compress_ctx(&cc);
> +	f2fs_destroy_compress_ctx(&cc, false);
>   
>   	return first_index;
>   }
> @@ -1350,7 +1351,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>   	f2fs_put_rpages(cc);
>   	page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
>   	cc->cpages = NULL;
> -	f2fs_destroy_compress_ctx(cc);
> +	f2fs_destroy_compress_ctx(cc, false);
>   	return 0;
>   
>   out_destroy_crypt:
> @@ -1512,7 +1513,7 @@ int f2fs_write_multi_pages(struct compress_ctx *cc,
>   	err = f2fs_write_raw_pages(cc, submitted, wbc, io_type);
>   	f2fs_put_rpages_wbc(cc, wbc, false, 0);
>   destroy_out:
> -	f2fs_destroy_compress_ctx(cc);
> +	f2fs_destroy_compress_ctx(cc, false);
>   	return err;
>   }
>   
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 96f1a354f89f..33e56ae84e35 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2287,7 +2287,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
>   							max_nr_pages,
>   							&last_block_in_bio,
>   							rac != NULL, false);
> -				f2fs_destroy_compress_ctx(&cc);
> +				f2fs_destroy_compress_ctx(&cc, false);
>   				if (ret)
>   					goto set_error_page;
>   			}
> @@ -2332,7 +2332,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
>   							max_nr_pages,
>   							&last_block_in_bio,
>   							rac != NULL, false);
> -				f2fs_destroy_compress_ctx(&cc);
> +				f2fs_destroy_compress_ctx(&cc, false);
>   			}
>   		}
>   #endif
> @@ -3033,7 +3033,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>   		}
>   	}
>   	if (f2fs_compressed_file(inode))
> -		f2fs_destroy_compress_ctx(&cc);
> +		f2fs_destroy_compress_ctx(&cc, false);
>   #endif
>   	if (retry) {
>   		index = 0;
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

end of thread, other threads:[~2021-05-11  1:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10  9:30 [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block() Chao Yu
2021-05-10  9:30 ` [f2fs-dev] " Chao Yu
2021-05-10  9:30 ` [PATCH 2/3] f2fs: compress: fix race condition of overwrite vs truncate Chao Yu
2021-05-10  9:30   ` [f2fs-dev] " Chao Yu
2021-05-10  9:30 ` [PATCH 3/3] f2fs: compress: fix to assign cc.cluster_idx correctly Chao Yu
2021-05-10  9:30   ` [f2fs-dev] " Chao Yu
2021-05-10 16:09   ` Jaegeuk Kim
2021-05-10 16:09     ` [f2fs-dev] " Jaegeuk Kim
2021-05-11  1:38     ` Chao Yu
2021-05-11  1:38       ` [f2fs-dev] " Chao Yu
2021-05-10 15:51 ` [PATCH 1/3] f2fs: compress: fix to call f2fs_put_dnode() paired with f2fs_get_block() Jaegeuk Kim
2021-05-10 15:51   ` [f2fs-dev] " Jaegeuk Kim
2021-05-11  1:37   ` Chao Yu
2021-05-11  1:37     ` [f2fs-dev] " Chao Yu

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