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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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,
On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <[email protected]> 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 <[email protected]>
> > ---
> > 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,
On 2024/3/4 11:33, Zhiguo Niu wrote:
> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <[email protected]> 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 <[email protected]>
>>> ---
>>> 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,
On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <[email protected]> wrote:
>
> On 2024/3/4 11:33, Zhiguo Niu wrote:
> > On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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,
On 2024/3/4 15:52, Zhiguo Niu wrote:
> On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <[email protected]> wrote:
>>
>> On 2024/3/4 11:33, Zhiguo Niu wrote:
>>> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> 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,
On Mon, Mar 4, 2024 at 5:19 PM Chao Yu <[email protected]> wrote:
>
> On 2024/3/4 15:52, Zhiguo Niu wrote:
> > On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <[email protected]> wrote:
> >>
> >> On 2024/3/4 11:33, Zhiguo Niu wrote:
> >>> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <[email protected]> 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 <[email protected]>
> >>>>> ---
> >>>>> 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,