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 7262423EA9B for ; Sun, 3 May 2026 09:46:50 +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=1777801610; cv=none; b=OO51A/LS3TpXtsy99SNHyV9mJ8nkYdcpXiD/yFdely7eMf98B7lOUWk/CUgl1gaxTVcFSpz9LByn9KpJQh0qeLFqf7+cCqP6n1r6BwwDOe+7AmOqMpsP/mTg8vaObxW52BR8gyRE8vDzDeWmobXgGY1iqlSuLCt+gbjmDoCatDw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777801610; c=relaxed/simple; bh=mCe1FlnmnUjohQTntNZEx42H9cLeeMfqdNdggNwpMQc=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=nMJMcwfhgqiG5SklbVeJ94Jhu+SEc0S6BJxi4MIisxVc70irhZPPNSxLf9cD6i5AdGooFYNCNt6Eld/lyWy76LS/Iiq+2ymnDhDKknv/Jc5nMsuSCLKKVc7OkdNSyxWqK0NTUcW6bSE0TkEIGKuo/w2VTPbEaYuCtfKR6qaZ7jM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=M3l6QTq5; 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="M3l6QTq5" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 394BCC2BCB4; Sun, 3 May 2026 09:46:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1777801610; bh=mCe1FlnmnUjohQTntNZEx42H9cLeeMfqdNdggNwpMQc=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=M3l6QTq5MvpmGvEd6FCx74j2SCJAFnIZJ5oONS4prWRIB34Q11fBlpUiHuLr6JYGC iJL/h1xLZCibmyLsAUwOK7Lk8/dTprEaRINp01JTP7D7b91sq4Qf2hUxVIoS41L5w5 pUxmcrX569XcHm8GX3l4Hfzh3VhP6PuyCAp2+rTBQVuBDwHCWgp9OjaAC2C6p0dXuI i+tOBVuTTyBa/jaINtK3eTtZagpEt5c+SF6BZYTz4VCrtSrc2VmrLRlAEjQPIakE72 3ezlwNPDv+0nq9Z6dnGBkKhC73eim1xbYQASDVSOcYnS/wbKLgAsaog+z5itDuy1qR LpBvDNEWbEE6A== Message-ID: <2ba0b3ad-615e-43c8-a165-af631607a4c3@kernel.org> Date: Sun, 3 May 2026 17:46:45 +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] f2fs: start discard thread after mount recovery To: Cen Zhang , jaegeuk@kernel.org References: <20260503045820.973718-1-zzzccc427@gmail.com> Content-Language: en-US From: Chao Yu In-Reply-To: <20260503045820.973718-1-zzzccc427@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 5/3/26 12:58, Cen Zhang wrote: > The discard command control is built while the segment manager is > constructed, before the node manager is built and before mount recovery > has completed. Starting the discard thread from that constructor lets the > background thread run while f2fs_fill_super() is still publishing and > initializing mount-time state. > > After commit d6d2b491a82e ("f2fs: allow to change discard policy based > on cached discard cmds"), issue_discard_thread() may consult node-manager > memory thresholds through f2fs_available_free_memory(). It can therefore > observe sbi->nm_info while f2fs_build_node_manager() is publishing and > initializing it. The same early-start window also lets the thread test > the superblock read-only state while recovery paths still make temporary > SB_RDONLY transitions. Not sure I understood you correctly, do you mean this? - f2fs_fill_super - f2fs_build_segment_manager - create discard thread - f2fs_build_node_manager - allocate memory sbi->nm_info ... - f2fs_destroy_node_manager - issue_discard_thread - f2fs_available_free_memory() - free sbi->nm_info - UAF: access sbi->nm_info->ram_thresh Thanks, > > Keep create_discard_cmd_control() limited to allocating and publishing > the command-control object, which recovery can use to queue discard > commands. Start the discard thread later in f2fs_fill_super(), after POR, > recovery, checkpoint option handling, and discard tuning have completed. > If starting the thread fails, unwind the shrinker and any GC thread that > may already have been started before continuing through the existing > mount-failure cleanup path. > > Fixes: d6d2b491a82e1e411a6766fbfb87c697d8701554 ("f2fs: allow to change discard policy based on cached discard cmds") > Signed-off-by: Cen Zhang > --- > 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 8390994a8..deb98f564 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 40079fd78..8228be53d 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);