| Message ID | 1709292976-13118-1-git-send-email-zhiguo.niu@unisoc.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel+bounces-88365-ouuuleilei=gmail.com@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a05:7301:2097:b0:108:e6aa:91d0 with SMTP id
gs23csp1015229dyb;
Fri, 1 Mar 2024 03:37:55 -0800 (PST)
X-Forwarded-Encrypted: i=3;
AJvYcCXfWPRfutckdk/dtNj139rTnh0misclCLRgIU8hMH38sNUCtYbb298hX32qV1rZdqEh9vzoqxYpI1b6ggYxxmoJQ7PogA==
X-Google-Smtp-Source:
AGHT+IFu1b6n2tPhWdmi0ZGVpR6DsEQ6q4fBbxTtgzndhXEUCgO4tW9zUXZw+dhjDT2tz+9WcSVC
X-Received: by 2002:a17:903:41c8:b0:1dc:cd80:8f58 with SMTP id
u8-20020a17090341c800b001dccd808f58mr2303601ple.30.1709293075026;
Fri, 01 Mar 2024 03:37:55 -0800 (PST)
ARC-Seal: i=2; a=rsa-sha256; t=1709293075; cv=pass;
d=google.com; s=arc-20160816;
b=VDfj1Oy58E4YgWOFjrY3/WdU5k+uaRD6oQa7TGp35YC8NG7dGaVYDAgvOpYAlQpphX
nyqaD/CrstSkMfy6DG2oJYE6ctMFaGw8QmuWAad+vgr7sIYBt/a8Db/DZBoCED6pIh6R
f35dVzBAxJYnqi89seve/Z3TjBQ7/9NJtosn9oU7fg6h2EC1fLEIJwdtZmuFXNFCafh8
UBEm5c6xo9fu+cQAk+lT9ea+MW9QCFRWSoLB8S0MO5xgo/Eqxu1Zf3wZzhiIpXa5hXPz
PYnq+OGQ8gacIvT+an3q6r/Aob1IRWUMee2QSOxojU9E8ySjxao84+MV7s80F4NcqoKj
KLlA==
ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=mime-version:list-unsubscribe:list-subscribe:list-id:precedence
:message-id:date:subject:cc:to:from;
bh=DfxOKDksgm9ZQ62GXl9t0jLCRqsGCn25+BykDw29rmc=;
fh=r88ULmOhzmh3DE8RgcUXK+PF5Q+3czCYkx5yGhGfkR0=;
b=aaP9tRMi1elJexqU1ph7zt47+1naM1ZLFMD1CZnORqVPoDRZWe3QfRZa9qSCiBYFMS
uiINnYL68RsY6kVqrdHyWbnmUiD5Ful12vlDx1BvxGAMMSPduQViMGsEwB8DublCDYgg
0fRE5aysGyJQh9j1Oxom3i2/jPjx87svXWl+LbnYrfSzIqoc4Ok3b0KvHoT9nR5oBzFN
ccqqERowCGc8qikN099Y7JlvflfzR8GGDqtw+yt9y4UNWNl6bx+/T6ZjtSges3Pit1LC
s9Oqyp9il+Euux+A+WGbYASPPEgcWvlvLoI0ELEN0jFtUcn+lVfthH0zDHnMjcMVgJaD
vFFg==;
dara=google.com
ARC-Authentication-Results: i=2; mx.google.com;
arc=pass (i=1 spf=pass spfdomain=unisoc.com);
spf=pass (google.com: domain of
linux-kernel+bounces-88365-ouuuleilei=gmail.com@vger.kernel.org designates
2604:1380:45e3:2400::1 as permitted sender)
smtp.mailfrom="linux-kernel+bounces-88365-ouuuleilei=gmail.com@vger.kernel.org"
Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org.
[2604:1380:45e3:2400::1])
by mx.google.com with ESMTPS id
az9-20020a170902a58900b001db70a9a26fsi3192467plb.133.2024.03.01.03.37.54
for <ouuuleilei@gmail.com>
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Fri, 01 Mar 2024 03:37:55 -0800 (PST)
Received-SPF: pass (google.com: domain of
linux-kernel+bounces-88365-ouuuleilei=gmail.com@vger.kernel.org designates
2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Authentication-Results: mx.google.com;
arc=pass (i=1 spf=pass spfdomain=unisoc.com);
spf=pass (google.com: domain of
linux-kernel+bounces-88365-ouuuleilei=gmail.com@vger.kernel.org designates
2604:1380:45e3:2400::1 as permitted sender)
smtp.mailfrom="linux-kernel+bounces-88365-ouuuleilei=gmail.com@vger.kernel.org"
Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org
[52.25.139.140])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by sv.mirrors.kernel.org (Postfix) with ESMTPS id 70BAE28D575
for <ouuuleilei@gmail.com>; Fri, 1 Mar 2024 11:37:54 +0000 (UTC)
Received: from localhost.localdomain (localhost.localdomain [127.0.0.1])
by smtp.subspace.kernel.org (Postfix) with ESMTP id D2DB36EB50;
Fri, 1 Mar 2024 11:37:00 +0000 (UTC)
Received: from SHSQR01.spreadtrum.com (unknown [222.66.158.135])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by smtp.subspace.kernel.org (Postfix) with ESMTPS id CF16C6EB51
for <linux-kernel@vger.kernel.org>; Fri, 1 Mar 2024 11:36:55 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org;
arc=none smtp.client-ip=222.66.158.135
ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
t=1709293019; cv=none;
b=djTXL/CBoNDSrtQ2VmGTo2vGRa0ZoX+VMkkkaChUPNFjcZLD5yH7iZHjP0GVKsYTHxBaDFMNnW4peWRO4pwaEW6RPXfLfwDhB8OriQUmuae29yjGz6RSGwE9oej1EpqnOLhnFjDINermPawWVsQX4U8iTfSkd6/uYUGiC/fyRYM=
ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org;
s=arc-20240116; t=1709293019; c=relaxed/simple;
bh=kTOIpCx85MBExZD7m/gewjcjf04YHT5rYbCmo2npUI4=;
h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type;
b=swAeWRZSxMzOt2/+h6twhCqkDA6UMr7OybM8d7ytMjqFVG2HM4POa7ABV+gpHcwpXkJlwu6T8D7QzX/w5EDa/kDw4UttwH5ejs2GZwXdOd1Jh3GOOyURn1TZAp/7edv46vfZJWzQovO9/i6z256oBe4CMPA2cCmCkcLEwJ0uPn0=
ARC-Authentication-Results: i=1; smtp.subspace.kernel.org;
dmarc=none (p=none dis=none) header.from=unisoc.com;
spf=pass smtp.mailfrom=unisoc.com; arc=none smtp.client-ip=222.66.158.135
Authentication-Results: smtp.subspace.kernel.org;
dmarc=none (p=none dis=none) header.from=unisoc.com
Authentication-Results: smtp.subspace.kernel.org;
spf=pass smtp.mailfrom=unisoc.com
Received: from dlp.unisoc.com ([10.29.3.86])
by SHSQR01.spreadtrum.com with ESMTP id 421BaPmN084487;
Fri, 1 Mar 2024 19:36:25 +0800 (+08)
(envelope-from Zhiguo.Niu@unisoc.com)
Received: from SHDLP.spreadtrum.com (bjmbx02.spreadtrum.com [10.0.64.8])
by dlp.unisoc.com (SkyGuard) with ESMTPS id 4TmQxw6S0bz2KkZ9v;
Fri, 1 Mar 2024 19:35:32 +0800 (CST)
Received: from bj08434pcu.spreadtrum.com (10.0.73.87) by
BJMBX02.spreadtrum.com (10.0.64.8) with Microsoft SMTP Server (TLS) id
15.0.1497.23; Fri, 1 Mar 2024 19:36:23 +0800
From: Zhiguo Niu <zhiguo.niu@unisoc.com>
To: <jaegeuk@kernel.org>, <chao@kernel.org>
CC: <linux-f2fs-devel@lists.sourceforge.net>, <linux-kernel@vger.kernel.org>,
<niuzhiguo84@gmail.com>, <zhiguo.niu@unisoc.com>,
<ke.wang@unisoc.com>,
<hongyu.jin@unisoc.com>
Subject: [PATCH] f2fs: fix to check result of new_curseg in
f2fs_allocate_segment_for_resize
Date: Fri, 1 Mar 2024 19:36:16 +0800
Message-ID: <1709292976-13118-1-git-send-email-zhiguo.niu@unisoc.com>
X-Mailer: git-send-email 1.9.1
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain
X-ClientProxiedBy: SHCAS03.spreadtrum.com (10.0.1.207) To
BJMBX02.spreadtrum.com (10.0.64.8)
X-MAIL: SHSQR01.spreadtrum.com 421BaPmN084487
X-getmail-retrieved-from-mailbox: INBOX
X-GMAIL-THRID: 1792323695479943094
X-GMAIL-MSGID: 1792323695479943094
|
| Series |
f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize
|
|
Commit Message
Zhiguo Niu
March 1, 2024, 11:36 a.m. UTC
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(-)
Comments
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,
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,
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);
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,
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,
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,
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,