From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751486AbdB0DLw (ORCPT ); Sun, 26 Feb 2017 22:11:52 -0500 Received: from szxga02-in.huawei.com ([45.249.212.188]:3361 "EHLO dggrg02-dlp.huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751327AbdB0DLv (ORCPT ); Sun, 26 Feb 2017 22:11:51 -0500 Subject: Re: [PATCH V2] f2fs: introduce free nid bitmap To: Jaegeuk Kim References: <20170223025349.104160-1-yuchao0@huawei.com> <20170225190255.GD51627@jaegeuk.local> CC: , , From: Chao Yu Message-ID: <78bcc379-08b5-063f-d2a5-ce1740499a71@huawei.com> Date: Mon, 27 Feb 2017 11:11:21 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <20170225190255.GD51627@jaegeuk.local> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.134.22.195] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020202.58B398E0.012A,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 383c91ad412247753df90258796518d8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/2/26 3:02, Jaegeuk Kim wrote: > On 02/25, Chao Yu wrote: >> Hi Jaegeuk, >> >> I added below diff code into this patch in order to fix incorrectly nid status >> updating to reduce large latency of generic/251 in fstest, could you help to >> review code below? > > Understand the problem, and how about this? > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index 52db02396878..83b305689913 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1765,7 +1765,7 @@ static void __remove_nid_from_list(struct f2fs_sb_info *sbi, > radix_tree_delete(&nm_i->free_nid_root, i->nid); > } > > -static int add_free_nid(struct f2fs_sb_info *sbi, nid_t nid, bool build) > +static bool add_free_nid(struct f2fs_sb_info *sbi, nid_t nid, bool build) > { > struct f2fs_nm_info *nm_i = NM_I(sbi); > struct free_nid *i; > @@ -1774,14 +1774,14 @@ static int add_free_nid(struct f2fs_sb_info *sbi, nid_t nid, bool build) > > /* 0 nid should not be used */ > if (unlikely(nid == 0)) > - return 0; > + return false; > > if (build) { > /* do not add allocated nids */ > ne = __lookup_nat_cache(nm_i, nid); > if (ne && (!get_nat_flag(ne, IS_CHECKPOINTED) || > nat_get_blkaddr(ne) != NULL_ADDR)) > - return 0; > + return false; > } > > i = f2fs_kmem_cache_alloc(free_nid_slab, GFP_NOFS); > @@ -1790,7 +1790,7 @@ static int add_free_nid(struct f2fs_sb_info *sbi, nid_t nid, bool build) > > if (radix_tree_preload(GFP_NOFS)) { > kmem_cache_free(free_nid_slab, i); > - return 0; > + return false; If there is no memory, actually current free nid is still available, we need to return true (or other value -1?), then caller can set free_nid_bitmap correctly. > } > > spin_lock(&nm_i->nid_list_lock); > @@ -1799,9 +1799,9 @@ static int add_free_nid(struct f2fs_sb_info *sbi, nid_t nid, bool build) > radix_tree_preload_end(); > if (err) { > kmem_cache_free(free_nid_slab, i); > - return 0; > + return false; ditto. Thanks, > } > - return 1; > + return true; > } > > static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid) > @@ -1851,6 +1851,7 @@ static void scan_nat_page(struct f2fs_sb_info *sbi, > i = start_nid % NAT_ENTRY_PER_BLOCK; > > for (; i < NAT_ENTRY_PER_BLOCK; i++, start_nid++) { > + bool freed = false; > > if (unlikely(start_nid >= nm_i->max_nid)) > break; > @@ -1858,8 +1859,8 @@ static void scan_nat_page(struct f2fs_sb_info *sbi, > blk_addr = le32_to_cpu(nat_blk->entries[i].block_addr); > f2fs_bug_on(sbi, blk_addr == NEW_ADDR); > if (blk_addr == NULL_ADDR) > - add_free_nid(sbi, start_nid, true); > - update_free_nid_bitmap(sbi, start_nid, blk_addr == NULL_ADDR); > + freed = add_free_nid(sbi, start_nid, true); > + update_free_nid_bitmap(sbi, start_nid, freed); > } > } > >