* [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize @ 2024-03-01 11:36 Zhiguo Niu 2024-03-04 3:19 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Zhiguo Niu @ 2024-03-01 11:36 UTC (permalink / raw) To: jaegeuk, chao Cc: linux-f2fs-devel, linux-kernel, niuzhiguo84, zhiguo.niu, ke.wang, hongyu.jin new_curseg may return error if get_new_segment fail, so its result should be check in its caller f2fs_allocate_segment_for_resize, alos pass this results to free_segment_range. Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> --- fs/f2fs/f2fs.h | 2 +- fs/f2fs/gc.c | 7 +++++-- fs/f2fs/segment.c | 9 +++++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 4331012..39dda7d 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, void 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 c60b747..7a458fa 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -2037,8 +2037,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 1bb3019..2a07b9d 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -3071,11 +3071,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 err = 0; f2fs_down_read(&SM_I(sbi)->curseg_lock); mutex_lock(&curseg->curseg_mutex); @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, change_curseg(sbi, type); else new_curseg(sbi, type, true); - + if (curseg->segno == NULL_SEGNO) { + err = -ENOSPC; + goto unlock; + } stat_inc_seg_type(sbi, curseg); locate_dirty_segment(sbi, segno); @@ -3102,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 err; } static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, -- 1.9.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize 2024-03-01 11:36 [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize Zhiguo Niu @ 2024-03-04 3:19 ` Chao Yu 2024-03-04 3:33 ` Zhiguo Niu 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2024-03-04 3:19 UTC (permalink / raw) To: Zhiguo Niu, jaegeuk Cc: linux-f2fs-devel, linux-kernel, niuzhiguo84, ke.wang, hongyu.jin On 2024/3/1 19:36, Zhiguo Niu wrote: > new_curseg may return error if get_new_segment fail, so its result > should be check in its caller f2fs_allocate_segment_for_resize, > alos pass this results to free_segment_range. Zhiguo, What about handling all error paths of new_curseg() and change_curseg() in one patch? Thanks, > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > --- > fs/f2fs/f2fs.h | 2 +- > fs/f2fs/gc.c | 7 +++++-- > fs/f2fs/segment.c | 9 +++++++-- > 3 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 4331012..39dda7d 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, > void 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 c60b747..7a458fa 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -2037,8 +2037,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 1bb3019..2a07b9d 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -3071,11 +3071,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 err = 0; > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > mutex_lock(&curseg->curseg_mutex); > @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > change_curseg(sbi, type); > else > new_curseg(sbi, type, true); > - > + if (curseg->segno == NULL_SEGNO) { > + err = -ENOSPC; > + goto unlock; > + } > stat_inc_seg_type(sbi, curseg); > > locate_dirty_segment(sbi, segno); > @@ -3102,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 err; > } > > static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize 2024-03-04 3:19 ` Chao Yu @ 2024-03-04 3:33 ` Zhiguo Niu 2024-03-04 7:17 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Zhiguo Niu @ 2024-03-04 3:33 UTC (permalink / raw) To: Chao Yu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, hongyu.jin On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote: > > On 2024/3/1 19:36, Zhiguo Niu wrote: > > new_curseg may return error if get_new_segment fail, so its result > > should be check in its caller f2fs_allocate_segment_for_resize, > > alos pass this results to free_segment_range. > > Zhiguo, > > What about handling all error paths of new_curseg() and change_curseg() > in one patch? Dear Chao, Do you mean to merge it with the previous patch “f2fs: fix to check return value of f2fs_gc_range”? Because in addition to new_curseg/change_curseg error handling, there are some other changes in the previous patch. besides, I searched for new related codes, and there should be the only place left without error handling about new_curseg/ change_curseg . thanks! > > Thanks, > > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > --- > > fs/f2fs/f2fs.h | 2 +- > > fs/f2fs/gc.c | 7 +++++-- > > fs/f2fs/segment.c | 9 +++++++-- > > 3 files changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 4331012..39dda7d 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, > > void 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 c60b747..7a458fa 100644 > > --- a/fs/f2fs/gc.c > > +++ b/fs/f2fs/gc.c > > @@ -2037,8 +2037,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 1bb3019..2a07b9d 100644 > > --- a/fs/f2fs/segment.c > > +++ b/fs/f2fs/segment.c > > @@ -3071,11 +3071,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 err = 0; > > > > f2fs_down_read(&SM_I(sbi)->curseg_lock); > > mutex_lock(&curseg->curseg_mutex); > > @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > > change_curseg(sbi, type); > > else > > new_curseg(sbi, type, true); > > - > > + if (curseg->segno == NULL_SEGNO) { > > + err = -ENOSPC; > > + goto unlock; > > + } > > stat_inc_seg_type(sbi, curseg); > > > > locate_dirty_segment(sbi, segno); > > @@ -3102,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 err; > > } > > > > static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize 2024-03-04 3:33 ` Zhiguo Niu @ 2024-03-04 7:17 ` Chao Yu 2024-03-04 7:52 ` Zhiguo Niu 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2024-03-04 7:17 UTC (permalink / raw) To: Zhiguo Niu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, hongyu.jin On 2024/3/4 11:33, Zhiguo Niu wrote: > On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote: >> >> On 2024/3/1 19:36, Zhiguo Niu wrote: >>> new_curseg may return error if get_new_segment fail, so its result >>> should be check in its caller f2fs_allocate_segment_for_resize, >>> alos pass this results to free_segment_range. >> >> Zhiguo, >> >> What about handling all error paths of new_curseg() and change_curseg() >> in one patch? > Dear Chao, > > Do you mean to merge it with the previous patch “f2fs: fix to check > return value of f2fs_gc_range”? > Because in addition to new_curseg/change_curseg error handling, there > are some other changes in the previous patch. > besides, I searched for new related codes, and there should be the > only place left without error handling about new_curseg/ > change_curseg . Zhiguo, I meant something like this? Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg() --- fs/f2fs/f2fs.h | 4 +-- fs/f2fs/gc.c | 7 +++-- fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++---------------- fs/f2fs/super.c | 4 ++- 4 files changed, 54 insertions(+), 28 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 80789255bf68..03927f1b2ea1 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable); 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 f8314765246a..854ad0a3f6ea 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -2033,8 +2033,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 22241aba6564..2bcf01fde143 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,24 @@ 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; curseg->seg_type = target_type; @@ -2910,38 +2913,45 @@ 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; 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 +3079,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 +3095,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 +3111,8 @@ 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, @@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, { struct curseg_info *curseg = CURSEG_I(sbi, type); unsigned int old_segno; + int ret; if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited) goto allocate; @@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, allocate: old_segno = curseg->segno; - if (new_curseg(sbi, type, true)) - return -EAGAIN; + ret = new_curseg(sbi, type, true); + if (ret) + return ret; stat_inc_seg_type(sbi, curseg); locate_dirty_segment(sbi, old_segno); return 0; @@ -3480,14 +3495,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); @@ -3541,17 +3559,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; } @@ -3594,7 +3612,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; } @@ -3824,7 +3842,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); @@ -3850,12 +3869,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 0676c2dcbbf7..8c69257db8cc 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -4691,7 +4691,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); -- 2.40.1 > > thanks! >> >> Thanks, >> >>> >>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> >>> --- >>> fs/f2fs/f2fs.h | 2 +- >>> fs/f2fs/gc.c | 7 +++++-- >>> fs/f2fs/segment.c | 9 +++++++-- >>> 3 files changed, 13 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>> index 4331012..39dda7d 100644 >>> --- a/fs/f2fs/f2fs.h >>> +++ b/fs/f2fs/f2fs.h >>> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, >>> void 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 c60b747..7a458fa 100644 >>> --- a/fs/f2fs/gc.c >>> +++ b/fs/f2fs/gc.c >>> @@ -2037,8 +2037,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 1bb3019..2a07b9d 100644 >>> --- a/fs/f2fs/segment.c >>> +++ b/fs/f2fs/segment.c >>> @@ -3071,11 +3071,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 err = 0; >>> >>> f2fs_down_read(&SM_I(sbi)->curseg_lock); >>> mutex_lock(&curseg->curseg_mutex); >>> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, >>> change_curseg(sbi, type); >>> else >>> new_curseg(sbi, type, true); >>> - >>> + if (curseg->segno == NULL_SEGNO) { >>> + err = -ENOSPC; >>> + goto unlock; >>> + } >>> stat_inc_seg_type(sbi, curseg); >>> >>> locate_dirty_segment(sbi, segno); >>> @@ -3102,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 err; >>> } >>> >>> static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize 2024-03-04 7:17 ` Chao Yu @ 2024-03-04 7:52 ` Zhiguo Niu 2024-03-04 9:19 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Zhiguo Niu @ 2024-03-04 7:52 UTC (permalink / raw) To: Chao Yu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, hongyu.jin On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <chao@kernel.org> wrote: > > On 2024/3/4 11:33, Zhiguo Niu wrote: > > On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote: > >> > >> On 2024/3/1 19:36, Zhiguo Niu wrote: > >>> new_curseg may return error if get_new_segment fail, so its result > >>> should be check in its caller f2fs_allocate_segment_for_resize, > >>> alos pass this results to free_segment_range. > >> > >> Zhiguo, > >> > >> What about handling all error paths of new_curseg() and change_curseg() > >> in one patch? > > Dear Chao, > > > > Do you mean to merge it with the previous patch “f2fs: fix to check > > return value of f2fs_gc_range”? > > Because in addition to new_curseg/change_curseg error handling, there > > are some other changes in the previous patch. > > besides, I searched for new related codes, and there should be the > > only place left without error handling about new_curseg/ > > change_curseg . > > Zhiguo, I meant something like this? > > Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg() Dear Chao, I got your meaning and I think this patch looks good. Please ignore my patch and use your version:）. thanks! > > --- > fs/f2fs/f2fs.h | 4 +-- > fs/f2fs/gc.c | 7 +++-- > fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++---------------- > fs/f2fs/super.c | 4 ++- > 4 files changed, 54 insertions(+), 28 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 80789255bf68..03927f1b2ea1 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable); > 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 f8314765246a..854ad0a3f6ea 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -2033,8 +2033,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 22241aba6564..2bcf01fde143 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,24 @@ 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; > > curseg->seg_type = target_type; > > @@ -2910,38 +2913,45 @@ 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; > > 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 +3079,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 +3095,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 +3111,8 @@ 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, > @@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, > { > struct curseg_info *curseg = CURSEG_I(sbi, type); > unsigned int old_segno; > + int ret; > > if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited) > goto allocate; > @@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, > > allocate: > old_segno = curseg->segno; > - if (new_curseg(sbi, type, true)) > - return -EAGAIN; > + ret = new_curseg(sbi, type, true); > + if (ret) > + return ret; > stat_inc_seg_type(sbi, curseg); > locate_dirty_segment(sbi, old_segno); > return 0; > @@ -3480,14 +3495,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); > @@ -3541,17 +3559,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; > } > > @@ -3594,7 +3612,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; > > } > > @@ -3824,7 +3842,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); > @@ -3850,12 +3869,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 0676c2dcbbf7..8c69257db8cc 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -4691,7 +4691,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); > -- > 2.40.1 > > > > > > > thanks! > >> > >> Thanks, > >> > >>> > >>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > >>> --- > >>> fs/f2fs/f2fs.h | 2 +- > >>> fs/f2fs/gc.c | 7 +++++-- > >>> fs/f2fs/segment.c | 9 +++++++-- > >>> 3 files changed, 13 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >>> index 4331012..39dda7d 100644 > >>> --- a/fs/f2fs/f2fs.h > >>> +++ b/fs/f2fs/f2fs.h > >>> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, > >>> void 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 c60b747..7a458fa 100644 > >>> --- a/fs/f2fs/gc.c > >>> +++ b/fs/f2fs/gc.c > >>> @@ -2037,8 +2037,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 1bb3019..2a07b9d 100644 > >>> --- a/fs/f2fs/segment.c > >>> +++ b/fs/f2fs/segment.c > >>> @@ -3071,11 +3071,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 err = 0; > >>> > >>> f2fs_down_read(&SM_I(sbi)->curseg_lock); > >>> mutex_lock(&curseg->curseg_mutex); > >>> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > >>> change_curseg(sbi, type); > >>> else > >>> new_curseg(sbi, type, true); > >>> - > >>> + if (curseg->segno == NULL_SEGNO) { > >>> + err = -ENOSPC; > >>> + goto unlock; > >>> + } > >>> stat_inc_seg_type(sbi, curseg); > >>> > >>> locate_dirty_segment(sbi, segno); > >>> @@ -3102,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 err; > >>> } > >>> > >>> static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize 2024-03-04 7:52 ` Zhiguo Niu @ 2024-03-04 9:19 ` Chao Yu 2024-03-04 10:43 ` Zhiguo Niu 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2024-03-04 9:19 UTC (permalink / raw) To: Zhiguo Niu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, hongyu.jin On 2024/3/4 15:52, Zhiguo Niu wrote: > On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <chao@kernel.org> wrote: >> >> On 2024/3/4 11:33, Zhiguo Niu wrote: >>> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote: >>>> >>>> On 2024/3/1 19:36, Zhiguo Niu wrote: >>>>> new_curseg may return error if get_new_segment fail, so its result >>>>> should be check in its caller f2fs_allocate_segment_for_resize, >>>>> alos pass this results to free_segment_range. >>>> >>>> Zhiguo, >>>> >>>> What about handling all error paths of new_curseg() and change_curseg() >>>> in one patch? >>> Dear Chao, >>> >>> Do you mean to merge it with the previous patch “f2fs: fix to check >>> return value of f2fs_gc_range”? >>> Because in addition to new_curseg/change_curseg error handling, there >>> are some other changes in the previous patch. >>> besides, I searched for new related codes, and there should be the >>> only place left without error handling about new_curseg/ >>> change_curseg . >> >> Zhiguo, I meant something like this? >> >> Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg() > Dear Chao, > I got your meaning and I think this patch looks good. > Please ignore my patch and use your version:）. Zhiguo, This is a raw patch, can you please write commit message for it, and resend it once you've tested it. Thanks, > thanks! >> >> --- >> fs/f2fs/f2fs.h | 4 +-- >> fs/f2fs/gc.c | 7 +++-- >> fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++---------------- >> fs/f2fs/super.c | 4 ++- >> 4 files changed, 54 insertions(+), 28 deletions(-) >> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 80789255bf68..03927f1b2ea1 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable); >> 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 f8314765246a..854ad0a3f6ea 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -2033,8 +2033,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 22241aba6564..2bcf01fde143 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,24 @@ 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; >> >> curseg->seg_type = target_type; >> >> @@ -2910,38 +2913,45 @@ 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; >> >> 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 +3079,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 +3095,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 +3111,8 @@ 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, >> @@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, >> { >> struct curseg_info *curseg = CURSEG_I(sbi, type); >> unsigned int old_segno; >> + int ret; >> >> if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited) >> goto allocate; >> @@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, >> >> allocate: >> old_segno = curseg->segno; >> - if (new_curseg(sbi, type, true)) >> - return -EAGAIN; >> + ret = new_curseg(sbi, type, true); >> + if (ret) >> + return ret; >> stat_inc_seg_type(sbi, curseg); >> locate_dirty_segment(sbi, old_segno); >> return 0; >> @@ -3480,14 +3495,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); >> @@ -3541,17 +3559,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; >> } >> >> @@ -3594,7 +3612,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; >> >> } >> >> @@ -3824,7 +3842,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); >> @@ -3850,12 +3869,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 0676c2dcbbf7..8c69257db8cc 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -4691,7 +4691,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); >> -- >> 2.40.1 >> >> >> >>> >>> thanks! >>>> >>>> Thanks, >>>> >>>>> >>>>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> >>>>> --- >>>>> fs/f2fs/f2fs.h | 2 +- >>>>> fs/f2fs/gc.c | 7 +++++-- >>>>> fs/f2fs/segment.c | 9 +++++++-- >>>>> 3 files changed, 13 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>>> index 4331012..39dda7d 100644 >>>>> --- a/fs/f2fs/f2fs.h >>>>> +++ b/fs/f2fs/f2fs.h >>>>> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, >>>>> void 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 c60b747..7a458fa 100644 >>>>> --- a/fs/f2fs/gc.c >>>>> +++ b/fs/f2fs/gc.c >>>>> @@ -2037,8 +2037,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 1bb3019..2a07b9d 100644 >>>>> --- a/fs/f2fs/segment.c >>>>> +++ b/fs/f2fs/segment.c >>>>> @@ -3071,11 +3071,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 err = 0; >>>>> >>>>> f2fs_down_read(&SM_I(sbi)->curseg_lock); >>>>> mutex_lock(&curseg->curseg_mutex); >>>>> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, >>>>> change_curseg(sbi, type); >>>>> else >>>>> new_curseg(sbi, type, true); >>>>> - >>>>> + if (curseg->segno == NULL_SEGNO) { >>>>> + err = -ENOSPC; >>>>> + goto unlock; >>>>> + } >>>>> stat_inc_seg_type(sbi, curseg); >>>>> >>>>> locate_dirty_segment(sbi, segno); >>>>> @@ -3102,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 err; >>>>> } >>>>> >>>>> static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize 2024-03-04 9:19 ` Chao Yu @ 2024-03-04 10:43 ` Zhiguo Niu 0 siblings, 0 replies; 7+ messages in thread From: Zhiguo Niu @ 2024-03-04 10:43 UTC (permalink / raw) To: Chao Yu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, hongyu.jin On Mon, Mar 4, 2024 at 5:19 PM Chao Yu <chao@kernel.org> wrote: > > On 2024/3/4 15:52, Zhiguo Niu wrote: > > On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <chao@kernel.org> wrote: > >> > >> On 2024/3/4 11:33, Zhiguo Niu wrote: > >>> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote: > >>>> > >>>> On 2024/3/1 19:36, Zhiguo Niu wrote: > >>>>> new_curseg may return error if get_new_segment fail, so its result > >>>>> should be check in its caller f2fs_allocate_segment_for_resize, > >>>>> alos pass this results to free_segment_range. > >>>> > >>>> Zhiguo, > >>>> > >>>> What about handling all error paths of new_curseg() and change_curseg() > >>>> in one patch? > >>> Dear Chao, > >>> > >>> Do you mean to merge it with the previous patch “f2fs: fix to check > >>> return value of f2fs_gc_range”? > >>> Because in addition to new_curseg/change_curseg error handling, there > >>> are some other changes in the previous patch. > >>> besides, I searched for new related codes, and there should be the > >>> only place left without error handling about new_curseg/ > >>> change_curseg . > >> > >> Zhiguo, I meant something like this? > >> > >> Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg() > > Dear Chao, > > I got your meaning and I think this patch looks good. > > Please ignore my patch and use your version:）. > > Zhiguo, > > This is a raw patch, can you please write commit message for it, and > resend it once you've tested it. Dear Chao, OK, no problem. thanks! > > Thanks, > > > thanks! > >> > >> --- > >> fs/f2fs/f2fs.h | 4 +-- > >> fs/f2fs/gc.c | 7 +++-- > >> fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++---------------- > >> fs/f2fs/super.c | 4 ++- > >> 4 files changed, 54 insertions(+), 28 deletions(-) > >> > >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >> index 80789255bf68..03927f1b2ea1 100644 > >> --- a/fs/f2fs/f2fs.h > >> +++ b/fs/f2fs/f2fs.h > >> @@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable); > >> 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 f8314765246a..854ad0a3f6ea 100644 > >> --- a/fs/f2fs/gc.c > >> +++ b/fs/f2fs/gc.c > >> @@ -2033,8 +2033,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 22241aba6564..2bcf01fde143 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,24 @@ 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; > >> > >> curseg->seg_type = target_type; > >> > >> @@ -2910,38 +2913,45 @@ 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; > >> > >> 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 +3079,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 +3095,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 +3111,8 @@ 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, > >> @@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, > >> { > >> struct curseg_info *curseg = CURSEG_I(sbi, type); > >> unsigned int old_segno; > >> + int ret; > >> > >> if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited) > >> goto allocate; > >> @@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, > >> > >> allocate: > >> old_segno = curseg->segno; > >> - if (new_curseg(sbi, type, true)) > >> - return -EAGAIN; > >> + ret = new_curseg(sbi, type, true); > >> + if (ret) > >> + return ret; > >> stat_inc_seg_type(sbi, curseg); > >> locate_dirty_segment(sbi, old_segno); > >> return 0; > >> @@ -3480,14 +3495,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); > >> @@ -3541,17 +3559,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; > >> } > >> > >> @@ -3594,7 +3612,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; > >> > >> } > >> > >> @@ -3824,7 +3842,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); > >> @@ -3850,12 +3869,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 0676c2dcbbf7..8c69257db8cc 100644 > >> --- a/fs/f2fs/super.c > >> +++ b/fs/f2fs/super.c > >> @@ -4691,7 +4691,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); > >> -- > >> 2.40.1 > >> > >> > >> > >>> > >>> thanks! > >>>> > >>>> Thanks, > >>>> > >>>>> > >>>>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > >>>>> --- > >>>>> fs/f2fs/f2fs.h | 2 +- > >>>>> fs/f2fs/gc.c | 7 +++++-- > >>>>> fs/f2fs/segment.c | 9 +++++++-- > >>>>> 3 files changed, 13 insertions(+), 5 deletions(-) > >>>>> > >>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >>>>> index 4331012..39dda7d 100644 > >>>>> --- a/fs/f2fs/f2fs.h > >>>>> +++ b/fs/f2fs/f2fs.h > >>>>> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, > >>>>> void 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 c60b747..7a458fa 100644 > >>>>> --- a/fs/f2fs/gc.c > >>>>> +++ b/fs/f2fs/gc.c > >>>>> @@ -2037,8 +2037,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 1bb3019..2a07b9d 100644 > >>>>> --- a/fs/f2fs/segment.c > >>>>> +++ b/fs/f2fs/segment.c > >>>>> @@ -3071,11 +3071,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 err = 0; > >>>>> > >>>>> f2fs_down_read(&SM_I(sbi)->curseg_lock); > >>>>> mutex_lock(&curseg->curseg_mutex); > >>>>> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type, > >>>>> change_curseg(sbi, type); > >>>>> else > >>>>> new_curseg(sbi, type, true); > >>>>> - > >>>>> + if (curseg->segno == NULL_SEGNO) { > >>>>> + err = -ENOSPC; > >>>>> + goto unlock; > >>>>> + } > >>>>> stat_inc_seg_type(sbi, curseg); > >>>>> > >>>>> locate_dirty_segment(sbi, segno); > >>>>> @@ -3102,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 err; > >>>>> } > >>>>> > >>>>> static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-04 10:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-01 11:36 [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize Zhiguo Niu 2024-03-04 3:19 ` Chao Yu 2024-03-04 3:33 ` Zhiguo Niu 2024-03-04 7:17 ` Chao Yu 2024-03-04 7:52 ` Zhiguo Niu 2024-03-04 9:19 ` Chao Yu 2024-03-04 10:43 ` Zhiguo Niu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®