Thread: [f2fs-dev] [PATCH v2 1/2] f2fs: avoid redundant clean nat entry move in lru list
Brought to you by:
kjgkr
Menu
▾
▴
linux-f2fs-devel
|
From: wangzijie <wan...@ho...> - 2025-07-22 14:36:41
|
__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 <wan...@ho...>
---
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
|
|
From: wangzijie <wan...@ho...> - 2025-07-28 05:02:51
|
__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 <wan...@ho...>
---
v3:
- followed by Chao's suggestion to update comments
v2:
- followed by Jaegeuk's suggestion to add a parameter in __lookup_nat_cache
---
fs/f2fs/node.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 76aba1961..940b52d38 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -204,14 +204,17 @@ 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 nat entry which will not be dirtied soon
+ * later, 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 +386,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 +403,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 +417,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 +442,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 +463,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 +505,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 +565,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 +2374,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 +2939,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
|
|
From: wangzijie <wan...@ho...> - 2025-07-28 05:02:52
|
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 <wan...@ho...>
---
v3:
- followed by Chao's suggestion to clean up code
---
fs/f2fs/node.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 940b52d38..27743b93e 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,12 @@ 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) {
+ INIT_LIST_HEAD(&ne->list);
+ 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);
@@ -259,7 +265,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;
@@ -282,7 +288,8 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
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);
@@ -444,7 +451,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) !=
@@ -461,11 +468,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) {
@@ -501,7 +510,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)
@@ -2927,6 +2936,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++) {
@@ -2937,12 +2947,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);
}
/*
@@ -2957,7 +2970,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
|
|
From: Chao Yu <ch...@ke...> - 2025-07-28 07:47:32
|
On 7/28/25 13:02, 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 <wan...@ho...> Reviewed-by: Chao Yu <ch...@ke...> Thanks, |
|
From: Chao Yu <ch...@ke...> - 2025-07-28 07:39:53
|
On 7/28/25 13:02, 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 <wan...@ho...> Reviewed-by: Chao Yu <ch...@ke...> Thanks, |
|
From: <pat...@ke...> - 2025-07-28 16:50:10
|
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <ja...@ke...>:
On Mon, 28 Jul 2025 13:02:35 +0800 you 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 <wan...@ho...>
> ---
> v3:
> - followed by Chao's suggestion to update comments
> v2:
> - followed by Jaegeuk's suggestion to add a parameter in __lookup_nat_cache
>
> [...]
Here is the summary with links:
- [f2fs-dev,v3,1/2] f2fs: avoid redundant clean nat entry move in lru list
https://git.kernel.org/jaegeuk/f2fs/c/0349b7f95c80
- [f2fs-dev,v3,2/2] f2fs: directly add newly allocated pre-dirty nat entry to dirty set list
https://git.kernel.org/jaegeuk/f2fs/c/40aa9e1223fd
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
|
|
From: wangzijie <wan...@ho...> - 2025-07-22 14:36:42
|
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 <wan...@ho...>
---
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
|
|
From: Chao Yu <ch...@ke...> - 2025-07-24 08:27:10
|
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 <wan...@ho...>
> ---
> 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);
|
|
From: wangzijie <wan...@ho...> - 2025-07-25 02:17:47
|
> 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 <wan...@ho...>
> > ---
> > 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);
|
|
From: Chao Yu <ch...@ke...> - 2025-07-25 02:36:56
|
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 <wan...@ho...>
>>> ---
>>> 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);
>
|
|
From: Chao Yu <ch...@ke...> - 2025-07-24 06:00:44
|
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 <wan...@ho...>
> ---
> 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);
|
|
From: wangzijie <wan...@ho...> - 2025-07-24 06:57:50
|
> 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 <wan...@ho...>
> > ---
> > 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);
|
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X