From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 30598C761A6 for ; Sat, 1 Apr 2023 02:29:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233942AbjDAC3L (ORCPT ); Fri, 31 Mar 2023 22:29:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50740 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233727AbjDAC3I (ORCPT ); Fri, 31 Mar 2023 22:29:08 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 92B04EB54 for ; Fri, 31 Mar 2023 19:29:07 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 46C60B83320 for ; Sat, 1 Apr 2023 02:29:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4E4B9C433D2; Sat, 1 Apr 2023 02:29:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680316145; bh=OPadgu5eivq29B65tO77LvWicJc6enTlUvXBiOy1gts=; h=Date:To:References:From:Subject:In-Reply-To:From; b=enKpzenFm9js0MeDGIvKIwlbzJvnJov4Qoby9ncj+ub13XPpONbCRnnnRbYkU0V3f RIL2pcN7710JHqk6X1KjcQfqT+EFtY/c8X1zfF9Wv8BtW+ujpzDZ+tm7tPXrdsJDnD crZbACEgci+06C751wxnVYeYZVsZKezZU8TXiGmL2edBdlDCWBjwM4vTzybbIVV4Bw Sv1+Tz7MeJ84V+11eDp077en5a7JJVhhANOybpcnsEXvvnuYRIP6adCzFGUBrk3B/K whM+EaT3OhKFMPN73WxU6Ob39DWa+bNhM2ZSWDUBzi1SYfQ/hErB+gbd2SFxXAgYzF oTQausXGpaKcQ== Message-ID: <48f18a16-c6d8-3df4-55c5-11546e7dde35@kernel.org> Date: Sat, 1 Apr 2023 10:29:01 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Content-Language: en-US To: yonggil.song@samsung.com, "jaegeuk@kernel.org" , "linux-f2fs-devel@lists.sourceforge.net" , "linux-kernel@vger.kernel.org" References: <20230321001251epcms2p4c1fd48495643dbfca2cf82a433490bb8@epcms2p4> From: Chao Yu Subject: Re: [PATCH v2] f2fs: Fix system crash due to lack of free space in LFS In-Reply-To: <20230321001251epcms2p4c1fd48495643dbfca2cf82a433490bb8@epcms2p4> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2023/3/21 8:12, Yonggil Song wrote: > When f2fs tries to checkpoint during foreground gc in LFS mode, system > crash occurs due to lack of free space if the amount of dirty node and > dentry pages generated by data migration exceeds free space. > The reproduction sequence is as follows. > > - 20GiB capacity block device (null_blk) > - format and mount with LFS mode > - create a file and write 20,000MiB > - 4k random write on full range of the file > > RIP: 0010:new_curseg+0x48a/0x510 [f2fs] > Code: 55 e7 f5 89 c0 48 0f af c3 48 8b 5d c0 48 c1 e8 20 83 c0 01 89 43 6c 48 83 c4 28 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc <0f> 0b f0 41 80 4f 48 04 45 85 f6 0f 84 ba fd ff ff e9 ef fe ff ff > RSP: 0018:ffff977bc397b218 EFLAGS: 00010246 > RAX: 00000000000027b9 RBX: 0000000000000000 RCX: 00000000000027c0 > RDX: 0000000000000000 RSI: 00000000000027b9 RDI: ffff8c25ab4e74f8 > RBP: ffff977bc397b268 R08: 00000000000027b9 R09: ffff8c29e4a34b40 > R10: 0000000000000001 R11: ffff977bc397b0d8 R12: 0000000000000000 > R13: ffff8c25b4dd81a0 R14: 0000000000000000 R15: ffff8c2f667f9000 > FS: 0000000000000000(0000) GS:ffff8c344ec80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c00055d000 CR3: 0000000e30810003 CR4: 00000000003706e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > > allocate_segment_by_default+0x9c/0x110 [f2fs] > f2fs_allocate_data_block+0x243/0xa30 [f2fs] > ? __mod_lruvec_page_state+0xa0/0x150 > do_write_page+0x80/0x160 [f2fs] > f2fs_do_write_node_page+0x32/0x50 [f2fs] > __write_node_page+0x339/0x730 [f2fs] > f2fs_sync_node_pages+0x5a6/0x780 [f2fs] > block_operations+0x257/0x340 [f2fs] > f2fs_write_checkpoint+0x102/0x1050 [f2fs] > f2fs_gc+0x27c/0x630 [f2fs] > ? folio_mark_dirty+0x36/0x70 > f2fs_balance_fs+0x16f/0x180 [f2fs] > > This patch adds checking whether free sections are enough before checkpoint > during gc. > > Signed-off-by: Yonggil Song > --- > fs/f2fs/gc.c | 10 ++++++++-- > fs/f2fs/gc.h | 2 ++ > fs/f2fs/segment.h | 27 ++++++++++++++++++++++----- > 3 files changed, 32 insertions(+), 7 deletions(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 4546e01b2ee0..dd563866d3c9 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -1773,6 +1773,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > .iroot = RADIX_TREE_INIT(gc_list.iroot, GFP_NOFS), > }; > unsigned int skipped_round = 0, round = 0; > + unsigned int need_lower = 0, need_upper = 0; > > trace_f2fs_gc_begin(sbi->sb, gc_type, gc_control->no_bg_gc, > gc_control->nr_free_secs, > @@ -1858,8 +1859,13 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > } > } > > - /* Write checkpoint to reclaim prefree segments */ > - if (free_sections(sbi) < NR_CURSEG_PERSIST_TYPE && > + ret = get_need_secs(sbi, &need_lower, &need_upper); Can we avoid calling has_curseg_enough_space() for this case? Maybe we can add one parameter curseg_no_space for get_need_secs() to get result of has_curseg_enough_space()? static inline void get_need_secs(struct f2fs_sb_info *sbi, unsigned int *lower, unsigned int *upper, bool *curseg_no_space); { ... *lower = node_secs + dent_secs; *upper = *lower + (node_blocks ? 1 : 0) + (dent_blocks ? 1 : 0); if (curseg_no_space) *curseg_no_space = !has_curseg_enough_space(sbi, node_blocks, dent_blocks); } Then we can use get_need_secs(, , NULL) in f2fs_gc(), and use get_need_secs(, , &curseg_no_space) in has_not_enough_free_secs()? Thoughts? Thanks, > + > + /* > + * Write checkpoint to reclaim prefree segments. > + * We need more three extra sections for writer's data/node/dentry. > + */ > + if (free_sections(sbi) <= need_upper + NR_GC_CHECKPOINT_SECS && > prefree_segments(sbi)) { > ret = f2fs_write_checkpoint(sbi, &cpc); > if (ret) > diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h > index 19b956c2d697..e81d22bf3772 100644 > --- a/fs/f2fs/gc.h > +++ b/fs/f2fs/gc.h > @@ -30,6 +30,8 @@ > /* Search max. number of dirty segments to select a victim segment */ > #define DEF_MAX_VICTIM_SEARCH 4096 /* covers 8GB */ > > +#define NR_GC_CHECKPOINT_SECS (3) /* data/node/dentry sections */ > + > struct f2fs_gc_kthread { > struct task_struct *f2fs_gc_task; > wait_queue_head_t gc_wait_queue_head; > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index be8f2d7d007b..52a6d1ed4f24 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -605,8 +605,12 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi, > return true; > } > > -static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi, > - int freed, int needed) > +/* > + * calculate needed sections for dirty node/dentry > + * and call has_curseg_enough_space > + */ > +static inline bool get_need_secs(struct f2fs_sb_info *sbi, > + unsigned int *lower, unsigned int *upper) > { > unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) + > get_pages(sbi, F2FS_DIRTY_DENTS) + > @@ -616,20 +620,33 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi, > unsigned int dent_secs = total_dent_blocks / CAP_BLKS_PER_SEC(sbi); > unsigned int node_blocks = total_node_blocks % CAP_BLKS_PER_SEC(sbi); > unsigned int dent_blocks = total_dent_blocks % CAP_BLKS_PER_SEC(sbi); > + > + *lower = node_secs + dent_secs; > + *upper = *lower + (node_blocks ? 1 : 0) + (dent_blocks ? 1 : 0); > + > + return !has_curseg_enough_space(sbi, node_blocks, dent_blocks); > +} > + > +static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi, > + int freed, int needed) > +{ > unsigned int free, need_lower, need_upper; > + bool curseg_enough; > > if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) > return false; > > + curseg_enough = get_need_secs(sbi, &need_lower, &need_upper); > + > free = free_sections(sbi) + freed; > - need_lower = node_secs + dent_secs + reserved_sections(sbi) + needed; > - need_upper = need_lower + (node_blocks ? 1 : 0) + (dent_blocks ? 1 : 0); > + need_lower += (needed + reserved_sections(sbi)); > + need_upper += (needed + reserved_sections(sbi)); > > if (free > need_upper) > return false; > else if (free <= need_lower) > return true; > - return !has_curseg_enough_space(sbi, node_blocks, dent_blocks); > + return curseg_enough; > } > > static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)