[f2fs-dev] [PATCH v2 1/2] f2fs: avoid redundant clean nat entry move in lru list

[f2fs-dev] [PATCH v2 1/2] f2fs: avoid redundant clean nat entry move in lru list

[wangzijie]

__lookup_nat_cache follows LRU manner to move clean nat entry, when nat
entries are going to be dirty, no need to move them to tail of lru list.
Introduce a parameter 'for_dirty' to avoid it.

Signed-off-by: wangzijie <wangzijie1@honor.com>
---
v2:
- followed by Jaegeuk's suggestion to add a parameter in __lookup_nat_cache
---
 fs/f2fs/node.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 76aba1961..a23db6238 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -204,14 +204,14 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
 	return ne;
 }
 
-static struct nat_entry *__lookup_nat_cache(struct f2fs_nm_info *nm_i, nid_t n)
+static struct nat_entry *__lookup_nat_cache(struct f2fs_nm_info *nm_i, nid_t n, bool for_dirty)
 {
 	struct nat_entry *ne;
 
 	ne = radix_tree_lookup(&nm_i->nat_root, n);
 
-	/* for recent accessed nat entry, move it to tail of lru list */
-	if (ne && !get_nat_flag(ne, IS_DIRTY)) {
+	/* for recent accessed(not used to set dirty) nat entry, move it to tail of lru list */
+	if (ne && !get_nat_flag(ne, IS_DIRTY) && !for_dirty) {
 		spin_lock(&nm_i->nat_list_lock);
 		if (!list_empty(&ne->list))
 			list_move_tail(&ne->list, &nm_i->nat_entries);
@@ -383,7 +383,7 @@ int f2fs_need_dentry_mark(struct f2fs_sb_info *sbi, nid_t nid)
 	bool need = false;
 
 	f2fs_down_read(&nm_i->nat_tree_lock);
-	e = __lookup_nat_cache(nm_i, nid);
+	e = __lookup_nat_cache(nm_i, nid, false);
 	if (e) {
 		if (!get_nat_flag(e, IS_CHECKPOINTED) &&
 				!get_nat_flag(e, HAS_FSYNCED_INODE))
@@ -400,7 +400,7 @@ bool f2fs_is_checkpointed_node(struct f2fs_sb_info *sbi, nid_t nid)
 	bool is_cp = true;
 
 	f2fs_down_read(&nm_i->nat_tree_lock);
-	e = __lookup_nat_cache(nm_i, nid);
+	e = __lookup_nat_cache(nm_i, nid, false);
 	if (e && !get_nat_flag(e, IS_CHECKPOINTED))
 		is_cp = false;
 	f2fs_up_read(&nm_i->nat_tree_lock);
@@ -414,7 +414,7 @@ bool f2fs_need_inode_block_update(struct f2fs_sb_info *sbi, nid_t ino)
 	bool need_update = true;
 
 	f2fs_down_read(&nm_i->nat_tree_lock);
-	e = __lookup_nat_cache(nm_i, ino);
+	e = __lookup_nat_cache(nm_i, ino, false);
 	if (e && get_nat_flag(e, HAS_LAST_FSYNC) &&
 			(get_nat_flag(e, IS_CHECKPOINTED) ||
 			 get_nat_flag(e, HAS_FSYNCED_INODE)))
@@ -439,7 +439,7 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi, nid_t nid,
 		return;
 
 	f2fs_down_write(&nm_i->nat_tree_lock);
-	e = __lookup_nat_cache(nm_i, nid);
+	e = __lookup_nat_cache(nm_i, nid, false);
 	if (!e)
 		e = __init_nat_entry(nm_i, new, ne, false);
 	else
@@ -460,7 +460,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
 	struct nat_entry *new = __alloc_nat_entry(sbi, ni->nid, true);
 
 	f2fs_down_write(&nm_i->nat_tree_lock);
-	e = __lookup_nat_cache(nm_i, ni->nid);
+	e = __lookup_nat_cache(nm_i, ni->nid, true);
 	if (!e) {
 		e = __init_nat_entry(nm_i, new, NULL, true);
 		copy_node_info(&e->ni, ni);
@@ -502,7 +502,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
 
 	/* update fsync_mark if its inode nat entry is still alive */
 	if (ni->nid != ni->ino)
-		e = __lookup_nat_cache(nm_i, ni->ino);
+		e = __lookup_nat_cache(nm_i, ni->ino, false);
 	if (e) {
 		if (fsync_done && ni->nid == ni->ino)
 			set_nat_flag(e, HAS_FSYNCED_INODE, true);
@@ -562,7 +562,7 @@ int f2fs_get_node_info(struct f2fs_sb_info *sbi, nid_t nid,
 retry:
 	/* Check nat cache */
 	f2fs_down_read(&nm_i->nat_tree_lock);
-	e = __lookup_nat_cache(nm_i, nid);
+	e = __lookup_nat_cache(nm_i, nid, false);
 	if (e) {
 		ni->ino = nat_get_ino(e);
 		ni->blk_addr = nat_get_blkaddr(e);
@@ -2371,7 +2371,7 @@ static bool add_free_nid(struct f2fs_sb_info *sbi,
 		 *   - __remove_nid_from_list(PREALLOC_NID)
 		 *                         - __insert_nid_to_list(FREE_NID)
 		 */
-		ne = __lookup_nat_cache(nm_i, nid);
+		ne = __lookup_nat_cache(nm_i, nid, false);
 		if (ne && (!get_nat_flag(ne, IS_CHECKPOINTED) ||
 				nat_get_blkaddr(ne) != NULL_ADDR))
 			goto err_out;
@@ -2936,7 +2936,7 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
 
 		raw_ne = nat_in_journal(journal, i);
 
-		ne = __lookup_nat_cache(nm_i, nid);
+		ne = __lookup_nat_cache(nm_i, nid, true);
 		if (!ne) {
 			ne = __alloc_nat_entry(sbi, nid, true);
 			__init_nat_entry(nm_i, ne, &raw_ne, true);
-- 
2.25.1

[f2fs-dev] [PATCH v2 2/2] f2fs: directly add newly allocated pre-dirty nat entry to dirty set list

[wangzijie]

When we need to alloc nat entry and set it dirty, we can directly add it to
dirty set list(or initialize its list_head for new_ne) instead of adding it
to clean list and make a move. Introduce init_dirty flag to do it.

Signed-off-by: wangzijie <wangzijie1@honor.com>
---
 fs/f2fs/node.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index a23db6238..20bcf8559 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -185,7 +185,7 @@ static void __free_nat_entry(struct nat_entry *e)
 
 /* must be locked by nat_tree_lock */
 static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
-	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail)
+	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail, bool init_dirty)
 {
 	if (no_fail)
 		f2fs_radix_tree_insert(&nm_i->nat_root, nat_get_nid(ne), ne);
@@ -195,6 +195,11 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
 	if (raw_ne)
 		node_info_from_raw_nat(&ne->ni, raw_ne);
 
+	if (init_dirty) {
+		nm_i->nat_cnt[TOTAL_NAT]++;
+		return ne;
+	}
+
 	spin_lock(&nm_i->nat_list_lock);
 	list_add_tail(&ne->list, &nm_i->nat_entries);
 	spin_unlock(&nm_i->nat_list_lock);
@@ -256,7 +261,7 @@ static struct nat_entry_set *__grab_nat_entry_set(struct f2fs_nm_info *nm_i,
 }
 
 static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
-						struct nat_entry *ne)
+					struct nat_entry *ne, bool init_dirty)
 {
 	struct nat_entry_set *head;
 	bool new_ne = nat_get_blkaddr(ne) == NEW_ADDR;
@@ -275,6 +280,18 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
 
 	set_nat_flag(ne, IS_PREALLOC, new_ne);
 
+	if (init_dirty) {
+		nm_i->nat_cnt[DIRTY_NAT]++;
+		set_nat_flag(ne, IS_DIRTY, true);
+		spin_lock(&nm_i->nat_list_lock);
+		if (new_ne)
+			INIT_LIST_HEAD(&ne->list);
+		else
+			list_add_tail(&ne->list, &head->entry_list);
+		spin_unlock(&nm_i->nat_list_lock);
+		return;
+    }
+
 	if (get_nat_flag(ne, IS_DIRTY))
 		goto refresh_list;
 
@@ -441,7 +458,7 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi, nid_t nid,
 	f2fs_down_write(&nm_i->nat_tree_lock);
 	e = __lookup_nat_cache(nm_i, nid, false);
 	if (!e)
-		e = __init_nat_entry(nm_i, new, ne, false);
+		e = __init_nat_entry(nm_i, new, ne, false, false);
 	else
 		f2fs_bug_on(sbi, nat_get_ino(e) != le32_to_cpu(ne->ino) ||
 				nat_get_blkaddr(e) !=
@@ -458,11 +475,13 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
 	struct f2fs_nm_info *nm_i = NM_I(sbi);
 	struct nat_entry *e;
 	struct nat_entry *new = __alloc_nat_entry(sbi, ni->nid, true);
+	bool init_dirty = false;
 
 	f2fs_down_write(&nm_i->nat_tree_lock);
 	e = __lookup_nat_cache(nm_i, ni->nid, true);
 	if (!e) {
-		e = __init_nat_entry(nm_i, new, NULL, true);
+		init_dirty = true;
+		e = __init_nat_entry(nm_i, new, NULL, true, true);
 		copy_node_info(&e->ni, ni);
 		f2fs_bug_on(sbi, ni->blk_addr == NEW_ADDR);
 	} else if (new_blkaddr == NEW_ADDR) {
@@ -498,7 +517,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
 	nat_set_blkaddr(e, new_blkaddr);
 	if (!__is_valid_data_blkaddr(new_blkaddr))
 		set_nat_flag(e, IS_CHECKPOINTED, false);
-	__set_nat_cache_dirty(nm_i, e);
+	__set_nat_cache_dirty(nm_i, e, init_dirty);
 
 	/* update fsync_mark if its inode nat entry is still alive */
 	if (ni->nid != ni->ino)
@@ -2924,6 +2943,7 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
 	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
 	struct f2fs_journal *journal = curseg->journal;
 	int i;
+	bool init_dirty;
 
 	down_write(&curseg->journal_rwsem);
 	for (i = 0; i < nats_in_cursum(journal); i++) {
@@ -2934,12 +2954,15 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
 		if (f2fs_check_nid_range(sbi, nid))
 			continue;
 
+		init_dirty = false;
+
 		raw_ne = nat_in_journal(journal, i);
 
 		ne = __lookup_nat_cache(nm_i, nid, true);
 		if (!ne) {
+			init_dirty = true;
 			ne = __alloc_nat_entry(sbi, nid, true);
-			__init_nat_entry(nm_i, ne, &raw_ne, true);
+			__init_nat_entry(nm_i, ne, &raw_ne, true, true);
 		}
 
 		/*
@@ -2954,7 +2977,7 @@ 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(nm_i, ne, init_dirty);
 	}
 	update_nats_in_cursum(journal, -i);
 	up_write(&curseg->journal_rwsem);
-- 
2.25.1

Re: [f2fs-dev] [PATCH v2 1/2] f2fs: avoid redundant clean nat entry move in lru list

[Chao Yu]

On 7/22/25 22:36, wangzijie wrote:
> __lookup_nat_cache follows LRU manner to move clean nat entry, when nat
> entries are going to be dirty, no need to move them to tail of lru list.
> Introduce a parameter 'for_dirty' to avoid it.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
> v2:
> - followed by Jaegeuk's suggestion to add a parameter in __lookup_nat_cache
> ---
>  fs/f2fs/node.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 76aba1961..a23db6238 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -204,14 +204,14 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
>  	return ne;
>  }
>  
> -static struct nat_entry *__lookup_nat_cache(struct f2fs_nm_info *nm_i, nid_t n)
> +static struct nat_entry *__lookup_nat_cache(struct f2fs_nm_info *nm_i, nid_t n, bool for_dirty)
>  {
>  	struct nat_entry *ne;
>  
>  	ne = radix_tree_lookup(&nm_i->nat_root, n);
>  
> -	/* for recent accessed nat entry, move it to tail of lru list */
> -	if (ne && !get_nat_flag(ne, IS_DIRTY)) {
> +	/* for recent accessed(not used to set dirty) nat entry, move it to tail of lru list */

What do you think of this?

	/*
	 * for recent accessed nat entry which will not be dirtied soon
	 * later, move it to tail of lru list.
	 */

Thanks,

> +	if (ne && !get_nat_flag(ne, IS_DIRTY) && !for_dirty) {
>  		spin_lock(&nm_i->nat_list_lock);
>  		if (!list_empty(&ne->list))
>  			list_move_tail(&ne->list, &nm_i->nat_entries);
> @@ -383,7 +383,7 @@ int f2fs_need_dentry_mark(struct f2fs_sb_info *sbi, nid_t nid)
>  	bool need = false;
>  
>  	f2fs_down_read(&nm_i->nat_tree_lock);
> -	e = __lookup_nat_cache(nm_i, nid);
> +	e = __lookup_nat_cache(nm_i, nid, false);
>  	if (e) {
>  		if (!get_nat_flag(e, IS_CHECKPOINTED) &&
>  				!get_nat_flag(e, HAS_FSYNCED_INODE))
> @@ -400,7 +400,7 @@ bool f2fs_is_checkpointed_node(struct f2fs_sb_info *sbi, nid_t nid)
>  	bool is_cp = true;
>  
>  	f2fs_down_read(&nm_i->nat_tree_lock);
> -	e = __lookup_nat_cache(nm_i, nid);
> +	e = __lookup_nat_cache(nm_i, nid, false);
>  	if (e && !get_nat_flag(e, IS_CHECKPOINTED))
>  		is_cp = false;
>  	f2fs_up_read(&nm_i->nat_tree_lock);
> @@ -414,7 +414,7 @@ bool f2fs_need_inode_block_update(struct f2fs_sb_info *sbi, nid_t ino)
>  	bool need_update = true;
>  
>  	f2fs_down_read(&nm_i->nat_tree_lock);
> -	e = __lookup_nat_cache(nm_i, ino);
> +	e = __lookup_nat_cache(nm_i, ino, false);
>  	if (e && get_nat_flag(e, HAS_LAST_FSYNC) &&
>  			(get_nat_flag(e, IS_CHECKPOINTED) ||
>  			 get_nat_flag(e, HAS_FSYNCED_INODE)))
> @@ -439,7 +439,7 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi, nid_t nid,
>  		return;
>  
>  	f2fs_down_write(&nm_i->nat_tree_lock);
> -	e = __lookup_nat_cache(nm_i, nid);
> +	e = __lookup_nat_cache(nm_i, nid, false);
>  	if (!e)
>  		e = __init_nat_entry(nm_i, new, ne, false);
>  	else
> @@ -460,7 +460,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>  	struct nat_entry *new = __alloc_nat_entry(sbi, ni->nid, true);
>  
>  	f2fs_down_write(&nm_i->nat_tree_lock);
> -	e = __lookup_nat_cache(nm_i, ni->nid);
> +	e = __lookup_nat_cache(nm_i, ni->nid, true);
>  	if (!e) {
>  		e = __init_nat_entry(nm_i, new, NULL, true);
>  		copy_node_info(&e->ni, ni);
> @@ -502,7 +502,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>  
>  	/* update fsync_mark if its inode nat entry is still alive */
>  	if (ni->nid != ni->ino)
> -		e = __lookup_nat_cache(nm_i, ni->ino);
> +		e = __lookup_nat_cache(nm_i, ni->ino, false);
>  	if (e) {
>  		if (fsync_done && ni->nid == ni->ino)
>  			set_nat_flag(e, HAS_FSYNCED_INODE, true);
> @@ -562,7 +562,7 @@ int f2fs_get_node_info(struct f2fs_sb_info *sbi, nid_t nid,
>  retry:
>  	/* Check nat cache */
>  	f2fs_down_read(&nm_i->nat_tree_lock);
> -	e = __lookup_nat_cache(nm_i, nid);
> +	e = __lookup_nat_cache(nm_i, nid, false);
>  	if (e) {
>  		ni->ino = nat_get_ino(e);
>  		ni->blk_addr = nat_get_blkaddr(e);
> @@ -2371,7 +2371,7 @@ static bool add_free_nid(struct f2fs_sb_info *sbi,
>  		 *   - __remove_nid_from_list(PREALLOC_NID)
>  		 *                         - __insert_nid_to_list(FREE_NID)
>  		 */
> -		ne = __lookup_nat_cache(nm_i, nid);
> +		ne = __lookup_nat_cache(nm_i, nid, false);
>  		if (ne && (!get_nat_flag(ne, IS_CHECKPOINTED) ||
>  				nat_get_blkaddr(ne) != NULL_ADDR))
>  			goto err_out;
> @@ -2936,7 +2936,7 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>  
>  		raw_ne = nat_in_journal(journal, i);
>  
> -		ne = __lookup_nat_cache(nm_i, nid);
> +		ne = __lookup_nat_cache(nm_i, nid, true);
>  		if (!ne) {
>  			ne = __alloc_nat_entry(sbi, nid, true);
>  			__init_nat_entry(nm_i, ne, &raw_ne, true);

Re: [f2fs-dev] [PATCH v2 1/2] f2fs: avoid redundant clean nat entry move in lru list

[wangzijie]

> On 7/22/25 22:36, wangzijie wrote:
> > __lookup_nat_cache follows LRU manner to move clean nat entry, when nat
> > entries are going to be dirty, no need to move them to tail of lru list.
> > Introduce a parameter 'for_dirty' to avoid it.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> > v2:
> > - followed by Jaegeuk's suggestion to add a parameter in __lookup_nat_cache
> > ---
> >  fs/f2fs/node.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index 76aba1961..a23db6238 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -204,14 +204,14 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
> >  	return ne;
> >  }
> >  
> > -static struct nat_entry *__lookup_nat_cache(struct f2fs_nm_info *nm_i, nid_t n)
> > +static struct nat_entry *__lookup_nat_cache(struct f2fs_nm_info *nm_i, nid_t n, bool for_dirty)
> >  {
> >  	struct nat_entry *ne;
> >  
> >  	ne = radix_tree_lookup(&nm_i->nat_root, n);
> >  
> > -	/* for recent accessed nat entry, move it to tail of lru list */
> > -	if (ne && !get_nat_flag(ne, IS_DIRTY)) {
> > +	/* for recent accessed(not used to set dirty) nat entry, move it to tail of lru list */
> 
> What do you think of this?
> 
> 	/*
> 	 * for recent accessed nat entry which will not be dirtied soon
> 	 * later, move it to tail of lru list.
> 	 */
> 
> Thanks,

Hi, Chao
Thank you for your suggestion. Let me update comment in next version.

> > +	if (ne && !get_nat_flag(ne, IS_DIRTY) && !for_dirty) {
> >  		spin_lock(&nm_i->nat_list_lock);
> >  		if (!list_empty(&ne->list))
> >  			list_move_tail(&ne->list, &nm_i->nat_entries);
> > @@ -383,7 +383,7 @@ int f2fs_need_dentry_mark(struct f2fs_sb_info *sbi, nid_t nid)
> >  	bool need = false;
> >  
> >  	f2fs_down_read(&nm_i->nat_tree_lock);
> > -	e = __lookup_nat_cache(nm_i, nid);
> > +	e = __lookup_nat_cache(nm_i, nid, false);
> >  	if (e) {
> >  		if (!get_nat_flag(e, IS_CHECKPOINTED) &&
> >  				!get_nat_flag(e, HAS_FSYNCED_INODE))
> > @@ -400,7 +400,7 @@ bool f2fs_is_checkpointed_node(struct f2fs_sb_info *sbi, nid_t nid)
> >  	bool is_cp = true;
> >  
> >  	f2fs_down_read(&nm_i->nat_tree_lock);
> > -	e = __lookup_nat_cache(nm_i, nid);
> > +	e = __lookup_nat_cache(nm_i, nid, false);
> >  	if (e && !get_nat_flag(e, IS_CHECKPOINTED))
> >  		is_cp = false;
> >  	f2fs_up_read(&nm_i->nat_tree_lock);
> > @@ -414,7 +414,7 @@ bool f2fs_need_inode_block_update(struct f2fs_sb_info *sbi, nid_t ino)
> >  	bool need_update = true;
> >  
> >  	f2fs_down_read(&nm_i->nat_tree_lock);
> > -	e = __lookup_nat_cache(nm_i, ino);
> > +	e = __lookup_nat_cache(nm_i, ino, false);
> >  	if (e && get_nat_flag(e, HAS_LAST_FSYNC) &&
> >  			(get_nat_flag(e, IS_CHECKPOINTED) ||
> >  			 get_nat_flag(e, HAS_FSYNCED_INODE)))
> > @@ -439,7 +439,7 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi, nid_t nid,
> >  		return;
> >  
> >  	f2fs_down_write(&nm_i->nat_tree_lock);
> > -	e = __lookup_nat_cache(nm_i, nid);
> > +	e = __lookup_nat_cache(nm_i, nid, false);
> >  	if (!e)
> >  		e = __init_nat_entry(nm_i, new, ne, false);
> >  	else
> > @@ -460,7 +460,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
> >  	struct nat_entry *new = __alloc_nat_entry(sbi, ni->nid, true);
> >  
> >  	f2fs_down_write(&nm_i->nat_tree_lock);
> > -	e = __lookup_nat_cache(nm_i, ni->nid);
> > +	e = __lookup_nat_cache(nm_i, ni->nid, true);
> >  	if (!e) {
> >  		e = __init_nat_entry(nm_i, new, NULL, true);
> >  		copy_node_info(&e->ni, ni);
> > @@ -502,7 +502,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
> >  
> >  	/* update fsync_mark if its inode nat entry is still alive */
> >  	if (ni->nid != ni->ino)
> > -		e = __lookup_nat_cache(nm_i, ni->ino);
> > +		e = __lookup_nat_cache(nm_i, ni->ino, false);
> >  	if (e) {
> >  		if (fsync_done && ni->nid == ni->ino)
> >  			set_nat_flag(e, HAS_FSYNCED_INODE, true);
> > @@ -562,7 +562,7 @@ int f2fs_get_node_info(struct f2fs_sb_info *sbi, nid_t nid,
> >  retry:
> >  	/* Check nat cache */
> >  	f2fs_down_read(&nm_i->nat_tree_lock);
> > -	e = __lookup_nat_cache(nm_i, nid);
> > +	e = __lookup_nat_cache(nm_i, nid, false);
> >  	if (e) {
> >  		ni->ino = nat_get_ino(e);
> >  		ni->blk_addr = nat_get_blkaddr(e);
> > @@ -2371,7 +2371,7 @@ static bool add_free_nid(struct f2fs_sb_info *sbi,
> >  		 *   - __remove_nid_from_list(PREALLOC_NID)
> >  		 *                         - __insert_nid_to_list(FREE_NID)
> >  		 */
> > -		ne = __lookup_nat_cache(nm_i, nid);
> > +		ne = __lookup_nat_cache(nm_i, nid, false);
> >  		if (ne && (!get_nat_flag(ne, IS_CHECKPOINTED) ||
> >  				nat_get_blkaddr(ne) != NULL_ADDR))
> >  			goto err_out;
> > @@ -2936,7 +2936,7 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
> >  
> >  		raw_ne = nat_in_journal(journal, i);
> >  
> > -		ne = __lookup_nat_cache(nm_i, nid);
> > +		ne = __lookup_nat_cache(nm_i, nid, true);
> >  		if (!ne) {
> >  			ne = __alloc_nat_entry(sbi, nid, true);
> >  			__init_nat_entry(nm_i, ne, &raw_ne, true);

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: directly add newly allocated pre-dirty nat entry to dirty set list

[Chao Yu]

On 7/22/25 22:36, wangzijie wrote:
> When we need to alloc nat entry and set it dirty, we can directly add it to
> dirty set list(or initialize its list_head for new_ne) instead of adding it
> to clean list and make a move. Introduce init_dirty flag to do it.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
>  fs/f2fs/node.c | 37 ++++++++++++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index a23db6238..20bcf8559 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -185,7 +185,7 @@ static void __free_nat_entry(struct nat_entry *e)
>  
>  /* must be locked by nat_tree_lock */
>  static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
> -	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail)
> +	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail, bool init_dirty)
>  {
>  	if (no_fail)
>  		f2fs_radix_tree_insert(&nm_i->nat_root, nat_get_nid(ne), ne);
> @@ -195,6 +195,11 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
>  	if (raw_ne)
>  		node_info_from_raw_nat(&ne->ni, raw_ne);
>  
> +	if (init_dirty) {
> +		nm_i->nat_cnt[TOTAL_NAT]++;
> +		return ne;
> +	}
> +
>  	spin_lock(&nm_i->nat_list_lock);
>  	list_add_tail(&ne->list, &nm_i->nat_entries);
>  	spin_unlock(&nm_i->nat_list_lock);
> @@ -256,7 +261,7 @@ static struct nat_entry_set *__grab_nat_entry_set(struct f2fs_nm_info *nm_i,
>  }
>  
>  static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> -						struct nat_entry *ne)
> +					struct nat_entry *ne, bool init_dirty)
>  {
>  	struct nat_entry_set *head;
>  	bool new_ne = nat_get_blkaddr(ne) == NEW_ADDR;
> @@ -275,6 +280,18 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>  
>  	set_nat_flag(ne, IS_PREALLOC, new_ne);
>  
> +	if (init_dirty) {
> +		nm_i->nat_cnt[DIRTY_NAT]++;
> +		set_nat_flag(ne, IS_DIRTY, true);
> +		spin_lock(&nm_i->nat_list_lock);
> +		if (new_ne)
> +			INIT_LIST_HEAD(&ne->list);
> +		else
> +			list_add_tail(&ne->list, &head->entry_list);
> +		spin_unlock(&nm_i->nat_list_lock);
> +		return;
> +    }

Nit issue, above blanks should be replaced w/ tab.

Can we clean up like this?

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index de99b42437c6..60fc2c7b8e10 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -280,30 +280,23 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,

 	set_nat_flag(ne, IS_PREALLOC, new_ne);

-	if (init_dirty) {
-		nm_i->nat_cnt[DIRTY_NAT]++;
-		set_nat_flag(ne, IS_DIRTY, true);
-		spin_lock(&nm_i->nat_list_lock);
-		if (new_ne)
-			INIT_LIST_HEAD(&ne->list);
-		else
-			list_add_tail(&ne->list, &head->entry_list);
-		spin_unlock(&nm_i->nat_list_lock);
-		return;
-    }
-
 	if (get_nat_flag(ne, IS_DIRTY))
 		goto refresh_list;

 	nm_i->nat_cnt[DIRTY_NAT]++;
-	nm_i->nat_cnt[RECLAIMABLE_NAT]--;
+	if (!init_dirty)
+		nm_i->nat_cnt[RECLAIMABLE_NAT]--;
 	set_nat_flag(ne, IS_DIRTY, true);
 refresh_list:
 	spin_lock(&nm_i->nat_list_lock);
-	if (new_ne)
-		list_del_init(&ne->list);
-	else
+	if (new_ne) {
+		if (init_dirty)
+			INIT_LIST_HEAD(&ne->list);
+		else
+			list_del_init(&ne->list);
+	} else {
 		list_move_tail(&ne->list, &head->entry_list);
+	}
 	spin_unlock(&nm_i->nat_list_lock);
 }

Thanks,

> +
>  	if (get_nat_flag(ne, IS_DIRTY))
>  		goto refresh_list;
>  
> @@ -441,7 +458,7 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi, nid_t nid,
>  	f2fs_down_write(&nm_i->nat_tree_lock);
>  	e = __lookup_nat_cache(nm_i, nid, false);
>  	if (!e)
> -		e = __init_nat_entry(nm_i, new, ne, false);
> +		e = __init_nat_entry(nm_i, new, ne, false, false);
>  	else
>  		f2fs_bug_on(sbi, nat_get_ino(e) != le32_to_cpu(ne->ino) ||
>  				nat_get_blkaddr(e) !=
> @@ -458,11 +475,13 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
>  	struct nat_entry *e;
>  	struct nat_entry *new = __alloc_nat_entry(sbi, ni->nid, true);
> +	bool init_dirty = false;
>  
>  	f2fs_down_write(&nm_i->nat_tree_lock);
>  	e = __lookup_nat_cache(nm_i, ni->nid, true);
>  	if (!e) {
> -		e = __init_nat_entry(nm_i, new, NULL, true);
> +		init_dirty = true;
> +		e = __init_nat_entry(nm_i, new, NULL, true, true);
>  		copy_node_info(&e->ni, ni);
>  		f2fs_bug_on(sbi, ni->blk_addr == NEW_ADDR);
>  	} else if (new_blkaddr == NEW_ADDR) {
> @@ -498,7 +517,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>  	nat_set_blkaddr(e, new_blkaddr);
>  	if (!__is_valid_data_blkaddr(new_blkaddr))
>  		set_nat_flag(e, IS_CHECKPOINTED, false);
> -	__set_nat_cache_dirty(nm_i, e);
> +	__set_nat_cache_dirty(nm_i, e, init_dirty);
>  
>  	/* update fsync_mark if its inode nat entry is still alive */
>  	if (ni->nid != ni->ino)
> @@ -2924,6 +2943,7 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>  	struct f2fs_journal *journal = curseg->journal;
>  	int i;
> +	bool init_dirty;
>  
>  	down_write(&curseg->journal_rwsem);
>  	for (i = 0; i < nats_in_cursum(journal); i++) {
> @@ -2934,12 +2954,15 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>  		if (f2fs_check_nid_range(sbi, nid))
>  			continue;
>  
> +		init_dirty = false;
> +
>  		raw_ne = nat_in_journal(journal, i);
>  
>  		ne = __lookup_nat_cache(nm_i, nid, true);
>  		if (!ne) {
> +			init_dirty = true;
>  			ne = __alloc_nat_entry(sbi, nid, true);
> -			__init_nat_entry(nm_i, ne, &raw_ne, true);
> +			__init_nat_entry(nm_i, ne, &raw_ne, true, true);
>  		}
>  
>  		/*
> @@ -2954,7 +2977,7 @@ 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(nm_i, ne, init_dirty);
>  	}
>  	update_nats_in_cursum(journal, -i);
>  	up_write(&curseg->journal_rwsem);

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: directly add newly allocated pre-dirty nat entry to dirty set list

[wangzijie]

> On 7/22/25 22:36, wangzijie wrote:
> > When we need to alloc nat entry and set it dirty, we can directly add it to
> > dirty set list(or initialize its list_head for new_ne) instead of adding it
> > to clean list and make a move. Introduce init_dirty flag to do it.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> >  fs/f2fs/node.c | 37 ++++++++++++++++++++++++++++++-------
> >  1 file changed, 30 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index a23db6238..20bcf8559 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -185,7 +185,7 @@ static void __free_nat_entry(struct nat_entry *e)
> >  
> >  /* must be locked by nat_tree_lock */
> >  static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
> > -	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail)
> > +	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail, bool init_dirty)
> >  {
> >  	if (no_fail)
> >  		f2fs_radix_tree_insert(&nm_i->nat_root, nat_get_nid(ne), ne);
> > @@ -195,6 +195,11 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
> >  	if (raw_ne)
> >  		node_info_from_raw_nat(&ne->ni, raw_ne);
> >  
> > +	if (init_dirty) {
> > +		nm_i->nat_cnt[TOTAL_NAT]++;
> > +		return ne;
> > +	}
> > +
> >  	spin_lock(&nm_i->nat_list_lock);
> >  	list_add_tail(&ne->list, &nm_i->nat_entries);
> >  	spin_unlock(&nm_i->nat_list_lock);
> > @@ -256,7 +261,7 @@ static struct nat_entry_set *__grab_nat_entry_set(struct f2fs_nm_info *nm_i,
> >  }
> >  
> >  static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> > -						struct nat_entry *ne)
> > +					struct nat_entry *ne, bool init_dirty)
> >  {
> >  	struct nat_entry_set *head;
> >  	bool new_ne = nat_get_blkaddr(ne) == NEW_ADDR;
> > @@ -275,6 +280,18 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> >  
> >  	set_nat_flag(ne, IS_PREALLOC, new_ne);
> >  
> > +	if (init_dirty) {
> > +		nm_i->nat_cnt[DIRTY_NAT]++;
> > +		set_nat_flag(ne, IS_DIRTY, true);
> > +		spin_lock(&nm_i->nat_list_lock);
> > +		if (new_ne)
> > +			INIT_LIST_HEAD(&ne->list);
> > +		else
> > +			list_add_tail(&ne->list, &head->entry_list);
> > +		spin_unlock(&nm_i->nat_list_lock);
> > +		return;
> > +    }
> 
> Nit issue, above blanks should be replaced w/ tab.

Ah...my bad :-(

> Can we clean up like this?
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index de99b42437c6..60fc2c7b8e10 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -280,30 +280,23 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> 
>  	set_nat_flag(ne, IS_PREALLOC, new_ne);
> 
> -	if (init_dirty) {
> -		nm_i->nat_cnt[DIRTY_NAT]++;
> -		set_nat_flag(ne, IS_DIRTY, true);
> -		spin_lock(&nm_i->nat_list_lock);
> -		if (new_ne)
> -			INIT_LIST_HEAD(&ne->list);
> -		else
> -			list_add_tail(&ne->list, &head->entry_list);
> -		spin_unlock(&nm_i->nat_list_lock);
> -		return;
> -    }
> -
>  	if (get_nat_flag(ne, IS_DIRTY))
>  		goto refresh_list;
> 
>  	nm_i->nat_cnt[DIRTY_NAT]++;
> -	nm_i->nat_cnt[RECLAIMABLE_NAT]--;
> +	if (!init_dirty)
> +		nm_i->nat_cnt[RECLAIMABLE_NAT]--;
>  	set_nat_flag(ne, IS_DIRTY, true);
>  refresh_list:
>  	spin_lock(&nm_i->nat_list_lock);
> -	if (new_ne)
> -		list_del_init(&ne->list);
> -	else
> +	if (new_ne) {
> +		if (init_dirty)
> +			INIT_LIST_HEAD(&ne->list);
> +		else
> +			list_del_init(&ne->list);
> +	} else {
>  		list_move_tail(&ne->list, &head->entry_list);
> +	}
>  	spin_unlock(&nm_i->nat_list_lock);
>  }
> 
> Thanks,

We need to init list_head before using list_move_tail.
I think we can do more clean up like this, keep refresh_list part code.

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 20bcf8559..ebb624fa1 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -196,6 +196,7 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
 		node_info_from_raw_nat(&ne->ni, raw_ne);
 
 	if (init_dirty) {
+		INIT_LIST_HEAD(&ne->list);
 		nm_i->nat_cnt[TOTAL_NAT]++;
 		return ne;
 	}
@@ -280,23 +281,12 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
 
 	set_nat_flag(ne, IS_PREALLOC, new_ne);
 
-	if (init_dirty) {
-		nm_i->nat_cnt[DIRTY_NAT]++;
-		set_nat_flag(ne, IS_DIRTY, true);
-		spin_lock(&nm_i->nat_list_lock);
-		if (new_ne)
-			INIT_LIST_HEAD(&ne->list);
-		else
-			list_add_tail(&ne->list, &head->entry_list);
-		spin_unlock(&nm_i->nat_list_lock);
-		return;
-    }
-
 	if (get_nat_flag(ne, IS_DIRTY))
 		goto refresh_list;
 
 	nm_i->nat_cnt[DIRTY_NAT]++;
-	nm_i->nat_cnt[RECLAIMABLE_NAT]--;
+	if (!init_dirty)
+		nm_i->nat_cnt[RECLAIMABLE_NAT]--;
 	set_nat_flag(ne, IS_DIRTY, true);
 refresh_list:
 	spin_lock(&nm_i->nat_list_lock);

> > +
> >  	if (get_nat_flag(ne, IS_DIRTY))
> >  		goto refresh_list;
> >  
> > @@ -441,7 +458,7 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi, nid_t nid,
> >  	f2fs_down_write(&nm_i->nat_tree_lock);
> >  	e = __lookup_nat_cache(nm_i, nid, false);
> >  	if (!e)
> > -		e = __init_nat_entry(nm_i, new, ne, false);
> > +		e = __init_nat_entry(nm_i, new, ne, false, false);
> >  	else
> >  		f2fs_bug_on(sbi, nat_get_ino(e) != le32_to_cpu(ne->ino) ||
> >  				nat_get_blkaddr(e) !=
> > @@ -458,11 +475,13 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
> >  	struct f2fs_nm_info *nm_i = NM_I(sbi);
> >  	struct nat_entry *e;
> >  	struct nat_entry *new = __alloc_nat_entry(sbi, ni->nid, true);
> > +	bool init_dirty = false;
> >  
> >  	f2fs_down_write(&nm_i->nat_tree_lock);
> >  	e = __lookup_nat_cache(nm_i, ni->nid, true);
> >  	if (!e) {
> > -		e = __init_nat_entry(nm_i, new, NULL, true);
> > +		init_dirty = true;
> > +		e = __init_nat_entry(nm_i, new, NULL, true, true);
> >  		copy_node_info(&e->ni, ni);
> >  		f2fs_bug_on(sbi, ni->blk_addr == NEW_ADDR);
> >  	} else if (new_blkaddr == NEW_ADDR) {
> > @@ -498,7 +517,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
> >  	nat_set_blkaddr(e, new_blkaddr);
> >  	if (!__is_valid_data_blkaddr(new_blkaddr))
> >  		set_nat_flag(e, IS_CHECKPOINTED, false);
> > -	__set_nat_cache_dirty(nm_i, e);
> > +	__set_nat_cache_dirty(nm_i, e, init_dirty);
> >  
> >  	/* update fsync_mark if its inode nat entry is still alive */
> >  	if (ni->nid != ni->ino)
> > @@ -2924,6 +2943,7 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
> >  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
> >  	struct f2fs_journal *journal = curseg->journal;
> >  	int i;
> > +	bool init_dirty;
> >  
> >  	down_write(&curseg->journal_rwsem);
> >  	for (i = 0; i < nats_in_cursum(journal); i++) {
> > @@ -2934,12 +2954,15 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
> >  		if (f2fs_check_nid_range(sbi, nid))
> >  			continue;
> >  
> > +		init_dirty = false;
> > +
> >  		raw_ne = nat_in_journal(journal, i);
> >  
> >  		ne = __lookup_nat_cache(nm_i, nid, true);
> >  		if (!ne) {
> > +			init_dirty = true;
> >  			ne = __alloc_nat_entry(sbi, nid, true);
> > -			__init_nat_entry(nm_i, ne, &raw_ne, true);
> > +			__init_nat_entry(nm_i, ne, &raw_ne, true, true);
> >  		}
> >  
> >  		/*
> > @@ -2954,7 +2977,7 @@ 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(nm_i, ne, init_dirty);
> >  	}
> >  	update_nats_in_cursum(journal, -i);
> >  	up_write(&curseg->journal_rwsem);

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: directly add newly allocated pre-dirty nat entry to dirty set list

[Chao Yu]

On 7/25/2025 10:17 AM, wangzijie wrote:
>> On 7/22/25 22:36, wangzijie wrote:
>>> When we need to alloc nat entry and set it dirty, we can directly add it to
>>> dirty set list(or initialize its list_head for new_ne) instead of adding it
>>> to clean list and make a move. Introduce init_dirty flag to do it.
>>>
>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>> ---
>>>   fs/f2fs/node.c | 37 ++++++++++++++++++++++++++++++-------
>>>   1 file changed, 30 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>> index a23db6238..20bcf8559 100644
>>> --- a/fs/f2fs/node.c
>>> +++ b/fs/f2fs/node.c
>>> @@ -185,7 +185,7 @@ static void __free_nat_entry(struct nat_entry *e)
>>>   
>>>   /* must be locked by nat_tree_lock */
>>>   static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
>>> -	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail)
>>> +	struct nat_entry *ne, struct f2fs_nat_entry *raw_ne, bool no_fail, bool init_dirty)
>>>   {
>>>   	if (no_fail)
>>>   		f2fs_radix_tree_insert(&nm_i->nat_root, nat_get_nid(ne), ne);
>>> @@ -195,6 +195,11 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
>>>   	if (raw_ne)
>>>   		node_info_from_raw_nat(&ne->ni, raw_ne);
>>>   
>>> +	if (init_dirty) {
>>> +		nm_i->nat_cnt[TOTAL_NAT]++;
>>> +		return ne;
>>> +	}
>>> +
>>>   	spin_lock(&nm_i->nat_list_lock);
>>>   	list_add_tail(&ne->list, &nm_i->nat_entries);
>>>   	spin_unlock(&nm_i->nat_list_lock);
>>> @@ -256,7 +261,7 @@ static struct nat_entry_set *__grab_nat_entry_set(struct f2fs_nm_info *nm_i,
>>>   }
>>>   
>>>   static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>> -						struct nat_entry *ne)
>>> +					struct nat_entry *ne, bool init_dirty)
>>>   {
>>>   	struct nat_entry_set *head;
>>>   	bool new_ne = nat_get_blkaddr(ne) == NEW_ADDR;
>>> @@ -275,6 +280,18 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>>   
>>>   	set_nat_flag(ne, IS_PREALLOC, new_ne);
>>>   
>>> +	if (init_dirty) {
>>> +		nm_i->nat_cnt[DIRTY_NAT]++;
>>> +		set_nat_flag(ne, IS_DIRTY, true);
>>> +		spin_lock(&nm_i->nat_list_lock);
>>> +		if (new_ne)
>>> +			INIT_LIST_HEAD(&ne->list);
>>> +		else
>>> +			list_add_tail(&ne->list, &head->entry_list);
>>> +		spin_unlock(&nm_i->nat_list_lock);
>>> +		return;
>>> +    }
>>
>> Nit issue, above blanks should be replaced w/ tab.
> 
> Ah...my bad :-(
> 
>> Can we clean up like this?
>>
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index de99b42437c6..60fc2c7b8e10 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -280,30 +280,23 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>>
>>   	set_nat_flag(ne, IS_PREALLOC, new_ne);
>>
>> -	if (init_dirty) {
>> -		nm_i->nat_cnt[DIRTY_NAT]++;
>> -		set_nat_flag(ne, IS_DIRTY, true);
>> -		spin_lock(&nm_i->nat_list_lock);
>> -		if (new_ne)
>> -			INIT_LIST_HEAD(&ne->list);
>> -		else
>> -			list_add_tail(&ne->list, &head->entry_list);
>> -		spin_unlock(&nm_i->nat_list_lock);
>> -		return;
>> -    }
>> -
>>   	if (get_nat_flag(ne, IS_DIRTY))
>>   		goto refresh_list;
>>
>>   	nm_i->nat_cnt[DIRTY_NAT]++;
>> -	nm_i->nat_cnt[RECLAIMABLE_NAT]--;
>> +	if (!init_dirty)
>> +		nm_i->nat_cnt[RECLAIMABLE_NAT]--;
>>   	set_nat_flag(ne, IS_DIRTY, true);
>>   refresh_list:
>>   	spin_lock(&nm_i->nat_list_lock);
>> -	if (new_ne)
>> -		list_del_init(&ne->list);
>> -	else
>> +	if (new_ne) {
>> +		if (init_dirty)
>> +			INIT_LIST_HEAD(&ne->list);
>> +		else
>> +			list_del_init(&ne->list);
>> +	} else {
>>   		list_move_tail(&ne->list, &head->entry_list);
>> +	}
>>   	spin_unlock(&nm_i->nat_list_lock);
>>   }
>>
>> Thanks,
> 
> We need to init list_head before using list_move_tail.
> I think we can do more clean up like this, keep refresh_list part code.

Ah, that's better.

Thanks,

> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 20bcf8559..ebb624fa1 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -196,6 +196,7 @@ static struct nat_entry *__init_nat_entry(struct f2fs_nm_info *nm_i,
>   		node_info_from_raw_nat(&ne->ni, raw_ne);
>   
>   	if (init_dirty) {
> +		INIT_LIST_HEAD(&ne->list);
>   		nm_i->nat_cnt[TOTAL_NAT]++;
>   		return ne;
>   	}
> @@ -280,23 +281,12 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
>   
>   	set_nat_flag(ne, IS_PREALLOC, new_ne);
>   
> -	if (init_dirty) {
> -		nm_i->nat_cnt[DIRTY_NAT]++;
> -		set_nat_flag(ne, IS_DIRTY, true);
> -		spin_lock(&nm_i->nat_list_lock);
> -		if (new_ne)
> -			INIT_LIST_HEAD(&ne->list);
> -		else
> -			list_add_tail(&ne->list, &head->entry_list);
> -		spin_unlock(&nm_i->nat_list_lock);
> -		return;
> -    }
> -
>   	if (get_nat_flag(ne, IS_DIRTY))
>   		goto refresh_list;
>   
>   	nm_i->nat_cnt[DIRTY_NAT]++;
> -	nm_i->nat_cnt[RECLAIMABLE_NAT]--;
> +	if (!init_dirty)
> +		nm_i->nat_cnt[RECLAIMABLE_NAT]--;
>   	set_nat_flag(ne, IS_DIRTY, true);
>   refresh_list:
>   	spin_lock(&nm_i->nat_list_lock);
> 
>>> +
>>>   	if (get_nat_flag(ne, IS_DIRTY))
>>>   		goto refresh_list;
>>>   
>>> @@ -441,7 +458,7 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi, nid_t nid,
>>>   	f2fs_down_write(&nm_i->nat_tree_lock);
>>>   	e = __lookup_nat_cache(nm_i, nid, false);
>>>   	if (!e)
>>> -		e = __init_nat_entry(nm_i, new, ne, false);
>>> +		e = __init_nat_entry(nm_i, new, ne, false, false);
>>>   	else
>>>   		f2fs_bug_on(sbi, nat_get_ino(e) != le32_to_cpu(ne->ino) ||
>>>   				nat_get_blkaddr(e) !=
>>> @@ -458,11 +475,13 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>   	struct f2fs_nm_info *nm_i = NM_I(sbi);
>>>   	struct nat_entry *e;
>>>   	struct nat_entry *new = __alloc_nat_entry(sbi, ni->nid, true);
>>> +	bool init_dirty = false;
>>>   
>>>   	f2fs_down_write(&nm_i->nat_tree_lock);
>>>   	e = __lookup_nat_cache(nm_i, ni->nid, true);
>>>   	if (!e) {
>>> -		e = __init_nat_entry(nm_i, new, NULL, true);
>>> +		init_dirty = true;
>>> +		e = __init_nat_entry(nm_i, new, NULL, true, true);
>>>   		copy_node_info(&e->ni, ni);
>>>   		f2fs_bug_on(sbi, ni->blk_addr == NEW_ADDR);
>>>   	} else if (new_blkaddr == NEW_ADDR) {
>>> @@ -498,7 +517,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>   	nat_set_blkaddr(e, new_blkaddr);
>>>   	if (!__is_valid_data_blkaddr(new_blkaddr))
>>>   		set_nat_flag(e, IS_CHECKPOINTED, false);
>>> -	__set_nat_cache_dirty(nm_i, e);
>>> +	__set_nat_cache_dirty(nm_i, e, init_dirty);
>>>   
>>>   	/* update fsync_mark if its inode nat entry is still alive */
>>>   	if (ni->nid != ni->ino)
>>> @@ -2924,6 +2943,7 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>>>   	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>>   	struct f2fs_journal *journal = curseg->journal;
>>>   	int i;
>>> +	bool init_dirty;
>>>   
>>>   	down_write(&curseg->journal_rwsem);
>>>   	for (i = 0; i < nats_in_cursum(journal); i++) {
>>> @@ -2934,12 +2954,15 @@ static void remove_nats_in_journal(struct f2fs_sb_info *sbi)
>>>   		if (f2fs_check_nid_range(sbi, nid))
>>>   			continue;
>>>   
>>> +		init_dirty = false;
>>> +
>>>   		raw_ne = nat_in_journal(journal, i);
>>>   
>>>   		ne = __lookup_nat_cache(nm_i, nid, true);
>>>   		if (!ne) {
>>> +			init_dirty = true;
>>>   			ne = __alloc_nat_entry(sbi, nid, true);
>>> -			__init_nat_entry(nm_i, ne, &raw_ne, true);
>>> +			__init_nat_entry(nm_i, ne, &raw_ne, true, true);
>>>   		}
>>>   
>>>   		/*
>>> @@ -2954,7 +2977,7 @@ 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(nm_i, ne, init_dirty);
>>>   	}
>>>   	update_nats_in_cursum(journal, -i);
>>>   	up_write(&curseg->journal_rwsem);
>