* [PATCH 1/3] f2fs: use folio_test_writeback
@ 2024-04-09 20:34 Jaegeuk Kim
2024-04-09 20:34 ` [PATCH 2/3] f2fs: clear writeback when compression failed Jaegeuk Kim
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2024-04-09 20:34 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim
Let's convert PageWriteback to folio_test_writeback.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/compress.c | 2 +-
fs/f2fs/data.c | 3 +--
fs/f2fs/f2fs.h | 2 +-
fs/f2fs/gc.c | 2 +-
fs/f2fs/inline.c | 2 +-
fs/f2fs/inode.c | 3 ++-
fs/f2fs/node.c | 2 +-
fs/f2fs/segment.c | 10 +++++-----
8 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 8892c8262141..d67c471ab5df 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1484,7 +1484,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
if (!PageDirty(cc->rpages[i]))
goto continue_unlock;
- if (PageWriteback(cc->rpages[i])) {
+ if (folio_test_writeback(page_folio(cc->rpages[i]))) {
if (wbc->sync_mode == WB_SYNC_NONE)
goto continue_unlock;
f2fs_wait_on_page_writeback(cc->rpages[i], DATA, true, true);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 60056b9a51be..19f1e573297d 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2707,8 +2707,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
if (err) {
if (fscrypt_inode_uses_fs_layer_crypto(inode))
fscrypt_finalize_bounce_page(&fio->encrypted_page);
- if (PageWriteback(page))
- end_page_writeback(page);
+ end_page_writeback(page);
} else {
set_inode_flag(inode, FI_UPDATE_WRITE);
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e9ef971f4dba..dd530dc70005 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4660,7 +4660,7 @@ static inline void f2fs_truncate_meta_inode_pages(struct f2fs_sb_info *sbi,
page = find_get_page(META_MAPPING(sbi), blkaddr + i);
if (page) {
- if (PageWriteback(page))
+ if (folio_test_writeback(page_folio(page)))
need_submit = true;
f2fs_put_page(page, 0);
}
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 8852814dab7f..ac4cbbe50c2f 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1434,7 +1434,7 @@ static int move_data_page(struct inode *inode, block_t bidx, int gc_type,
goto out;
if (gc_type == BG_GC) {
- if (PageWriteback(page)) {
+ if (folio_test_writeback(page_folio(page))) {
err = -EAGAIN;
goto out;
}
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index ac00423f117b..3d3218a4b29d 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -164,7 +164,7 @@ int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page)
return -EFSCORRUPTED;
}
- f2fs_bug_on(F2FS_P_SB(page), PageWriteback(page));
+ f2fs_bug_on(F2FS_P_SB(page), folio_test_writeback(page_folio(page)));
f2fs_do_read_inline_data(page, dn->inode_page);
set_page_dirty(page);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 12b1fef31f43..d7a5a88a1a5e 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -161,7 +161,8 @@ bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page)
if (!f2fs_enable_inode_chksum(sbi, page))
#else
if (!f2fs_enable_inode_chksum(sbi, page) ||
- PageDirty(page) || PageWriteback(page))
+ PageDirty(page) ||
+ folio_test_writeback(page_folio(page)))
#endif
return true;
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index bb57bbaff7b4..3b9eb5693683 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1743,7 +1743,7 @@ int f2fs_move_node_page(struct page *node_page, int gc_type)
goto release_page;
} else {
/* set page dirty and write it */
- if (!PageWriteback(node_page))
+ if (!folio_test_writeback(page_folio(node_page)))
set_page_dirty(node_page);
}
out_page:
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 4fd76e867e0a..065fd5919b48 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3612,13 +3612,13 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
mutex_unlock(&curseg->curseg_mutex);
f2fs_up_read(&SM_I(sbi)->curseg_lock);
return 0;
+
out_err:
*new_blkaddr = NULL_ADDR;
up_write(&sit_i->sentry_lock);
mutex_unlock(&curseg->curseg_mutex);
f2fs_up_read(&SM_I(sbi)->curseg_lock);
return ret;
-
}
void f2fs_update_device_state(struct f2fs_sb_info *sbi, nid_t ino,
@@ -3660,8 +3660,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
&fio->new_blkaddr, sum, type, fio)) {
if (fscrypt_inode_uses_fs_layer_crypto(fio->page->mapping->host))
fscrypt_finalize_bounce_page(&fio->encrypted_page);
- if (PageWriteback(fio->page))
- end_page_writeback(fio->page);
+ end_page_writeback(fio->page);
if (f2fs_in_warm_node_list(fio->sbi, fio->page))
f2fs_del_fsync_node_entry(fio->sbi, fio->page);
goto out;
@@ -3904,7 +3903,7 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
void f2fs_wait_on_page_writeback(struct page *page,
enum page_type type, bool ordered, bool locked)
{
- if (PageWriteback(page)) {
+ if (folio_test_writeback(page_folio(page))) {
struct f2fs_sb_info *sbi = F2FS_P_SB(page);
/* submit cached LFS IO */
@@ -3913,7 +3912,8 @@ void f2fs_wait_on_page_writeback(struct page *page,
f2fs_submit_merged_ipu_write(sbi, NULL, page);
if (ordered) {
wait_on_page_writeback(page);
- f2fs_bug_on(sbi, locked && PageWriteback(page));
+ f2fs_bug_on(sbi, locked &&
+ folio_test_writeback(page_folio(page)));
} else {
wait_for_stable_page(page);
}
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] f2fs: clear writeback when compression failed
2024-04-09 20:34 [PATCH 1/3] f2fs: use folio_test_writeback Jaegeuk Kim
@ 2024-04-09 20:34 ` Jaegeuk Kim
2024-04-11 9:05 ` [f2fs-dev] " Chao Yu
2024-04-16 16:49 ` [PATCH 2/3 v2] " Jaegeuk Kim
2024-04-09 20:34 ` [PATCH 3/3] f2fs: fix false alarm on invalid block address Jaegeuk Kim
` (3 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2024-04-09 20:34 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim
Let's stop issuing compressed writes and clear their writeback flags.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/compress.c | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d67c471ab5df..3a8ecc6aee84 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1031,6 +1031,25 @@ static void set_cluster_writeback(struct compress_ctx *cc)
}
}
+static void cancel_cluster_writeback(struct compress_ctx *cc, int submitted)
+{
+ int i;
+
+ for (i = 0; i < cc->cluster_size; i++) {
+ if (!cc->rpages[i])
+ continue;
+ if (i < submitted) {
+ if (i)
+ f2fs_wait_on_page_writeback(cc->rpages[i],
+ DATA, true, true);
+ inode_inc_dirty_pages(cc->inode);
+ lock_page(cc->rpages[i]);
+ }
+ clear_page_private_gcing(cc->rpages[i]);
+ end_page_writeback(cc->rpages[i]);
+ }
+}
+
static void set_cluster_dirty(struct compress_ctx *cc)
{
int i;
@@ -1232,7 +1251,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
.page = NULL,
.encrypted_page = NULL,
.compressed_page = NULL,
- .submitted = 0,
.io_type = io_type,
.io_wbc = wbc,
.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
@@ -1358,7 +1376,15 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
fio.compressed_page = cc->cpages[i - 1];
cc->cpages[i - 1] = NULL;
+ fio.submitted = 0;
f2fs_outplace_write_data(&dn, &fio);
+ if (unlikely(!fio.submitted)) {
+ cancel_cluster_writeback(cc, i);
+
+ /* To call fscrypt_finalize_bounce_page */
+ i = cc->valid_nr_cpages;
+ goto out_destroy_crypt;
+ }
(*submitted)++;
unlock_continue:
inode_dec_dirty_pages(cc->inode);
@@ -1392,8 +1418,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
out_destroy_crypt:
page_array_free(cc->inode, cic->rpages, cc->cluster_size);
- for (--i; i >= 0; i--)
+ for (--i; i >= 0; i--) {
+ if (!cc->cpages[i])
+ continue;
fscrypt_finalize_bounce_page(&cc->cpages[i]);
+ }
out_put_cic:
kmem_cache_free(cic_entry_slab, cic);
out_put_dnode:
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-09 20:34 [PATCH 1/3] f2fs: use folio_test_writeback Jaegeuk Kim
2024-04-09 20:34 ` [PATCH 2/3] f2fs: clear writeback when compression failed Jaegeuk Kim
@ 2024-04-09 20:34 ` Jaegeuk Kim
2024-04-11 9:08 ` [f2fs-dev] " Chao Yu
2024-04-11 8:56 ` [f2fs-dev] [PATCH 1/3] f2fs: use folio_test_writeback Chao Yu
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Jaegeuk Kim @ 2024-04-09 20:34 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim
f2fs_ra_meta_pages can try to read ahead on invalid block address which is
not the corruption case.
Fixes: 31f85ccc84b8 ("f2fs: unify the error handling of f2fs_is_valid_blkaddr")
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/checkpoint.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index eac698b8dd38..b01320502624 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -179,22 +179,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
break;
case META_SIT:
if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
- goto err;
+ goto check_only;
break;
case META_SSA:
if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
blkaddr < SM_I(sbi)->ssa_blkaddr))
- goto err;
+ goto check_only;
break;
case META_CP:
if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
blkaddr < __start_cp_addr(sbi)))
- goto err;
+ goto check_only;
break;
case META_POR:
if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
blkaddr < MAIN_BLKADDR(sbi)))
- goto err;
+ goto check_only;
break;
case DATA_GENERIC:
case DATA_GENERIC_ENHANCE:
@@ -228,6 +228,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
return true;
err:
f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
+check_only:
return false;
}
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 1/3] f2fs: use folio_test_writeback
2024-04-09 20:34 [PATCH 1/3] f2fs: use folio_test_writeback Jaegeuk Kim
2024-04-09 20:34 ` [PATCH 2/3] f2fs: clear writeback when compression failed Jaegeuk Kim
2024-04-09 20:34 ` [PATCH 3/3] f2fs: fix false alarm on invalid block address Jaegeuk Kim
@ 2024-04-11 8:56 ` Chao Yu
2024-04-14 15:32 ` patchwork-bot+f2fs
2024-04-24 17:40 ` patchwork-bot+f2fs
4 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2024-04-11 8:56 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2024/4/10 4:34, Jaegeuk Kim wrote:
> Let's convert PageWriteback to folio_test_writeback.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: clear writeback when compression failed
2024-04-09 20:34 ` [PATCH 2/3] f2fs: clear writeback when compression failed Jaegeuk Kim
@ 2024-04-11 9:05 ` Chao Yu
2024-04-12 20:57 ` Jaegeuk Kim
2024-04-16 16:49 ` [PATCH 2/3 v2] " Jaegeuk Kim
1 sibling, 1 reply; 18+ messages in thread
From: Chao Yu @ 2024-04-11 9:05 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2024/4/10 4:34, Jaegeuk Kim wrote:
> Let's stop issuing compressed writes and clear their writeback flags.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/compress.c | 33 +++++++++++++++++++++++++++++++--
> 1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index d67c471ab5df..3a8ecc6aee84 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1031,6 +1031,25 @@ static void set_cluster_writeback(struct compress_ctx *cc)
> }
> }
>
> +static void cancel_cluster_writeback(struct compress_ctx *cc, int submitted)
> +{
> + int i;
> +
> + for (i = 0; i < cc->cluster_size; i++) {
> + if (!cc->rpages[i])
> + continue;
> + if (i < submitted) {
> + if (i)
> + f2fs_wait_on_page_writeback(cc->rpages[i],
> + DATA, true, true);
> + inode_inc_dirty_pages(cc->inode);
> + lock_page(cc->rpages[i]);
> + }
> + clear_page_private_gcing(cc->rpages[i]);
> + end_page_writeback(cc->rpages[i]);
> + }
> +}
> +
> static void set_cluster_dirty(struct compress_ctx *cc)
> {
> int i;
> @@ -1232,7 +1251,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> .page = NULL,
> .encrypted_page = NULL,
> .compressed_page = NULL,
> - .submitted = 0,
> .io_type = io_type,
> .io_wbc = wbc,
> .encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
> @@ -1358,7 +1376,15 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> fio.compressed_page = cc->cpages[i - 1];
>
> cc->cpages[i - 1] = NULL;
> + fio.submitted = 0;
> f2fs_outplace_write_data(&dn, &fio);
> + if (unlikely(!fio.submitted)) {
> + cancel_cluster_writeback(cc, i);
> +
> + /* To call fscrypt_finalize_bounce_page */
> + i = cc->valid_nr_cpages;
*submitted = 0; ?
Thanks,
> + goto out_destroy_crypt;
> + }
> (*submitted)++;
> unlock_continue:
> inode_dec_dirty_pages(cc->inode);
> @@ -1392,8 +1418,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> out_destroy_crypt:
> page_array_free(cc->inode, cic->rpages, cc->cluster_size);
>
> - for (--i; i >= 0; i--)
> + for (--i; i >= 0; i--) {
> + if (!cc->cpages[i])
> + continue;
> fscrypt_finalize_bounce_page(&cc->cpages[i]);
> + }
> out_put_cic:
> kmem_cache_free(cic_entry_slab, cic);
> out_put_dnode:
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-09 20:34 ` [PATCH 3/3] f2fs: fix false alarm on invalid block address Jaegeuk Kim
@ 2024-04-11 9:08 ` Chao Yu
2024-04-12 20:56 ` Jaegeuk Kim
0 siblings, 1 reply; 18+ messages in thread
From: Chao Yu @ 2024-04-11 9:08 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2024/4/10 4:34, Jaegeuk Kim wrote:
> f2fs_ra_meta_pages can try to read ahead on invalid block address which is
> not the corruption case.
In which case we will read ahead invalid meta pages? recovery w/ META_POR?
Thanks,
>
> Fixes: 31f85ccc84b8 ("f2fs: unify the error handling of f2fs_is_valid_blkaddr")
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/checkpoint.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index eac698b8dd38..b01320502624 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -179,22 +179,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> break;
> case META_SIT:
> if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
> - goto err;
> + goto check_only;
> break;
> case META_SSA:
> if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
> blkaddr < SM_I(sbi)->ssa_blkaddr))
> - goto err;
> + goto check_only;
> break;
> case META_CP:
> if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
> blkaddr < __start_cp_addr(sbi)))
> - goto err;
> + goto check_only;
> break;
> case META_POR:
> if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> blkaddr < MAIN_BLKADDR(sbi)))
> - goto err;
> + goto check_only;
> break;
> case DATA_GENERIC:
> case DATA_GENERIC_ENHANCE:
> @@ -228,6 +228,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> return true;
> err:
> f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
> +check_only:
> return false;
> }
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-11 9:08 ` [f2fs-dev] " Chao Yu
@ 2024-04-12 20:56 ` Jaegeuk Kim
2024-04-19 10:27 ` Juhyung Park
0 siblings, 1 reply; 18+ messages in thread
From: Jaegeuk Kim @ 2024-04-12 20:56 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 04/11, Chao Yu wrote:
> On 2024/4/10 4:34, Jaegeuk Kim wrote:
> > f2fs_ra_meta_pages can try to read ahead on invalid block address which is
> > not the corruption case.
>
> In which case we will read ahead invalid meta pages? recovery w/ META_POR?
I was trying to debug another issue, but found the root cause. Let me drop this
patch.
>
> Thanks,
>
> >
> > Fixes: 31f85ccc84b8 ("f2fs: unify the error handling of f2fs_is_valid_blkaddr")
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> > fs/f2fs/checkpoint.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index eac698b8dd38..b01320502624 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -179,22 +179,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> > break;
> > case META_SIT:
> > if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
> > - goto err;
> > + goto check_only;
> > break;
> > case META_SSA:
> > if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
> > blkaddr < SM_I(sbi)->ssa_blkaddr))
> > - goto err;
> > + goto check_only;
> > break;
> > case META_CP:
> > if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
> > blkaddr < __start_cp_addr(sbi)))
> > - goto err;
> > + goto check_only;
> > break;
> > case META_POR:
> > if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> > blkaddr < MAIN_BLKADDR(sbi)))
> > - goto err;
> > + goto check_only;
> > break;
> > case DATA_GENERIC:
> > case DATA_GENERIC_ENHANCE:
> > @@ -228,6 +228,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> > return true;
> > err:
> > f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
> > +check_only:
> > return false;
> > }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: clear writeback when compression failed
2024-04-11 9:05 ` [f2fs-dev] " Chao Yu
@ 2024-04-12 20:57 ` Jaegeuk Kim
0 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2024-04-12 20:57 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 04/11, Chao Yu wrote:
> On 2024/4/10 4:34, Jaegeuk Kim wrote:
> > Let's stop issuing compressed writes and clear their writeback flags.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> > fs/f2fs/compress.c | 33 +++++++++++++++++++++++++++++++--
> > 1 file changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index d67c471ab5df..3a8ecc6aee84 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -1031,6 +1031,25 @@ static void set_cluster_writeback(struct compress_ctx *cc)
> > }
> > }
> > +static void cancel_cluster_writeback(struct compress_ctx *cc, int submitted)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < cc->cluster_size; i++) {
> > + if (!cc->rpages[i])
> > + continue;
> > + if (i < submitted) {
> > + if (i)
> > + f2fs_wait_on_page_writeback(cc->rpages[i],
> > + DATA, true, true);
> > + inode_inc_dirty_pages(cc->inode);
> > + lock_page(cc->rpages[i]);
> > + }
> > + clear_page_private_gcing(cc->rpages[i]);
> > + end_page_writeback(cc->rpages[i]);
> > + }
> > +}
> > +
> > static void set_cluster_dirty(struct compress_ctx *cc)
> > {
> > int i;
> > @@ -1232,7 +1251,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> > .page = NULL,
> > .encrypted_page = NULL,
> > .compressed_page = NULL,
> > - .submitted = 0,
> > .io_type = io_type,
> > .io_wbc = wbc,
> > .encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
> > @@ -1358,7 +1376,15 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> > fio.compressed_page = cc->cpages[i - 1];
> > cc->cpages[i - 1] = NULL;
> > + fio.submitted = 0;
> > f2fs_outplace_write_data(&dn, &fio);
> > + if (unlikely(!fio.submitted)) {
> > + cancel_cluster_writeback(cc, i);
> > +
> > + /* To call fscrypt_finalize_bounce_page */
> > + i = cc->valid_nr_cpages;
>
> *submitted = 0; ?
And, it seems this is not enough to address kernel hang on wait_on_writeback
while running fsstress + shutdown test. Stay tuned.
>
> Thanks,
>
> > + goto out_destroy_crypt;
> > + }
> > (*submitted)++;
> > unlock_continue:
> > inode_dec_dirty_pages(cc->inode);
> > @@ -1392,8 +1418,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> > out_destroy_crypt:
> > page_array_free(cc->inode, cic->rpages, cc->cluster_size);
> > - for (--i; i >= 0; i--)
> > + for (--i; i >= 0; i--) {
> > + if (!cc->cpages[i])
> > + continue;
> > fscrypt_finalize_bounce_page(&cc->cpages[i]);
> > + }
> > out_put_cic:
> > kmem_cache_free(cic_entry_slab, cic);
> > out_put_dnode:
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 1/3] f2fs: use folio_test_writeback
2024-04-09 20:34 [PATCH 1/3] f2fs: use folio_test_writeback Jaegeuk Kim
` (2 preceding siblings ...)
2024-04-11 8:56 ` [f2fs-dev] [PATCH 1/3] f2fs: use folio_test_writeback Chao Yu
@ 2024-04-14 15:32 ` patchwork-bot+f2fs
2024-04-24 17:40 ` patchwork-bot+f2fs
4 siblings, 0 replies; 18+ messages in thread
From: patchwork-bot+f2fs @ 2024-04-14 15:32 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Tue, 9 Apr 2024 20:34:09 +0000 you wrote:
> Let's convert PageWriteback to folio_test_writeback.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/compress.c | 2 +-
> fs/f2fs/data.c | 3 +--
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/gc.c | 2 +-
> fs/f2fs/inline.c | 2 +-
> fs/f2fs/inode.c | 3 ++-
> fs/f2fs/node.c | 2 +-
> fs/f2fs/segment.c | 10 +++++-----
> 8 files changed, 13 insertions(+), 13 deletions(-)
Here is the summary with links:
- [f2fs-dev,1/3] f2fs: use folio_test_writeback
https://git.kernel.org/jaegeuk/f2fs/c/16778aea9186
- [f2fs-dev,2/3] f2fs: clear writeback when compression failed
(no matching commit)
- [f2fs-dev,3/3] f2fs: fix false alarm on invalid block address
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3 v2] f2fs: clear writeback when compression failed
2024-04-09 20:34 ` [PATCH 2/3] f2fs: clear writeback when compression failed Jaegeuk Kim
2024-04-11 9:05 ` [f2fs-dev] " Chao Yu
@ 2024-04-16 16:49 ` Jaegeuk Kim
2024-04-23 19:47 ` [f2fs-dev] " Daeho Jeong
2024-04-25 2:15 ` Chao Yu
1 sibling, 2 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2024-04-16 16:49 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel
Let's stop issuing compressed writes and clear their writeback flags.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
Now, I don't see any kernel hang for 24hours.
Change log from v1:
- fix bugs
fs/f2fs/compress.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d67c471ab5df..b12d3a49bfda 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1031,6 +1031,31 @@ static void set_cluster_writeback(struct compress_ctx *cc)
}
}
+static void cancel_cluster_writeback(struct compress_ctx *cc,
+ struct compress_io_ctx *cic, int submitted)
+{
+ int i;
+
+ /* Wait for submitted IOs. */
+ if (submitted > 1) {
+ f2fs_submit_merged_write(F2FS_I_SB(cc->inode), DATA);
+ while (atomic_read(&cic->pending_pages) !=
+ (cc->valid_nr_cpages - submitted + 1))
+ f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
+ }
+
+ /* Cancel writeback and stay locked. */
+ for (i = 0; i < cc->cluster_size; i++) {
+ if (i < submitted) {
+ inode_inc_dirty_pages(cc->inode);
+ lock_page(cc->rpages[i]);
+ }
+ clear_page_private_gcing(cc->rpages[i]);
+ if (folio_test_writeback(page_folio(cc->rpages[i])))
+ end_page_writeback(cc->rpages[i]);
+ }
+}
+
static void set_cluster_dirty(struct compress_ctx *cc)
{
int i;
@@ -1232,7 +1257,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
.page = NULL,
.encrypted_page = NULL,
.compressed_page = NULL,
- .submitted = 0,
.io_type = io_type,
.io_wbc = wbc,
.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
@@ -1358,7 +1382,16 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
fio.compressed_page = cc->cpages[i - 1];
cc->cpages[i - 1] = NULL;
+ fio.submitted = 0;
f2fs_outplace_write_data(&dn, &fio);
+ if (unlikely(!fio.submitted)) {
+ cancel_cluster_writeback(cc, cic, i);
+
+ /* To call fscrypt_finalize_bounce_page */
+ i = cc->valid_nr_cpages;
+ *submitted = 0;
+ goto out_destroy_crypt;
+ }
(*submitted)++;
unlock_continue:
inode_dec_dirty_pages(cc->inode);
@@ -1392,8 +1425,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
out_destroy_crypt:
page_array_free(cc->inode, cic->rpages, cc->cluster_size);
- for (--i; i >= 0; i--)
+ for (--i; i >= 0; i--) {
+ if (!cc->cpages[i])
+ continue;
fscrypt_finalize_bounce_page(&cc->cpages[i]);
+ }
out_put_cic:
kmem_cache_free(cic_entry_slab, cic);
out_put_dnode:
--
2.44.0.683.g7961c838ac-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-12 20:56 ` Jaegeuk Kim
@ 2024-04-19 10:27 ` Juhyung Park
2024-04-19 17:50 ` Jaegeuk Kim
2024-04-25 7:29 ` Chao Yu
0 siblings, 2 replies; 18+ messages in thread
From: Juhyung Park @ 2024-04-19 10:27 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: Chao Yu, linux-kernel, linux-f2fs-devel
On Sat, Apr 13, 2024 at 5:57 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> On 04/11, Chao Yu wrote:
> > On 2024/4/10 4:34, Jaegeuk Kim wrote:
> > > f2fs_ra_meta_pages can try to read ahead on invalid block address which is
> > > not the corruption case.
> >
> > In which case we will read ahead invalid meta pages? recovery w/ META_POR?
In my case, it seems like it's META_SIT, and it's triggered right after mount.
fsck detects invalid_blkaddr, and when the kernel mounts it, it
immediately flags invalid_blkaddr again:
[ 6.333498] init: [libfs_mgr] Running /system/bin/fsck.f2fs -a -c
10000 --debug-cache /dev/block/sda13
[ 6.337671] fsck.f2fs: Info: Fix the reported corruption.
[ 6.337947] fsck.f2fs: Info: not exist /proc/version!
[ 6.338010] fsck.f2fs: Info: can't find /sys, assuming normal block device
[ 6.338294] fsck.f2fs: Info: MKFS version
[ 6.338319] fsck.f2fs: "5.10.160-android12-9-ge5cfec41c8e2"
[ 6.338366] fsck.f2fs: Info: FSCK version
[ 6.338380] fsck.f2fs: from "5.10-arter97"
[ 6.338393] fsck.f2fs: to "5.10-arter97"
[ 6.338414] fsck.f2fs: Info: superblock features = 1499 : encrypt
verity extra_attr project_quota quota_ino casefold
[ 6.338429] fsck.f2fs: Info: superblock encrypt level = 0, salt =
00000000000000000000000000000000
[ 6.338442] fsck.f2fs: Info: checkpoint stop reason: shutdown(180)
[ 6.338455] fsck.f2fs: Info: fs errors: invalid_blkaddr
[ 6.338468] fsck.f2fs: Info: Segments per section = 1
[ 6.338480] fsck.f2fs: Info: Sections per zone = 1
[ 6.338492] fsck.f2fs: Info: total FS sectors = 58971571 (230357 MB)
[ 6.340599] fsck.f2fs: Info: CKPT version = 2b7e3b29
[ 6.340620] fsck.f2fs: Info: version timestamp cur: 19789296, prev: 18407008
[ 6.677041] fsck.f2fs: Info: checkpoint state = 46 : crc
compacted_summary orphan_inodes sudden-power-off
[ 6.677052] fsck.f2fs: [FSCK] Check node 1 / 712937 (0.00%)
[ 8.997922] fsck.f2fs: [FSCK] Check node 71294 / 712937 (10.00%)
[ 10.629205] fsck.f2fs: [FSCK] Check node 142587 / 712937 (20.00%)
[ 12.278186] fsck.f2fs: [FSCK] Check node 213880 / 712937 (30.00%)
[ 13.768177] fsck.f2fs: [FSCK] Check node 285173 / 712937 (40.00%)
[ 17.446971] fsck.f2fs: [FSCK] Check node 356466 / 712937 (50.00%)
[ 19.891623] fsck.f2fs: [FSCK] Check node 427759 / 712937 (60.00%)
[ 23.251327] fsck.f2fs: [FSCK] Check node 499052 / 712937 (70.00%)
[ 28.493457] fsck.f2fs: [FSCK] Check node 570345 / 712937 (80.00%)
[ 29.640800] fsck.f2fs: [FSCK] Check node 641638 / 712937 (90.00%)
[ 30.718347] fsck.f2fs: [FSCK] Check node 712931 / 712937 (100.00%)
[ 30.724176] fsck.f2fs:
[ 30.737160] fsck.f2fs: [FSCK] Max image size: 167506 MB, Free space: 62850 MB
[ 30.737164] fsck.f2fs: [FSCK] Unreachable nat entries
[Ok..] [0x0]
[ 30.737638] fsck.f2fs: [FSCK] SIT valid block bitmap checking
[Ok..]
[ 30.737640] fsck.f2fs: [FSCK] Hard link checking for regular file
[Ok..] [0xd]
[ 30.737641] fsck.f2fs: [FSCK] valid_block_count matching with CP
[Ok..] [0x28b98e6]
[ 30.737644] fsck.f2fs: [FSCK] valid_node_count matching with CP (de
lookup) [Ok..] [0xae0e9]
[ 30.737646] fsck.f2fs: [FSCK] valid_node_count matching with CP
(nat lookup) [Ok..] [0xae0e9]
[ 30.737647] fsck.f2fs: [FSCK] valid_inode_count matched with CP
[Ok..] [0xa74a3]
[ 30.737649] fsck.f2fs: [FSCK] free segment_count matched with CP
[Ok..] [0x7aa3]
[ 30.737662] fsck.f2fs: [FSCK] next block offset is free
[Ok..]
[ 30.737663] fsck.f2fs: [FSCK] fixing SIT types
[ 30.737867] fsck.f2fs: [FSCK] other corrupted bugs
[Ok..]
[ 30.737893] fsck.f2fs: [update_superblock: 765] Info: Done to
update superblock
[ 30.960610] fsck.f2fs:
[ 30.960618] fsck.f2fs: Done: 24.622956 secs
[ 30.960620] fsck.f2fs:
[ 30.960622] fsck.f2fs: c, u, RA, CH, CM, Repl=
[ 30.960627] fsck.f2fs: 10000 10000 43600517 42605434 995083 985083
[ 30.963274] F2FS-fs (sda13): Using encoding defined by superblock:
utf8-12.1.0 with flags 0x0
[ 30.995360] __f2fs_is_valid_blkaddr: type=2
(Manually added that print ^)
[ 30.995369] ------------[ cut here ]------------
[ 30.995375] WARNING: CPU: 7 PID: 1 at f2fs_handle_error+0x18/0x3c
[ 30.995378] CPU: 7 PID: 1 Comm: init Tainted: G S W
5.10.209-arter97-r15-kernelsu-g0867d0e4f1d2 #6
[ 30.995379] Hardware name: Qualcomm Technologies, Inc. Cape QRD
with PM8010 (DT)
[ 30.995380] pstate: 22400005 (nzCv daif +PAN -UAO +TCO BTYPE=--)
[ 30.995382] pc : f2fs_handle_error+0x18/0x3c
[ 30.995384] lr : __f2fs_is_valid_blkaddr+0x2a4/0x2b0
[ 30.995385] sp : ffffff80209e79b0
[ 30.995386] x29: ffffff80209e79b0 x28: 0000000000000037
[ 30.995388] x27: 00000000000001c7 x26: 0000000020120121
[ 30.995389] x25: 00000000000000d9 x24: 0000000000000000
[ 30.995390] x23: ffffffff00f1a700 x22: 0000000000000828
[ 30.995391] x21: ffffff80462aa000 x20: ffffff80462aa000
[ 30.995392] x19: 0000000000000002 x18: ffffffffffffffff
[ 30.995393] x17: 0000000000000000 x16: 00000000ffff0000
[ 30.995394] x15: 0000000000000004 x14: ffffffd1675ac6d0
[ 30.995395] x13: 0000000000000003 x12: 0000000000000003
[ 30.995396] x11: 00000000ffffffff x10: 0000000000000000
[ 30.995397] x9 : 0000000100000001 x8 : 0000000100000000
[ 30.995398] x7 : 64696c61765f7369 x6 : ffffffd1681279e8
[ 30.995399] x5 : 000000000000001f x4 : 0000000000000001
[ 30.995400] x3 : 0000000000000000 x2 : ffffff89f03dedc8
[ 30.995401] x1 : 0000000000000002 x0 : ffffff80462aa000
[ 30.995403] Call trace:
[ 30.995404] f2fs_handle_error+0x18/0x3c
[ 30.995405] __f2fs_is_valid_blkaddr+0x2a4/0x2b0
[ 30.995406] f2fs_is_valid_blkaddr+0x10/0x20
[ 30.995407] f2fs_ra_meta_pages+0xe0/0x230
[ 30.995409] build_sit_entries+0xa8/0x580
[ 30.995411] f2fs_build_segment_manager+0x124/0x170
[ 30.995412] f2fs_fill_super+0x78c/0xd1c
[ 30.995415] mount_bdev+0x168/0x1ac
[ 30.995416] f2fs_mount+0x18/0x24
[ 30.995418] legacy_get_tree.llvm.9147845779559715083+0x30/0x5c
[ 30.995419] vfs_get_tree+0x30/0xe0
[ 30.995421] do_new_mount+0x140/0x358
[ 30.995422] path_mount+0x1fc/0x4e8
[ 30.995423] __arm64_sys_mount+0x150/0x294
[ 30.995425] el0_svc_common.llvm.15698454952154965787+0xa8/0x138
[ 30.995426] do_el0_svc+0x24/0x90
[ 30.995429] el0_svc+0x10/0x1c
[ 30.995430] el0_sync_handler+0xcc/0xe4
[ 30.995432] el0_sync+0x1a0/0x1c0
[ 30.995433] ---[ end trace 3b83295e0cdac94e ]---
[ 31.005011] F2FS-fs (sda13): Mounted with checkpoint version = 2b7e3b29
[ 31.005176] init: [libfs_mgr]
__mount(source=/dev/block/bootdevice/by-name/userdata,target=/data,type=f2fs)=0:
Success
[ 31.007749] init: Userdata mounted using /vendor/etc/fstab.qcom result : 0
I was bisecting a long boot time (24 additional seconds) issue, which
is always reproducible, and found commit 31f85ccc84b8 ("f2fs: unify
the error handling of f2fs_is_valid_blkaddr") to be causing it.
I'll just revert that patch locally. Seems like Jaegeuk's dev branch
doesn't have the fix for this specifically yet.
Thanks.
>
> I was trying to debug another issue, but found the root cause. Let me drop this
> patch.
>
> >
> > Thanks,
> >
> > >
> > > Fixes: 31f85ccc84b8 ("f2fs: unify the error handling of f2fs_is_valid_blkaddr")
> > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > ---
> > > fs/f2fs/checkpoint.c | 9 +++++----
> > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > > index eac698b8dd38..b01320502624 100644
> > > --- a/fs/f2fs/checkpoint.c
> > > +++ b/fs/f2fs/checkpoint.c
> > > @@ -179,22 +179,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> > > break;
> > > case META_SIT:
> > > if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
> > > - goto err;
> > > + goto check_only;
> > > break;
> > > case META_SSA:
> > > if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
> > > blkaddr < SM_I(sbi)->ssa_blkaddr))
> > > - goto err;
> > > + goto check_only;
> > > break;
> > > case META_CP:
> > > if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
> > > blkaddr < __start_cp_addr(sbi)))
> > > - goto err;
> > > + goto check_only;
> > > break;
> > > case META_POR:
> > > if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> > > blkaddr < MAIN_BLKADDR(sbi)))
> > > - goto err;
> > > + goto check_only;
> > > break;
> > > case DATA_GENERIC:
> > > case DATA_GENERIC_ENHANCE:
> > > @@ -228,6 +228,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> > > return true;
> > > err:
> > > f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
> > > +check_only:
> > > return false;
> > > }
>
>
> _______________________________________________
> 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] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-19 10:27 ` Juhyung Park
@ 2024-04-19 17:50 ` Jaegeuk Kim
2024-04-25 7:29 ` Chao Yu
1 sibling, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2024-04-19 17:50 UTC (permalink / raw)
To: Juhyung Park; +Cc: Chao Yu, linux-kernel, linux-f2fs-devel
On 04/19, Juhyung Park wrote:
> On Sat, Apr 13, 2024 at 5:57 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> >
> > On 04/11, Chao Yu wrote:
> > > On 2024/4/10 4:34, Jaegeuk Kim wrote:
> > > > f2fs_ra_meta_pages can try to read ahead on invalid block address which is
> > > > not the corruption case.
> > >
> > > In which case we will read ahead invalid meta pages? recovery w/ META_POR?
>
> In my case, it seems like it's META_SIT, and it's triggered right after mount.
> fsck detects invalid_blkaddr, and when the kernel mounts it, it
> immediately flags invalid_blkaddr again:
>
> [ 6.333498] init: [libfs_mgr] Running /system/bin/fsck.f2fs -a -c
> 10000 --debug-cache /dev/block/sda13
> [ 6.337671] fsck.f2fs: Info: Fix the reported corruption.
> [ 6.337947] fsck.f2fs: Info: not exist /proc/version!
> [ 6.338010] fsck.f2fs: Info: can't find /sys, assuming normal block device
> [ 6.338294] fsck.f2fs: Info: MKFS version
> [ 6.338319] fsck.f2fs: "5.10.160-android12-9-ge5cfec41c8e2"
> [ 6.338366] fsck.f2fs: Info: FSCK version
> [ 6.338380] fsck.f2fs: from "5.10-arter97"
> [ 6.338393] fsck.f2fs: to "5.10-arter97"
> [ 6.338414] fsck.f2fs: Info: superblock features = 1499 : encrypt
> verity extra_attr project_quota quota_ino casefold
> [ 6.338429] fsck.f2fs: Info: superblock encrypt level = 0, salt =
> 00000000000000000000000000000000
> [ 6.338442] fsck.f2fs: Info: checkpoint stop reason: shutdown(180)
> [ 6.338455] fsck.f2fs: Info: fs errors: invalid_blkaddr
> [ 6.338468] fsck.f2fs: Info: Segments per section = 1
> [ 6.338480] fsck.f2fs: Info: Sections per zone = 1
> [ 6.338492] fsck.f2fs: Info: total FS sectors = 58971571 (230357 MB)
> [ 6.340599] fsck.f2fs: Info: CKPT version = 2b7e3b29
> [ 6.340620] fsck.f2fs: Info: version timestamp cur: 19789296, prev: 18407008
> [ 6.677041] fsck.f2fs: Info: checkpoint state = 46 : crc
> compacted_summary orphan_inodes sudden-power-off
> [ 6.677052] fsck.f2fs: [FSCK] Check node 1 / 712937 (0.00%)
> [ 8.997922] fsck.f2fs: [FSCK] Check node 71294 / 712937 (10.00%)
> [ 10.629205] fsck.f2fs: [FSCK] Check node 142587 / 712937 (20.00%)
> [ 12.278186] fsck.f2fs: [FSCK] Check node 213880 / 712937 (30.00%)
> [ 13.768177] fsck.f2fs: [FSCK] Check node 285173 / 712937 (40.00%)
> [ 17.446971] fsck.f2fs: [FSCK] Check node 356466 / 712937 (50.00%)
> [ 19.891623] fsck.f2fs: [FSCK] Check node 427759 / 712937 (60.00%)
> [ 23.251327] fsck.f2fs: [FSCK] Check node 499052 / 712937 (70.00%)
> [ 28.493457] fsck.f2fs: [FSCK] Check node 570345 / 712937 (80.00%)
> [ 29.640800] fsck.f2fs: [FSCK] Check node 641638 / 712937 (90.00%)
> [ 30.718347] fsck.f2fs: [FSCK] Check node 712931 / 712937 (100.00%)
> [ 30.724176] fsck.f2fs:
> [ 30.737160] fsck.f2fs: [FSCK] Max image size: 167506 MB, Free space: 62850 MB
> [ 30.737164] fsck.f2fs: [FSCK] Unreachable nat entries
> [Ok..] [0x0]
> [ 30.737638] fsck.f2fs: [FSCK] SIT valid block bitmap checking
> [Ok..]
> [ 30.737640] fsck.f2fs: [FSCK] Hard link checking for regular file
> [Ok..] [0xd]
> [ 30.737641] fsck.f2fs: [FSCK] valid_block_count matching with CP
> [Ok..] [0x28b98e6]
> [ 30.737644] fsck.f2fs: [FSCK] valid_node_count matching with CP (de
> lookup) [Ok..] [0xae0e9]
> [ 30.737646] fsck.f2fs: [FSCK] valid_node_count matching with CP
> (nat lookup) [Ok..] [0xae0e9]
> [ 30.737647] fsck.f2fs: [FSCK] valid_inode_count matched with CP
> [Ok..] [0xa74a3]
> [ 30.737649] fsck.f2fs: [FSCK] free segment_count matched with CP
> [Ok..] [0x7aa3]
> [ 30.737662] fsck.f2fs: [FSCK] next block offset is free
> [Ok..]
> [ 30.737663] fsck.f2fs: [FSCK] fixing SIT types
> [ 30.737867] fsck.f2fs: [FSCK] other corrupted bugs
> [Ok..]
> [ 30.737893] fsck.f2fs: [update_superblock: 765] Info: Done to
> update superblock
> [ 30.960610] fsck.f2fs:
> [ 30.960618] fsck.f2fs: Done: 24.622956 secs
> [ 30.960620] fsck.f2fs:
> [ 30.960622] fsck.f2fs: c, u, RA, CH, CM, Repl=
> [ 30.960627] fsck.f2fs: 10000 10000 43600517 42605434 995083 985083
> [ 30.963274] F2FS-fs (sda13): Using encoding defined by superblock:
> utf8-12.1.0 with flags 0x0
> [ 30.995360] __f2fs_is_valid_blkaddr: type=2
>
> (Manually added that print ^)
>
> [ 30.995369] ------------[ cut here ]------------
> [ 30.995375] WARNING: CPU: 7 PID: 1 at f2fs_handle_error+0x18/0x3c
> [ 30.995378] CPU: 7 PID: 1 Comm: init Tainted: G S W
> 5.10.209-arter97-r15-kernelsu-g0867d0e4f1d2 #6
> [ 30.995379] Hardware name: Qualcomm Technologies, Inc. Cape QRD
> with PM8010 (DT)
> [ 30.995380] pstate: 22400005 (nzCv daif +PAN -UAO +TCO BTYPE=--)
> [ 30.995382] pc : f2fs_handle_error+0x18/0x3c
> [ 30.995384] lr : __f2fs_is_valid_blkaddr+0x2a4/0x2b0
> [ 30.995385] sp : ffffff80209e79b0
> [ 30.995386] x29: ffffff80209e79b0 x28: 0000000000000037
> [ 30.995388] x27: 00000000000001c7 x26: 0000000020120121
> [ 30.995389] x25: 00000000000000d9 x24: 0000000000000000
> [ 30.995390] x23: ffffffff00f1a700 x22: 0000000000000828
> [ 30.995391] x21: ffffff80462aa000 x20: ffffff80462aa000
> [ 30.995392] x19: 0000000000000002 x18: ffffffffffffffff
> [ 30.995393] x17: 0000000000000000 x16: 00000000ffff0000
> [ 30.995394] x15: 0000000000000004 x14: ffffffd1675ac6d0
> [ 30.995395] x13: 0000000000000003 x12: 0000000000000003
> [ 30.995396] x11: 00000000ffffffff x10: 0000000000000000
> [ 30.995397] x9 : 0000000100000001 x8 : 0000000100000000
> [ 30.995398] x7 : 64696c61765f7369 x6 : ffffffd1681279e8
> [ 30.995399] x5 : 000000000000001f x4 : 0000000000000001
> [ 30.995400] x3 : 0000000000000000 x2 : ffffff89f03dedc8
> [ 30.995401] x1 : 0000000000000002 x0 : ffffff80462aa000
> [ 30.995403] Call trace:
> [ 30.995404] f2fs_handle_error+0x18/0x3c
> [ 30.995405] __f2fs_is_valid_blkaddr+0x2a4/0x2b0
> [ 30.995406] f2fs_is_valid_blkaddr+0x10/0x20
> [ 30.995407] f2fs_ra_meta_pages+0xe0/0x230
> [ 30.995409] build_sit_entries+0xa8/0x580
> [ 30.995411] f2fs_build_segment_manager+0x124/0x170
> [ 30.995412] f2fs_fill_super+0x78c/0xd1c
> [ 30.995415] mount_bdev+0x168/0x1ac
> [ 30.995416] f2fs_mount+0x18/0x24
> [ 30.995418] legacy_get_tree.llvm.9147845779559715083+0x30/0x5c
> [ 30.995419] vfs_get_tree+0x30/0xe0
> [ 30.995421] do_new_mount+0x140/0x358
> [ 30.995422] path_mount+0x1fc/0x4e8
> [ 30.995423] __arm64_sys_mount+0x150/0x294
> [ 30.995425] el0_svc_common.llvm.15698454952154965787+0xa8/0x138
> [ 30.995426] do_el0_svc+0x24/0x90
> [ 30.995429] el0_svc+0x10/0x1c
> [ 30.995430] el0_sync_handler+0xcc/0xe4
> [ 30.995432] el0_sync+0x1a0/0x1c0
> [ 30.995433] ---[ end trace 3b83295e0cdac94e ]---
> [ 31.005011] F2FS-fs (sda13): Mounted with checkpoint version = 2b7e3b29
> [ 31.005176] init: [libfs_mgr]
> __mount(source=/dev/block/bootdevice/by-name/userdata,target=/data,type=f2fs)=0:
> Success
> [ 31.007749] init: Userdata mounted using /vendor/etc/fstab.qcom result : 0
>
>
> I was bisecting a long boot time (24 additional seconds) issue, which
> is always reproducible, and found commit 31f85ccc84b8 ("f2fs: unify
> the error handling of f2fs_is_valid_blkaddr") to be causing it.
>
> I'll just revert that patch locally. Seems like Jaegeuk's dev branch
> doesn't have the fix for this specifically yet.
I was suspecting f2fs_ra_meta_pages() could try to read out beyond the boundary
before, but it seems you hit the case. Can you check this patch address that?
>
> Thanks.
>
> >
> > I was trying to debug another issue, but found the root cause. Let me drop this
> > patch.
> >
> > >
> > > Thanks,
> > >
> > > >
> > > > Fixes: 31f85ccc84b8 ("f2fs: unify the error handling of f2fs_is_valid_blkaddr")
> > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > > ---
> > > > fs/f2fs/checkpoint.c | 9 +++++----
> > > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > > > index eac698b8dd38..b01320502624 100644
> > > > --- a/fs/f2fs/checkpoint.c
> > > > +++ b/fs/f2fs/checkpoint.c
> > > > @@ -179,22 +179,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> > > > break;
> > > > case META_SIT:
> > > > if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
> > > > - goto err;
> > > > + goto check_only;
> > > > break;
> > > > case META_SSA:
> > > > if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
> > > > blkaddr < SM_I(sbi)->ssa_blkaddr))
> > > > - goto err;
> > > > + goto check_only;
> > > > break;
> > > > case META_CP:
> > > > if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
> > > > blkaddr < __start_cp_addr(sbi)))
> > > > - goto err;
> > > > + goto check_only;
> > > > break;
> > > > case META_POR:
> > > > if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> > > > blkaddr < MAIN_BLKADDR(sbi)))
> > > > - goto err;
> > > > + goto check_only;
> > > > break;
> > > > case DATA_GENERIC:
> > > > case DATA_GENERIC_ENHANCE:
> > > > @@ -228,6 +228,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> > > > return true;
> > > > err:
> > > > f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
> > > > +check_only:
> > > > return false;
> > > > }
> >
> >
> > _______________________________________________
> > 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] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3 v2] f2fs: clear writeback when compression failed
2024-04-16 16:49 ` [PATCH 2/3 v2] " Jaegeuk Kim
@ 2024-04-23 19:47 ` Daeho Jeong
2024-04-25 2:15 ` Chao Yu
1 sibling, 0 replies; 18+ messages in thread
From: Daeho Jeong @ 2024-04-23 19:47 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On Tue, Apr 16, 2024 at 9:50 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> Let's stop issuing compressed writes and clear their writeback flags.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>
> Now, I don't see any kernel hang for 24hours.
>
> Change log from v1:
> - fix bugs
>
> fs/f2fs/compress.c | 40 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index d67c471ab5df..b12d3a49bfda 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1031,6 +1031,31 @@ static void set_cluster_writeback(struct compress_ctx *cc)
> }
> }
>
> +static void cancel_cluster_writeback(struct compress_ctx *cc,
> + struct compress_io_ctx *cic, int submitted)
> +{
> + int i;
> +
> + /* Wait for submitted IOs. */
> + if (submitted > 1) {
> + f2fs_submit_merged_write(F2FS_I_SB(cc->inode), DATA);
> + while (atomic_read(&cic->pending_pages) !=
> + (cc->valid_nr_cpages - submitted + 1))
> + f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
> + }
> +
> + /* Cancel writeback and stay locked. */
> + for (i = 0; i < cc->cluster_size; i++) {
> + if (i < submitted) {
> + inode_inc_dirty_pages(cc->inode);
> + lock_page(cc->rpages[i]);
> + }
> + clear_page_private_gcing(cc->rpages[i]);
> + if (folio_test_writeback(page_folio(cc->rpages[i])))
> + end_page_writeback(cc->rpages[i]);
> + }
> +}
> +
> static void set_cluster_dirty(struct compress_ctx *cc)
> {
> int i;
> @@ -1232,7 +1257,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> .page = NULL,
> .encrypted_page = NULL,
> .compressed_page = NULL,
> - .submitted = 0,
> .io_type = io_type,
> .io_wbc = wbc,
> .encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
> @@ -1358,7 +1382,16 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> fio.compressed_page = cc->cpages[i - 1];
>
> cc->cpages[i - 1] = NULL;
> + fio.submitted = 0;
> f2fs_outplace_write_data(&dn, &fio);
> + if (unlikely(!fio.submitted)) {
> + cancel_cluster_writeback(cc, cic, i);
> +
> + /* To call fscrypt_finalize_bounce_page */
> + i = cc->valid_nr_cpages;
> + *submitted = 0;
> + goto out_destroy_crypt;
> + }
> (*submitted)++;
> unlock_continue:
> inode_dec_dirty_pages(cc->inode);
> @@ -1392,8 +1425,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> out_destroy_crypt:
> page_array_free(cc->inode, cic->rpages, cc->cluster_size);
>
> - for (--i; i >= 0; i--)
> + for (--i; i >= 0; i--) {
> + if (!cc->cpages[i])
> + continue;
> fscrypt_finalize_bounce_page(&cc->cpages[i]);
> + }
> out_put_cic:
> kmem_cache_free(cic_entry_slab, cic);
> out_put_dnode:
> --
> 2.44.0.683.g7961c838ac-goog
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Reviewed-by: Daeho Jeong <daehojeong@google.com>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 1/3] f2fs: use folio_test_writeback
2024-04-09 20:34 [PATCH 1/3] f2fs: use folio_test_writeback Jaegeuk Kim
` (3 preceding siblings ...)
2024-04-14 15:32 ` patchwork-bot+f2fs
@ 2024-04-24 17:40 ` patchwork-bot+f2fs
4 siblings, 0 replies; 18+ messages in thread
From: patchwork-bot+f2fs @ 2024-04-24 17:40 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Tue, 9 Apr 2024 20:34:09 +0000 you wrote:
> Let's convert PageWriteback to folio_test_writeback.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/compress.c | 2 +-
> fs/f2fs/data.c | 3 +--
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/gc.c | 2 +-
> fs/f2fs/inline.c | 2 +-
> fs/f2fs/inode.c | 3 ++-
> fs/f2fs/node.c | 2 +-
> fs/f2fs/segment.c | 10 +++++-----
> 8 files changed, 13 insertions(+), 13 deletions(-)
Here is the summary with links:
- [f2fs-dev,1/3] f2fs: use folio_test_writeback
(no matching commit)
- [f2fs-dev,2/3] f2fs: clear writeback when compression failed
(no matching commit)
- [f2fs-dev,3/3] f2fs: fix false alarm on invalid block address
https://git.kernel.org/jaegeuk/f2fs/c/04890641bf1f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3 v2] f2fs: clear writeback when compression failed
2024-04-16 16:49 ` [PATCH 2/3 v2] " Jaegeuk Kim
2024-04-23 19:47 ` [f2fs-dev] " Daeho Jeong
@ 2024-04-25 2:15 ` Chao Yu
1 sibling, 0 replies; 18+ messages in thread
From: Chao Yu @ 2024-04-25 2:15 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2024/4/17 0:49, Jaegeuk Kim wrote:
> Let's stop issuing compressed writes and clear their writeback flags.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-19 10:27 ` Juhyung Park
2024-04-19 17:50 ` Jaegeuk Kim
@ 2024-04-25 7:29 ` Chao Yu
2024-04-28 1:23 ` Daeho Jeong
1 sibling, 1 reply; 18+ messages in thread
From: Chao Yu @ 2024-04-25 7:29 UTC (permalink / raw)
To: Juhyung Park, Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2024/4/19 18:27, Juhyung Park wrote:
> On Sat, Apr 13, 2024 at 5:57 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>>
>> On 04/11, Chao Yu wrote:
>>> On 2024/4/10 4:34, Jaegeuk Kim wrote:
>>>> f2fs_ra_meta_pages can try to read ahead on invalid block address which is
>>>> not the corruption case.
>>>
>>> In which case we will read ahead invalid meta pages? recovery w/ META_POR?
>
> In my case, it seems like it's META_SIT, and it's triggered right after mount.
Ah, I see, actually it hits at this case, thanks for the information.
Thanks,
> fsck detects invalid_blkaddr, and when the kernel mounts it, it
> immediately flags invalid_blkaddr again:
>
> [ 6.333498] init: [libfs_mgr] Running /system/bin/fsck.f2fs -a -c
> 10000 --debug-cache /dev/block/sda13
> [ 6.337671] fsck.f2fs: Info: Fix the reported corruption.
> [ 6.337947] fsck.f2fs: Info: not exist /proc/version!
> [ 6.338010] fsck.f2fs: Info: can't find /sys, assuming normal block device
> [ 6.338294] fsck.f2fs: Info: MKFS version
> [ 6.338319] fsck.f2fs: "5.10.160-android12-9-ge5cfec41c8e2"
> [ 6.338366] fsck.f2fs: Info: FSCK version
> [ 6.338380] fsck.f2fs: from "5.10-arter97"
> [ 6.338393] fsck.f2fs: to "5.10-arter97"
> [ 6.338414] fsck.f2fs: Info: superblock features = 1499 : encrypt
> verity extra_attr project_quota quota_ino casefold
> [ 6.338429] fsck.f2fs: Info: superblock encrypt level = 0, salt =
> 00000000000000000000000000000000
> [ 6.338442] fsck.f2fs: Info: checkpoint stop reason: shutdown(180)
> [ 6.338455] fsck.f2fs: Info: fs errors: invalid_blkaddr
> [ 6.338468] fsck.f2fs: Info: Segments per section = 1
> [ 6.338480] fsck.f2fs: Info: Sections per zone = 1
> [ 6.338492] fsck.f2fs: Info: total FS sectors = 58971571 (230357 MB)
> [ 6.340599] fsck.f2fs: Info: CKPT version = 2b7e3b29
> [ 6.340620] fsck.f2fs: Info: version timestamp cur: 19789296, prev: 18407008
> [ 6.677041] fsck.f2fs: Info: checkpoint state = 46 : crc
> compacted_summary orphan_inodes sudden-power-off
> [ 6.677052] fsck.f2fs: [FSCK] Check node 1 / 712937 (0.00%)
> [ 8.997922] fsck.f2fs: [FSCK] Check node 71294 / 712937 (10.00%)
> [ 10.629205] fsck.f2fs: [FSCK] Check node 142587 / 712937 (20.00%)
> [ 12.278186] fsck.f2fs: [FSCK] Check node 213880 / 712937 (30.00%)
> [ 13.768177] fsck.f2fs: [FSCK] Check node 285173 / 712937 (40.00%)
> [ 17.446971] fsck.f2fs: [FSCK] Check node 356466 / 712937 (50.00%)
> [ 19.891623] fsck.f2fs: [FSCK] Check node 427759 / 712937 (60.00%)
> [ 23.251327] fsck.f2fs: [FSCK] Check node 499052 / 712937 (70.00%)
> [ 28.493457] fsck.f2fs: [FSCK] Check node 570345 / 712937 (80.00%)
> [ 29.640800] fsck.f2fs: [FSCK] Check node 641638 / 712937 (90.00%)
> [ 30.718347] fsck.f2fs: [FSCK] Check node 712931 / 712937 (100.00%)
> [ 30.724176] fsck.f2fs:
> [ 30.737160] fsck.f2fs: [FSCK] Max image size: 167506 MB, Free space: 62850 MB
> [ 30.737164] fsck.f2fs: [FSCK] Unreachable nat entries
> [Ok..] [0x0]
> [ 30.737638] fsck.f2fs: [FSCK] SIT valid block bitmap checking
> [Ok..]
> [ 30.737640] fsck.f2fs: [FSCK] Hard link checking for regular file
> [Ok..] [0xd]
> [ 30.737641] fsck.f2fs: [FSCK] valid_block_count matching with CP
> [Ok..] [0x28b98e6]
> [ 30.737644] fsck.f2fs: [FSCK] valid_node_count matching with CP (de
> lookup) [Ok..] [0xae0e9]
> [ 30.737646] fsck.f2fs: [FSCK] valid_node_count matching with CP
> (nat lookup) [Ok..] [0xae0e9]
> [ 30.737647] fsck.f2fs: [FSCK] valid_inode_count matched with CP
> [Ok..] [0xa74a3]
> [ 30.737649] fsck.f2fs: [FSCK] free segment_count matched with CP
> [Ok..] [0x7aa3]
> [ 30.737662] fsck.f2fs: [FSCK] next block offset is free
> [Ok..]
> [ 30.737663] fsck.f2fs: [FSCK] fixing SIT types
> [ 30.737867] fsck.f2fs: [FSCK] other corrupted bugs
> [Ok..]
> [ 30.737893] fsck.f2fs: [update_superblock: 765] Info: Done to
> update superblock
> [ 30.960610] fsck.f2fs:
> [ 30.960618] fsck.f2fs: Done: 24.622956 secs
> [ 30.960620] fsck.f2fs:
> [ 30.960622] fsck.f2fs: c, u, RA, CH, CM, Repl=
> [ 30.960627] fsck.f2fs: 10000 10000 43600517 42605434 995083 985083
> [ 30.963274] F2FS-fs (sda13): Using encoding defined by superblock:
> utf8-12.1.0 with flags 0x0
> [ 30.995360] __f2fs_is_valid_blkaddr: type=2
>
> (Manually added that print ^)
>
> [ 30.995369] ------------[ cut here ]------------
> [ 30.995375] WARNING: CPU: 7 PID: 1 at f2fs_handle_error+0x18/0x3c
> [ 30.995378] CPU: 7 PID: 1 Comm: init Tainted: G S W
> 5.10.209-arter97-r15-kernelsu-g0867d0e4f1d2 #6
> [ 30.995379] Hardware name: Qualcomm Technologies, Inc. Cape QRD
> with PM8010 (DT)
> [ 30.995380] pstate: 22400005 (nzCv daif +PAN -UAO +TCO BTYPE=--)
> [ 30.995382] pc : f2fs_handle_error+0x18/0x3c
> [ 30.995384] lr : __f2fs_is_valid_blkaddr+0x2a4/0x2b0
> [ 30.995385] sp : ffffff80209e79b0
> [ 30.995386] x29: ffffff80209e79b0 x28: 0000000000000037
> [ 30.995388] x27: 00000000000001c7 x26: 0000000020120121
> [ 30.995389] x25: 00000000000000d9 x24: 0000000000000000
> [ 30.995390] x23: ffffffff00f1a700 x22: 0000000000000828
> [ 30.995391] x21: ffffff80462aa000 x20: ffffff80462aa000
> [ 30.995392] x19: 0000000000000002 x18: ffffffffffffffff
> [ 30.995393] x17: 0000000000000000 x16: 00000000ffff0000
> [ 30.995394] x15: 0000000000000004 x14: ffffffd1675ac6d0
> [ 30.995395] x13: 0000000000000003 x12: 0000000000000003
> [ 30.995396] x11: 00000000ffffffff x10: 0000000000000000
> [ 30.995397] x9 : 0000000100000001 x8 : 0000000100000000
> [ 30.995398] x7 : 64696c61765f7369 x6 : ffffffd1681279e8
> [ 30.995399] x5 : 000000000000001f x4 : 0000000000000001
> [ 30.995400] x3 : 0000000000000000 x2 : ffffff89f03dedc8
> [ 30.995401] x1 : 0000000000000002 x0 : ffffff80462aa000
> [ 30.995403] Call trace:
> [ 30.995404] f2fs_handle_error+0x18/0x3c
> [ 30.995405] __f2fs_is_valid_blkaddr+0x2a4/0x2b0
> [ 30.995406] f2fs_is_valid_blkaddr+0x10/0x20
> [ 30.995407] f2fs_ra_meta_pages+0xe0/0x230
> [ 30.995409] build_sit_entries+0xa8/0x580
> [ 30.995411] f2fs_build_segment_manager+0x124/0x170
> [ 30.995412] f2fs_fill_super+0x78c/0xd1c
> [ 30.995415] mount_bdev+0x168/0x1ac
> [ 30.995416] f2fs_mount+0x18/0x24
> [ 30.995418] legacy_get_tree.llvm.9147845779559715083+0x30/0x5c
> [ 30.995419] vfs_get_tree+0x30/0xe0
> [ 30.995421] do_new_mount+0x140/0x358
> [ 30.995422] path_mount+0x1fc/0x4e8
> [ 30.995423] __arm64_sys_mount+0x150/0x294
> [ 30.995425] el0_svc_common.llvm.15698454952154965787+0xa8/0x138
> [ 30.995426] do_el0_svc+0x24/0x90
> [ 30.995429] el0_svc+0x10/0x1c
> [ 30.995430] el0_sync_handler+0xcc/0xe4
> [ 30.995432] el0_sync+0x1a0/0x1c0
> [ 30.995433] ---[ end trace 3b83295e0cdac94e ]---
> [ 31.005011] F2FS-fs (sda13): Mounted with checkpoint version = 2b7e3b29
> [ 31.005176] init: [libfs_mgr]
> __mount(source=/dev/block/bootdevice/by-name/userdata,target=/data,type=f2fs)=0:
> Success
> [ 31.007749] init: Userdata mounted using /vendor/etc/fstab.qcom result : 0
>
>
> I was bisecting a long boot time (24 additional seconds) issue, which
> is always reproducible, and found commit 31f85ccc84b8 ("f2fs: unify
> the error handling of f2fs_is_valid_blkaddr") to be causing it.
>
> I'll just revert that patch locally. Seems like Jaegeuk's dev branch
> doesn't have the fix for this specifically yet.
>
> Thanks.
>
>>
>> I was trying to debug another issue, but found the root cause. Let me drop this
>> patch.
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Fixes: 31f85ccc84b8 ("f2fs: unify the error handling of f2fs_is_valid_blkaddr")
>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>> ---
>>>> fs/f2fs/checkpoint.c | 9 +++++----
>>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>> index eac698b8dd38..b01320502624 100644
>>>> --- a/fs/f2fs/checkpoint.c
>>>> +++ b/fs/f2fs/checkpoint.c
>>>> @@ -179,22 +179,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
>>>> break;
>>>> case META_SIT:
>>>> if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
>>>> - goto err;
>>>> + goto check_only;
>>>> break;
>>>> case META_SSA:
>>>> if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
>>>> blkaddr < SM_I(sbi)->ssa_blkaddr))
>>>> - goto err;
>>>> + goto check_only;
>>>> break;
>>>> case META_CP:
>>>> if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
>>>> blkaddr < __start_cp_addr(sbi)))
>>>> - goto err;
>>>> + goto check_only;
>>>> break;
>>>> case META_POR:
>>>> if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
>>>> blkaddr < MAIN_BLKADDR(sbi)))
>>>> - goto err;
>>>> + goto check_only;
>>>> break;
>>>> case DATA_GENERIC:
>>>> case DATA_GENERIC_ENHANCE:
>>>> @@ -228,6 +228,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
>>>> return true;
>>>> err:
>>>> f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
>>>> +check_only:
>>>> return false;
>>>> }
>>
>>
>> _______________________________________________
>> 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] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-25 7:29 ` Chao Yu
@ 2024-04-28 1:23 ` Daeho Jeong
2024-04-28 9:08 ` Chao Yu
0 siblings, 1 reply; 18+ messages in thread
From: Daeho Jeong @ 2024-04-28 1:23 UTC (permalink / raw)
To: Chao Yu; +Cc: Juhyung Park, Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On Thu, Apr 25, 2024 at 12:30 AM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/4/19 18:27, Juhyung Park wrote:
> > On Sat, Apr 13, 2024 at 5:57 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> >>
> >> On 04/11, Chao Yu wrote:
> >>> On 2024/4/10 4:34, Jaegeuk Kim wrote:
> >>>> f2fs_ra_meta_pages can try to read ahead on invalid block address which is
> >>>> not the corruption case.
> >>>
> >>> In which case we will read ahead invalid meta pages? recovery w/ META_POR?
> >
> > In my case, it seems like it's META_SIT, and it's triggered right after mount.
>
> Ah, I see, actually it hits at this case, thanks for the information.
>
> Thanks,
>
> > fsck detects invalid_blkaddr, and when the kernel mounts it, it
> > immediately flags invalid_blkaddr again:
> >
> > [ 6.333498] init: [libfs_mgr] Running /system/bin/fsck.f2fs -a -c
> > 10000 --debug-cache /dev/block/sda13
> > [ 6.337671] fsck.f2fs: Info: Fix the reported corruption.
> > [ 6.337947] fsck.f2fs: Info: not exist /proc/version!
> > [ 6.338010] fsck.f2fs: Info: can't find /sys, assuming normal block device
> > [ 6.338294] fsck.f2fs: Info: MKFS version
> > [ 6.338319] fsck.f2fs: "5.10.160-android12-9-ge5cfec41c8e2"
> > [ 6.338366] fsck.f2fs: Info: FSCK version
> > [ 6.338380] fsck.f2fs: from "5.10-arter97"
> > [ 6.338393] fsck.f2fs: to "5.10-arter97"
> > [ 6.338414] fsck.f2fs: Info: superblock features = 1499 : encrypt
> > verity extra_attr project_quota quota_ino casefold
> > [ 6.338429] fsck.f2fs: Info: superblock encrypt level = 0, salt =
> > 00000000000000000000000000000000
> > [ 6.338442] fsck.f2fs: Info: checkpoint stop reason: shutdown(180)
> > [ 6.338455] fsck.f2fs: Info: fs errors: invalid_blkaddr
> > [ 6.338468] fsck.f2fs: Info: Segments per section = 1
> > [ 6.338480] fsck.f2fs: Info: Sections per zone = 1
> > [ 6.338492] fsck.f2fs: Info: total FS sectors = 58971571 (230357 MB)
> > [ 6.340599] fsck.f2fs: Info: CKPT version = 2b7e3b29
> > [ 6.340620] fsck.f2fs: Info: version timestamp cur: 19789296, prev: 18407008
> > [ 6.677041] fsck.f2fs: Info: checkpoint state = 46 : crc
> > compacted_summary orphan_inodes sudden-power-off
> > [ 6.677052] fsck.f2fs: [FSCK] Check node 1 / 712937 (0.00%)
> > [ 8.997922] fsck.f2fs: [FSCK] Check node 71294 / 712937 (10.00%)
> > [ 10.629205] fsck.f2fs: [FSCK] Check node 142587 / 712937 (20.00%)
> > [ 12.278186] fsck.f2fs: [FSCK] Check node 213880 / 712937 (30.00%)
> > [ 13.768177] fsck.f2fs: [FSCK] Check node 285173 / 712937 (40.00%)
> > [ 17.446971] fsck.f2fs: [FSCK] Check node 356466 / 712937 (50.00%)
> > [ 19.891623] fsck.f2fs: [FSCK] Check node 427759 / 712937 (60.00%)
> > [ 23.251327] fsck.f2fs: [FSCK] Check node 499052 / 712937 (70.00%)
> > [ 28.493457] fsck.f2fs: [FSCK] Check node 570345 / 712937 (80.00%)
> > [ 29.640800] fsck.f2fs: [FSCK] Check node 641638 / 712937 (90.00%)
> > [ 30.718347] fsck.f2fs: [FSCK] Check node 712931 / 712937 (100.00%)
> > [ 30.724176] fsck.f2fs:
> > [ 30.737160] fsck.f2fs: [FSCK] Max image size: 167506 MB, Free space: 62850 MB
> > [ 30.737164] fsck.f2fs: [FSCK] Unreachable nat entries
> > [Ok..] [0x0]
> > [ 30.737638] fsck.f2fs: [FSCK] SIT valid block bitmap checking
> > [Ok..]
> > [ 30.737640] fsck.f2fs: [FSCK] Hard link checking for regular file
> > [Ok..] [0xd]
> > [ 30.737641] fsck.f2fs: [FSCK] valid_block_count matching with CP
> > [Ok..] [0x28b98e6]
> > [ 30.737644] fsck.f2fs: [FSCK] valid_node_count matching with CP (de
> > lookup) [Ok..] [0xae0e9]
> > [ 30.737646] fsck.f2fs: [FSCK] valid_node_count matching with CP
> > (nat lookup) [Ok..] [0xae0e9]
> > [ 30.737647] fsck.f2fs: [FSCK] valid_inode_count matched with CP
> > [Ok..] [0xa74a3]
> > [ 30.737649] fsck.f2fs: [FSCK] free segment_count matched with CP
> > [Ok..] [0x7aa3]
> > [ 30.737662] fsck.f2fs: [FSCK] next block offset is free
> > [Ok..]
> > [ 30.737663] fsck.f2fs: [FSCK] fixing SIT types
> > [ 30.737867] fsck.f2fs: [FSCK] other corrupted bugs
> > [Ok..]
> > [ 30.737893] fsck.f2fs: [update_superblock: 765] Info: Done to
> > update superblock
> > [ 30.960610] fsck.f2fs:
> > [ 30.960618] fsck.f2fs: Done: 24.622956 secs
> > [ 30.960620] fsck.f2fs:
> > [ 30.960622] fsck.f2fs: c, u, RA, CH, CM, Repl=
> > [ 30.960627] fsck.f2fs: 10000 10000 43600517 42605434 995083 985083
> > [ 30.963274] F2FS-fs (sda13): Using encoding defined by superblock:
> > utf8-12.1.0 with flags 0x0
> > [ 30.995360] __f2fs_is_valid_blkaddr: type=2
> >
> > (Manually added that print ^)
> >
> > [ 30.995369] ------------[ cut here ]------------
> > [ 30.995375] WARNING: CPU: 7 PID: 1 at f2fs_handle_error+0x18/0x3c
> > [ 30.995378] CPU: 7 PID: 1 Comm: init Tainted: G S W
> > 5.10.209-arter97-r15-kernelsu-g0867d0e4f1d2 #6
> > [ 30.995379] Hardware name: Qualcomm Technologies, Inc. Cape QRD
> > with PM8010 (DT)
> > [ 30.995380] pstate: 22400005 (nzCv daif +PAN -UAO +TCO BTYPE=--)
> > [ 30.995382] pc : f2fs_handle_error+0x18/0x3c
> > [ 30.995384] lr : __f2fs_is_valid_blkaddr+0x2a4/0x2b0
> > [ 30.995385] sp : ffffff80209e79b0
> > [ 30.995386] x29: ffffff80209e79b0 x28: 0000000000000037
> > [ 30.995388] x27: 00000000000001c7 x26: 0000000020120121
> > [ 30.995389] x25: 00000000000000d9 x24: 0000000000000000
> > [ 30.995390] x23: ffffffff00f1a700 x22: 0000000000000828
> > [ 30.995391] x21: ffffff80462aa000 x20: ffffff80462aa000
> > [ 30.995392] x19: 0000000000000002 x18: ffffffffffffffff
> > [ 30.995393] x17: 0000000000000000 x16: 00000000ffff0000
> > [ 30.995394] x15: 0000000000000004 x14: ffffffd1675ac6d0
> > [ 30.995395] x13: 0000000000000003 x12: 0000000000000003
> > [ 30.995396] x11: 00000000ffffffff x10: 0000000000000000
> > [ 30.995397] x9 : 0000000100000001 x8 : 0000000100000000
> > [ 30.995398] x7 : 64696c61765f7369 x6 : ffffffd1681279e8
> > [ 30.995399] x5 : 000000000000001f x4 : 0000000000000001
> > [ 30.995400] x3 : 0000000000000000 x2 : ffffff89f03dedc8
> > [ 30.995401] x1 : 0000000000000002 x0 : ffffff80462aa000
> > [ 30.995403] Call trace:
> > [ 30.995404] f2fs_handle_error+0x18/0x3c
> > [ 30.995405] __f2fs_is_valid_blkaddr+0x2a4/0x2b0
> > [ 30.995406] f2fs_is_valid_blkaddr+0x10/0x20
> > [ 30.995407] f2fs_ra_meta_pages+0xe0/0x230
> > [ 30.995409] build_sit_entries+0xa8/0x580
> > [ 30.995411] f2fs_build_segment_manager+0x124/0x170
> > [ 30.995412] f2fs_fill_super+0x78c/0xd1c
> > [ 30.995415] mount_bdev+0x168/0x1ac
> > [ 30.995416] f2fs_mount+0x18/0x24
> > [ 30.995418] legacy_get_tree.llvm.9147845779559715083+0x30/0x5c
> > [ 30.995419] vfs_get_tree+0x30/0xe0
> > [ 30.995421] do_new_mount+0x140/0x358
> > [ 30.995422] path_mount+0x1fc/0x4e8
> > [ 30.995423] __arm64_sys_mount+0x150/0x294
> > [ 30.995425] el0_svc_common.llvm.15698454952154965787+0xa8/0x138
> > [ 30.995426] do_el0_svc+0x24/0x90
> > [ 30.995429] el0_svc+0x10/0x1c
> > [ 30.995430] el0_sync_handler+0xcc/0xe4
> > [ 30.995432] el0_sync+0x1a0/0x1c0
> > [ 30.995433] ---[ end trace 3b83295e0cdac94e ]---
> > [ 31.005011] F2FS-fs (sda13): Mounted with checkpoint version = 2b7e3b29
> > [ 31.005176] init: [libfs_mgr]
> > __mount(source=/dev/block/bootdevice/by-name/userdata,target=/data,type=f2fs)=0:
> > Success
> > [ 31.007749] init: Userdata mounted using /vendor/etc/fstab.qcom result : 0
> >
> >
> > I was bisecting a long boot time (24 additional seconds) issue, which
> > is always reproducible, and found commit 31f85ccc84b8 ("f2fs: unify
> > the error handling of f2fs_is_valid_blkaddr") to be causing it.
> >
> > I'll just revert that patch locally. Seems like Jaegeuk's dev branch
> > doesn't have the fix for this specifically yet.
> >
> > Thanks.
> >
> >>
> >> I was trying to debug another issue, but found the root cause. Let me drop this
> >> patch.
> >>
> >>>
> >>> Thanks,
> >>>
> >>>>
> >>>> Fixes: 31f85ccc84b8 ("f2fs: unify the error handling of f2fs_is_valid_blkaddr")
> >>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >>>> ---
> >>>> fs/f2fs/checkpoint.c | 9 +++++----
> >>>> 1 file changed, 5 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >>>> index eac698b8dd38..b01320502624 100644
> >>>> --- a/fs/f2fs/checkpoint.c
> >>>> +++ b/fs/f2fs/checkpoint.c
> >>>> @@ -179,22 +179,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> >>>> break;
> >>>> case META_SIT:
> >>>> if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
> >>>> - goto err;
> >>>> + goto check_only;
> >>>> break;
> >>>> case META_SSA:
> >>>> if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
> >>>> blkaddr < SM_I(sbi)->ssa_blkaddr))
> >>>> - goto err;
> >>>> + goto check_only;
> >>>> break;
> >>>> case META_CP:
> >>>> if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
> >>>> blkaddr < __start_cp_addr(sbi)))
> >>>> - goto err;
> >>>> + goto check_only;
> >>>> break;
> >>>> case META_POR:
> >>>> if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> >>>> blkaddr < MAIN_BLKADDR(sbi)))
> >>>> - goto err;
> >>>> + goto check_only;
> >>>> break;
> >>>> case DATA_GENERIC:
> >>>> case DATA_GENERIC_ENHANCE:
> >>>> @@ -228,6 +228,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
> >>>> return true;
> >>>> err:
> >>>> f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
> >>>> +check_only:
> >>>> return false;
> >>>> }
> >>
> >>
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> Linux-f2fs-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
I have a question. Is it okay for META_GENERIC?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix false alarm on invalid block address
2024-04-28 1:23 ` Daeho Jeong
@ 2024-04-28 9:08 ` Chao Yu
0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2024-04-28 9:08 UTC (permalink / raw)
To: Daeho Jeong; +Cc: Juhyung Park, Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2024/4/28 9:23, Daeho Jeong wrote:
> I have a question. Is it okay for META_GENERIC?
It seems all users of META_GENERIC comes from IO paths:
a) f2fs_merge_page_bio
b) f2fs_submit_page_bio
c) f2fs_submit_page_write - verify_fio_blkaddr
They are all impossible cases? so it's fine to record the error
for this case?
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2024-04-28 9:09 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-09 20:34 [PATCH 1/3] f2fs: use folio_test_writeback Jaegeuk Kim
2024-04-09 20:34 ` [PATCH 2/3] f2fs: clear writeback when compression failed Jaegeuk Kim
2024-04-11 9:05 ` [f2fs-dev] " Chao Yu
2024-04-12 20:57 ` Jaegeuk Kim
2024-04-16 16:49 ` [PATCH 2/3 v2] " Jaegeuk Kim
2024-04-23 19:47 ` [f2fs-dev] " Daeho Jeong
2024-04-25 2:15 ` Chao Yu
2024-04-09 20:34 ` [PATCH 3/3] f2fs: fix false alarm on invalid block address Jaegeuk Kim
2024-04-11 9:08 ` [f2fs-dev] " Chao Yu
2024-04-12 20:56 ` Jaegeuk Kim
2024-04-19 10:27 ` Juhyung Park
2024-04-19 17:50 ` Jaegeuk Kim
2024-04-25 7:29 ` Chao Yu
2024-04-28 1:23 ` Daeho Jeong
2024-04-28 9:08 ` Chao Yu
2024-04-11 8:56 ` [f2fs-dev] [PATCH 1/3] f2fs: use folio_test_writeback Chao Yu
2024-04-14 15:32 ` patchwork-bot+f2fs
2024-04-24 17:40 ` patchwork-bot+f2fs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).