>-----Original Message-----
>From: Jaegeuk Kim [mailto:[email protected]]
>Sent: Friday, November 10, 2017 2:05 AM
>To: heyunlei
>Cc: Yuchao (T); [email protected]; Wangbintian; Jianing
>(Euler)
>Subject: Re: [f2fs-dev][PATCH v5] f2fs: Separate nat entry mem alloc from
>nat_tree_locks
>
>On 11/07, Yunlei He wrote:
>> This path separate nat_entry mem alloc from nat_tree_lock,
>> which will benefit in some low mem case.
>>
>> Signed-off-by: Yunlei He <[email protected]>
>> ---
>> fs/f2fs/node.c | 56 +++++++++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 39 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index fef5c68..c6aea33 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -257,22 +257,15 @@ static struct nat_entry *grab_nat_entry(struct
>> f2fs_nm_info *nm_i, nid_t nid,
>>
>> if (no_fail) {
>> new = f2fs_kmem_cache_alloc(nat_entry_slab, GFP_NOFS);
>> - f2fs_radix_tree_insert(&nm_i->nat_root, nid, new);
>> } else {
>> new = kmem_cache_alloc(nat_entry_slab, GFP_NOFS);
>> if (!new)
>> return NULL;
>> - if (radix_tree_insert(&nm_i->nat_root, nid, new)) {
>> - kmem_cache_free(nat_entry_slab, new);
>> - return NULL;
>> - }
>> }
>>
>> memset(new, 0, sizeof(struct nat_entry));
>> nat_set_nid(new, nid);
>> nat_reset_flag(new);
>> - list_add_tail(&new->list, &nm_i->nat_entries);
>> - nm_i->nat_cnt++;
>> return new;
>> }
>>
>> @@ -282,17 +275,32 @@ static void cache_nat_entry(struct f2fs_sb_info *sbi,
>> nid_t nid,
>> struct f2fs_nm_info *nm_i = NM_I(sbi);
>> struct nat_entry *e;
>>
>> +#ifdef CONFIG_F2FS_CHECK_FS
>> + down_read(&nm_i->nat_tree_lock);
>> e = __lookup_nat_cache(nm_i, nid);
>> - if (!e) {
>> - e = grab_nat_entry(nm_i, nid, false);
>> - if (e)
>> - node_info_from_raw_nat(&e->ni, ne);
>> - } else {
>> + if (unlikely(e)) {
>> f2fs_bug_on(sbi, nat_get_ino(e) != le32_to_cpu(ne->ino) ||
>> - nat_get_blkaddr(e) !=
>> - le32_to_cpu(ne->block_addr) ||
>> + nat_get_blkaddr(e) != le32_to_cpu(ne->block_addr) ||
>> nat_get_version(e) != ne->version);
>> + up_read(&nm_i->nat_tree_lock);
>> + return;
>> + }
>> + up_read(&nm_i->nat_tree_lock);
>> +#endif
>
>Please avoid #ifdef as much as possible, which is totally undesirable.
>BTW, it'd be better to give the mallocated e to grab_nat_entry() to consolidate
>initialization stuffs?
Do you mean that delete the found nat entry and use the new alloced one？
Thanks.
>
>> + e = grab_nat_entry(nm_i, nid, false);
>> + down_write(&nm_i->nat_tree_lock);
>> + if (!e)
>> + goto out;
>> +
>> + if (!radix_tree_insert(&nm_i->nat_root, nid, e)) {
>> + list_add_tail(&e->list, &nm_i->nat_entries);
>> + nm_i->nat_cnt++;
>> + node_info_from_raw_nat(&e->ni, ne);
>> + } else {
>> + kmem_cache_free(nat_entry_slab, e);
>> }
>> +out:
>> + up_write(&nm_i->nat_tree_lock);
>> }
>>
>> static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
>> @@ -300,11 +308,21 @@ 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 = grab_nat_entry(nm_i, ni->nid, true);
>>
>> +retry:
>> down_write(&nm_i->nat_tree_lock);
>> e = __lookup_nat_cache(nm_i, ni->nid);
>> if (!e) {
>> - e = grab_nat_entry(nm_i, ni->nid, true);
>> + e = new;
>> + if (radix_tree_insert(&nm_i->nat_root, ni->nid, e)) {
>> + up_write(&nm_i->nat_tree_lock);
>> + cond_resched();
>> + goto retry;
>> + }
>> +
>> + list_add_tail(&e->list, &nm_i->nat_entries);
>> + nm_i->nat_cnt++;
>> copy_node_info(&e->ni, ni);
>> f2fs_bug_on(sbi, ni->blk_addr == NEW_ADDR);
>> } else if (new_blkaddr == NEW_ADDR) {
>> @@ -315,6 +333,9 @@ static void set_node_addr(struct f2fs_sb_info *sbi,
>> struct node_info *ni,
>> */
>> copy_node_info(&e->ni, ni);
>> f2fs_bug_on(sbi, ni->blk_addr != NULL_ADDR);
>> + kmem_cache_free(nat_entry_slab, new);
>> + } else {
>> + kmem_cache_free(nat_entry_slab, new);
>> }
>>
>> /* sanity check */
>> @@ -424,9 +445,7 @@ void get_node_info(struct f2fs_sb_info *sbi, nid_t nid,
>> struct node_info *ni)
>> f2fs_put_page(page, 1);
>> cache:
>> /* cache nat entry */
>> - down_write(&nm_i->nat_tree_lock);
>> cache_nat_entry(sbi, nid, &ne);
>> - up_write(&nm_i->nat_tree_lock);
>> }
>>
>> /*
>> @@ -2375,6 +2394,9 @@ static void remove_nats_in_journal(struct f2fs_sb_info
>> *sbi)
>> ne = __lookup_nat_cache(nm_i, nid);
>> if (!ne) {
>> ne = grab_nat_entry(nm_i, nid, true);
>> + f2fs_radix_tree_insert(&nm_i->nat_root, nid, ne);
>> + list_add_tail(&ne->list, &nm_i->nat_entries);
>> + nm_i->nat_cnt++;
>> node_info_from_raw_nat(&ne->ni, &raw_ne);
>> }
>>
>> --
>> 1.9.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
