From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 D5F1534DCC7 for ; Thu, 20 Aug 2026 07:20:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787210459; cv=none; b=sBKtVuaAnQV7puZA84/TaHE2O/Ey+YwMU+xgcnFejxgRxlt+8UumI/lTirCPJ5cjaJ/9oay24COIivaBzhHmCos1hvzOuYx2ljHS0tfSvCkt6LE1TOx73rU51q9MaP/1RiGXTUOFl1ZF0+b/YkZ6/KyJMsJcfR1A3Ab7pAXTJSU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787210459; c=relaxed/simple; bh=oBIvPl4Ez1tJESmcfVX2qKWKnuArVn/e/Pn/9R+EvNg=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=KASOT/N1EMp1egd20M0LupGla6oCmZz9Qu/0oVEygIbhB595fAPa1TI5N8X3C8X1v1+0p7BXm5tPJENB69BAeTOYmQxrT21tPWmAyPNc0Sc8xsXqSq4Dw2mm+EbwcslFydNurU+qTBvJbCkjeOsJnbEJyHezDeNkXOkH34ci3iU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=afRmFp+U; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="afRmFp+U" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 98E401F000E9; Thu, 20 Aug 2026 07:20:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787210457; bh=/fqovBIP09GTfmPeRatEDRe7nIZ9H7E/7p0NxkSlJGw=; h=Date:Cc:Subject:To:References:From:In-Reply-To; b=afRmFp+UkMwdTZvTFqW3g/F/TdnUJt6GVYLUtdEyw9yRVGEvwpWOW8GInBkxlwptV miE3ed/VI7UtSMssjx6AIBoNgadDpL+aNkcDCOWaAaAjTItrt3kHDqdZsp0YQEGq21 l3XUY9p59PjZZ/WLOO2DLcWqnufGXqZbz3N1eZBNOviDnImWdlhc+cIpoqCxt1m3HX HHDd05eaqaTETBIpHwPHggzTws9J2ESQBa6bwzSKxrrbFN+5qdrdTOpm1hxYPUBbiM 8LqvtLWfEMs+5nHjV1zMXRVcOKRKj5jCKgBLhZ5rrInE+nMjMgA/3+jT40QuuS7VKL xLT+Z4SiUWDyQ== Message-ID: <746c8f52-cf7c-4f63-b2e1-36e0cad940a1@kernel.org> Date: Thu, 20 Aug 2026 15:20:54 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: chao@kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com, Daeho Jeong Subject: Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN To: Daeho Jeong References: <20260818170535.3190869-1-daeho43@gmail.com> <6b55a4e9-e4a0-421c-9bb6-98ffc2c915c9@kernel.org> Content-Language: en-US From: Chao Yu In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 8/20/26 02:21, Daeho Jeong wrote: > On Tue, Aug 18, 2026 at 7:34 PM Chao Yu wrote: >> >> On 8/19/26 01:05, Daeho Jeong wrote: >>> From: Daeho Jeong >>> >>> When f2fs_resize_fs() fails due to transient lock contention or retryable >>> GC failure in free_segment_range() returning -EAGAIN, no filesystem >>> metadata has been modified on-disk yet. The filesystem remains completely >>> consistent and clean. >> >> Any way to make sure EAGAIN is from free_segment_range(), in case we return >> EAGAIN from 1) any other places that we may miss to check now or 2) we changed >> the code to return EAGAIN in future. > > Makes sense. > > In fact, if free_segment_range() fails with ANY error (e.g., -EAGAIN, > -ENOMEM, -ERESTARTSYS, etc.), no on-disk superblock or filesystem metadata > has been modified yet, and free_segment_range() safely restores all in-memory > counters (MAIN_SECS, free_sections) before returning. Therefore, setting > SBI_NEED_FSCK is not needed for all errors occurring in free_segment_range(). Yeah, better. > > To make this explicit and defensive against any future changes, we can > separate the error recovery path with dedicated labels as follows: > > err = free_segment_range(sbi, secs, false); > if (err) > - goto recover_out; > + goto recover_user_blocks; > > update_sb_metadata(sbi, -secs); > > ... > > recover_out: > + set_sbi_flag(sbi, SBI_NEED_FSCK); > + f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); > +recover_user_blocks: How about this? recover_out: if (err) { f2fs_bug_on(sbi, err == -EAGAIN); set_sbi_flag(sbi, SBI_NEED_FSCK); f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); } recover_user_blocks: Thanks, > clear_sbi_flag(sbi, SBI_IS_RESIZEFS); > if (err) { > - set_sbi_flag(sbi, SBI_NEED_FSCK); > - f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); > - > spin_lock(&sbi->stat_lock); > sbi->user_block_count += shrunk_blocks; > spin_unlock(&sbi->stat_lock); > > Thanks, > >> >> Thanks, >> >>> >>> However, the current error recovery path unconditionally sets the >>> SBI_NEED_FSCK flag on any error, forcing an unnecessary and time-consuming >>> fsck.f2fs repair on the subsequent mount/reboot. >>> >>> Fix this by guarding set_sbi_flag(sbi, SBI_NEED_FSCK) with >>> `if (err != -EAGAIN)`, avoiding false-positive filesystem corruption >>> flags on transient resize retries. >>> >>> Signed-off-by: Daeho Jeong >>> Signed-off-by: Sunmin Jeong >>> --- >>> fs/f2fs/gc.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >>> index 192b16ac02f8..787133ee2eb2 100644 >>> --- a/fs/f2fs/gc.c >>> +++ b/fs/f2fs/gc.c >>> @@ -2446,8 +2446,10 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) >>> recover_out: >>> clear_sbi_flag(sbi, SBI_IS_RESIZEFS); >>> if (err) { >>> - set_sbi_flag(sbi, SBI_NEED_FSCK); >>> - f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); >>> + if (err != -EAGAIN) { >>> + set_sbi_flag(sbi, SBI_NEED_FSCK); >>> + f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); >>> + } >>> >>> spin_lock(&sbi->stat_lock); >>> sbi->user_block_count += shrunk_blocks; >>