It would be better to use atomic variable for total_extent_tree.
Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/debug.c | 5 +++--
fs/f2fs/extent_cache.c | 8 ++++----
fs/f2fs/f2fs.h | 2 +-
fs/f2fs/node.c | 3 ++-
fs/f2fs/shrinker.c | 3 ++-
5 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index bb307e6..ed5dfcc 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -38,7 +38,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
si->hit_rbtree = atomic64_read(&sbi->read_hit_rbtree);
si->hit_total = si->hit_largest + si->hit_cached + si->hit_rbtree;
si->total_ext = atomic64_read(&sbi->total_hit_ext);
- si->ext_tree = sbi->total_ext_tree;
+ si->ext_tree = atomic_read(&sbi->total_ext_tree);
si->ext_node = atomic_read(&sbi->total_ext_node);
si->ndirty_node = get_pages(sbi, F2FS_DIRTY_NODES);
si->ndirty_dent = get_pages(sbi, F2FS_DIRTY_DENTS);
@@ -193,7 +193,8 @@ get_cache:
si->cache_mem += si->inmem_pages * sizeof(struct inmem_pages);
for (i = 0; i <= UPDATE_INO; i++)
si->cache_mem += sbi->im[i].ino_num * sizeof(struct ino_entry);
- si->cache_mem += sbi->total_ext_tree * sizeof(struct extent_tree);
+ si->cache_mem += atomic_read(&sbi->total_ext_tree) *
+ sizeof(struct extent_tree);
si->cache_mem += atomic_read(&sbi->total_ext_node) *
sizeof(struct extent_node);
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index e86e9f1e..0e97d6af 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -70,7 +70,7 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
rwlock_init(&et->lock);
atomic_set(&et->refcount, 0);
et->count = 0;
- sbi->total_ext_tree++;
+ atomic_inc(&sbi->total_ext_tree);
}
atomic_inc(&et->refcount);
up_write(&sbi->extent_tree_lock);
@@ -570,7 +570,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
radix_tree_delete(root, et->ino);
kmem_cache_free(extent_tree_slab, et);
- sbi->total_ext_tree--;
+ atomic_dec(&sbi->total_ext_tree);
tree_cnt++;
if (node_cnt + tree_cnt >= nr_shrink)
@@ -663,7 +663,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
f2fs_bug_on(sbi, atomic_read(&et->refcount) || et->count);
radix_tree_delete(&sbi->extent_tree_root, inode->i_ino);
kmem_cache_free(extent_tree_slab, et);
- sbi->total_ext_tree--;
+ atomic_dec(&sbi->total_ext_tree);
up_write(&sbi->extent_tree_lock);
F2FS_I(inode)->extent_tree = NULL;
@@ -715,7 +715,7 @@ void init_extent_cache_info(struct f2fs_sb_info *sbi)
init_rwsem(&sbi->extent_tree_lock);
INIT_LIST_HEAD(&sbi->extent_list);
spin_lock_init(&sbi->extent_lock);
- sbi->total_ext_tree = 0;
+ atomic_set(&sbi->total_ext_tree, 0);
atomic_set(&sbi->total_ext_node, 0);
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 19beabe..a7f6191 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -762,7 +762,7 @@ struct f2fs_sb_info {
struct rw_semaphore extent_tree_lock; /* locking extent radix tree */
struct list_head extent_list; /* lru list for shrinker */
spinlock_t extent_lock; /* locking extent lru list */
- int total_ext_tree; /* extent tree count */
+ atomic_t total_ext_tree; /* extent tree count */
atomic_t total_ext_node; /* extent info count */
/* basic filesystem units */
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index d842b19..6cc8ac7 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -65,7 +65,8 @@ bool available_free_memory(struct f2fs_sb_info *sbi, int type)
sizeof(struct ino_entry)) >> PAGE_CACHE_SHIFT;
res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
} else if (type == EXTENT_CACHE) {
- mem_size = (sbi->total_ext_tree * sizeof(struct extent_tree) +
+ mem_size = (atomic_read(&sbi->total_ext_tree) *
+ sizeof(struct extent_tree) +
atomic_read(&sbi->total_ext_node) *
sizeof(struct extent_node)) >> PAGE_CACHE_SHIFT;
res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
index da0d8e0..a11e099 100644
--- a/fs/f2fs/shrinker.c
+++ b/fs/f2fs/shrinker.c
@@ -32,7 +32,8 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
{
- return sbi->total_ext_tree + atomic_read(&sbi->total_ext_node);
+ return atomic_read(&sbi->total_ext_tree) +
+ atomic_read(&sbi->total_ext_node);
}
unsigned long f2fs_shrink_count(struct shrinker *shrink,
--
2.5.4 (Apple Git-61)
If there is no candidates for shrinking slab entries, we don't need to traverse
any trees at all.
Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/extent_cache.c | 12 ++++++++++++
fs/f2fs/f2fs.h | 1 +
fs/f2fs/shrinker.c | 2 +-
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 0e97d6af..32693af 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -71,6 +71,8 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
atomic_set(&et->refcount, 0);
et->count = 0;
atomic_inc(&sbi->total_ext_tree);
+ } else {
+ atomic_dec(&sbi->total_zombie_tree);
}
atomic_inc(&et->refcount);
up_write(&sbi->extent_tree_lock);
@@ -547,10 +549,14 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
unsigned int found;
unsigned int node_cnt = 0, tree_cnt = 0;
int remained;
+ bool do_free = false;
if (!test_opt(sbi, EXTENT_CACHE))
return 0;
+ if (!atomic_read(&sbi->total_zombie_tree))
+ goto free_node;
+
if (!down_write_trylock(&sbi->extent_tree_lock))
goto out;
@@ -580,6 +586,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
}
up_write(&sbi->extent_tree_lock);
+free_node:
/* 2. remove LRU extent entries */
if (!down_write_trylock(&sbi->extent_tree_lock))
goto out;
@@ -591,9 +598,13 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
if (!remained--)
break;
list_del_init(&en->list);
+ do_free = true;
}
spin_unlock(&sbi->extent_lock);
+ if (do_free == false)
+ goto unlock_out;
+
/*
* reset ino for searching victims from beginning of global extent tree.
*/
@@ -651,6 +662,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
if (inode->i_nlink && !is_bad_inode(inode) && et->count) {
atomic_dec(&et->refcount);
+ atomic_dec(&sbi->total_zombie_tree);
return;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index a7f6191..90fb970 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -763,6 +763,7 @@ struct f2fs_sb_info {
struct list_head extent_list; /* lru list for shrinker */
spinlock_t extent_lock; /* locking extent lru list */
atomic_t total_ext_tree; /* extent tree count */
+ atomic_t total_zombie_tree; /* extent zombie tree count */
atomic_t total_ext_node; /* extent info count */
/* basic filesystem units */
diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
index a11e099..93606f2 100644
--- a/fs/f2fs/shrinker.c
+++ b/fs/f2fs/shrinker.c
@@ -32,7 +32,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
{
- return atomic_read(&sbi->total_ext_tree) +
+ return atomic_read(&sbi->total_zombie_tree) +
atomic_read(&sbi->total_ext_node);
}
--
2.5.4 (Apple Git-61)
On Mon, Dec 21, 2015 at 07:38:41PM -0800, Jaegeuk Kim wrote:
> If there is no candidates for shrinking slab entries, we don't need to traverse
> any trees at all.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/extent_cache.c | 12 ++++++++++++
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/shrinker.c | 2 +-
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 0e97d6af..32693af 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -71,6 +71,8 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> atomic_set(&et->refcount, 0);
> et->count = 0;
> atomic_inc(&sbi->total_ext_tree);
> + } else {
> + atomic_dec(&sbi->total_zombie_tree);
> }
> atomic_inc(&et->refcount);
> up_write(&sbi->extent_tree_lock);
> @@ -547,10 +549,14 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
> unsigned int found;
> unsigned int node_cnt = 0, tree_cnt = 0;
> int remained;
> + bool do_free = false;
>
> if (!test_opt(sbi, EXTENT_CACHE))
> return 0;
>
> + if (!atomic_read(&sbi->total_zombie_tree))
> + goto free_node;
> +
> if (!down_write_trylock(&sbi->extent_tree_lock))
> goto out;
>
> @@ -580,6 +586,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
> }
> up_write(&sbi->extent_tree_lock);
>
> +free_node:
> /* 2. remove LRU extent entries */
> if (!down_write_trylock(&sbi->extent_tree_lock))
> goto out;
> @@ -591,9 +598,13 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
> if (!remained--)
> break;
> list_del_init(&en->list);
> + do_free = true;
> }
> spin_unlock(&sbi->extent_lock);
>
> + if (do_free == false)
> + goto unlock_out;
> +
> /*
> * reset ino for searching victims from beginning of global extent tree.
> */
> @@ -651,6 +662,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
>
> if (inode->i_nlink && !is_bad_inode(inode) && et->count) {
> atomic_dec(&et->refcount);
> + atomic_dec(&sbi->total_zombie_tree);
This will be changed to atomic_inc().
Sorry for confusion.
Thanks,
> return;
> }
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index a7f6191..90fb970 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -763,6 +763,7 @@ struct f2fs_sb_info {
> struct list_head extent_list; /* lru list for shrinker */
> spinlock_t extent_lock; /* locking extent lru list */
> atomic_t total_ext_tree; /* extent tree count */
> + atomic_t total_zombie_tree; /* extent zombie tree count */
> atomic_t total_ext_node; /* extent info count */
>
> /* basic filesystem units */
> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> index a11e099..93606f2 100644
> --- a/fs/f2fs/shrinker.c
> +++ b/fs/f2fs/shrinker.c
> @@ -32,7 +32,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
>
> static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> {
> - return atomic_read(&sbi->total_ext_tree) +
> + return atomic_read(&sbi->total_zombie_tree) +
> atomic_read(&sbi->total_ext_node);
> }
>
> --
> 2.5.4 (Apple Git-61)
Hi Jaegeuk,
We should update &total_zombie_tree whenever removing unreferenced
extent tree during shrinking:
- f2fs_shrink_extent_tree
if (!atomic_read(&et->refcount)) {
...
atomic_dec(&sbi->total_ext_tree);
atomic_dec(&sbi->total_zombie_tree);
...
}
Other parts look good to me. :)
Reviewed-by: Chao Yu <[email protected]>
Thanks,
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Tuesday, December 22, 2015 11:39 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 2/2] f2fs: speed up shrinking extent tree entries
>
> If there is no candidates for shrinking slab entries, we don't need to traverse
> any trees at all.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/extent_cache.c | 12 ++++++++++++
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/shrinker.c | 2 +-
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 0e97d6af..32693af 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -71,6 +71,8 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> atomic_set(&et->refcount, 0);
> et->count = 0;
> atomic_inc(&sbi->total_ext_tree);
> + } else {
> + atomic_dec(&sbi->total_zombie_tree);
> }
> atomic_inc(&et->refcount);
> up_write(&sbi->extent_tree_lock);
> @@ -547,10 +549,14 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> nr_shrink)
> unsigned int found;
> unsigned int node_cnt = 0, tree_cnt = 0;
> int remained;
> + bool do_free = false;
>
> if (!test_opt(sbi, EXTENT_CACHE))
> return 0;
>
> + if (!atomic_read(&sbi->total_zombie_tree))
> + goto free_node;
> +
> if (!down_write_trylock(&sbi->extent_tree_lock))
> goto out;
>
> @@ -580,6 +586,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> nr_shrink)
> }
> up_write(&sbi->extent_tree_lock);
>
> +free_node:
> /* 2. remove LRU extent entries */
> if (!down_write_trylock(&sbi->extent_tree_lock))
> goto out;
> @@ -591,9 +598,13 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> nr_shrink)
> if (!remained--)
> break;
> list_del_init(&en->list);
> + do_free = true;
> }
> spin_unlock(&sbi->extent_lock);
>
> + if (do_free == false)
> + goto unlock_out;
> +
> /*
> * reset ino for searching victims from beginning of global extent tree.
> */
> @@ -651,6 +662,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
>
> if (inode->i_nlink && !is_bad_inode(inode) && et->count) {
> atomic_dec(&et->refcount);
> + atomic_dec(&sbi->total_zombie_tree);
> return;
> }
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index a7f6191..90fb970 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -763,6 +763,7 @@ struct f2fs_sb_info {
> struct list_head extent_list; /* lru list for shrinker */
> spinlock_t extent_lock; /* locking extent lru list */
> atomic_t total_ext_tree; /* extent tree count */
> + atomic_t total_zombie_tree; /* extent zombie tree count */
> atomic_t total_ext_node; /* extent info count */
>
> /* basic filesystem units */
> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> index a11e099..93606f2 100644
> --- a/fs/f2fs/shrinker.c
> +++ b/fs/f2fs/shrinker.c
> @@ -32,7 +32,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
>
> static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> {
> - return atomic_read(&sbi->total_ext_tree) +
> + return atomic_read(&sbi->total_zombie_tree) +
> atomic_read(&sbi->total_ext_node);
> }
>
> --
> 2.5.4 (Apple Git-61)
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Hi Jaegeuk,
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Tuesday, December 22, 2015 11:39 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 1/2] f2fs: use atomic variable for total_extent_tree
>
> It would be better to use atomic variable for total_extent_tree.
total_extent_tree was protected by extent_tree_lock semaphore, so intention here
is to make related calculation in available_free_memory or update_general_status
more accurate, right?
Thanks,
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/debug.c | 5 +++--
> fs/f2fs/extent_cache.c | 8 ++++----
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/node.c | 3 ++-
> fs/f2fs/shrinker.c | 3 ++-
> 5 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index bb307e6..ed5dfcc 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -38,7 +38,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> si->hit_rbtree = atomic64_read(&sbi->read_hit_rbtree);
> si->hit_total = si->hit_largest + si->hit_cached + si->hit_rbtree;
> si->total_ext = atomic64_read(&sbi->total_hit_ext);
> - si->ext_tree = sbi->total_ext_tree;
> + si->ext_tree = atomic_read(&sbi->total_ext_tree);
> si->ext_node = atomic_read(&sbi->total_ext_node);
> si->ndirty_node = get_pages(sbi, F2FS_DIRTY_NODES);
> si->ndirty_dent = get_pages(sbi, F2FS_DIRTY_DENTS);
> @@ -193,7 +193,8 @@ get_cache:
> si->cache_mem += si->inmem_pages * sizeof(struct inmem_pages);
> for (i = 0; i <= UPDATE_INO; i++)
> si->cache_mem += sbi->im[i].ino_num * sizeof(struct ino_entry);
> - si->cache_mem += sbi->total_ext_tree * sizeof(struct extent_tree);
> + si->cache_mem += atomic_read(&sbi->total_ext_tree) *
> + sizeof(struct extent_tree);
> si->cache_mem += atomic_read(&sbi->total_ext_node) *
> sizeof(struct extent_node);
>
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index e86e9f1e..0e97d6af 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -70,7 +70,7 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> rwlock_init(&et->lock);
> atomic_set(&et->refcount, 0);
> et->count = 0;
> - sbi->total_ext_tree++;
> + atomic_inc(&sbi->total_ext_tree);
> }
> atomic_inc(&et->refcount);
> up_write(&sbi->extent_tree_lock);
> @@ -570,7 +570,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> nr_shrink)
>
> radix_tree_delete(root, et->ino);
> kmem_cache_free(extent_tree_slab, et);
> - sbi->total_ext_tree--;
> + atomic_dec(&sbi->total_ext_tree);
> tree_cnt++;
>
> if (node_cnt + tree_cnt >= nr_shrink)
> @@ -663,7 +663,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
> f2fs_bug_on(sbi, atomic_read(&et->refcount) || et->count);
> radix_tree_delete(&sbi->extent_tree_root, inode->i_ino);
> kmem_cache_free(extent_tree_slab, et);
> - sbi->total_ext_tree--;
> + atomic_dec(&sbi->total_ext_tree);
> up_write(&sbi->extent_tree_lock);
>
> F2FS_I(inode)->extent_tree = NULL;
> @@ -715,7 +715,7 @@ void init_extent_cache_info(struct f2fs_sb_info *sbi)
> init_rwsem(&sbi->extent_tree_lock);
> INIT_LIST_HEAD(&sbi->extent_list);
> spin_lock_init(&sbi->extent_lock);
> - sbi->total_ext_tree = 0;
> + atomic_set(&sbi->total_ext_tree, 0);
> atomic_set(&sbi->total_ext_node, 0);
> }
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 19beabe..a7f6191 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -762,7 +762,7 @@ struct f2fs_sb_info {
> struct rw_semaphore extent_tree_lock; /* locking extent radix tree */
> struct list_head extent_list; /* lru list for shrinker */
> spinlock_t extent_lock; /* locking extent lru list */
> - int total_ext_tree; /* extent tree count */
> + atomic_t total_ext_tree; /* extent tree count */
> atomic_t total_ext_node; /* extent info count */
>
> /* basic filesystem units */
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index d842b19..6cc8ac7 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -65,7 +65,8 @@ bool available_free_memory(struct f2fs_sb_info *sbi, int type)
> sizeof(struct ino_entry)) >> PAGE_CACHE_SHIFT;
> res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> } else if (type == EXTENT_CACHE) {
> - mem_size = (sbi->total_ext_tree * sizeof(struct extent_tree) +
> + mem_size = (atomic_read(&sbi->total_ext_tree) *
> + sizeof(struct extent_tree) +
> atomic_read(&sbi->total_ext_node) *
> sizeof(struct extent_node)) >> PAGE_CACHE_SHIFT;
> res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> index da0d8e0..a11e099 100644
> --- a/fs/f2fs/shrinker.c
> +++ b/fs/f2fs/shrinker.c
> @@ -32,7 +32,8 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
>
> static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> {
> - return sbi->total_ext_tree + atomic_read(&sbi->total_ext_node);
> + return atomic_read(&sbi->total_ext_tree) +
> + atomic_read(&sbi->total_ext_node);
> }
>
> unsigned long f2fs_shrink_count(struct shrinker *shrink,
> --
> 2.5.4 (Apple Git-61)
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On Tue, Dec 22, 2015 at 01:28:09PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:[email protected]]
> > Sent: Tuesday, December 22, 2015 11:39 AM
> > To: [email protected]; [email protected];
> > [email protected]
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 1/2] f2fs: use atomic variable for total_extent_tree
> >
> > It would be better to use atomic variable for total_extent_tree.
>
> total_extent_tree was protected by extent_tree_lock semaphore, so intention here
> is to make related calculation in available_free_memory or update_general_status
> more accurate, right?
Moreover, another major thing is to specify it is atomic along with other extent
counts.
Thanks,
>
> Thanks,
>
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/debug.c | 5 +++--
> > fs/f2fs/extent_cache.c | 8 ++++----
> > fs/f2fs/f2fs.h | 2 +-
> > fs/f2fs/node.c | 3 ++-
> > fs/f2fs/shrinker.c | 3 ++-
> > 5 files changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > index bb307e6..ed5dfcc 100644
> > --- a/fs/f2fs/debug.c
> > +++ b/fs/f2fs/debug.c
> > @@ -38,7 +38,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> > si->hit_rbtree = atomic64_read(&sbi->read_hit_rbtree);
> > si->hit_total = si->hit_largest + si->hit_cached + si->hit_rbtree;
> > si->total_ext = atomic64_read(&sbi->total_hit_ext);
> > - si->ext_tree = sbi->total_ext_tree;
> > + si->ext_tree = atomic_read(&sbi->total_ext_tree);
> > si->ext_node = atomic_read(&sbi->total_ext_node);
> > si->ndirty_node = get_pages(sbi, F2FS_DIRTY_NODES);
> > si->ndirty_dent = get_pages(sbi, F2FS_DIRTY_DENTS);
> > @@ -193,7 +193,8 @@ get_cache:
> > si->cache_mem += si->inmem_pages * sizeof(struct inmem_pages);
> > for (i = 0; i <= UPDATE_INO; i++)
> > si->cache_mem += sbi->im[i].ino_num * sizeof(struct ino_entry);
> > - si->cache_mem += sbi->total_ext_tree * sizeof(struct extent_tree);
> > + si->cache_mem += atomic_read(&sbi->total_ext_tree) *
> > + sizeof(struct extent_tree);
> > si->cache_mem += atomic_read(&sbi->total_ext_node) *
> > sizeof(struct extent_node);
> >
> > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> > index e86e9f1e..0e97d6af 100644
> > --- a/fs/f2fs/extent_cache.c
> > +++ b/fs/f2fs/extent_cache.c
> > @@ -70,7 +70,7 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> > rwlock_init(&et->lock);
> > atomic_set(&et->refcount, 0);
> > et->count = 0;
> > - sbi->total_ext_tree++;
> > + atomic_inc(&sbi->total_ext_tree);
> > }
> > atomic_inc(&et->refcount);
> > up_write(&sbi->extent_tree_lock);
> > @@ -570,7 +570,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> > nr_shrink)
> >
> > radix_tree_delete(root, et->ino);
> > kmem_cache_free(extent_tree_slab, et);
> > - sbi->total_ext_tree--;
> > + atomic_dec(&sbi->total_ext_tree);
> > tree_cnt++;
> >
> > if (node_cnt + tree_cnt >= nr_shrink)
> > @@ -663,7 +663,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
> > f2fs_bug_on(sbi, atomic_read(&et->refcount) || et->count);
> > radix_tree_delete(&sbi->extent_tree_root, inode->i_ino);
> > kmem_cache_free(extent_tree_slab, et);
> > - sbi->total_ext_tree--;
> > + atomic_dec(&sbi->total_ext_tree);
> > up_write(&sbi->extent_tree_lock);
> >
> > F2FS_I(inode)->extent_tree = NULL;
> > @@ -715,7 +715,7 @@ void init_extent_cache_info(struct f2fs_sb_info *sbi)
> > init_rwsem(&sbi->extent_tree_lock);
> > INIT_LIST_HEAD(&sbi->extent_list);
> > spin_lock_init(&sbi->extent_lock);
> > - sbi->total_ext_tree = 0;
> > + atomic_set(&sbi->total_ext_tree, 0);
> > atomic_set(&sbi->total_ext_node, 0);
> > }
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 19beabe..a7f6191 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -762,7 +762,7 @@ struct f2fs_sb_info {
> > struct rw_semaphore extent_tree_lock; /* locking extent radix tree */
> > struct list_head extent_list; /* lru list for shrinker */
> > spinlock_t extent_lock; /* locking extent lru list */
> > - int total_ext_tree; /* extent tree count */
> > + atomic_t total_ext_tree; /* extent tree count */
> > atomic_t total_ext_node; /* extent info count */
> >
> > /* basic filesystem units */
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index d842b19..6cc8ac7 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -65,7 +65,8 @@ bool available_free_memory(struct f2fs_sb_info *sbi, int type)
> > sizeof(struct ino_entry)) >> PAGE_CACHE_SHIFT;
> > res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> > } else if (type == EXTENT_CACHE) {
> > - mem_size = (sbi->total_ext_tree * sizeof(struct extent_tree) +
> > + mem_size = (atomic_read(&sbi->total_ext_tree) *
> > + sizeof(struct extent_tree) +
> > atomic_read(&sbi->total_ext_node) *
> > sizeof(struct extent_node)) >> PAGE_CACHE_SHIFT;
> > res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> > diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> > index da0d8e0..a11e099 100644
> > --- a/fs/f2fs/shrinker.c
> > +++ b/fs/f2fs/shrinker.c
> > @@ -32,7 +32,8 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
> >
> > static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> > {
> > - return sbi->total_ext_tree + atomic_read(&sbi->total_ext_node);
> > + return atomic_read(&sbi->total_ext_tree) +
> > + atomic_read(&sbi->total_ext_node);
> > }
> >
> > unsigned long f2fs_shrink_count(struct shrinker *shrink,
> > --
> > 2.5.4 (Apple Git-61)
> >
> >
> > ------------------------------------------------------------------------------
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On Tue, Dec 22, 2015 at 01:20:13PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> We should update &total_zombie_tree whenever removing unreferenced
> extent tree during shrinking:
> - f2fs_shrink_extent_tree
> if (!atomic_read(&et->refcount)) {
> ...
> atomic_dec(&sbi->total_ext_tree);
> atomic_dec(&sbi->total_zombie_tree);
> ...
Sure. :)
Thanks,
> }
>
> Other parts look good to me. :)
>
> Reviewed-by: Chao Yu <[email protected]>
>
> Thanks,
>
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:[email protected]]
> > Sent: Tuesday, December 22, 2015 11:39 AM
> > To: [email protected]; [email protected];
> > [email protected]
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 2/2] f2fs: speed up shrinking extent tree entries
> >
> > If there is no candidates for shrinking slab entries, we don't need to traverse
> > any trees at all.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/extent_cache.c | 12 ++++++++++++
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/shrinker.c | 2 +-
> > 3 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> > index 0e97d6af..32693af 100644
> > --- a/fs/f2fs/extent_cache.c
> > +++ b/fs/f2fs/extent_cache.c
> > @@ -71,6 +71,8 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> > atomic_set(&et->refcount, 0);
> > et->count = 0;
> > atomic_inc(&sbi->total_ext_tree);
> > + } else {
> > + atomic_dec(&sbi->total_zombie_tree);
> > }
> > atomic_inc(&et->refcount);
> > up_write(&sbi->extent_tree_lock);
> > @@ -547,10 +549,14 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> > nr_shrink)
> > unsigned int found;
> > unsigned int node_cnt = 0, tree_cnt = 0;
> > int remained;
> > + bool do_free = false;
> >
> > if (!test_opt(sbi, EXTENT_CACHE))
> > return 0;
> >
> > + if (!atomic_read(&sbi->total_zombie_tree))
> > + goto free_node;
> > +
> > if (!down_write_trylock(&sbi->extent_tree_lock))
> > goto out;
> >
> > @@ -580,6 +586,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> > nr_shrink)
> > }
> > up_write(&sbi->extent_tree_lock);
> >
> > +free_node:
> > /* 2. remove LRU extent entries */
> > if (!down_write_trylock(&sbi->extent_tree_lock))
> > goto out;
> > @@ -591,9 +598,13 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> > nr_shrink)
> > if (!remained--)
> > break;
> > list_del_init(&en->list);
> > + do_free = true;
> > }
> > spin_unlock(&sbi->extent_lock);
> >
> > + if (do_free == false)
> > + goto unlock_out;
> > +
> > /*
> > * reset ino for searching victims from beginning of global extent tree.
> > */
> > @@ -651,6 +662,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
> >
> > if (inode->i_nlink && !is_bad_inode(inode) && et->count) {
> > atomic_dec(&et->refcount);
> > + atomic_dec(&sbi->total_zombie_tree);
> > return;
> > }
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index a7f6191..90fb970 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -763,6 +763,7 @@ struct f2fs_sb_info {
> > struct list_head extent_list; /* lru list for shrinker */
> > spinlock_t extent_lock; /* locking extent lru list */
> > atomic_t total_ext_tree; /* extent tree count */
> > + atomic_t total_zombie_tree; /* extent zombie tree count */
> > atomic_t total_ext_node; /* extent info count */
> >
> > /* basic filesystem units */
> > diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> > index a11e099..93606f2 100644
> > --- a/fs/f2fs/shrinker.c
> > +++ b/fs/f2fs/shrinker.c
> > @@ -32,7 +32,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
> >
> > static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> > {
> > - return atomic_read(&sbi->total_ext_tree) +
> > + return atomic_read(&sbi->total_zombie_tree) +
> > atomic_read(&sbi->total_ext_node);
> > }
> >
> > --
> > 2.5.4 (Apple Git-61)
> >
> >
> > ------------------------------------------------------------------------------
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Tuesday, December 22, 2015 3:35 PM
> To: Chao Yu
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: use atomic variable for total_extent_tree
>
> On Tue, Dec 22, 2015 at 01:28:09PM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > > -----Original Message-----
> > > From: Jaegeuk Kim [mailto:[email protected]]
> > > Sent: Tuesday, December 22, 2015 11:39 AM
> > > To: [email protected]; [email protected];
> > > [email protected]
> > > Cc: Jaegeuk Kim
> > > Subject: [f2fs-dev] [PATCH 1/2] f2fs: use atomic variable for total_extent_tree
> > >
> > > It would be better to use atomic variable for total_extent_tree.
> >
> > total_extent_tree was protected by extent_tree_lock semaphore, so intention here
> > is to make related calculation in available_free_memory or update_general_status
> > more accurate, right?
>
> Moreover, another major thing is to specify it is atomic along with other extent
> counts.
Right, :) Please add:
Reviewed-by: Chao Yu <[email protected]>
On 2015/12/22 13:20, Chao Yu wrote:
> Hi Jaegeuk,
>
> We should update &total_zombie_tree whenever removing unreferenced
> extent tree during shrinking:
> - f2fs_shrink_extent_tree
> if (!atomic_read(&et->refcount)) {
> ...
> atomic_dec(&sbi->total_ext_tree);
> atomic_dec(&sbi->total_zombie_tree);
> ...
> }
>
> Other parts look good to me. :)
>
> Reviewed-by: Chao Yu <[email protected]>
>
> Thanks,
>
>> -----Original Message-----
>> From: Jaegeuk Kim [mailto:[email protected]]
>> Sent: Tuesday, December 22, 2015 11:39 AM
>> To: [email protected]; [email protected];
>> [email protected]
>> Cc: Jaegeuk Kim
>> Subject: [f2fs-dev] [PATCH 2/2] f2fs: speed up shrinking extent tree entries
>>
>> If there is no candidates for shrinking slab entries, we don't need to traverse
>> any trees at all.
>>
>> Signed-off-by: Jaegeuk Kim <[email protected]>
>> ---
>> fs/f2fs/extent_cache.c | 12 ++++++++++++
>> fs/f2fs/f2fs.h | 1 +
>> fs/f2fs/shrinker.c | 2 +-
>> 3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index 0e97d6af..32693af 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -71,6 +71,8 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
>> atomic_set(&et->refcount, 0);
>> et->count = 0;
>> atomic_inc(&sbi->total_ext_tree);
>> + } else {
>> + atomic_dec(&sbi->total_zombie_tree);
>> }
>> atomic_inc(&et->refcount);
>> up_write(&sbi->extent_tree_lock);
>> @@ -547,10 +549,14 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
>> nr_shrink)
>> unsigned int found;
>> unsigned int node_cnt = 0, tree_cnt = 0;
>> int remained;
>> + bool do_free = false;
>>
>> if (!test_opt(sbi, EXTENT_CACHE))
>> return 0;
>>
>> + if (!atomic_read(&sbi->total_zombie_tree))
>> + goto free_node;
>> +
>> if (!down_write_trylock(&sbi->extent_tree_lock))
>> goto out;
>>
>> @@ -580,6 +586,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
>> nr_shrink)
>> }
>> up_write(&sbi->extent_tree_lock);
>>
>> +free_node:
>> /* 2. remove LRU extent entries */
>> if (!down_write_trylock(&sbi->extent_tree_lock))
>> goto out;
>> @@ -591,9 +598,13 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
>> nr_shrink)
>> if (!remained--)
>> break;
>> list_del_init(&en->list);
>> + do_free = true;
>> }
>> spin_unlock(&sbi->extent_lock);
>>
>> + if (do_free == false)
>> + goto unlock_out;
>> +
>> /*
>> * reset ino for searching victims from beginning of global extent tree.
>> */
>> @@ -651,6 +662,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
>>
>> if (inode->i_nlink && !is_bad_inode(inode) && et->count) {
>> atomic_dec(&et->refcount);
>> + atomic_dec(&sbi->total_zombie_tree);
>> return;
>> }
Hi,all
here, sbi->total_ext_tree-- also should change to
atomic_dec(&sbi->total_ext_tree);
Thanks,
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index a7f6191..90fb970 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -763,6 +763,7 @@ struct f2fs_sb_info {
>> struct list_head extent_list; /* lru list for shrinker */
>> spinlock_t extent_lock; /* locking extent lru list */
>> atomic_t total_ext_tree; /* extent tree count */
>> + atomic_t total_zombie_tree; /* extent zombie tree count */
>> atomic_t total_ext_node; /* extent info count */
>>
>> /* basic filesystem units */
>> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
>> index a11e099..93606f2 100644
>> --- a/fs/f2fs/shrinker.c
>> +++ b/fs/f2fs/shrinker.c
>> @@ -32,7 +32,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
>>
>> static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
>> {
>> - return atomic_read(&sbi->total_ext_tree) +
>> + return atomic_read(&sbi->total_zombie_tree) +
>> atomic_read(&sbi->total_ext_node);
>> }
>>
>> --
>> 2.5.4 (Apple Git-61)
>>
>>
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
> .
>
> -----Original Message-----
> From: He YunLei [mailto:[email protected]]
> Sent: Tuesday, December 22, 2015 8:35 PM
> To: Chao Yu
> Cc: 'Jaegeuk Kim'; [email protected]; [email protected];
> [email protected]
> Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: speed up shrinking extent tree entries
>
> On 2015/12/22 13:20, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > We should update &total_zombie_tree whenever removing unreferenced
> > extent tree during shrinking:
> > - f2fs_shrink_extent_tree
> > if (!atomic_read(&et->refcount)) {
> > ...
> > atomic_dec(&sbi->total_ext_tree);
> > atomic_dec(&sbi->total_zombie_tree);
> > ...
> > }
> >
> > Other parts look good to me. :)
> >
> > Reviewed-by: Chao Yu <[email protected]>
> >
> > Thanks,
> >
> >> -----Original Message-----
> >> From: Jaegeuk Kim [mailto:[email protected]]
> >> Sent: Tuesday, December 22, 2015 11:39 AM
> >> To: [email protected]; [email protected];
> >> [email protected]
> >> Cc: Jaegeuk Kim
> >> Subject: [f2fs-dev] [PATCH 2/2] f2fs: speed up shrinking extent tree entries
> >>
> >> If there is no candidates for shrinking slab entries, we don't need to traverse
> >> any trees at all.
> >>
> >> Signed-off-by: Jaegeuk Kim <[email protected]>
> >> ---
> >> fs/f2fs/extent_cache.c | 12 ++++++++++++
> >> fs/f2fs/f2fs.h | 1 +
> >> fs/f2fs/shrinker.c | 2 +-
> >> 3 files changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> >> index 0e97d6af..32693af 100644
> >> --- a/fs/f2fs/extent_cache.c
> >> +++ b/fs/f2fs/extent_cache.c
> >> @@ -71,6 +71,8 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> >> atomic_set(&et->refcount, 0);
> >> et->count = 0;
> >> atomic_inc(&sbi->total_ext_tree);
> >> + } else {
> >> + atomic_dec(&sbi->total_zombie_tree);
> >> }
> >> atomic_inc(&et->refcount);
> >> up_write(&sbi->extent_tree_lock);
> >> @@ -547,10 +549,14 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> >> nr_shrink)
> >> unsigned int found;
> >> unsigned int node_cnt = 0, tree_cnt = 0;
> >> int remained;
> >> + bool do_free = false;
> >>
> >> if (!test_opt(sbi, EXTENT_CACHE))
> >> return 0;
> >>
> >> + if (!atomic_read(&sbi->total_zombie_tree))
> >> + goto free_node;
> >> +
> >> if (!down_write_trylock(&sbi->extent_tree_lock))
> >> goto out;
> >>
> >> @@ -580,6 +586,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> >> nr_shrink)
> >> }
> >> up_write(&sbi->extent_tree_lock);
> >>
> >> +free_node:
> >> /* 2. remove LRU extent entries */
> >> if (!down_write_trylock(&sbi->extent_tree_lock))
> >> goto out;
> >> @@ -591,9 +598,13 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> >> nr_shrink)
> >> if (!remained--)
> >> break;
> >> list_del_init(&en->list);
> >> + do_free = true;
> >> }
> >> spin_unlock(&sbi->extent_lock);
> >>
> >> + if (do_free == false)
> >> + goto unlock_out;
> >> +
> >> /*
> >> * reset ino for searching victims from beginning of global extent tree.
> >> */
> >> @@ -651,6 +662,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
> >>
> >> if (inode->i_nlink && !is_bad_inode(inode) && et->count) {
> >> atomic_dec(&et->refcount);
> >> + atomic_dec(&sbi->total_zombie_tree);
> >> return;
> >> }
> Hi,all
> here, sbi->total_ext_tree-- also should change to
> atomic_dec(&sbi->total_ext_tree);
Yunlei,
Seems not right.
Thanks,
> Thanks,
>
> >>
> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >> index a7f6191..90fb970 100644
> >> --- a/fs/f2fs/f2fs.h
> >> +++ b/fs/f2fs/f2fs.h
> >> @@ -763,6 +763,7 @@ struct f2fs_sb_info {
> >> struct list_head extent_list; /* lru list for shrinker */
> >> spinlock_t extent_lock; /* locking extent lru list */
> >> atomic_t total_ext_tree; /* extent tree count */
> >> + atomic_t total_zombie_tree; /* extent zombie tree count */
> >> atomic_t total_ext_node; /* extent info count */
> >>
> >> /* basic filesystem units */
> >> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> >> index a11e099..93606f2 100644
> >> --- a/fs/f2fs/shrinker.c
> >> +++ b/fs/f2fs/shrinker.c
> >> @@ -32,7 +32,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
> >>
> >> static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> >> {
> >> - return atomic_read(&sbi->total_ext_tree) +
> >> + return atomic_read(&sbi->total_zombie_tree) +
> >> atomic_read(&sbi->total_ext_node);
> >> }
> >>
> >> --
> >> 2.5.4 (Apple Git-61)
> >>
> >>
> >> ------------------------------------------------------------------------------
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> [email protected]
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >
> >
> > ------------------------------------------------------------------------------
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >
> > .
> >
On 2015/12/22 11:38, Jaegeuk Kim wrote:
> It would be better to use atomic variable for total_extent_tree.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/debug.c | 5 +++--
> fs/f2fs/extent_cache.c | 8 ++++----
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/node.c | 3 ++-
> fs/f2fs/shrinker.c | 3 ++-
> 5 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index bb307e6..ed5dfcc 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -38,7 +38,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> si->hit_rbtree = atomic64_read(&sbi->read_hit_rbtree);
> si->hit_total = si->hit_largest + si->hit_cached + si->hit_rbtree;
> si->total_ext = atomic64_read(&sbi->total_hit_ext);
> - si->ext_tree = sbi->total_ext_tree;
> + si->ext_tree = atomic_read(&sbi->total_ext_tree);
> si->ext_node = atomic_read(&sbi->total_ext_node);
> si->ndirty_node = get_pages(sbi, F2FS_DIRTY_NODES);
> si->ndirty_dent = get_pages(sbi, F2FS_DIRTY_DENTS);
> @@ -193,7 +193,8 @@ get_cache:
> si->cache_mem += si->inmem_pages * sizeof(struct inmem_pages);
> for (i = 0; i <= UPDATE_INO; i++)
> si->cache_mem += sbi->im[i].ino_num * sizeof(struct ino_entry);
> - si->cache_mem += sbi->total_ext_tree * sizeof(struct extent_tree);
> + si->cache_mem += atomic_read(&sbi->total_ext_tree) *
> + sizeof(struct extent_tree);
> si->cache_mem += atomic_read(&sbi->total_ext_node) *
> sizeof(struct extent_node);
>
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index e86e9f1e..0e97d6af 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -70,7 +70,7 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> rwlock_init(&et->lock);
> atomic_set(&et->refcount, 0);
> et->count = 0;
> - sbi->total_ext_tree++;
> + atomic_inc(&sbi->total_ext_tree);
> }
> atomic_inc(&et->refcount);
> up_write(&sbi->extent_tree_lock);
> @@ -570,7 +570,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
>
> radix_tree_delete(root, et->ino);
> kmem_cache_free(extent_tree_slab, et);
> - sbi->total_ext_tree--;
> + atomic_dec(&sbi->total_ext_tree);
> tree_cnt++;
>
> if (node_cnt + tree_cnt >= nr_shrink)
> @@ -663,7 +663,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
> f2fs_bug_on(sbi, atomic_read(&et->refcount) || et->count);
> radix_tree_delete(&sbi->extent_tree_root, inode->i_ino);
> kmem_cache_free(extent_tree_slab, et);
> - sbi->total_ext_tree--;
> + atomic_dec(&sbi->total_ext_tree);
> up_write(&sbi->extent_tree_lock);
>
> F2FS_I(inode)->extent_tree = NULL;
> @@ -715,7 +715,7 @@ void init_extent_cache_info(struct f2fs_sb_info *sbi)
> init_rwsem(&sbi->extent_tree_lock);
> INIT_LIST_HEAD(&sbi->extent_list);
> spin_lock_init(&sbi->extent_lock);
> - sbi->total_ext_tree = 0;
> + atomic_set(&sbi->total_ext_tree, 0);
Hi,
here we'd better to init total_zombie_tree:
atomic_set(&sbi->total_zombie_tree, 0);
Thanks,
> atomic_set(&sbi->total_ext_node, 0);
> }
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 19beabe..a7f6191 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -762,7 +762,7 @@ struct f2fs_sb_info {
> struct rw_semaphore extent_tree_lock; /* locking extent radix tree */
> struct list_head extent_list; /* lru list for shrinker */
> spinlock_t extent_lock; /* locking extent lru list */
> - int total_ext_tree; /* extent tree count */
> + atomic_t total_ext_tree; /* extent tree count */
> atomic_t total_ext_node; /* extent info count */
>
> /* basic filesystem units */
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index d842b19..6cc8ac7 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -65,7 +65,8 @@ bool available_free_memory(struct f2fs_sb_info *sbi, int type)
> sizeof(struct ino_entry)) >> PAGE_CACHE_SHIFT;
> res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> } else if (type == EXTENT_CACHE) {
> - mem_size = (sbi->total_ext_tree * sizeof(struct extent_tree) +
> + mem_size = (atomic_read(&sbi->total_ext_tree) *
> + sizeof(struct extent_tree) +
> atomic_read(&sbi->total_ext_node) *
> sizeof(struct extent_node)) >> PAGE_CACHE_SHIFT;
> res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> index da0d8e0..a11e099 100644
> --- a/fs/f2fs/shrinker.c
> +++ b/fs/f2fs/shrinker.c
> @@ -32,7 +32,8 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
>
> static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> {
> - return sbi->total_ext_tree + atomic_read(&sbi->total_ext_node);
> + return atomic_read(&sbi->total_ext_tree) +
> + atomic_read(&sbi->total_ext_node);
> }
>
> unsigned long f2fs_shrink_count(struct shrinker *shrink,
>
Hi Yunlei,
Thanks for your report, but the fix should be done in another patch. :)
Hi Jaegeuk,
We should add to init total_zombie_tree in ("f2fs: speed up shrinking
extent tree entries") as Yunlei reported.
Thanks,
> -----Original Message-----
> From: He YunLei [mailto:[email protected]]
> Sent: Tuesday, December 29, 2015 7:11 PM
> To: Jaegeuk Kim
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: use atomic variable for total_extent_tree
>
> On 2015/12/22 11:38, Jaegeuk Kim wrote:
> > It would be better to use atomic variable for total_extent_tree.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/debug.c | 5 +++--
> > fs/f2fs/extent_cache.c | 8 ++++----
> > fs/f2fs/f2fs.h | 2 +-
> > fs/f2fs/node.c | 3 ++-
> > fs/f2fs/shrinker.c | 3 ++-
> > 5 files changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > index bb307e6..ed5dfcc 100644
> > --- a/fs/f2fs/debug.c
> > +++ b/fs/f2fs/debug.c
> > @@ -38,7 +38,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> > si->hit_rbtree = atomic64_read(&sbi->read_hit_rbtree);
> > si->hit_total = si->hit_largest + si->hit_cached + si->hit_rbtree;
> > si->total_ext = atomic64_read(&sbi->total_hit_ext);
> > - si->ext_tree = sbi->total_ext_tree;
> > + si->ext_tree = atomic_read(&sbi->total_ext_tree);
> > si->ext_node = atomic_read(&sbi->total_ext_node);
> > si->ndirty_node = get_pages(sbi, F2FS_DIRTY_NODES);
> > si->ndirty_dent = get_pages(sbi, F2FS_DIRTY_DENTS);
> > @@ -193,7 +193,8 @@ get_cache:
> > si->cache_mem += si->inmem_pages * sizeof(struct inmem_pages);
> > for (i = 0; i <= UPDATE_INO; i++)
> > si->cache_mem += sbi->im[i].ino_num * sizeof(struct ino_entry);
> > - si->cache_mem += sbi->total_ext_tree * sizeof(struct extent_tree);
> > + si->cache_mem += atomic_read(&sbi->total_ext_tree) *
> > + sizeof(struct extent_tree);
> > si->cache_mem += atomic_read(&sbi->total_ext_node) *
> > sizeof(struct extent_node);
> >
> > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> > index e86e9f1e..0e97d6af 100644
> > --- a/fs/f2fs/extent_cache.c
> > +++ b/fs/f2fs/extent_cache.c
> > @@ -70,7 +70,7 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
> > rwlock_init(&et->lock);
> > atomic_set(&et->refcount, 0);
> > et->count = 0;
> > - sbi->total_ext_tree++;
> > + atomic_inc(&sbi->total_ext_tree);
> > }
> > atomic_inc(&et->refcount);
> > up_write(&sbi->extent_tree_lock);
> > @@ -570,7 +570,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int
> nr_shrink)
> >
> > radix_tree_delete(root, et->ino);
> > kmem_cache_free(extent_tree_slab, et);
> > - sbi->total_ext_tree--;
> > + atomic_dec(&sbi->total_ext_tree);
> > tree_cnt++;
> >
> > if (node_cnt + tree_cnt >= nr_shrink)
> > @@ -663,7 +663,7 @@ void f2fs_destroy_extent_tree(struct inode *inode)
> > f2fs_bug_on(sbi, atomic_read(&et->refcount) || et->count);
> > radix_tree_delete(&sbi->extent_tree_root, inode->i_ino);
> > kmem_cache_free(extent_tree_slab, et);
> > - sbi->total_ext_tree--;
> > + atomic_dec(&sbi->total_ext_tree);
> > up_write(&sbi->extent_tree_lock);
> >
> > F2FS_I(inode)->extent_tree = NULL;
> > @@ -715,7 +715,7 @@ void init_extent_cache_info(struct f2fs_sb_info *sbi)
> > init_rwsem(&sbi->extent_tree_lock);
> > INIT_LIST_HEAD(&sbi->extent_list);
> > spin_lock_init(&sbi->extent_lock);
> > - sbi->total_ext_tree = 0;
> > + atomic_set(&sbi->total_ext_tree, 0);
> Hi,
> here we'd better to init total_zombie_tree:
> atomic_set(&sbi->total_zombie_tree, 0);
> Thanks,
> > atomic_set(&sbi->total_ext_node, 0);
> > }
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 19beabe..a7f6191 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -762,7 +762,7 @@ struct f2fs_sb_info {
> > struct rw_semaphore extent_tree_lock; /* locking extent radix tree */
> > struct list_head extent_list; /* lru list for shrinker */
> > spinlock_t extent_lock; /* locking extent lru list */
> > - int total_ext_tree; /* extent tree count */
> > + atomic_t total_ext_tree; /* extent tree count */
> > atomic_t total_ext_node; /* extent info count */
> >
> > /* basic filesystem units */
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index d842b19..6cc8ac7 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -65,7 +65,8 @@ bool available_free_memory(struct f2fs_sb_info *sbi, int type)
> > sizeof(struct ino_entry)) >> PAGE_CACHE_SHIFT;
> > res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> > } else if (type == EXTENT_CACHE) {
> > - mem_size = (sbi->total_ext_tree * sizeof(struct extent_tree) +
> > + mem_size = (atomic_read(&sbi->total_ext_tree) *
> > + sizeof(struct extent_tree) +
> > atomic_read(&sbi->total_ext_node) *
> > sizeof(struct extent_node)) >> PAGE_CACHE_SHIFT;
> > res = mem_size < ((avail_ram * nm_i->ram_thresh / 100) >> 1);
> > diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> > index da0d8e0..a11e099 100644
> > --- a/fs/f2fs/shrinker.c
> > +++ b/fs/f2fs/shrinker.c
> > @@ -32,7 +32,8 @@ static unsigned long __count_free_nids(struct f2fs_sb_info *sbi)
> >
> > static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
> > {
> > - return sbi->total_ext_tree + atomic_read(&sbi->total_ext_node);
> > + return atomic_read(&sbi->total_ext_tree) +
> > + atomic_read(&sbi->total_ext_node);
> > }
> >
> > unsigned long f2fs_shrink_count(struct shrinker *shrink,
> >
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel