Messages in this thread Patch in this message |  | | Date | Sun, 17 Apr 2022 14:41:51 +0800 | Subject | Re: [f2fs-dev] [PATCH 2/2 v2] f2fs: avoid infinite loop to flush node pages | From | Chao Yu <> |
| |
On 2022/4/15 6:27, Jaegeuk Kim wrote: > xfstests/generic/475 can give EIO all the time which give an infinite loop > to flush node page like below. Let's avoid it. > > [16418.518551] Call Trace: > [16418.518553] ? dm_submit_bio+0x48/0x400 > [16418.518574] ? submit_bio_checks+0x1ac/0x5a0 > [16418.525207] __submit_bio+0x1a9/0x230 > [16418.525210] ? kmem_cache_alloc+0x29e/0x3c0 > [16418.525223] submit_bio_noacct+0xa8/0x2b0 > [16418.525226] submit_bio+0x4d/0x130 > [16418.525238] __submit_bio+0x49/0x310 [f2fs] > [16418.525339] ? bio_add_page+0x6a/0x90 > [16418.525344] f2fs_submit_page_bio+0x134/0x1f0 [f2fs] > [16418.525365] read_node_page+0x125/0x1b0 [f2fs] > [16418.525388] __get_node_page.part.0+0x58/0x3f0 [f2fs] > [16418.525409] __get_node_page+0x2f/0x60 [f2fs] > [16418.525431] f2fs_get_dnode_of_data+0x423/0x860 [f2fs] > [16418.525452] ? asm_sysvec_apic_timer_interrupt+0x12/0x20 > [16418.525458] ? __mod_memcg_state.part.0+0x2a/0x30 > [16418.525465] ? __mod_memcg_lruvec_state+0x27/0x40 > [16418.525467] ? __xa_set_mark+0x57/0x70 > [16418.525472] f2fs_do_write_data_page+0x10e/0x7b0 [f2fs] > [16418.525493] f2fs_write_single_data_page+0x555/0x830 [f2fs] > [16418.525514] ? sysvec_apic_timer_interrupt+0x4e/0x90 > [16418.525518] ? asm_sysvec_apic_timer_interrupt+0x12/0x20 > [16418.525523] f2fs_write_cache_pages+0x303/0x880 [f2fs] > [16418.525545] ? blk_flush_plug_list+0x47/0x100 > [16418.525548] f2fs_write_data_pages+0xfd/0x320 [f2fs] > [16418.525569] do_writepages+0xd5/0x210 > [16418.525648] filemap_fdatawrite_wbc+0x7d/0xc0 > [16418.525655] filemap_fdatawrite+0x50/0x70 > [16418.525658] f2fs_sync_dirty_inodes+0xa4/0x230 [f2fs] > [16418.525679] f2fs_write_checkpoint+0x16d/0x1720 [f2fs] > [16418.525699] ? ttwu_do_wakeup+0x1c/0x160 > [16418.525709] ? ttwu_do_activate+0x6d/0xd0 > [16418.525711] ? __wait_for_common+0x11d/0x150 > [16418.525715] kill_f2fs_super+0xca/0x100 [f2fs] > [16418.525733] deactivate_locked_super+0x3b/0xb0 > [16418.525739] deactivate_super+0x40/0x50 > [16418.525741] cleanup_mnt+0x139/0x190 > [16418.525747] __cleanup_mnt+0x12/0x20 > [16418.525749] task_work_run+0x6d/0xa0 > [16418.525765] exit_to_user_mode_prepare+0x1ad/0x1b0 > [16418.525771] syscall_exit_to_user_mode+0x27/0x50 > [16418.525774] do_syscall_64+0x48/0xc0 > [16418.525776] entry_SYSCALL_64_after_hwframe+0x44/0xae > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > --- > > Change log from v1: > - cover one more error case > > fs/f2fs/checkpoint.c | 8 +------- > fs/f2fs/f2fs.h | 20 ++++++++++++++++---- > fs/f2fs/node.c | 2 ++ > 3 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index 909085a78f9c..319903c2b34f 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -98,13 +98,7 @@ static struct page *__get_meta_page(struct f2fs_sb_info *sbi, pgoff_t index, > } > > if (unlikely(!PageUptodate(page))) { > - if (page->index == sbi->metapage_eio_ofs) { > - if (sbi->metapage_eio_cnt++ == MAX_RETRY_META_PAGE_EIO) > - set_ckpt_flags(sbi, CP_ERROR_FLAG); > - } else { > - sbi->metapage_eio_ofs = page->index; > - sbi->metapage_eio_cnt = 0; > - } > + f2fs_handle_page_eio(sbi, page->index, META); > f2fs_put_page(page, 1); > return ERR_PTR(-EIO); > } > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index cd1e65bcf0b0..977826a22568 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -579,8 +579,8 @@ enum { > /* maximum retry quota flush count */ > #define DEFAULT_RETRY_QUOTA_FLUSH_COUNT 8 > > -/* maximum retry of EIO'ed meta page */ > -#define MAX_RETRY_META_PAGE_EIO 100 > +/* maximum retry of EIO'ed page */ > +#define MAX_RETRY_PAGE_EIO 100 > > #define F2FS_LINK_MAX 0xffffffff /* maximum link count per file */ > > @@ -1621,8 +1621,8 @@ struct f2fs_sb_info { > /* keep migration IO order for LFS mode */ > struct f2fs_rwsem io_order_lock; > mempool_t *write_io_dummy; /* Dummy pages */ > - pgoff_t metapage_eio_ofs; /* EIO page offset */ > - int metapage_eio_cnt; /* EIO count */ > + pgoff_t page_eio_ofs[NR_PAGE_TYPE]; /* EIO page offset */ > + int page_eio_cnt[NR_PAGE_TYPE]; /* EIO count */ > > /* for checkpoint */ > struct f2fs_checkpoint *ckpt; /* raw checkpoint pointer */ > @@ -4543,6 +4543,18 @@ static inline void f2fs_io_schedule_timeout(long timeout) > io_schedule_timeout(timeout); > } > > +static inline void f2fs_handle_page_eio(struct f2fs_sb_info *sbi, pgoff_t ofs, > + enum page_type type) > +{
If we have already set cp_error, it doesn't need to process below logic redundantly?
if (f2fs_cp_error()) return;
> + if (ofs == sbi->page_eio_ofs[type]) { > + if (sbi->page_eio_cnt[type]++ == MAX_RETRY_PAGE_EIO) > + set_ckpt_flags(sbi, CP_ERROR_FLAG); > + } else { > + sbi->page_eio_ofs[type] = ofs; > + sbi->page_eio_cnt[type] = 0; > + } > +} > + > #define EFSBADCRC EBADMSG /* Bad CRC detected */ > #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */ > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index c45d341dcf6e..c280f482c741 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1416,6 +1416,7 @@ static struct page *__get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid, > > err = read_node_page(page, 0); > if (err < 0) { > + f2fs_handle_page_eio(sbi, page->index, NODE);
How about:
goto out_put;
> f2fs_put_page(page, 1); > return ERR_PTR(err); > } else if (err == LOCKED_PAGE) { > @@ -1452,6 +1453,7 @@ static struct page *__get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid, > err = -EINVAL; > out_err: > ClearPageUptodate(page);
out_put:
Thanks,
> + f2fs_handle_page_eio(sbi, page->index, NODE); > f2fs_put_page(page, 1); > return ERR_PTR(err); > }
|  |