From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 EBBC43002A0 for ; Sat, 9 May 2026 08:54:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778316875; cv=none; b=TF0li78D+JFY9rFjKxRx8Ipkm+xuzAasIlgCrTcYxupaqXvq/BtMuqEITKTE8XmyZ9+DfbJ4qgvGGBOEKKlIl4jeoQfxDtRlztwWIjGXbn75hm3hWtjQWQufLkKEA26Xd1eMjD3BwAk4CqQTXLJYVsEgXPG3xL4th1oWrEdb2kc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778316875; c=relaxed/simple; bh=kACYnYiEPp138gxpFCBVt443jU5FbdAoCxCKB42Pyhw=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=gfimhymsOYao+VVP/PQBOaq8IdVhy8LcZQA8eAKiQsU+S1G5vh5afNcmjZ7dji+qgKi7akyz8YDIQWtYzGOzXQAiiyuycdkz8Pz8oYmrE803UwPBlCBSoVi0V8k32f0Jdf87JDu3tagcluvlFD2w4R8YJ+z+uomET3rmCAidYAo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=t57Giu1k; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="t57Giu1k" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 99E5AC2BCB2; Sat, 9 May 2026 08:54:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778316874; bh=kACYnYiEPp138gxpFCBVt443jU5FbdAoCxCKB42Pyhw=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=t57Giu1krAmBI55Kze9IRY4xhgiKyLaZ0OFnOimJPhUgOEy0WOtDpctEtFMICxfN5 Q2URHkl9QWJhGE89dAC3UXEkGqYC2MLHVNCbHBQCvTeSTLQo6K075sPWgEOIyKpWGI Hl7N2Ygex/qxUAQqH5vaFS/FlVVHP8q7b0foYKyAiCCz9iJOW6US5VKrbIB74HTaLf OipADClPXM5Cin6piHrbTIhSsKgaMicwXebjIrk887miLRA7kZy4Hwm+Cn8polxi1O oDqSquqgb5no8SjTufy8o8HkfHINGrBntrvFgCAoRZQIPH61xaBhaJMrXJ8Ey9Vye+ ZhWnvzDglKgWg== Message-ID: Date: Sat, 9 May 2026 16:54:30 +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, quic_stummala@quicinc.com, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, baijiaju1990@gmail.com Subject: Re: [PATCH v2] f2fs: start discard thread after mount recovery To: Cen Zhang , jaegeuk@kernel.org References: <20260504133552.1902811-1-zzzccc427@gmail.com> Content-Language: en-US From: Chao Yu In-Reply-To: <20260504133552.1902811-1-zzzccc427@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 5/4/26 21:35, Cen Zhang wrote: > create_discard_cmd_control() is called from f2fs_build_segment_manager(), > before f2fs_build_node_manager() and mount recovery have completed. It > currently starts the discard thread immediately, so issue_discard_thread() > can run while f2fs_fill_super() is still initializing mount-time state. > > This is not the failure-unwind case where free_nm stops the discard > thread before f2fs_destroy_node_manager() frees nm_info. The window is > earlier: the thread may run while f2fs_build_node_manager() has published > sbi->nm_info but init_node_manager() is still initializing it. After > commit d6d2b491a82e, issue_discard_thread() may call > f2fs_available_free_memory() and read fields such as nm_i->ram_thresh. > > The same early-start window also lets the discard thread observe the > superblock read-only state while mount recovery is still making temporary > SB_RDONLY transitions. > > Keep the discard command control available early, but start the discard > thread later in f2fs_fill_super(), after node-manager initialization and > mount recovery have completed. > > Cc: stable@kernel.org > Fixes: d6d2b491a82e1e411a6766fbfb87c697d8701554 ("f2fs: allow to change discard policy based on cached discard cmds") Fixes: d6d2b491a82e ("f2fs: allow to change discard policy based on cached discard cmds") otherwise, ./scripts/checkpatch.pl will complain as below: ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 'commit d6d2b491a82e ("f2fs: allow to change discard policy based on cached discard cmds")' Thanks, > Signed-off-by: Cen Zhang <zzzccc427@gmail.com> > --- > fs/f2fs/segment.c | 17 ++++------------- > fs/f2fs/super.c | 12 ++++++++++++ > 2 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 8390994a8826..deb98f564165 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -2302,12 +2302,10 @@ int f2fs_start_discard_thread(struct f2fs_sb_info *sbi) > static int create_discard_cmd_control(struct f2fs_sb_info *sbi) > { > struct discard_cmd_control *dcc; > - int err = 0, i; > + int i; > > - if (SM_I(sbi)->dcc_info) { > - dcc = SM_I(sbi)->dcc_info; > - goto init_thread; > - } > + if (SM_I(sbi)->dcc_info) > + return 0; > > dcc = f2fs_kzalloc(sbi, sizeof(struct discard_cmd_control), GFP_KERNEL); > if (!dcc) > @@ -2344,14 +2342,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi) > > init_waitqueue_head(&dcc->discard_wait_queue); > SM_I(sbi)->dcc_info = dcc; > -init_thread: > - err = f2fs_start_discard_thread(sbi); > - if (err) { > - kfree(dcc); > - SM_I(sbi)->dcc_info = NULL; > - } > - > - return err; > + return 0; > } > > static void destroy_discard_cmd_control(struct f2fs_sb_info *sbi) > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 40079fd7886b..8228be53d036 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -5340,6 +5340,15 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) > > f2fs_tuning_parameters(sbi); > > + /* > + * After POR and mount-time recovery, we can run the discard thread. It > + * reads node-manager memory thresholds and the superblock read-only > + * state, so keep it out of the fill_super() initialization window. > + */ > + err = f2fs_start_discard_thread(sbi); > + if (err) > + goto leave_shrinker; > + > f2fs_notice(sbi, "Mounted with checkpoint version = %llx", > cur_cp_version(F2FS_CKPT(sbi))); > f2fs_update_time(sbi, CP_TIME); > @@ -5349,6 +5358,9 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) > sbi->umount_lock_holder = NULL; > return 0; > > +leave_shrinker: > + f2fs_leave_shrinker(sbi); > + f2fs_stop_gc_thread(sbi); > sync_free_meta: > /* safe to flush all the data */ > sync_filesystem(sbi->sb);