From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751278AbdJ1Ls6 (ORCPT ); Sat, 28 Oct 2017 07:48:58 -0400 Received: from mail.kernel.org ([198.145.29.99]:39560 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751029AbdJ1Ls4 (ORCPT ); Sat, 28 Oct 2017 07:48:56 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9E66721939 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=chao@kernel.org Subject: Re: [f2fs-dev] [PATCH] f2fs: add a function to move nid To: Fan Li , "'Chao Yu'" , jaegeuk@kernel.org Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net References: <000001d34fdc$7d461260$77d23720$@samsung.com> From: Chao Yu Message-ID: <70867f4c-e865-2acb-472a-8f24a7588d91@kernel.org> Date: Sat, 28 Oct 2017 19:48:39 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <000001d34fdc$7d461260$77d23720$@samsung.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/10/28 19:03, Fan Li wrote: > This patch add a new function to move nid from one state to another. > Move operation is heavily used, by adding a new function for it > we can cut down some branches from several flow. > > > Signed-off-by: Fan li > --- > fs/f2fs/node.c | 51 ++++++++++++++++++++++++++++++--------------------- > 1 file changed, 30 insertions(+), 21 deletions(-) > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index ac629d6..8116b50 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1763,15 +1763,13 @@ static struct free_nid *__lookup_free_nid_list(struct f2fs_nm_info *nm_i, > } > > static int __insert_free_nid(struct f2fs_sb_info *sbi, > - struct free_nid *i, enum nid_state state, bool new) > + struct free_nid *i, enum nid_state state) > { > struct f2fs_nm_info *nm_i = NM_I(sbi); > > - if (new) { > - int err = radix_tree_insert(&nm_i->free_nid_root, i->nid, i); > - if (err) > - return err; > - } > + int err = radix_tree_insert(&nm_i->free_nid_root, i->nid, i); > + if (err) > + return err; > > f2fs_bug_on(sbi, state != i->state); > nm_i->nid_cnt[state]++; > @@ -1781,7 +1779,7 @@ static int __insert_free_nid(struct f2fs_sb_info *sbi, > } > > static void __remove_free_nid(struct f2fs_sb_info *sbi, > - struct free_nid *i, enum nid_state state, bool reuse) > + struct free_nid *i, enum nid_state state) > { > struct f2fs_nm_info *nm_i = NM_I(sbi); > > @@ -1789,8 +1787,23 @@ static void __remove_free_nid(struct f2fs_sb_info *sbi, > nm_i->nid_cnt[state]--; > if (state == FREE_NID) > list_del(&i->list); > - if (!reuse) > - radix_tree_delete(&nm_i->free_nid_root, i->nid); > + radix_tree_delete(&nm_i->free_nid_root, i->nid); > +} > + > +static void __move_free_nid(struct f2fs_sb_info *sbi, struct free_nid *i, > + enum nid_state org_state, enum nid_state dst_state) > +{ > + struct f2fs_nm_info *nm_i = NM_I(sbi); > + > + f2fs_bug_on(sbi, org_state != i->state); > + i->state = dst_state; > + nm_i->nid_cnt[org_state]--; > + nm_i->nid_cnt[dst_state]++; > + > + if (org_state == FREE_NID) Welcome back to community, Fan. :) Minor, how about "if (dst_stat == PREALLOC_NID)" ? Otherwise this patch looks good to me, anyway, you can add: Reviewed-by: Chao Yu Thanks, > + list_del(&i->list); > + else if (dst_state == FREE_NID) > + list_add_tail(&i->list, &nm_i->free_nid_list); > } > > /* return if the nid is recognized as free */ > @@ -1850,7 +1863,7 @@ static bool add_free_nid(struct f2fs_sb_info *sbi, nid_t nid, bool build) > } > } > ret = true; > - err = __insert_free_nid(sbi, i, FREE_NID, true); > + err = __insert_free_nid(sbi, i, FREE_NID); > err_out: > spin_unlock(&nm_i->nid_list_lock); > radix_tree_preload_end(); > @@ -1869,7 +1882,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid) > spin_lock(&nm_i->nid_list_lock); > i = __lookup_free_nid_list(nm_i, nid); > if (i && i->state == FREE_NID) { > - __remove_free_nid(sbi, i, FREE_NID, false); > + __remove_free_nid(sbi, i, FREE_NID); > need_free = true; > } > spin_unlock(&nm_i->nid_list_lock); > @@ -2080,9 +2093,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid) > struct free_nid, list); > *nid = i->nid; > > - __remove_free_nid(sbi, i, FREE_NID, true); > - i->state = PREALLOC_NID; > - __insert_free_nid(sbi, i, PREALLOC_NID, false); > + __move_free_nid(sbi, i, FREE_NID, PREALLOC_NID); > nm_i->available_nids--; > > update_free_nid_bitmap(sbi, *nid, false, false); > @@ -2108,7 +2119,7 @@ void alloc_nid_done(struct f2fs_sb_info *sbi, nid_t nid) > spin_lock(&nm_i->nid_list_lock); > i = __lookup_free_nid_list(nm_i, nid); > f2fs_bug_on(sbi, !i); > - __remove_free_nid(sbi, i, PREALLOC_NID, false); > + __remove_free_nid(sbi, i, PREALLOC_NID); > spin_unlock(&nm_i->nid_list_lock); > > kmem_cache_free(free_nid_slab, i); > @@ -2131,12 +2142,10 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid) > f2fs_bug_on(sbi, !i); > > if (!available_free_memory(sbi, FREE_NIDS)) { > - __remove_free_nid(sbi, i, PREALLOC_NID, false); > + __remove_free_nid(sbi, i, PREALLOC_NID); > need_free = true; > } else { > - __remove_free_nid(sbi, i, PREALLOC_NID, true); > - i->state = FREE_NID; > - __insert_free_nid(sbi, i, FREE_NID, false); > + __move_free_nid(sbi, i, PREALLOC_NID, FREE_NID); > } > > nm_i->available_nids++; > @@ -2167,7 +2176,7 @@ int try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink) > nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS) > break; > > - __remove_free_nid(sbi, i, FREE_NID, false); > + __remove_free_nid(sbi, i, FREE_NID); > kmem_cache_free(free_nid_slab, i); > nr_shrink--; > } > @@ -2746,7 +2755,7 @@ void destroy_node_manager(struct f2fs_sb_info *sbi) > /* destroy free nid list */ > spin_lock(&nm_i->nid_list_lock); > list_for_each_entry_safe(i, next_i, &nm_i->free_nid_list, list) { > - __remove_free_nid(sbi, i, FREE_NID, false); > + __remove_free_nid(sbi, i, FREE_NID); > spin_unlock(&nm_i->nid_list_lock); > kmem_cache_free(free_nid_slab, i); > spin_lock(&nm_i->nid_list_lock); >