linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
To: wangzijie <wangzijie1@honor.com>, jaegeuk@kernel.org
Cc: linux-kernel@vger.kernel.org, feng.han@honor.com,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v2 2/2] f2fs: directly add newly allocated pre-dirty nat entry to dirty set list
Date: Thu, 24 Jul 2025 16:26:55 +0800	[thread overview]
Message-ID: <524de02e-31a1-4b98-8601-edaa51d40d56@kernel.org> (raw)
In-Reply-To: <20250722143628.430776-2-wangzijie1@honor.com>

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);



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2025-07-24  8:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-22 14:36 [f2fs-dev] [PATCH v2 1/2] f2fs: avoid redundant clean nat entry move in lru list wangzijie
2025-07-22 14:36 ` [f2fs-dev] [PATCH v2 2/2] f2fs: directly add newly allocated pre-dirty nat entry to dirty set list wangzijie
2025-07-24  8:26   ` Chao Yu via Linux-f2fs-devel [this message]
2025-07-25  2:17     ` wangzijie
2025-07-25  2:36       ` Chao Yu via Linux-f2fs-devel
2025-07-24  6:00 ` [f2fs-dev] [PATCH v2 1/2] f2fs: avoid redundant clean nat entry move in lru list Chao Yu via Linux-f2fs-devel
2025-07-24  6:57   ` wangzijie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=524de02e-31a1-4b98-8601-edaa51d40d56@kernel.org \
    --to=linux-f2fs-devel@lists.sourceforge.net \
    --cc=chao@kernel.org \
    --cc=feng.han@honor.com \
    --cc=jaegeuk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wangzijie1@honor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).