All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] f2fs: introduce a method to make nat journal more fresh
@ 2018-03-14  6:33 Yunlei He
  2018-03-16  8:49 ` Chao Yu
  0 siblings, 1 reply; 7+ messages in thread
From: Yunlei He @ 2018-03-14  6:33 UTC (permalink / raw)
  To: jaegeuk, yuchao0, linux-f2fs-devel; +Cc: heyunlei, zhangdianfang

This patch introduce a method to make nat journal more fresh:
i.  sort set list using dirty entry number and cp version
    average value.
ii. if meet with cache hit, update average version valus with
    current cp version.

With this patch, newly modified nat set will flush to journal,
and flush old nat set with same dirty entry number to nat area.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/node.c | 45 ++++++++++++++++++++++++++-------------------
 fs/f2fs/node.h |  4 +++-
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 177c438..74f6079 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
 	__free_nat_entry(e);
 }
 
-static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
-						struct nat_entry *ne)
+static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool remove_journal,
+			struct f2fs_nm_info *nm_i, struct nat_entry *ne)
 {
 	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
 	struct nat_entry_set *head;
@@ -207,9 +207,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
 		INIT_LIST_HEAD(&head->set_list);
 		head->set = set;
 		head->entry_cnt = 0;
+		head->cp_ver = 0;
 		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
 	}
 
+	/* journal hit case, try to locate set in journal */
+	if (!remove_journal && head->entry_cnt <= NAT_JOURNAL_ENTRIES)
+		head->cp_ver += div_u64(cur_cp_version(F2FS_CKPT(sbi)),
+							NAT_JOURNAL_ENTRIES);
+
 	if (get_nat_flag(ne, IS_DIRTY))
 		goto refresh_list;
 
@@ -217,10 +223,7 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
 	head->entry_cnt++;
 	set_nat_flag(ne, IS_DIRTY, true);
 refresh_list:
-	if (nat_get_blkaddr(ne) == NEW_ADDR)
-		list_del_init(&ne->list);
-	else
-		list_move_tail(&ne->list, &head->entry_list);
+	list_move_tail(&ne->list, &head->entry_list);
 }
 
 static void __clear_nat_cache_dirty(struct f2fs_nm_info *nm_i,
@@ -357,7 +360,8 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
 	nat_set_blkaddr(e, new_blkaddr);
 	if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
 		set_nat_flag(e, IS_CHECKPOINTED, false);
-	__set_nat_cache_dirty(nm_i, e);
+	if (new_blkaddr != NEW_ADDR)
+		__set_nat_cache_dirty(sbi, false, nm_i, e);
 
 	/* update fsync_mark if its inode nat entry is still alive */
 	if (ni->nid != ni->ino)
@@ -2395,14 +2399,14 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
 			spin_unlock(&nm_i->nid_list_lock);
 		}
 
-		__set_nat_cache_dirty(nm_i, ne);
+		__set_nat_cache_dirty(sbi, true, nm_i, ne);
 	}
 	update_nats_in_cursum(journal, -i);
 	up_write(&curseg->journal_rwsem);
 }
 
-static void __adjust_nat_entry_set(struct nat_entry_set *nes,
-						struct list_head *head, int max)
+static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
+		struct nat_entry_set *nes, struct list_head *head, int max)
 {
 	struct nat_entry_set *cur;
 
@@ -2410,7 +2414,9 @@ static void __adjust_nat_entry_set(struct nat_entry_set *nes,
 		goto add_out;
 
 	list_for_each_entry(cur, head, set_list) {
-		if (cur->entry_cnt >= nes->entry_cnt) {
+		if (cur->entry_cnt > nes->entry_cnt ||
+			(cur->entry_cnt == nes->entry_cnt &&
+			cur->cp_ver < nes->cp_ver)) {
 			list_add(&nes->set_list, cur->set_list.prev);
 			return;
 		}
@@ -2458,7 +2464,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
 	struct f2fs_journal *journal = curseg->journal;
 	nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
-	bool to_journal = true;
 	struct f2fs_nat_block *nat_blk;
 	struct nat_entry *ne, *cur;
 	struct page *page = NULL;
@@ -2468,11 +2473,14 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 	 * #1, flush nat entries to journal in current hot data summary block.
 	 * #2, flush nat entries to nat page.
 	 */
+
+	set->to_journal = true;
+
 	if (enabled_nat_bits(sbi, cpc) ||
 		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))
-		to_journal = false;
+		set->to_journal = false;
 
-	if (to_journal) {
+	if (set->to_journal) {
 		down_write(&curseg->journal_rwsem);
 	} else {
 		page = get_next_nat_page(sbi, start_nid);
@@ -2488,7 +2496,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 
 		f2fs_bug_on(sbi, nat_get_blkaddr(ne) == NEW_ADDR);
 
-		if (to_journal) {
+		if (set->to_journal) {
 			offset = lookup_journal_in_cursum(journal,
 							NAT_JOURNAL, nid, 1);
 			f2fs_bug_on(sbi, offset < 0);
@@ -2509,7 +2517,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 		}
 	}
 
-	if (to_journal) {
+	if (set->to_journal) {
 		up_write(&curseg->journal_rwsem);
 	} else {
 		__update_nat_bits(sbi, start_nid, page);
@@ -2517,7 +2525,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 	}
 
 	/* Allow dirty nats by node block allocation in write_begin */
-	if (!set->entry_cnt) {
+	if (!set->to_journal) {
 		radix_tree_delete(&NM_I(sbi)->nat_set_root, set->set);
 		kmem_cache_free(nat_entry_set_slab, set);
 	}
@@ -2556,7 +2564,7 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 		unsigned idx;
 		set_idx = setvec[found - 1]->set + 1;
 		for (idx = 0; idx < found; idx++)
-			__adjust_nat_entry_set(setvec[idx], &sets,
+			__adjust_nat_entry_set(sbi, setvec[idx], &sets,
 						MAX_NAT_JENTRIES(journal));
 	}
 
@@ -2565,7 +2573,6 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 		__flush_nat_entry_set(sbi, set, cpc);
 
 	up_write(&nm_i->nat_tree_lock);
-	/* Allow dirty nats by node block allocation in write_begin */
 }
 
 static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
index 081ef0d..3cf9c8b6 100644
--- a/fs/f2fs/node.h
+++ b/fs/f2fs/node.h
@@ -148,7 +148,9 @@ struct nat_entry_set {
 	struct list_head set_list;	/* link with other nat sets */
 	struct list_head entry_list;	/* link with dirty nat entries */
 	nid_t set;			/* set number*/
-	unsigned int entry_cnt;		/* the # of nat entries in set */
+	unsigned int entry_cnt:9;	/* the # of nat entries in set */
+	unsigned int to_journal:1;	/* set flush to journal */
+	__u64 cp_ver;			/* cp version of this set modify */
 };
 
 struct free_nid {
-- 
1.9.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH v3] f2fs: introduce a method to make nat journal more fresh
  2018-03-14  6:33 [PATCH v3] f2fs: introduce a method to make nat journal more fresh Yunlei He
@ 2018-03-16  8:49 ` Chao Yu
  2018-03-16  9:14   ` heyunlei
  0 siblings, 1 reply; 7+ messages in thread
From: Chao Yu @ 2018-03-16  8:49 UTC (permalink / raw)
  To: Yunlei He, jaegeuk, linux-f2fs-devel; +Cc: zhangdianfang



On 2018/3/14 14:33, Yunlei He wrote:
> This patch introduce a method to make nat journal more fresh:
> i.  sort set list using dirty entry number and cp version
>     average value.
> ii. if meet with cache hit, update average version valus with
>     current cp version.
> 
> With this patch, newly modified nat set will flush to journal,
> and flush old nat set with same dirty entry number to nat area.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/node.c | 45 ++++++++++++++++++++++++++-------------------
>  fs/f2fs/node.h |  4 +++-
>  2 files changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 177c438..74f6079 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
>  	__free_nat_entry(e);
>  }
>  
> -static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> -						struct nat_entry *ne)
> +static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool remove_journal,
> +			struct f2fs_nm_info *nm_i, struct nat_entry *ne)
>  {
>  	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
>  	struct nat_entry_set *head;
> @@ -207,9 +207,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>  		INIT_LIST_HEAD(&head->set_list);
>  		head->set = set;
>  		head->entry_cnt = 0;
> +		head->cp_ver = 0;
>  		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
>  	}
>  
> +	/* journal hit case, try to locate set in journal */
> +	if (!remove_journal && head->entry_cnt <= NAT_JOURNAL_ENTRIES)
> +		head->cp_ver += div_u64(cur_cp_version(F2FS_CKPT(sbi)),
> +							NAT_JOURNAL_ENTRIES);
> +
>  	if (get_nat_flag(ne, IS_DIRTY))
>  		goto refresh_list;

	else {
		update head version here?
	}

>  
> @@ -217,10 +223,7 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>  	head->entry_cnt++;
>  	set_nat_flag(ne, IS_DIRTY, true);
>  refresh_list:
> -	if (nat_get_blkaddr(ne) == NEW_ADDR)
> -		list_del_init(&ne->list);
> -	else
> -		list_move_tail(&ne->list, &head->entry_list);
> +	list_move_tail(&ne->list, &head->entry_list);

For NEW_ADDR case, why we need to track this nat entry in entry_list?

>  }
>  
>  static void __clear_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> @@ -357,7 +360,8 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>  	nat_set_blkaddr(e, new_blkaddr);
>  	if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
>  		set_nat_flag(e, IS_CHECKPOINTED, false);
> -	__set_nat_cache_dirty(nm_i, e);
> +	if (new_blkaddr != NEW_ADDR)
> +		__set_nat_cache_dirty(sbi, false, nm_i, e);
>  
>  	/* update fsync_mark if its inode nat entry is still alive */
>  	if (ni->nid != ni->ino)
> @@ -2395,14 +2399,14 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>  			spin_unlock(&nm_i->nid_list_lock);
>  		}
>  
> -		__set_nat_cache_dirty(nm_i, ne);
> +		__set_nat_cache_dirty(sbi, true, nm_i, ne);
>  	}
>  	update_nats_in_cursum(journal, -i);
>  	up_write(&curseg->journal_rwsem);
>  }
>  
> -static void __adjust_nat_entry_set(struct nat_entry_set *nes,
> -						struct list_head *head, int max)
> +static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
> +		struct nat_entry_set *nes, struct list_head *head, int max)
>  {
>  	struct nat_entry_set *cur;
>  
> @@ -2410,7 +2414,9 @@ static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>  		goto add_out;
>  
>  	list_for_each_entry(cur, head, set_list) {
> -		if (cur->entry_cnt >= nes->entry_cnt) {
> +		if (cur->entry_cnt > nes->entry_cnt ||
> +			(cur->entry_cnt == nes->entry_cnt &&
> +			cur->cp_ver < nes->cp_ver)) {
>  			list_add(&nes->set_list, cur->set_list.prev);
>  			return;
>  		}
> @@ -2458,7 +2464,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>  	struct f2fs_journal *journal = curseg->journal;
>  	nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
> -	bool to_journal = true;
>  	struct f2fs_nat_block *nat_blk;
>  	struct nat_entry *ne, *cur;
>  	struct page *page = NULL;
> @@ -2468,11 +2473,14 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>  	 * #1, flush nat entries to journal in current hot data summary block.
>  	 * #2, flush nat entries to nat page.
>  	 */
> +
> +	set->to_journal = true;
> +
>  	if (enabled_nat_bits(sbi, cpc) ||
>  		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))
> -		to_journal = false;
> +		set->to_journal = false;
>  
> -	if (to_journal) {
> +	if (set->to_journal) {
>  		down_write(&curseg->journal_rwsem);
>  	} else {
>  		page = get_next_nat_page(sbi, start_nid);
> @@ -2488,7 +2496,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>  
>  		f2fs_bug_on(sbi, nat_get_blkaddr(ne) == NEW_ADDR);
>  
> -		if (to_journal) {
> +		if (set->to_journal) {
>  			offset = lookup_journal_in_cursum(journal,
>  							NAT_JOURNAL, nid, 1);
>  			f2fs_bug_on(sbi, offset < 0);
> @@ -2509,7 +2517,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>  		}
>  	}
>  
> -	if (to_journal) {
> +	if (set->to_journal) {
>  		up_write(&curseg->journal_rwsem);
>  	} else {
>  		__update_nat_bits(sbi, start_nid, page);
> @@ -2517,7 +2525,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>  	}
>  
>  	/* Allow dirty nats by node block allocation in write_begin */
> -	if (!set->entry_cnt) {
> +	if (!set->to_journal) {

if (!set->to_journal && !set->entry_cnt) ?

Thanks,

>  		radix_tree_delete(&NM_I(sbi)->nat_set_root, set->set);
>  		kmem_cache_free(nat_entry_set_slab, set);
>  	}
> @@ -2556,7 +2564,7 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  		unsigned idx;
>  		set_idx = setvec[found - 1]->set + 1;
>  		for (idx = 0; idx < found; idx++)
> -			__adjust_nat_entry_set(setvec[idx], &sets,
> +			__adjust_nat_entry_set(sbi, setvec[idx], &sets,
>  						MAX_NAT_JENTRIES(journal));
>  	}
>  
> @@ -2565,7 +2573,6 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  		__flush_nat_entry_set(sbi, set, cpc);
>  
>  	up_write(&nm_i->nat_tree_lock);
> -	/* Allow dirty nats by node block allocation in write_begin */
>  }
>  
>  static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
> index 081ef0d..3cf9c8b6 100644
> --- a/fs/f2fs/node.h
> +++ b/fs/f2fs/node.h
> @@ -148,7 +148,9 @@ struct nat_entry_set {
>  	struct list_head set_list;	/* link with other nat sets */
>  	struct list_head entry_list;	/* link with dirty nat entries */
>  	nid_t set;			/* set number*/
> -	unsigned int entry_cnt;		/* the # of nat entries in set */
> +	unsigned int entry_cnt:9;	/* the # of nat entries in set */
> +	unsigned int to_journal:1;	/* set flush to journal */
> +	__u64 cp_ver;			/* cp version of this set modify */
>  };
>  
>  struct free_nid {
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3] f2fs: introduce a method to make nat journal more fresh
  2018-03-16  8:49 ` Chao Yu
@ 2018-03-16  9:14   ` heyunlei
  2018-03-16  9:46     ` Chao Yu
  0 siblings, 1 reply; 7+ messages in thread
From: heyunlei @ 2018-03-16  9:14 UTC (permalink / raw)
  To: Yuchao (T), jaegeuk, linux-f2fs-devel; +Cc: Zhangdianfang (Euler)



>-----Original Message-----
>From: Yuchao (T)
>Sent: Friday, March 16, 2018 4:49 PM
>To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>Cc: Wangbintian; Zhangdianfang (Euler)
>Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>
>
>
>On 2018/3/14 14:33, Yunlei He wrote:
>> This patch introduce a method to make nat journal more fresh:
>> i.  sort set list using dirty entry number and cp version
>>     average value.
>> ii. if meet with cache hit, update average version valus with
>>     current cp version.
>>
>> With this patch, newly modified nat set will flush to journal,
>> and flush old nat set with same dirty entry number to nat area.
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>>  fs/f2fs/node.c | 45 ++++++++++++++++++++++++++-------------------
>>  fs/f2fs/node.h |  4 +++-
>>  2 files changed, 29 insertions(+), 20 deletions(-)
>>
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index 177c438..74f6079 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
>>  	__free_nat_entry(e);
>>  }
>>
>> -static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>> -						struct nat_entry *ne)
>> +static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool remove_journal,
>> +			struct f2fs_nm_info *nm_i, struct nat_entry *ne)
>>  {
>>  	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
>>  	struct nat_entry_set *head;
>> @@ -207,9 +207,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>  		INIT_LIST_HEAD(&head->set_list);
>>  		head->set = set;
>>  		head->entry_cnt = 0;
>> +		head->cp_ver = 0;
>>  		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
>>  	}
>>
>> +	/* journal hit case, try to locate set in journal */
>> +	if (!remove_journal && head->entry_cnt <= NAT_JOURNAL_ENTRIES)
>> +		head->cp_ver += div_u64(cur_cp_version(F2FS_CKPT(sbi)),
>> +							NAT_JOURNAL_ENTRIES);
>> +
>>  	if (get_nat_flag(ne, IS_DIRTY))
>>  		goto refresh_list;
>
>	else {
>		update head version here?
>	}

Okay, if one nat entry update more than once, here will be overflow.

>
>>
>> @@ -217,10 +223,7 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>  	head->entry_cnt++;
>>  	set_nat_flag(ne, IS_DIRTY, true);
>>  refresh_list:
>> -	if (nat_get_blkaddr(ne) == NEW_ADDR)
>> -		list_del_init(&ne->list);
>> -	else
>> -		list_move_tail(&ne->list, &head->entry_list);
>> +	list_move_tail(&ne->list, &head->entry_list);
>
>For NEW_ADDR case, why we need to track this nat entry in entry_list?

If new_blkaddr == NEW_ADDR, we don't need to call __set_nat_cache_dirty:
	i  nat entry with NEW_ADDR will increase dirty nat entry count in nat set，which will cause
      a nat set has large nat entry count（most is NEW_ADDR）， but fewer of nat entries should
	  be written back in this checkpoint
    ii  nat entry with NEW_ADDR will call __set_nat_cache_dirty again, when we write back its node
      page, in this time, new_blkaddr != NEW_ADDR
So, I think we'd better bypass _set_nat_cache_dirty if blkaddr == NEW_ADDR

Thanks.

>
>>  }
>>
>>  static void __clear_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>> @@ -357,7 +360,8 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>>  	nat_set_blkaddr(e, new_blkaddr);
>>  	if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
>>  		set_nat_flag(e, IS_CHECKPOINTED, false);
>> -	__set_nat_cache_dirty(nm_i, e);
>> +	if (new_blkaddr != NEW_ADDR)
>> +		__set_nat_cache_dirty(sbi, false, nm_i, e);
>>
>>  	/* update fsync_mark if its inode nat entry is still alive */
>>  	if (ni->nid != ni->ino)
>> @@ -2395,14 +2399,14 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>>  			spin_unlock(&nm_i->nid_list_lock);
>>  		}
>>
>> -		__set_nat_cache_dirty(nm_i, ne);
>> +		__set_nat_cache_dirty(sbi, true, nm_i, ne);
>>  	}
>>  	update_nats_in_cursum(journal, -i);
>>  	up_write(&curseg->journal_rwsem);
>>  }
>>
>> -static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>> -						struct list_head *head, int max)
>> +static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
>> +		struct nat_entry_set *nes, struct list_head *head, int max)
>>  {
>>  	struct nat_entry_set *cur;
>>
>> @@ -2410,7 +2414,9 @@ static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>  		goto add_out;
>>
>>  	list_for_each_entry(cur, head, set_list) {
>> -		if (cur->entry_cnt >= nes->entry_cnt) {
>> +		if (cur->entry_cnt > nes->entry_cnt ||
>> +			(cur->entry_cnt == nes->entry_cnt &&
>> +			cur->cp_ver < nes->cp_ver)) {
>>  			list_add(&nes->set_list, cur->set_list.prev);
>>  			return;
>>  		}
>> @@ -2458,7 +2464,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>  	struct f2fs_journal *journal = curseg->journal;
>>  	nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
>> -	bool to_journal = true;
>>  	struct f2fs_nat_block *nat_blk;
>>  	struct nat_entry *ne, *cur;
>>  	struct page *page = NULL;
>> @@ -2468,11 +2473,14 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>  	 * #1, flush nat entries to journal in current hot data summary block.
>>  	 * #2, flush nat entries to nat page.
>>  	 */
>> +
>> +	set->to_journal = true;
>> +
>>  	if (enabled_nat_bits(sbi, cpc) ||
>>  		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))
>> -		to_journal = false;
>> +		set->to_journal = false;
>>
>> -	if (to_journal) {
>> +	if (set->to_journal) {
>>  		down_write(&curseg->journal_rwsem);
>>  	} else {
>>  		page = get_next_nat_page(sbi, start_nid);
>> @@ -2488,7 +2496,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>
>>  		f2fs_bug_on(sbi, nat_get_blkaddr(ne) == NEW_ADDR);
>>
>> -		if (to_journal) {
>> +		if (set->to_journal) {
>>  			offset = lookup_journal_in_cursum(journal,
>>  							NAT_JOURNAL, nid, 1);
>>  			f2fs_bug_on(sbi, offset < 0);
>> @@ -2509,7 +2517,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>  		}
>>  	}
>>
>> -	if (to_journal) {
>> +	if (set->to_journal) {
>>  		up_write(&curseg->journal_rwsem);
>>  	} else {
>>  		__update_nat_bits(sbi, start_nid, page);
>> @@ -2517,7 +2525,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>  	}
>>
>>  	/* Allow dirty nats by node block allocation in write_begin */
>> -	if (!set->entry_cnt) {
>> +	if (!set->to_journal) {
>
>if (!set->to_journal && !set->entry_cnt) ?
>
>Thanks,
>
>>  		radix_tree_delete(&NM_I(sbi)->nat_set_root, set->set);
>>  		kmem_cache_free(nat_entry_set_slab, set);
>>  	}
>> @@ -2556,7 +2564,7 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>  		unsigned idx;
>>  		set_idx = setvec[found - 1]->set + 1;
>>  		for (idx = 0; idx < found; idx++)
>> -			__adjust_nat_entry_set(setvec[idx], &sets,
>> +			__adjust_nat_entry_set(sbi, setvec[idx], &sets,
>>  						MAX_NAT_JENTRIES(journal));
>>  	}
>>
>> @@ -2565,7 +2573,6 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>  		__flush_nat_entry_set(sbi, set, cpc);
>>
>>  	up_write(&nm_i->nat_tree_lock);
>> -	/* Allow dirty nats by node block allocation in write_begin */
>>  }
>>
>>  static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
>> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
>> index 081ef0d..3cf9c8b6 100644
>> --- a/fs/f2fs/node.h
>> +++ b/fs/f2fs/node.h
>> @@ -148,7 +148,9 @@ struct nat_entry_set {
>>  	struct list_head set_list;	/* link with other nat sets */
>>  	struct list_head entry_list;	/* link with dirty nat entries */
>>  	nid_t set;			/* set number*/
>> -	unsigned int entry_cnt;		/* the # of nat entries in set */
>> +	unsigned int entry_cnt:9;	/* the # of nat entries in set */
>> +	unsigned int to_journal:1;	/* set flush to journal */
>> +	__u64 cp_ver;			/* cp version of this set modify */
>>  };
>>
>>  struct free_nid {
>>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3] f2fs: introduce a method to make nat journal more fresh
  2018-03-16  9:14   ` heyunlei
@ 2018-03-16  9:46     ` Chao Yu
  2018-03-16  9:52       ` heyunlei
  0 siblings, 1 reply; 7+ messages in thread
From: Chao Yu @ 2018-03-16  9:46 UTC (permalink / raw)
  To: heyunlei, jaegeuk, linux-f2fs-devel; +Cc: Zhangdianfang (Euler)

On 2018/3/16 17:14, heyunlei wrote:
> 
> 
>> -----Original Message-----
>> From: Yuchao (T)
>> Sent: Friday, March 16, 2018 4:49 PM
>> To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>> Cc: Wangbintian; Zhangdianfang (Euler)
>> Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>>
>>
>>
>> On 2018/3/14 14:33, Yunlei He wrote:
>>> This patch introduce a method to make nat journal more fresh:
>>> i.  sort set list using dirty entry number and cp version
>>>     average value.
>>> ii. if meet with cache hit, update average version valus with
>>>     current cp version.
>>>
>>> With this patch, newly modified nat set will flush to journal,
>>> and flush old nat set with same dirty entry number to nat area.
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>  fs/f2fs/node.c | 45 ++++++++++++++++++++++++++-------------------
>>>  fs/f2fs/node.h |  4 +++-
>>>  2 files changed, 29 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>> index 177c438..74f6079 100644
>>> --- a/fs/f2fs/node.c
>>> +++ b/fs/f2fs/node.c
>>> @@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
>>>  	__free_nat_entry(e);
>>>  }
>>>
>>> -static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>> -						struct nat_entry *ne)
>>> +static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool remove_journal,
>>> +			struct f2fs_nm_info *nm_i, struct nat_entry *ne)
>>>  {
>>>  	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
>>>  	struct nat_entry_set *head;
>>> @@ -207,9 +207,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>  		INIT_LIST_HEAD(&head->set_list);
>>>  		head->set = set;
>>>  		head->entry_cnt = 0;
>>> +		head->cp_ver = 0;
>>>  		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
>>>  	}
>>>
>>> +	/* journal hit case, try to locate set in journal */
>>> +	if (!remove_journal && head->entry_cnt <= NAT_JOURNAL_ENTRIES)
>>> +		head->cp_ver += div_u64(cur_cp_version(F2FS_CKPT(sbi)),
>>> +							NAT_JOURNAL_ENTRIES);
>>> +
>>>  	if (get_nat_flag(ne, IS_DIRTY))
>>>  		goto refresh_list;
>>
>> 	else {
>> 		update head version here?
>> 	}
> 
> Okay, if one nat entry update more than once, here will be overflow.
> 
>>
>>>
>>> @@ -217,10 +223,7 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>  	head->entry_cnt++;
>>>  	set_nat_flag(ne, IS_DIRTY, true);
>>>  refresh_list:
>>> -	if (nat_get_blkaddr(ne) == NEW_ADDR)
>>> -		list_del_init(&ne->list);
>>> -	else
>>> -		list_move_tail(&ne->list, &head->entry_list);
>>> +	list_move_tail(&ne->list, &head->entry_list);
>>
>> For NEW_ADDR case, why we need to track this nat entry in entry_list?
> 
> If new_blkaddr == NEW_ADDR, we don't need to call __set_nat_cache_dirty:

But such entry is still in clean list, shrinker can easily remove it in
out-of-memory condition.

Thanks,

> 	i  nat entry with NEW_ADDR will increase dirty nat entry count in nat set，which will cause
>       a nat set has large nat entry count（most is NEW_ADDR）， but fewer of nat entries should
> 	  be written back in this checkpoint
>     ii  nat entry with NEW_ADDR will call __set_nat_cache_dirty again, when we write back its node
>       page, in this time, new_blkaddr != NEW_ADDR
> So, I think we'd better bypass _set_nat_cache_dirty if blkaddr == NEW_ADDR
> 
> Thanks.
> 
>>
>>>  }
>>>
>>>  static void __clear_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>> @@ -357,7 +360,8 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>  	nat_set_blkaddr(e, new_blkaddr);
>>>  	if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
>>>  		set_nat_flag(e, IS_CHECKPOINTED, false);
>>> -	__set_nat_cache_dirty(nm_i, e);
>>> +	if (new_blkaddr != NEW_ADDR)
>>> +		__set_nat_cache_dirty(sbi, false, nm_i, e);
>>>
>>>  	/* update fsync_mark if its inode nat entry is still alive */
>>>  	if (ni->nid != ni->ino)
>>> @@ -2395,14 +2399,14 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>>>  			spin_unlock(&nm_i->nid_list_lock);
>>>  		}
>>>
>>> -		__set_nat_cache_dirty(nm_i, ne);
>>> +		__set_nat_cache_dirty(sbi, true, nm_i, ne);
>>>  	}
>>>  	update_nats_in_cursum(journal, -i);
>>>  	up_write(&curseg->journal_rwsem);
>>>  }
>>>
>>> -static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>> -						struct list_head *head, int max)
>>> +static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
>>> +		struct nat_entry_set *nes, struct list_head *head, int max)
>>>  {
>>>  	struct nat_entry_set *cur;
>>>
>>> @@ -2410,7 +2414,9 @@ static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>>  		goto add_out;
>>>
>>>  	list_for_each_entry(cur, head, set_list) {
>>> -		if (cur->entry_cnt >= nes->entry_cnt) {
>>> +		if (cur->entry_cnt > nes->entry_cnt ||
>>> +			(cur->entry_cnt == nes->entry_cnt &&
>>> +			cur->cp_ver < nes->cp_ver)) {
>>>  			list_add(&nes->set_list, cur->set_list.prev);
>>>  			return;
>>>  		}
>>> @@ -2458,7 +2464,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>>  	struct f2fs_journal *journal = curseg->journal;
>>>  	nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
>>> -	bool to_journal = true;
>>>  	struct f2fs_nat_block *nat_blk;
>>>  	struct nat_entry *ne, *cur;
>>>  	struct page *page = NULL;
>>> @@ -2468,11 +2473,14 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>  	 * #1, flush nat entries to journal in current hot data summary block.
>>>  	 * #2, flush nat entries to nat page.
>>>  	 */
>>> +
>>> +	set->to_journal = true;
>>> +
>>>  	if (enabled_nat_bits(sbi, cpc) ||
>>>  		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))
>>> -		to_journal = false;
>>> +		set->to_journal = false;
>>>
>>> -	if (to_journal) {
>>> +	if (set->to_journal) {
>>>  		down_write(&curseg->journal_rwsem);
>>>  	} else {
>>>  		page = get_next_nat_page(sbi, start_nid);
>>> @@ -2488,7 +2496,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>
>>>  		f2fs_bug_on(sbi, nat_get_blkaddr(ne) == NEW_ADDR);
>>>
>>> -		if (to_journal) {
>>> +		if (set->to_journal) {
>>>  			offset = lookup_journal_in_cursum(journal,
>>>  							NAT_JOURNAL, nid, 1);
>>>  			f2fs_bug_on(sbi, offset < 0);
>>> @@ -2509,7 +2517,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>  		}
>>>  	}
>>>
>>> -	if (to_journal) {
>>> +	if (set->to_journal) {
>>>  		up_write(&curseg->journal_rwsem);
>>>  	} else {
>>>  		__update_nat_bits(sbi, start_nid, page);
>>> @@ -2517,7 +2525,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>  	}
>>>
>>>  	/* Allow dirty nats by node block allocation in write_begin */
>>> -	if (!set->entry_cnt) {
>>> +	if (!set->to_journal) {
>>
>> if (!set->to_journal && !set->entry_cnt) ?
>>
>> Thanks,
>>
>>>  		radix_tree_delete(&NM_I(sbi)->nat_set_root, set->set);
>>>  		kmem_cache_free(nat_entry_set_slab, set);
>>>  	}
>>> @@ -2556,7 +2564,7 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>  		unsigned idx;
>>>  		set_idx = setvec[found - 1]->set + 1;
>>>  		for (idx = 0; idx < found; idx++)
>>> -			__adjust_nat_entry_set(setvec[idx], &sets,
>>> +			__adjust_nat_entry_set(sbi, setvec[idx], &sets,
>>>  						MAX_NAT_JENTRIES(journal));
>>>  	}
>>>
>>> @@ -2565,7 +2573,6 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>  		__flush_nat_entry_set(sbi, set, cpc);
>>>
>>>  	up_write(&nm_i->nat_tree_lock);
>>> -	/* Allow dirty nats by node block allocation in write_begin */
>>>  }
>>>
>>>  static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
>>> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
>>> index 081ef0d..3cf9c8b6 100644
>>> --- a/fs/f2fs/node.h
>>> +++ b/fs/f2fs/node.h
>>> @@ -148,7 +148,9 @@ struct nat_entry_set {
>>>  	struct list_head set_list;	/* link with other nat sets */
>>>  	struct list_head entry_list;	/* link with dirty nat entries */
>>>  	nid_t set;			/* set number*/
>>> -	unsigned int entry_cnt;		/* the # of nat entries in set */
>>> +	unsigned int entry_cnt:9;	/* the # of nat entries in set */
>>> +	unsigned int to_journal:1;	/* set flush to journal */
>>> +	__u64 cp_ver;			/* cp version of this set modify */
>>>  };
>>>
>>>  struct free_nid {
>>>
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3] f2fs: introduce a method to make nat journal more fresh
  2018-03-16  9:46     ` Chao Yu
@ 2018-03-16  9:52       ` heyunlei
  2018-03-19  2:09         ` Chao Yu
  0 siblings, 1 reply; 7+ messages in thread
From: heyunlei @ 2018-03-16  9:52 UTC (permalink / raw)
  To: Yuchao (T), jaegeuk, linux-f2fs-devel; +Cc: Zhangdianfang (Euler)



>-----Original Message-----
>From: Yuchao (T)
>Sent: Friday, March 16, 2018 5:47 PM
>To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>Cc: Wangbintian; Zhangdianfang (Euler)
>Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>
>On 2018/3/16 17:14, heyunlei wrote:
>>
>>
>>> -----Original Message-----
>>> From: Yuchao (T)
>>> Sent: Friday, March 16, 2018 4:49 PM
>>> To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>>> Cc: Wangbintian; Zhangdianfang (Euler)
>>> Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>>>
>>>
>>>
>>> On 2018/3/14 14:33, Yunlei He wrote:
>>>> This patch introduce a method to make nat journal more fresh:
>>>> i.  sort set list using dirty entry number and cp version
>>>>     average value.
>>>> ii. if meet with cache hit, update average version valus with
>>>>     current cp version.
>>>>
>>>> With this patch, newly modified nat set will flush to journal,
>>>> and flush old nat set with same dirty entry number to nat area.
>>>>
>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>> ---
>>>>  fs/f2fs/node.c | 45 ++++++++++++++++++++++++++-------------------
>>>>  fs/f2fs/node.h |  4 +++-
>>>>  2 files changed, 29 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>>> index 177c438..74f6079 100644
>>>> --- a/fs/f2fs/node.c
>>>> +++ b/fs/f2fs/node.c
>>>> @@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
>>>>  	__free_nat_entry(e);
>>>>  }
>>>>
>>>> -static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>> -						struct nat_entry *ne)
>>>> +static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool remove_journal,
>>>> +			struct f2fs_nm_info *nm_i, struct nat_entry *ne)
>>>>  {
>>>>  	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
>>>>  	struct nat_entry_set *head;
>>>> @@ -207,9 +207,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>  		INIT_LIST_HEAD(&head->set_list);
>>>>  		head->set = set;
>>>>  		head->entry_cnt = 0;
>>>> +		head->cp_ver = 0;
>>>>  		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
>>>>  	}
>>>>
>>>> +	/* journal hit case, try to locate set in journal */
>>>> +	if (!remove_journal && head->entry_cnt <= NAT_JOURNAL_ENTRIES)
>>>> +		head->cp_ver += div_u64(cur_cp_version(F2FS_CKPT(sbi)),
>>>> +							NAT_JOURNAL_ENTRIES);
>>>> +
>>>>  	if (get_nat_flag(ne, IS_DIRTY))
>>>>  		goto refresh_list;
>>>
>>> 	else {
>>> 		update head version here?
>>> 	}
>>
>> Okay, if one nat entry update more than once, here will be overflow.
>>
>>>
>>>>
>>>> @@ -217,10 +223,7 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>  	head->entry_cnt++;
>>>>  	set_nat_flag(ne, IS_DIRTY, true);
>>>>  refresh_list:
>>>> -	if (nat_get_blkaddr(ne) == NEW_ADDR)
>>>> -		list_del_init(&ne->list);
>>>> -	else
>>>> -		list_move_tail(&ne->list, &head->entry_list);
>>>> +	list_move_tail(&ne->list, &head->entry_list);
>>>
>>> For NEW_ADDR case, why we need to track this nat entry in entry_list?
>>
>> If new_blkaddr == NEW_ADDR, we don't need to call __set_nat_cache_dirty:
>
>But such entry is still in clean list, shrinker can easily remove it in
>out-of-memory condition.

Yes, you are right, my test doesn't cover low memory case. I 'll send a new version.

Thanks.

>
>Thanks,
>
>> 	i  nat entry with NEW_ADDR will increase dirty nat entry count in nat set，which will cause
>>       a nat set has large nat entry count（most is NEW_ADDR）， but fewer of nat entries should
>> 	  be written back in this checkpoint
>>     ii  nat entry with NEW_ADDR will call __set_nat_cache_dirty again, when we write back its node
>>       page, in this time, new_blkaddr != NEW_ADDR
>> So, I think we'd better bypass _set_nat_cache_dirty if blkaddr == NEW_ADDR
>>
>> Thanks.
>>
>>>
>>>>  }
>>>>
>>>>  static void __clear_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>> @@ -357,7 +360,8 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>>  	nat_set_blkaddr(e, new_blkaddr);
>>>>  	if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
>>>>  		set_nat_flag(e, IS_CHECKPOINTED, false);
>>>> -	__set_nat_cache_dirty(nm_i, e);
>>>> +	if (new_blkaddr != NEW_ADDR)
>>>> +		__set_nat_cache_dirty(sbi, false, nm_i, e);
>>>>
>>>>  	/* update fsync_mark if its inode nat entry is still alive */
>>>>  	if (ni->nid != ni->ino)
>>>> @@ -2395,14 +2399,14 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>>>>  			spin_unlock(&nm_i->nid_list_lock);
>>>>  		}
>>>>
>>>> -		__set_nat_cache_dirty(nm_i, ne);
>>>> +		__set_nat_cache_dirty(sbi, true, nm_i, ne);
>>>>  	}
>>>>  	update_nats_in_cursum(journal, -i);
>>>>  	up_write(&curseg->journal_rwsem);
>>>>  }
>>>>
>>>> -static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>>> -						struct list_head *head, int max)
>>>> +static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
>>>> +		struct nat_entry_set *nes, struct list_head *head, int max)
>>>>  {
>>>>  	struct nat_entry_set *cur;
>>>>
>>>> @@ -2410,7 +2414,9 @@ static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>>>  		goto add_out;
>>>>
>>>>  	list_for_each_entry(cur, head, set_list) {
>>>> -		if (cur->entry_cnt >= nes->entry_cnt) {
>>>> +		if (cur->entry_cnt > nes->entry_cnt ||
>>>> +			(cur->entry_cnt == nes->entry_cnt &&
>>>> +			cur->cp_ver < nes->cp_ver)) {
>>>>  			list_add(&nes->set_list, cur->set_list.prev);
>>>>  			return;
>>>>  		}
>>>> @@ -2458,7 +2464,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>>>  	struct f2fs_journal *journal = curseg->journal;
>>>>  	nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
>>>> -	bool to_journal = true;
>>>>  	struct f2fs_nat_block *nat_blk;
>>>>  	struct nat_entry *ne, *cur;
>>>>  	struct page *page = NULL;
>>>> @@ -2468,11 +2473,14 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>  	 * #1, flush nat entries to journal in current hot data summary block.
>>>>  	 * #2, flush nat entries to nat page.
>>>>  	 */
>>>> +
>>>> +	set->to_journal = true;
>>>> +
>>>>  	if (enabled_nat_bits(sbi, cpc) ||
>>>>  		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))
>>>> -		to_journal = false;
>>>> +		set->to_journal = false;
>>>>
>>>> -	if (to_journal) {
>>>> +	if (set->to_journal) {
>>>>  		down_write(&curseg->journal_rwsem);
>>>>  	} else {
>>>>  		page = get_next_nat_page(sbi, start_nid);
>>>> @@ -2488,7 +2496,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>
>>>>  		f2fs_bug_on(sbi, nat_get_blkaddr(ne) == NEW_ADDR);
>>>>
>>>> -		if (to_journal) {
>>>> +		if (set->to_journal) {
>>>>  			offset = lookup_journal_in_cursum(journal,
>>>>  							NAT_JOURNAL, nid, 1);
>>>>  			f2fs_bug_on(sbi, offset < 0);
>>>> @@ -2509,7 +2517,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>  		}
>>>>  	}
>>>>
>>>> -	if (to_journal) {
>>>> +	if (set->to_journal) {
>>>>  		up_write(&curseg->journal_rwsem);
>>>>  	} else {
>>>>  		__update_nat_bits(sbi, start_nid, page);
>>>> @@ -2517,7 +2525,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>  	}
>>>>
>>>>  	/* Allow dirty nats by node block allocation in write_begin */
>>>> -	if (!set->entry_cnt) {
>>>> +	if (!set->to_journal) {
>>>
>>> if (!set->to_journal && !set->entry_cnt) ?
>>>
>>> Thanks,
>>>
>>>>  		radix_tree_delete(&NM_I(sbi)->nat_set_root, set->set);
>>>>  		kmem_cache_free(nat_entry_set_slab, set);
>>>>  	}
>>>> @@ -2556,7 +2564,7 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>  		unsigned idx;
>>>>  		set_idx = setvec[found - 1]->set + 1;
>>>>  		for (idx = 0; idx < found; idx++)
>>>> -			__adjust_nat_entry_set(setvec[idx], &sets,
>>>> +			__adjust_nat_entry_set(sbi, setvec[idx], &sets,
>>>>  						MAX_NAT_JENTRIES(journal));
>>>>  	}
>>>>
>>>> @@ -2565,7 +2573,6 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>  		__flush_nat_entry_set(sbi, set, cpc);
>>>>
>>>>  	up_write(&nm_i->nat_tree_lock);
>>>> -	/* Allow dirty nats by node block allocation in write_begin */
>>>>  }
>>>>
>>>>  static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
>>>> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
>>>> index 081ef0d..3cf9c8b6 100644
>>>> --- a/fs/f2fs/node.h
>>>> +++ b/fs/f2fs/node.h
>>>> @@ -148,7 +148,9 @@ struct nat_entry_set {
>>>>  	struct list_head set_list;	/* link with other nat sets */
>>>>  	struct list_head entry_list;	/* link with dirty nat entries */
>>>>  	nid_t set;			/* set number*/
>>>> -	unsigned int entry_cnt;		/* the # of nat entries in set */
>>>> +	unsigned int entry_cnt:9;	/* the # of nat entries in set */
>>>> +	unsigned int to_journal:1;	/* set flush to journal */
>>>> +	__u64 cp_ver;			/* cp version of this set modify */
>>>>  };
>>>>
>>>>  struct free_nid {
>>>>
>>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3] f2fs: introduce a method to make nat journal more fresh
  2018-03-16  9:52       ` heyunlei
@ 2018-03-19  2:09         ` Chao Yu
  2018-03-19  8:46           ` heyunlei
  0 siblings, 1 reply; 7+ messages in thread
From: Chao Yu @ 2018-03-19  2:09 UTC (permalink / raw)
  To: heyunlei, jaegeuk, linux-f2fs-devel; +Cc: Zhangdianfang (Euler)

On 2018/3/16 17:52, heyunlei wrote:
> 
> 
>> -----Original Message-----
>> From: Yuchao (T)
>> Sent: Friday, March 16, 2018 5:47 PM
>> To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>> Cc: Wangbintian; Zhangdianfang (Euler)
>> Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>>
>> On 2018/3/16 17:14, heyunlei wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Yuchao (T)
>>>> Sent: Friday, March 16, 2018 4:49 PM
>>>> To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>>>> Cc: Wangbintian; Zhangdianfang (Euler)
>>>> Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>>>>
>>>>
>>>>
>>>> On 2018/3/14 14:33, Yunlei He wrote:
>>>>> This patch introduce a method to make nat journal more fresh:
>>>>> i.  sort set list using dirty entry number and cp version
>>>>>     average value.
>>>>> ii. if meet with cache hit, update average version valus with
>>>>>     current cp version.
>>>>>
>>>>> With this patch, newly modified nat set will flush to journal,
>>>>> and flush old nat set with same dirty entry number to nat area.
>>>>>
>>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>>> ---
>>>>>  fs/f2fs/node.c | 45 ++++++++++++++++++++++++++-------------------
>>>>>  fs/f2fs/node.h |  4 +++-
>>>>>  2 files changed, 29 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>>>> index 177c438..74f6079 100644
>>>>> --- a/fs/f2fs/node.c
>>>>> +++ b/fs/f2fs/node.c
>>>>> @@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
>>>>>  	__free_nat_entry(e);
>>>>>  }
>>>>>
>>>>> -static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>> -						struct nat_entry *ne)
>>>>> +static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool remove_journal,
>>>>> +			struct f2fs_nm_info *nm_i, struct nat_entry *ne)
>>>>>  {
>>>>>  	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
>>>>>  	struct nat_entry_set *head;
>>>>> @@ -207,9 +207,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>>  		INIT_LIST_HEAD(&head->set_list);
>>>>>  		head->set = set;
>>>>>  		head->entry_cnt = 0;
>>>>> +		head->cp_ver = 0;
>>>>>  		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
>>>>>  	}
>>>>>
>>>>> +	/* journal hit case, try to locate set in journal */
>>>>> +	if (!remove_journal && head->entry_cnt <= NAT_JOURNAL_ENTRIES)
>>>>> +		head->cp_ver += div_u64(cur_cp_version(F2FS_CKPT(sbi)),
>>>>> +							NAT_JOURNAL_ENTRIES);
>>>>> +
>>>>>  	if (get_nat_flag(ne, IS_DIRTY))
>>>>>  		goto refresh_list;
>>>>
>>>> 	else {
>>>> 		update head version here?
>>>> 	}
>>>
>>> Okay, if one nat entry update more than once, here will be overflow.
>>>
>>>>
>>>>>
>>>>> @@ -217,10 +223,7 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>>  	head->entry_cnt++;
>>>>>  	set_nat_flag(ne, IS_DIRTY, true);
>>>>>  refresh_list:
>>>>> -	if (nat_get_blkaddr(ne) == NEW_ADDR)
>>>>> -		list_del_init(&ne->list);
>>>>> -	else
>>>>> -		list_move_tail(&ne->list, &head->entry_list);
>>>>> +	list_move_tail(&ne->list, &head->entry_list);
>>>>
>>>> For NEW_ADDR case, why we need to track this nat entry in entry_list?
>>>
>>> If new_blkaddr == NEW_ADDR, we don't need to call __set_nat_cache_dirty:
>>
>> But such entry is still in clean list, shrinker can easily remove it in
>> out-of-memory condition.
> 
> Yes, you are right, my test doesn't cover low memory case. I 'll send a new version.

How about adding below patch before current one?

Subject: [PATCH] f2fs: don't track new nat entry in nat set

Nat entry set is used only in checkpoint(), and during checkpoint() we
won't flush new nat entry with unallocated address, so we don't need to
add new nat entry into nat set, then nat_entry_set::entry_cnt can
indicate actual entry count we need to flush in checkpoint().

---
 fs/f2fs/node.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 9dedd4b5e077..10cd8838c95c 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
 	__free_nat_entry(e);
 }

-static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
-						struct nat_entry *ne)
+static struct nat_entry_set *__grab_nat_entry_set(struct f2fs_nm_info *nm_i,
+							struct nat_entry *ne)
 {
 	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
 	struct nat_entry_set *head;
@@ -209,15 +209,27 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
 		head->entry_cnt = 0;
 		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
 	}
+	return head;
+}
+
+static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
+						struct nat_entry *ne)
+{
+	struct nat_entry_set *head;
+	bool new_ne = nat_get_blkaddr(ne) == NEW_ADDR;
+
+	if (!new_ne)
+		head = __grab_nat_entry_set(nm_i, ne);

 	if (get_nat_flag(ne, IS_DIRTY))
 		goto refresh_list;

 	nm_i->dirty_nat_cnt++;
-	head->entry_cnt++;
+	if (!new_ne)
+		head->entry_cnt++;
 	set_nat_flag(ne, IS_DIRTY, true);
 refresh_list:
-	if (nat_get_blkaddr(ne) == NEW_ADDR)
+	if (new_ne)
 		list_del_init(&ne->list);
 	else
 		list_move_tail(&ne->list, &head->entry_list);
-- 
2.15.0.55.gc2ece9dc4de6

Thanks,

> 
> Thanks.
> 
>>
>> Thanks,
>>
>>> 	i  nat entry with NEW_ADDR will increase dirty nat entry count in nat set，which will cause
>>>       a nat set has large nat entry count（most is NEW_ADDR）， but fewer of nat entries should
>>> 	  be written back in this checkpoint
>>>     ii  nat entry with NEW_ADDR will call __set_nat_cache_dirty again, when we write back its node
>>>       page, in this time, new_blkaddr != NEW_ADDR
>>> So, I think we'd better bypass _set_nat_cache_dirty if blkaddr == NEW_ADDR
>>>
>>> Thanks.
>>>
>>>>
>>>>>  }
>>>>>
>>>>>  static void __clear_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>> @@ -357,7 +360,8 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>>>  	nat_set_blkaddr(e, new_blkaddr);
>>>>>  	if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
>>>>>  		set_nat_flag(e, IS_CHECKPOINTED, false);
>>>>> -	__set_nat_cache_dirty(nm_i, e);
>>>>> +	if (new_blkaddr != NEW_ADDR)
>>>>> +		__set_nat_cache_dirty(sbi, false, nm_i, e);
>>>>>
>>>>>  	/* update fsync_mark if its inode nat entry is still alive */
>>>>>  	if (ni->nid != ni->ino)
>>>>> @@ -2395,14 +2399,14 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>>>>>  			spin_unlock(&nm_i->nid_list_lock);
>>>>>  		}
>>>>>
>>>>> -		__set_nat_cache_dirty(nm_i, ne);
>>>>> +		__set_nat_cache_dirty(sbi, true, nm_i, ne);
>>>>>  	}
>>>>>  	update_nats_in_cursum(journal, -i);
>>>>>  	up_write(&curseg->journal_rwsem);
>>>>>  }
>>>>>
>>>>> -static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>>>> -						struct list_head *head, int max)
>>>>> +static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>> +		struct nat_entry_set *nes, struct list_head *head, int max)
>>>>>  {
>>>>>  	struct nat_entry_set *cur;
>>>>>
>>>>> @@ -2410,7 +2414,9 @@ static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>>>>  		goto add_out;
>>>>>
>>>>>  	list_for_each_entry(cur, head, set_list) {
>>>>> -		if (cur->entry_cnt >= nes->entry_cnt) {
>>>>> +		if (cur->entry_cnt > nes->entry_cnt ||
>>>>> +			(cur->entry_cnt == nes->entry_cnt &&
>>>>> +			cur->cp_ver < nes->cp_ver)) {
>>>>>  			list_add(&nes->set_list, cur->set_list.prev);
>>>>>  			return;
>>>>>  		}
>>>>> @@ -2458,7 +2464,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>>>>  	struct f2fs_journal *journal = curseg->journal;
>>>>>  	nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
>>>>> -	bool to_journal = true;
>>>>>  	struct f2fs_nat_block *nat_blk;
>>>>>  	struct nat_entry *ne, *cur;
>>>>>  	struct page *page = NULL;
>>>>> @@ -2468,11 +2473,14 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>  	 * #1, flush nat entries to journal in current hot data summary block.
>>>>>  	 * #2, flush nat entries to nat page.
>>>>>  	 */
>>>>> +
>>>>> +	set->to_journal = true;
>>>>> +
>>>>>  	if (enabled_nat_bits(sbi, cpc) ||
>>>>>  		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))
>>>>> -		to_journal = false;
>>>>> +		set->to_journal = false;
>>>>>
>>>>> -	if (to_journal) {
>>>>> +	if (set->to_journal) {
>>>>>  		down_write(&curseg->journal_rwsem);
>>>>>  	} else {
>>>>>  		page = get_next_nat_page(sbi, start_nid);
>>>>> @@ -2488,7 +2496,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>
>>>>>  		f2fs_bug_on(sbi, nat_get_blkaddr(ne) == NEW_ADDR);
>>>>>
>>>>> -		if (to_journal) {
>>>>> +		if (set->to_journal) {
>>>>>  			offset = lookup_journal_in_cursum(journal,
>>>>>  							NAT_JOURNAL, nid, 1);
>>>>>  			f2fs_bug_on(sbi, offset < 0);
>>>>> @@ -2509,7 +2517,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>  		}
>>>>>  	}
>>>>>
>>>>> -	if (to_journal) {
>>>>> +	if (set->to_journal) {
>>>>>  		up_write(&curseg->journal_rwsem);
>>>>>  	} else {
>>>>>  		__update_nat_bits(sbi, start_nid, page);
>>>>> @@ -2517,7 +2525,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>  	}
>>>>>
>>>>>  	/* Allow dirty nats by node block allocation in write_begin */
>>>>> -	if (!set->entry_cnt) {
>>>>> +	if (!set->to_journal) {
>>>>
>>>> if (!set->to_journal && !set->entry_cnt) ?
>>>>
>>>> Thanks,
>>>>
>>>>>  		radix_tree_delete(&NM_I(sbi)->nat_set_root, set->set);
>>>>>  		kmem_cache_free(nat_entry_set_slab, set);
>>>>>  	}
>>>>> @@ -2556,7 +2564,7 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>>  		unsigned idx;
>>>>>  		set_idx = setvec[found - 1]->set + 1;
>>>>>  		for (idx = 0; idx < found; idx++)
>>>>> -			__adjust_nat_entry_set(setvec[idx], &sets,
>>>>> +			__adjust_nat_entry_set(sbi, setvec[idx], &sets,
>>>>>  						MAX_NAT_JENTRIES(journal));
>>>>>  	}
>>>>>
>>>>> @@ -2565,7 +2573,6 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>>  		__flush_nat_entry_set(sbi, set, cpc);
>>>>>
>>>>>  	up_write(&nm_i->nat_tree_lock);
>>>>> -	/* Allow dirty nats by node block allocation in write_begin */
>>>>>  }
>>>>>
>>>>>  static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
>>>>> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
>>>>> index 081ef0d..3cf9c8b6 100644
>>>>> --- a/fs/f2fs/node.h
>>>>> +++ b/fs/f2fs/node.h
>>>>> @@ -148,7 +148,9 @@ struct nat_entry_set {
>>>>>  	struct list_head set_list;	/* link with other nat sets */
>>>>>  	struct list_head entry_list;	/* link with dirty nat entries */
>>>>>  	nid_t set;			/* set number*/
>>>>> -	unsigned int entry_cnt;		/* the # of nat entries in set */
>>>>> +	unsigned int entry_cnt:9;	/* the # of nat entries in set */
>>>>> +	unsigned int to_journal:1;	/* set flush to journal */
>>>>> +	__u64 cp_ver;			/* cp version of this set modify */
>>>>>  };
>>>>>
>>>>>  struct free_nid {
>>>>>
>>>
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH v3] f2fs: introduce a method to make nat journal more fresh
  2018-03-19  2:09         ` Chao Yu
@ 2018-03-19  8:46           ` heyunlei
  0 siblings, 0 replies; 7+ messages in thread
From: heyunlei @ 2018-03-19  8:46 UTC (permalink / raw)
  To: Yuchao (T), jaegeuk, linux-f2fs-devel; +Cc: Zhangdianfang (Euler)



>-----Original Message-----
>From: Yuchao (T)
>Sent: Monday, March 19, 2018 10:10 AM
>To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>Cc: Wangbintian; Zhangdianfang (Euler)
>Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>
>On 2018/3/16 17:52, heyunlei wrote:
>>
>>
>>> -----Original Message-----
>>> From: Yuchao (T)
>>> Sent: Friday, March 16, 2018 5:47 PM
>>> To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>>> Cc: Wangbintian; Zhangdianfang (Euler)
>>> Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>>>
>>> On 2018/3/16 17:14, heyunlei wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Yuchao (T)
>>>>> Sent: Friday, March 16, 2018 4:49 PM
>>>>> To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>>>>> Cc: Wangbintian; Zhangdianfang (Euler)
>>>>> Subject: Re: [f2fs-dev][PATCH v3] f2fs: introduce a method to make nat journal more fresh
>>>>>
>>>>>
>>>>>
>>>>> On 2018/3/14 14:33, Yunlei He wrote:
>>>>>> This patch introduce a method to make nat journal more fresh:
>>>>>> i.  sort set list using dirty entry number and cp version
>>>>>>     average value.
>>>>>> ii. if meet with cache hit, update average version valus with
>>>>>>     current cp version.
>>>>>>
>>>>>> With this patch, newly modified nat set will flush to journal,
>>>>>> and flush old nat set with same dirty entry number to nat area.
>>>>>>
>>>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>>>> ---
>>>>>>  fs/f2fs/node.c | 45 ++++++++++++++++++++++++++-------------------
>>>>>>  fs/f2fs/node.h |  4 +++-
>>>>>>  2 files changed, 29 insertions(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>>>>> index 177c438..74f6079 100644
>>>>>> --- a/fs/f2fs/node.c
>>>>>> +++ b/fs/f2fs/node.c
>>>>>> @@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
>>>>>>  	__free_nat_entry(e);
>>>>>>  }
>>>>>>
>>>>>> -static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>>> -						struct nat_entry *ne)
>>>>>> +static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool remove_journal,
>>>>>> +			struct f2fs_nm_info *nm_i, struct nat_entry *ne)
>>>>>>  {
>>>>>>  	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
>>>>>>  	struct nat_entry_set *head;
>>>>>> @@ -207,9 +207,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>>>  		INIT_LIST_HEAD(&head->set_list);
>>>>>>  		head->set = set;
>>>>>>  		head->entry_cnt = 0;
>>>>>> +		head->cp_ver = 0;
>>>>>>  		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
>>>>>>  	}
>>>>>>
>>>>>> +	/* journal hit case, try to locate set in journal */
>>>>>> +	if (!remove_journal && head->entry_cnt <= NAT_JOURNAL_ENTRIES)
>>>>>> +		head->cp_ver += div_u64(cur_cp_version(F2FS_CKPT(sbi)),
>>>>>> +							NAT_JOURNAL_ENTRIES);
>>>>>> +
>>>>>>  	if (get_nat_flag(ne, IS_DIRTY))
>>>>>>  		goto refresh_list;
>>>>>
>>>>> 	else {
>>>>> 		update head version here?
>>>>> 	}
>>>>
>>>> Okay, if one nat entry update more than once, here will be overflow.
>>>>
>>>>>
>>>>>>
>>>>>> @@ -217,10 +223,7 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>>>  	head->entry_cnt++;
>>>>>>  	set_nat_flag(ne, IS_DIRTY, true);
>>>>>>  refresh_list:
>>>>>> -	if (nat_get_blkaddr(ne) == NEW_ADDR)
>>>>>> -		list_del_init(&ne->list);
>>>>>> -	else
>>>>>> -		list_move_tail(&ne->list, &head->entry_list);
>>>>>> +	list_move_tail(&ne->list, &head->entry_list);
>>>>>
>>>>> For NEW_ADDR case, why we need to track this nat entry in entry_list?
>>>>
>>>> If new_blkaddr == NEW_ADDR, we don't need to call __set_nat_cache_dirty:
>>>
>>> But such entry is still in clean list, shrinker can easily remove it in
>>> out-of-memory condition.
>>
>> Yes, you are right, my test doesn't cover low memory case. I 'll send a new version.
>
>How about adding below patch before current one?
>
>Subject: [PATCH] f2fs: don't track new nat entry in nat set
>
>Nat entry set is used only in checkpoint(), and during checkpoint() we
>won't flush new nat entry with unallocated address, so we don't need to
>add new nat entry into nat set, then nat_entry_set::entry_cnt can
>indicate actual entry count we need to flush in checkpoint().
>
>---
> fs/f2fs/node.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
>diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>index 9dedd4b5e077..10cd8838c95c 100644
>--- a/fs/f2fs/node.c
>+++ b/fs/f2fs/node.c
>@@ -193,8 +193,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info *nm_i, struct nat_entry *e)
> 	__free_nat_entry(e);
> }
>
>-static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>-						struct nat_entry *ne)
>+static struct nat_entry_set *__grab_nat_entry_set(struct f2fs_nm_info *nm_i,
>+							struct nat_entry *ne)
> {
> 	nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
> 	struct nat_entry_set *head;
>@@ -209,15 +209,27 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> 		head->entry_cnt = 0;
> 		f2fs_radix_tree_insert(&nm_i->nat_set_root, set, head);
> 	}
>+	return head;
>+}
>+
>+static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>+						struct nat_entry *ne)
>+{
>+	struct nat_entry_set *head;
>+	bool new_ne = nat_get_blkaddr(ne) == NEW_ADDR;
>+
>+	if (!new_ne)
>+		head = __grab_nat_entry_set(nm_i, ne);
>
> 	if (get_nat_flag(ne, IS_DIRTY))
> 		goto refresh_list;
>
> 	nm_i->dirty_nat_cnt++;
>-	head->entry_cnt++;
>+	if (!new_ne)
>+		head->entry_cnt++;
> 	set_nat_flag(ne, IS_DIRTY, true);
> refresh_list:
>-	if (nat_get_blkaddr(ne) == NEW_ADDR)
>+	if (new_ne)
> 		list_del_init(&ne->list);
> 	else
> 		list_move_tail(&ne->list, &head->entry_list);

Okay, it looks good to me.

Thanks.
>--
>2.15.0.55.gc2ece9dc4de6
>
>Thanks,
>
>>
>> Thanks.
>>
>>>
>>> Thanks,
>>>
>>>> 	i  nat entry with NEW_ADDR will increase dirty nat entry count in nat set，which will cause
>>>>       a nat set has large nat entry count（most is NEW_ADDR）， but fewer of nat entries should
>>>> 	  be written back in this checkpoint
>>>>     ii  nat entry with NEW_ADDR will call __set_nat_cache_dirty again, when we write back its node
>>>>       page, in this time, new_blkaddr != NEW_ADDR
>>>> So, I think we'd better bypass _set_nat_cache_dirty if blkaddr == NEW_ADDR
>>>>
>>>> Thanks.
>>>>
>>>>>
>>>>>>  }
>>>>>>
>>>>>>  static void __clear_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>>>> @@ -357,7 +360,8 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>>>>  	nat_set_blkaddr(e, new_blkaddr);
>>>>>>  	if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
>>>>>>  		set_nat_flag(e, IS_CHECKPOINTED, false);
>>>>>> -	__set_nat_cache_dirty(nm_i, e);
>>>>>> +	if (new_blkaddr != NEW_ADDR)
>>>>>> +		__set_nat_cache_dirty(sbi, false, nm_i, e);
>>>>>>
>>>>>>  	/* update fsync_mark if its inode nat entry is still alive */
>>>>>>  	if (ni->nid != ni->ino)
>>>>>> @@ -2395,14 +2399,14 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>>>>>>  			spin_unlock(&nm_i->nid_list_lock);
>>>>>>  		}
>>>>>>
>>>>>> -		__set_nat_cache_dirty(nm_i, ne);
>>>>>> +		__set_nat_cache_dirty(sbi, true, nm_i, ne);
>>>>>>  	}
>>>>>>  	update_nats_in_cursum(journal, -i);
>>>>>>  	up_write(&curseg->journal_rwsem);
>>>>>>  }
>>>>>>
>>>>>> -static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>>>>> -						struct list_head *head, int max)
>>>>>> +static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>> +		struct nat_entry_set *nes, struct list_head *head, int max)
>>>>>>  {
>>>>>>  	struct nat_entry_set *cur;
>>>>>>
>>>>>> @@ -2410,7 +2414,9 @@ static void __adjust_nat_entry_set(struct nat_entry_set *nes,
>>>>>>  		goto add_out;
>>>>>>
>>>>>>  	list_for_each_entry(cur, head, set_list) {
>>>>>> -		if (cur->entry_cnt >= nes->entry_cnt) {
>>>>>> +		if (cur->entry_cnt > nes->entry_cnt ||
>>>>>> +			(cur->entry_cnt == nes->entry_cnt &&
>>>>>> +			cur->cp_ver < nes->cp_ver)) {
>>>>>>  			list_add(&nes->set_list, cur->set_list.prev);
>>>>>>  			return;
>>>>>>  		}
>>>>>> @@ -2458,7 +2464,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>>>>>  	struct f2fs_journal *journal = curseg->journal;
>>>>>>  	nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
>>>>>> -	bool to_journal = true;
>>>>>>  	struct f2fs_nat_block *nat_blk;
>>>>>>  	struct nat_entry *ne, *cur;
>>>>>>  	struct page *page = NULL;
>>>>>> @@ -2468,11 +2473,14 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>>  	 * #1, flush nat entries to journal in current hot data summary block.
>>>>>>  	 * #2, flush nat entries to nat page.
>>>>>>  	 */
>>>>>> +
>>>>>> +	set->to_journal = true;
>>>>>> +
>>>>>>  	if (enabled_nat_bits(sbi, cpc) ||
>>>>>>  		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))
>>>>>> -		to_journal = false;
>>>>>> +		set->to_journal = false;
>>>>>>
>>>>>> -	if (to_journal) {
>>>>>> +	if (set->to_journal) {
>>>>>>  		down_write(&curseg->journal_rwsem);
>>>>>>  	} else {
>>>>>>  		page = get_next_nat_page(sbi, start_nid);
>>>>>> @@ -2488,7 +2496,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>>
>>>>>>  		f2fs_bug_on(sbi, nat_get_blkaddr(ne) == NEW_ADDR);
>>>>>>
>>>>>> -		if (to_journal) {
>>>>>> +		if (set->to_journal) {
>>>>>>  			offset = lookup_journal_in_cursum(journal,
>>>>>>  							NAT_JOURNAL, nid, 1);
>>>>>>  			f2fs_bug_on(sbi, offset < 0);
>>>>>> @@ -2509,7 +2517,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>>  		}
>>>>>>  	}
>>>>>>
>>>>>> -	if (to_journal) {
>>>>>> +	if (set->to_journal) {
>>>>>>  		up_write(&curseg->journal_rwsem);
>>>>>>  	} else {
>>>>>>  		__update_nat_bits(sbi, start_nid, page);
>>>>>> @@ -2517,7 +2525,7 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>>>>  	}
>>>>>>
>>>>>>  	/* Allow dirty nats by node block allocation in write_begin */
>>>>>> -	if (!set->entry_cnt) {
>>>>>> +	if (!set->to_journal) {
>>>>>
>>>>> if (!set->to_journal && !set->entry_cnt) ?
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>  		radix_tree_delete(&NM_I(sbi)->nat_set_root, set->set);
>>>>>>  		kmem_cache_free(nat_entry_set_slab, set);
>>>>>>  	}
>>>>>> @@ -2556,7 +2564,7 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>>>  		unsigned idx;
>>>>>>  		set_idx = setvec[found - 1]->set + 1;
>>>>>>  		for (idx = 0; idx < found; idx++)
>>>>>> -			__adjust_nat_entry_set(setvec[idx], &sets,
>>>>>> +			__adjust_nat_entry_set(sbi, setvec[idx], &sets,
>>>>>>  						MAX_NAT_JENTRIES(journal));
>>>>>>  	}
>>>>>>
>>>>>> @@ -2565,7 +2573,6 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>>>  		__flush_nat_entry_set(sbi, set, cpc);
>>>>>>
>>>>>>  	up_write(&nm_i->nat_tree_lock);
>>>>>> -	/* Allow dirty nats by node block allocation in write_begin */
>>>>>>  }
>>>>>>
>>>>>>  static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
>>>>>> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
>>>>>> index 081ef0d..3cf9c8b6 100644
>>>>>> --- a/fs/f2fs/node.h
>>>>>> +++ b/fs/f2fs/node.h
>>>>>> @@ -148,7 +148,9 @@ struct nat_entry_set {
>>>>>>  	struct list_head set_list;	/* link with other nat sets */
>>>>>>  	struct list_head entry_list;	/* link with dirty nat entries */
>>>>>>  	nid_t set;			/* set number*/
>>>>>> -	unsigned int entry_cnt;		/* the # of nat entries in set */
>>>>>> +	unsigned int entry_cnt:9;	/* the # of nat entries in set */
>>>>>> +	unsigned int to_journal:1;	/* set flush to journal */
>>>>>> +	__u64 cp_ver;			/* cp version of this set modify */
>>>>>>  };
>>>>>>
>>>>>>  struct free_nid {
>>>>>>
>>>>
>>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-19  8:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14  6:33 [PATCH v3] f2fs: introduce a method to make nat journal more fresh Yunlei He
2018-03-16  8:49 ` Chao Yu
2018-03-16  9:14   ` heyunlei
2018-03-16  9:46     ` Chao Yu
2018-03-16  9:52       ` heyunlei
2018-03-19  2:09         ` Chao Yu
2018-03-19  8:46           ` heyunlei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.