* [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry
@ 2024-03-08 10:12 Zhiguo Niu
2024-03-08 10:12 ` [PATCH 2/2] f2fs: fix to handle error paths of {new,change}_curseg() Zhiguo Niu
2024-03-11 3:54 ` [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry Chao Yu
0 siblings, 2 replies; 6+ messages in thread
From: Zhiguo Niu @ 2024-03-08 10:12 UTC (permalink / raw)
To: jaegeuk, chao
Cc: linux-f2fs-devel, linux-kernel, niuzhiguo84, zhiguo.niu, ke.wang,
hongyu.jin
add_bio_entry should not trigger system panic when bio_add_page fail,
fix to remove it.
Fixes: 0b20fcec8651 ("f2fs: cache global IPU bio")
Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
---
fs/f2fs/data.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index d9494b5..f8ae684 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -759,8 +759,10 @@ static void add_bio_entry(struct f2fs_sb_info *sbi, struct bio *bio,
be->bio = bio;
bio_get(bio);
- if (bio_add_page(bio, page, PAGE_SIZE, 0) != PAGE_SIZE)
- f2fs_bug_on(sbi, 1);
+ if (bio_add_page(bio, page, PAGE_SIZE, 0) != PAGE_SIZE) {
+ bio_put(bio);
+ return;
+ }
f2fs_down_write(&io->bio_list_lock);
list_add_tail(&be->list, &io->bio_list);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] f2fs: fix to handle error paths of {new,change}_curseg() 2024-03-08 10:12 [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry Zhiguo Niu @ 2024-03-08 10:12 ` Zhiguo Niu 2024-03-11 6:09 ` Chao Yu 2024-03-11 3:54 ` [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry Chao Yu 1 sibling, 1 reply; 6+ messages in thread From: Zhiguo Niu @ 2024-03-08 10:12 UTC (permalink / raw) To: jaegeuk, chao Cc: linux-f2fs-devel, linux-kernel, niuzhiguo84, zhiguo.niu, ke.wang, hongyu.jin {new,change}_curseg() may return error in some special cases, error handling should be did in their callers, and this will also facilitate subsequent error path expansion in {new,change}_curseg(). Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> Signed-off-by: Chao Yu <chao@kernel.org> --- fs/f2fs/extent_cache.c | 2 +- fs/f2fs/f2fs.h | 4 ++-- fs/f2fs/gc.c | 7 +++++-- fs/f2fs/segment.c | 57 +++++++++++++++++++++++++++++++------------------- fs/f2fs/super.c | 4 +++- 5 files changed, 46 insertions(+), 28 deletions(-) diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c index 48048fa..dce00cf 100644 --- a/fs/f2fs/extent_cache.c +++ b/fs/f2fs/extent_cache.c @@ -988,7 +988,7 @@ bool f2fs_lookup_read_extent_cache_block(struct inode *inode, pgoff_t index, void f2fs_update_read_extent_cache(struct dnode_of_data *dn) { - return __update_extent_cache(dn, EX_READ); + __update_extent_cache(dn, EX_READ); } void f2fs_update_read_extent_cache_range(struct dnode_of_data *dn, diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 4836e7c..7beb074 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3700,10 +3700,10 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi); int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra); bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno); -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi); +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi); void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi); void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi); -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, unsigned int start, unsigned int end); int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force); int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi); diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index ca1bf41..8852814 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -2035,8 +2035,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi, mutex_unlock(&DIRTY_I(sbi)->seglist_lock); /* Move out cursegs from the target range */ - for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) - f2fs_allocate_segment_for_resize(sbi, type, start, end); + for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) { + err = f2fs_allocate_segment_for_resize(sbi, type, start, end); + if (err) + goto out; + } /* do GC to move out valid blocks in the range */ err = f2fs_gc_range(sbi, start, end, dry_run, 0); diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 4e4a51a..c1c1308 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -2863,7 +2863,7 @@ bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno) * This function always allocates a used segment(from dirty seglist) by SSR * manner, so it should recover the existing segment information of valid blocks */ -static void change_curseg(struct f2fs_sb_info *sbi, int type) +static int change_curseg(struct f2fs_sb_info *sbi, int type) { struct dirty_seglist_info *dirty_i = DIRTY_I(sbi); struct curseg_info *curseg = CURSEG_I(sbi, type); @@ -2888,21 +2888,23 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type) if (IS_ERR(sum_page)) { /* GC won't be able to use stale summary pages by cp_error */ memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE); - return; + return PTR_ERR(sum_page); } sum_node = (struct f2fs_summary_block *)page_address(sum_page); memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE); f2fs_put_page(sum_page, 1); + return 0; } static int get_ssr_segment(struct f2fs_sb_info *sbi, int type, int alloc_mode, unsigned long long age); -static void get_atssr_segment(struct f2fs_sb_info *sbi, int type, +static int get_atssr_segment(struct f2fs_sb_info *sbi, int type, int target_type, int alloc_mode, unsigned long long age) { struct curseg_info *curseg = CURSEG_I(sbi, type); + int ret = 0; curseg->seg_type = target_type; @@ -2910,38 +2912,41 @@ static void get_atssr_segment(struct f2fs_sb_info *sbi, int type, struct seg_entry *se = get_seg_entry(sbi, curseg->next_segno); curseg->seg_type = se->type; - change_curseg(sbi, type); + ret = change_curseg(sbi, type); } else { /* allocate cold segment by default */ curseg->seg_type = CURSEG_COLD_DATA; - new_curseg(sbi, type, true); + ret = new_curseg(sbi, type, true); } stat_inc_seg_type(sbi, curseg); + return ret; } -static void __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi) +static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi) { struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC); + int ret = 0; if (!sbi->am.atgc_enabled) - return; + return 0; f2fs_down_read(&SM_I(sbi)->curseg_lock); mutex_lock(&curseg->curseg_mutex); down_write(&SIT_I(sbi)->sentry_lock); - get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, CURSEG_COLD_DATA, SSR, 0); + ret = get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, + CURSEG_COLD_DATA, SSR, 0); up_write(&SIT_I(sbi)->sentry_lock); mutex_unlock(&curseg->curseg_mutex); f2fs_up_read(&SM_I(sbi)->curseg_lock); - + return ret; } -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi) +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi) { - __f2fs_init_atgc_curseg(sbi); + return __f2fs_init_atgc_curseg(sbi); } static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type) @@ -3069,11 +3074,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type) return false; } -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, unsigned int start, unsigned int end) { struct curseg_info *curseg = CURSEG_I(sbi, type); unsigned int segno; + int ret = 0; f2fs_down_read(&SM_I(sbi)->curseg_lock); mutex_lock(&curseg->curseg_mutex); @@ -3084,9 +3090,9 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, goto unlock; if (f2fs_need_SSR(sbi) && get_ssr_segment(sbi, type, SSR, 0)) - change_curseg(sbi, type); + ret = change_curseg(sbi, type); else - new_curseg(sbi, type, true); + ret = new_curseg(sbi, type, true); stat_inc_seg_type(sbi, curseg); @@ -3100,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, mutex_unlock(&curseg->curseg_mutex); f2fs_up_read(&SM_I(sbi)->curseg_lock); + return ret; } static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, @@ -3486,14 +3493,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, bool from_gc = (type == CURSEG_ALL_DATA_ATGC); struct seg_entry *se = NULL; bool segment_full = false; + int ret = 0; f2fs_down_read(&SM_I(sbi)->curseg_lock); mutex_lock(&curseg->curseg_mutex); down_write(&sit_i->sentry_lock); - if (curseg->segno == NULL_SEGNO) + if (curseg->segno == NULL_SEGNO) { + ret = -ENOSPC; goto out_err; + } if (from_gc) { f2fs_bug_on(sbi, GET_SEGNO(sbi, old_blkaddr) == NULL_SEGNO); @@ -3546,17 +3556,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, } if (from_gc) { - get_atssr_segment(sbi, type, se->type, + ret = get_atssr_segment(sbi, type, se->type, AT_SSR, se->mtime); } else { if (need_new_seg(sbi, type)) - new_curseg(sbi, type, false); + ret = new_curseg(sbi, type, false); else - change_curseg(sbi, type); + ret = change_curseg(sbi, type); stat_inc_seg_type(sbi, curseg); } - if (curseg->segno == NULL_SEGNO) + if (ret) goto out_err; } @@ -3599,7 +3609,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, up_write(&sit_i->sentry_lock); mutex_unlock(&curseg->curseg_mutex); f2fs_up_read(&SM_I(sbi)->curseg_lock); - return -ENOSPC; + return ret; } @@ -3829,7 +3839,8 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum, /* change the current segment */ if (segno != curseg->segno) { curseg->next_segno = segno; - change_curseg(sbi, type); + if (change_curseg(sbi, type)) + goto out_unlock; } curseg->next_blkoff = GET_BLKOFF_FROM_SEG0(sbi, new_blkaddr); @@ -3855,12 +3866,14 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum, if (recover_curseg) { if (old_cursegno != curseg->segno) { curseg->next_segno = old_cursegno; - change_curseg(sbi, type); + if (change_curseg(sbi, type)) + goto out_unlock; } curseg->next_blkoff = old_blkoff; curseg->alloc_type = old_alloc_type; } +out_unlock: up_write(&sit_i->sentry_lock); mutex_unlock(&curseg->curseg_mutex); f2fs_up_write(&SM_I(sbi)->curseg_lock); diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 78a7658..f2b6d3f 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -4685,7 +4685,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) if (err) goto free_meta; - f2fs_init_inmem_curseg(sbi); + err = f2fs_init_inmem_curseg(sbi); + if (err) + goto sync_free_meta; /* f2fs_recover_fsync_data() cleared this already */ clear_sbi_flag(sbi, SBI_POR_DOING); -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to handle error paths of {new,change}_curseg() 2024-03-08 10:12 ` [PATCH 2/2] f2fs: fix to handle error paths of {new,change}_curseg() Zhiguo Niu @ 2024-03-11 6:09 ` Chao Yu 2024-03-11 7:19 ` Zhiguo Niu 0 siblings, 1 reply; 6+ messages in thread From: Chao Yu @ 2024-03-11 6:09 UTC (permalink / raw) To: Zhiguo Niu, jaegeuk Cc: linux-f2fs-devel, linux-kernel, niuzhiguo84, ke.wang, hongyu.jin On 2024/3/8 18:12, Zhiguo Niu wrote: > {new,change}_curseg() may return error in some special cases, > error handling should be did in their callers, and this will also > facilitate subsequent error path expansion in {new,change}_curseg(). > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > Signed-off-by: Chao Yu <chao@kernel.org> > --- > fs/f2fs/extent_cache.c | 2 +- > fs/f2fs/f2fs.h | 4 ++-- > fs/f2fs/gc.c | 7 +++++-- > fs/f2fs/segment.c | 57 +++++++++++++++++++++++++++++++------------------- > fs/f2fs/super.c | 4 +++- > 5 files changed, 46 insertions(+), 28 deletions(-) > > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c > index 48048fa..dce00cf 100644 > --- a/fs/f2fs/extent_cache.c > +++ b/fs/f2fs/extent_cache.c > @@ -988,7 +988,7 @@ bool f2fs_lookup_read_extent_cache_block(struct inode *inode, pgoff_t index, > > void f2fs_update_read_extent_cache(struct dnode_of_data *dn) > { > - return __update_extent_cache(dn, EX_READ); > + __update_extent_cache(dn, EX_READ); Above change is not related to this patch? Otherwise, it looks good to me. Thanks, > } > > void f2fs_update_read_extent_cache_range(struct dnode_of_data *dn, > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 4836e7c..7beb074 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3700,10 +3700,10 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, > void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi); > int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra); > bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno); > -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi); > +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi); > void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi); > void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi); > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > unsigned int start, unsigned int end); > int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force); > int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi); > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index ca1bf41..8852814 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -2035,8 +2035,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi, > mutex_unlock(&DIRTY_I(sbi)->seglist_lock); > > /* Move out cursegs from the target range */ > - for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) > - f2fs_allocate_segment_for_resize(sbi, type, start, end); > + for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) { > + err = f2fs_allocate_segment_for_resize(sbi, type, start, end); > + if (err) > + goto out; > + } > > /* do GC to move out valid blocks in the range */ > err = f2fs_gc_range(sbi, start, end, dry_run, 0); > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 4e4a51a..c1c1308 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -2863,7 +2863,7 @@ bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno) > * This function always allocates a used segment(from dirty seglist) by SSR > * manner, so it should recover the existing segment information of valid blocks > */ > -static void change_curseg(struct f2fs_sb_info *sbi, int type) > +static int change_curseg(struct f2fs_sb_info *sbi, int type) > { > struct dirty_seglist_info *dirty_i = DIRTY_I(sbi); > struct curseg_info *curseg = CURSEG_I(sbi, type); > @@ -2888,21 +2888,23 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type) > if (IS_ERR(sum_page)) { > /* GC won't be able to use stale summary pages by cp_error */ > memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE); > - return; > + return PTR_ERR(sum_page); > } > sum_node = (struct f2fs_summary_block *)page_address(sum_page); > memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE); > f2fs_put_page(sum_page, 1); > + return 0; > } > > static int get_ssr_segment(struct f2fs_sb_info *sbi, int type, > int alloc_mode, unsigned long long age); > > -static void get_atssr_segment(struct f2fs_sb_info *sbi, int type, > +static int get_atssr_segment(struct f2fs_sb_info *sbi, int type, > int target_type, int alloc_mode, > unsigned long long age) > { > struct curseg_info *curseg = CURSEG_I(sbi, type); > + int ret = 0; > > curseg->seg_type = target_type; > > @@ -2910,38 +2912,41 @@ static void get_atssr_segment(struct f2fs_sb_info *sbi, int type, > struct seg_entry *se = get_seg_entry(sbi, curseg->next_segno); > > curseg->seg_type = se->type; > - change_curseg(sbi, type); > + ret = change_curseg(sbi, type); > } else { > /* allocate cold segment by default */ > curseg->seg_type = CURSEG_COLD_DATA; > - new_curseg(sbi, type, true); > + ret = new_curseg(sbi, type, true); > } > stat_inc_seg_type(sbi, curseg); > + return ret; > } > > -static void __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi) > +static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi) > { > struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC); > + int ret = 0; > > if (!sbi->am.atgc_enabled) > - return; > + return 0; > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > > mutex_lock(&curseg->curseg_mutex); > down_write(&SIT_I(sbi)->sentry_lock); > > - get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, CURSEG_COLD_DATA, SSR, 0); > + ret = get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, > + CURSEG_COLD_DATA, SSR, 0); > > up_write(&SIT_I(sbi)->sentry_lock); > mutex_unlock(&curseg->curseg_mutex); > > f2fs_up_read(&SM_I(sbi)->curseg_lock); > - > + return ret; > } > -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi) > +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi) > { > - __f2fs_init_atgc_curseg(sbi); > + return __f2fs_init_atgc_curseg(sbi); > } > > static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type) > @@ -3069,11 +3074,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type) > return false; > } > > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > unsigned int start, unsigned int end) > { > struct curseg_info *curseg = CURSEG_I(sbi, type); > unsigned int segno; > + int ret = 0; > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > mutex_lock(&curseg->curseg_mutex); > @@ -3084,9 +3090,9 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > goto unlock; > > if (f2fs_need_SSR(sbi) && get_ssr_segment(sbi, type, SSR, 0)) > - change_curseg(sbi, type); > + ret = change_curseg(sbi, type); > else > - new_curseg(sbi, type, true); > + ret = new_curseg(sbi, type, true); > > stat_inc_seg_type(sbi, curseg); > > @@ -3100,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > mutex_unlock(&curseg->curseg_mutex); > f2fs_up_read(&SM_I(sbi)->curseg_lock); > + return ret; > } > > static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, > @@ -3486,14 +3493,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > bool from_gc = (type == CURSEG_ALL_DATA_ATGC); > struct seg_entry *se = NULL; > bool segment_full = false; > + int ret = 0; > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > > mutex_lock(&curseg->curseg_mutex); > down_write(&sit_i->sentry_lock); > > - if (curseg->segno == NULL_SEGNO) > + if (curseg->segno == NULL_SEGNO) { > + ret = -ENOSPC; > goto out_err; > + } > > if (from_gc) { > f2fs_bug_on(sbi, GET_SEGNO(sbi, old_blkaddr) == NULL_SEGNO); > @@ -3546,17 +3556,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > } > > if (from_gc) { > - get_atssr_segment(sbi, type, se->type, > + ret = get_atssr_segment(sbi, type, se->type, > AT_SSR, se->mtime); > } else { > if (need_new_seg(sbi, type)) > - new_curseg(sbi, type, false); > + ret = new_curseg(sbi, type, false); > else > - change_curseg(sbi, type); > + ret = change_curseg(sbi, type); > stat_inc_seg_type(sbi, curseg); > } > > - if (curseg->segno == NULL_SEGNO) > + if (ret) > goto out_err; > } > > @@ -3599,7 +3609,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > up_write(&sit_i->sentry_lock); > mutex_unlock(&curseg->curseg_mutex); > f2fs_up_read(&SM_I(sbi)->curseg_lock); > - return -ENOSPC; > + return ret; > > } > > @@ -3829,7 +3839,8 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum, > /* change the current segment */ > if (segno != curseg->segno) { > curseg->next_segno = segno; > - change_curseg(sbi, type); > + if (change_curseg(sbi, type)) > + goto out_unlock; > } > > curseg->next_blkoff = GET_BLKOFF_FROM_SEG0(sbi, new_blkaddr); > @@ -3855,12 +3866,14 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum, > if (recover_curseg) { > if (old_cursegno != curseg->segno) { > curseg->next_segno = old_cursegno; > - change_curseg(sbi, type); > + if (change_curseg(sbi, type)) > + goto out_unlock; > } > curseg->next_blkoff = old_blkoff; > curseg->alloc_type = old_alloc_type; > } > > +out_unlock: > up_write(&sit_i->sentry_lock); > mutex_unlock(&curseg->curseg_mutex); > f2fs_up_write(&SM_I(sbi)->curseg_lock); > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 78a7658..f2b6d3f 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -4685,7 +4685,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > if (err) > goto free_meta; > > - f2fs_init_inmem_curseg(sbi); > + err = f2fs_init_inmem_curseg(sbi); > + if (err) > + goto sync_free_meta; > > /* f2fs_recover_fsync_data() cleared this already */ > clear_sbi_flag(sbi, SBI_POR_DOING); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to handle error paths of {new,change}_curseg() 2024-03-11 6:09 ` Chao Yu @ 2024-03-11 7:19 ` Zhiguo Niu 0 siblings, 0 replies; 6+ messages in thread From: Zhiguo Niu @ 2024-03-11 7:19 UTC (permalink / raw) To: Chao Yu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, hongyu.jin On Mon, Mar 11, 2024 at 2:09 PM Chao Yu <chao@kernel.org> wrote: > > On 2024/3/8 18:12, Zhiguo Niu wrote: > > {new,change}_curseg() may return error in some special cases, > > error handling should be did in their callers, and this will also > > facilitate subsequent error path expansion in {new,change}_curseg(). > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > Signed-off-by: Chao Yu <chao@kernel.org> > > --- > > fs/f2fs/extent_cache.c | 2 +- > > fs/f2fs/f2fs.h | 4 ++-- > > fs/f2fs/gc.c | 7 +++++-- > > fs/f2fs/segment.c | 57 +++++++++++++++++++++++++++++++------------------- > > fs/f2fs/super.c | 4 +++- > > 5 files changed, 46 insertions(+), 28 deletions(-) > > > > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c > > index 48048fa..dce00cf 100644 > > --- a/fs/f2fs/extent_cache.c > > +++ b/fs/f2fs/extent_cache.c > > @@ -988,7 +988,7 @@ bool f2fs_lookup_read_extent_cache_block(struct inode *inode, pgoff_t index, > > > > void f2fs_update_read_extent_cache(struct dnode_of_data *dn) > > { > > - return __update_extent_cache(dn, EX_READ); > > + __update_extent_cache(dn, EX_READ); > > Above change is not related to this patch? > > Otherwise, it looks good to me. > > Thanks, Dear Chao, Okay, I see that both functions here are void type, so there is no need to use return^^. I will remove this part and update. By the way, I did a stability test on this patch and the result passed thanks! > > > } > > > > void f2fs_update_read_extent_cache_range(struct dnode_of_data *dn, > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 4836e7c..7beb074 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -3700,10 +3700,10 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, > > void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi); > > int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra); > > bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno); > > -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi); > > +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi); > > void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi); > > void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi); > > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > unsigned int start, unsigned int end); > > int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force); > > int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi); > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > > index ca1bf41..8852814 100644 > > --- a/fs/f2fs/gc.c > > +++ b/fs/f2fs/gc.c > > @@ -2035,8 +2035,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi, > > mutex_unlock(&DIRTY_I(sbi)->seglist_lock); > > > > /* Move out cursegs from the target range */ > > - for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) > > - f2fs_allocate_segment_for_resize(sbi, type, start, end); > > + for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) { > > + err = f2fs_allocate_segment_for_resize(sbi, type, start, end); > > + if (err) > > + goto out; > > + } > > > > /* do GC to move out valid blocks in the range */ > > err = f2fs_gc_range(sbi, start, end, dry_run, 0); > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > > index 4e4a51a..c1c1308 100644 > > --- a/fs/f2fs/segment.c > > +++ b/fs/f2fs/segment.c > > @@ -2863,7 +2863,7 @@ bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno) > > * This function always allocates a used segment(from dirty seglist) by SSR > > * manner, so it should recover the existing segment information of valid blocks > > */ > > -static void change_curseg(struct f2fs_sb_info *sbi, int type) > > +static int change_curseg(struct f2fs_sb_info *sbi, int type) > > { > > struct dirty_seglist_info *dirty_i = DIRTY_I(sbi); > > struct curseg_info *curseg = CURSEG_I(sbi, type); > > @@ -2888,21 +2888,23 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type) > > if (IS_ERR(sum_page)) { > > /* GC won't be able to use stale summary pages by cp_error */ > > memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE); > > - return; > > + return PTR_ERR(sum_page); > > } > > sum_node = (struct f2fs_summary_block *)page_address(sum_page); > > memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE); > > f2fs_put_page(sum_page, 1); > > + return 0; > > } > > > > static int get_ssr_segment(struct f2fs_sb_info *sbi, int type, > > int alloc_mode, unsigned long long age); > > > > -static void get_atssr_segment(struct f2fs_sb_info *sbi, int type, > > +static int get_atssr_segment(struct f2fs_sb_info *sbi, int type, > > int target_type, int alloc_mode, > > unsigned long long age) > > { > > struct curseg_info *curseg = CURSEG_I(sbi, type); > > + int ret = 0; > > > > curseg->seg_type = target_type; > > > > @@ -2910,38 +2912,41 @@ static void get_atssr_segment(struct f2fs_sb_info *sbi, int type, > > struct seg_entry *se = get_seg_entry(sbi, curseg->next_segno); > > > > curseg->seg_type = se->type; > > - change_curseg(sbi, type); > > + ret = change_curseg(sbi, type); > > } else { > > /* allocate cold segment by default */ > > curseg->seg_type = CURSEG_COLD_DATA; > > - new_curseg(sbi, type, true); > > + ret = new_curseg(sbi, type, true); > > } > > stat_inc_seg_type(sbi, curseg); > > + return ret; > > } > > > > -static void __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi) > > +static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi) > > { > > struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC); > > + int ret = 0; > > > > if (!sbi->am.atgc_enabled) > > - return; > > + return 0; > > > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > > > > mutex_lock(&curseg->curseg_mutex); > > down_write(&SIT_I(sbi)->sentry_lock); > > > > - get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, CURSEG_COLD_DATA, SSR, 0); > > + ret = get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, > > + CURSEG_COLD_DATA, SSR, 0); > > > > up_write(&SIT_I(sbi)->sentry_lock); > > mutex_unlock(&curseg->curseg_mutex); > > > > f2fs_up_read(&SM_I(sbi)->curseg_lock); > > - > > + return ret; > > } > > -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi) > > +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi) > > { > > - __f2fs_init_atgc_curseg(sbi); > > + return __f2fs_init_atgc_curseg(sbi); > > } > > > > static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type) > > @@ -3069,11 +3074,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type) > > return false; > > } > > > > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > unsigned int start, unsigned int end) > > { > > struct curseg_info *curseg = CURSEG_I(sbi, type); > > unsigned int segno; > > + int ret = 0; > > > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > > mutex_lock(&curseg->curseg_mutex); > > @@ -3084,9 +3090,9 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > goto unlock; > > > > if (f2fs_need_SSR(sbi) && get_ssr_segment(sbi, type, SSR, 0)) > > - change_curseg(sbi, type); > > + ret = change_curseg(sbi, type); > > else > > - new_curseg(sbi, type, true); > > + ret = new_curseg(sbi, type, true); > > > > stat_inc_seg_type(sbi, curseg); > > > > @@ -3100,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > > > mutex_unlock(&curseg->curseg_mutex); > > f2fs_up_read(&SM_I(sbi)->curseg_lock); > > + return ret; > > } > > > > static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, > > @@ -3486,14 +3493,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > > bool from_gc = (type == CURSEG_ALL_DATA_ATGC); > > struct seg_entry *se = NULL; > > bool segment_full = false; > > + int ret = 0; > > > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > > > > mutex_lock(&curseg->curseg_mutex); > > down_write(&sit_i->sentry_lock); > > > > - if (curseg->segno == NULL_SEGNO) > > + if (curseg->segno == NULL_SEGNO) { > > + ret = -ENOSPC; > > goto out_err; > > + } > > > > if (from_gc) { > > f2fs_bug_on(sbi, GET_SEGNO(sbi, old_blkaddr) == NULL_SEGNO); > > @@ -3546,17 +3556,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > > } > > > > if (from_gc) { > > - get_atssr_segment(sbi, type, se->type, > > + ret = get_atssr_segment(sbi, type, se->type, > > AT_SSR, se->mtime); > > } else { > > if (need_new_seg(sbi, type)) > > - new_curseg(sbi, type, false); > > + ret = new_curseg(sbi, type, false); > > else > > - change_curseg(sbi, type); > > + ret = change_curseg(sbi, type); > > stat_inc_seg_type(sbi, curseg); > > } > > > > - if (curseg->segno == NULL_SEGNO) > > + if (ret) > > goto out_err; > > } > > > > @@ -3599,7 +3609,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > > up_write(&sit_i->sentry_lock); > > mutex_unlock(&curseg->curseg_mutex); > > f2fs_up_read(&SM_I(sbi)->curseg_lock); > > - return -ENOSPC; > > + return ret; > > > > } > > > > @@ -3829,7 +3839,8 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum, > > /* change the current segment */ > > if (segno != curseg->segno) { > > curseg->next_segno = segno; > > - change_curseg(sbi, type); > > + if (change_curseg(sbi, type)) > > + goto out_unlock; > > } > > > > curseg->next_blkoff = GET_BLKOFF_FROM_SEG0(sbi, new_blkaddr); > > @@ -3855,12 +3866,14 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum, > > if (recover_curseg) { > > if (old_cursegno != curseg->segno) { > > curseg->next_segno = old_cursegno; > > - change_curseg(sbi, type); > > + if (change_curseg(sbi, type)) > > + goto out_unlock; > > } > > curseg->next_blkoff = old_blkoff; > > curseg->alloc_type = old_alloc_type; > > } > > > > +out_unlock: > > up_write(&sit_i->sentry_lock); > > mutex_unlock(&curseg->curseg_mutex); > > f2fs_up_write(&SM_I(sbi)->curseg_lock); > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > index 78a7658..f2b6d3f 100644 > > --- a/fs/f2fs/super.c > > +++ b/fs/f2fs/super.c > > @@ -4685,7 +4685,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > > if (err) > > goto free_meta; > > > > - f2fs_init_inmem_curseg(sbi); > > + err = f2fs_init_inmem_curseg(sbi); > > + if (err) > > + goto sync_free_meta; > > > > /* f2fs_recover_fsync_data() cleared this already */ > > clear_sbi_flag(sbi, SBI_POR_DOING); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry 2024-03-08 10:12 [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry Zhiguo Niu 2024-03-08 10:12 ` [PATCH 2/2] f2fs: fix to handle error paths of {new,change}_curseg() Zhiguo Niu @ 2024-03-11 3:54 ` Chao Yu 2024-03-11 7:26 ` Zhiguo Niu 1 sibling, 1 reply; 6+ messages in thread From: Chao Yu @ 2024-03-11 3:54 UTC (permalink / raw) To: Zhiguo Niu, jaegeuk Cc: linux-f2fs-devel, linux-kernel, niuzhiguo84, ke.wang, hongyu.jin On 2024/3/8 18:12, Zhiguo Niu wrote: > add_bio_entry should not trigger system panic when bio_add_page fail, > fix to remove it. > > Fixes: 0b20fcec8651 ("f2fs: cache global IPU bio") > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > --- > fs/f2fs/data.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index d9494b5..f8ae684 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -759,8 +759,10 @@ static void add_bio_entry(struct f2fs_sb_info *sbi, struct bio *bio, > be->bio = bio; > bio_get(bio); > > - if (bio_add_page(bio, page, PAGE_SIZE, 0) != PAGE_SIZE) > - f2fs_bug_on(sbi, 1); > + if (bio_add_page(bio, page, PAGE_SIZE, 0) != PAGE_SIZE) { > + bio_put(bio); I didn't get it, why new created bio has no space to store one page? Thanks, > + return; > + } > > f2fs_down_write(&io->bio_list_lock); > list_add_tail(&be->list, &io->bio_list); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry 2024-03-11 3:54 ` [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry Chao Yu @ 2024-03-11 7:26 ` Zhiguo Niu 0 siblings, 0 replies; 6+ messages in thread From: Zhiguo Niu @ 2024-03-11 7:26 UTC (permalink / raw) To: Chao Yu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, hongyu.jin On Mon, Mar 11, 2024 at 11:54 AM Chao Yu <chao@kernel.org> wrote: > > On 2024/3/8 18:12, Zhiguo Niu wrote: > > add_bio_entry should not trigger system panic when bio_add_page fail, > > fix to remove it. > > > > Fixes: 0b20fcec8651 ("f2fs: cache global IPU bio") > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > --- > > fs/f2fs/data.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > index d9494b5..f8ae684 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -759,8 +759,10 @@ static void add_bio_entry(struct f2fs_sb_info *sbi, struct bio *bio, > > be->bio = bio; > > bio_get(bio); > > > > - if (bio_add_page(bio, page, PAGE_SIZE, 0) != PAGE_SIZE) > > - f2fs_bug_on(sbi, 1); > > + if (bio_add_page(bio, page, PAGE_SIZE, 0) != PAGE_SIZE) { > > + bio_put(bio); > > I didn't get it, why new created bio has no space to store one page? > > Thanks, Dear Chao, I got what you mean. We are doing bio merge optimization in bio_add_page. After looking at all the locations where bio_add_page is called, and think it is unreasonable to panic the system if bio_add_page fails. but it is not impossible to panic in the current flow about bio_add_page. so keeping it as is is a good choice. thanks! > > + return; > > + } > > > > f2fs_down_write(&io->bio_list_lock); > > list_add_tail(&be->list, &io->bio_list); ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-11 7:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-08 10:12 [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry Zhiguo Niu
2024-03-08 10:12 ` [PATCH 2/2] f2fs: fix to handle error paths of {new,change}_curseg() Zhiguo Niu
2024-03-11 6:09 ` Chao Yu
2024-03-11 7:19 ` Zhiguo Niu
2024-03-11 3:54 ` [PATCH 1/2] f2fs: fix to remove f2fs_bug_on in add_bio_entry Chao Yu
2024-03-11 7:26 ` Zhiguo Niu
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 NNTP newsgroup(s).