Messages in this thread | | | Date | Sat, 22 Jul 2023 21:25:25 +0800 | From | Chao Yu <> | Subject | Re: [PATCH] f2fs: introduce two helper functions for the largest cached extent |
| |
On 2023/7/18 16:33, Chao Liu wrote: > From: Chao Liu <liuchao@coolpad.com> > > This patch is a cleanup: > 1. Merge __drop_largest_extent() since it has only one caller. > 2. Introduce __notify_largest_extent_updated() and > __drop_largest_extent() to help manage largest and largest_update > in extent_tree. > > Signed-off-by: Chao Liu <liuchao@coolpad.com> > --- > fs/f2fs/extent_cache.c | 60 +++++++++++++++++++----------------------- > fs/f2fs/f2fs.h | 4 +-- > 2 files changed, 29 insertions(+), 35 deletions(-) > > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c > index 0e2d49140c07f..45dfddd8c3ad0 100644 > --- a/fs/f2fs/extent_cache.c > +++ b/fs/f2fs/extent_cache.c > @@ -19,6 +19,12 @@ > #include "node.h" > #include <trace/events/f2fs.h> > > +static void __drop_largest_extent(struct extent_tree *et) > +{ > + et->largest.len = 0; > + et->largest_updated = true; > +} > + > bool sanity_check_extent_cache(struct inode *inode) > { > struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > @@ -35,8 +41,7 @@ bool sanity_check_extent_cache(struct inode *inode) > > /* Let's drop, if checkpoint got corrupted. */ > if (is_set_ckpt_flags(sbi, CP_ERROR_FLAG)) { > - ei->len = 0; > - et->largest_updated = true; > + __drop_largest_extent(et); > return true; > } > > @@ -310,6 +315,8 @@ static void __detach_extent_node(struct f2fs_sb_info *sbi, > > if (et->cached_en == en) > et->cached_en = NULL; > + > + /* keep the largest as we can still use it */ > kmem_cache_free(extent_node_slab, en); > } > > @@ -385,15 +392,6 @@ static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi, > return count - atomic_read(&et->node_cnt); > } > > -static void __drop_largest_extent(struct extent_tree *et, > - pgoff_t fofs, unsigned int len) > -{ > - if (fofs < et->largest.fofs + et->largest.len && > - fofs + len > et->largest.fofs) { > - et->largest.len = 0; > - et->largest_updated = true; > - } > -} > > void f2fs_init_read_extent_tree(struct inode *inode, struct page *ipage) > { > @@ -601,6 +599,15 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi, > return en; > } > > +static void __notify_largest_extent_updated(struct extent_tree *et, > + struct inode *inode) > +{ > + if (et->type == EX_READ && et->largest_updated) { > + f2fs_mark_inode_dirty_sync(inode, true); > + et->largest_updated = false; > + } > +} > + > static void __update_extent_tree_range(struct inode *inode, > struct extent_info *tei, enum extent_type type) > { > @@ -612,7 +619,6 @@ static void __update_extent_tree_range(struct inode *inode, > struct rb_node **insert_p = NULL, *insert_parent = NULL; > unsigned int fofs = tei->fofs, len = tei->len; > unsigned int end = fofs + len; > - bool updated = false; > bool leftmost = false; > > if (!et) > @@ -636,11 +642,10 @@ static void __update_extent_tree_range(struct inode *inode, > prev = et->largest; > dei.len = 0; > > - /* > - * drop largest extent before lookup, in case it's already > - * been shrunk from extent tree > - */ > - __drop_largest_extent(et, fofs, len); > + /* updates may cause largest extent cache to become invalid */ > + if (fofs < et->largest.fofs + et->largest.len && > + fofs + len > et->largest.fofs) > + __drop_largest_extent(et); > } > > /* 1. lookup first extent node in range [fofs, fofs + len - 1] */ > @@ -733,8 +738,7 @@ static void __update_extent_tree_range(struct inode *inode, > if (dei.len >= 1 && > prev.len < F2FS_MIN_EXTENT_LEN && > et->largest.len < F2FS_MIN_EXTENT_LEN) { > - et->largest.len = 0; > - et->largest_updated = true; > + __drop_largest_extent(et); > set_inode_flag(inode, FI_NO_EXTENT); > } > } > @@ -742,10 +746,6 @@ static void __update_extent_tree_range(struct inode *inode, > if (is_inode_flag_set(inode, FI_NO_EXTENT)) > __free_extent_tree(sbi, et); > > - if (et->largest_updated) { > - et->largest_updated = false; > - updated = true; > - } > goto out_read_extent_cache; > update_age_extent_cache: > if (!tei->last_blocks) > @@ -758,9 +758,7 @@ static void __update_extent_tree_range(struct inode *inode, > insert_p, insert_parent, leftmost); > out_read_extent_cache: > write_unlock(&et->lock); > - > - if (updated) > - f2fs_mark_inode_dirty_sync(inode, true); > + __notify_largest_extent_updated(et, inode);
et->largest_updated should be updated w/ &et->lock, this is why we checks local variable @updated here to decide whether setting inode dirty or not.
Thanks,
> } > > #ifdef CONFIG_F2FS_FS_COMPRESSION > @@ -1092,7 +1090,6 @@ static void __drop_extent_tree(struct inode *inode, enum extent_type type) > { > struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > struct extent_tree *et = F2FS_I(inode)->extent_tree[type]; > - bool updated = false; > > if (!__may_extent_tree(inode, type)) > return; > @@ -1101,14 +1098,11 @@ static void __drop_extent_tree(struct inode *inode, enum extent_type type) > __free_extent_tree(sbi, et); > if (type == EX_READ) { > set_inode_flag(inode, FI_NO_EXTENT); > - if (et->largest.len) { > - et->largest.len = 0; > - updated = true; > - } > + if (et->largest.len) > + __drop_largest_extent(et); > } > write_unlock(&et->lock); > - if (updated) > - f2fs_mark_inode_dirty_sync(inode, true); > + __notify_largest_extent_updated(et, inode); > } > > void f2fs_drop_extent_tree(struct inode *inode) > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index d372bedb0fe4e..da02e120e5ea6 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -665,7 +665,7 @@ struct extent_tree { > > struct extent_tree_info { > struct radix_tree_root extent_tree_root;/* cache extent cache entries */ > - struct mutex extent_tree_lock; /* locking extent radix tree */ > + struct mutex extent_tree_lock; /* locking extent radix tree */ > 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 */ > @@ -766,7 +766,7 @@ enum { > FI_ACL_MODE, /* indicate acl mode */ > FI_NO_ALLOC, /* should not allocate any blocks */ > FI_FREE_NID, /* free allocated nide */ > - FI_NO_EXTENT, /* not to use the extent cache */ > + FI_NO_EXTENT, /* not to use the read extent cache */ > FI_INLINE_XATTR, /* used for inline xattr */ > FI_INLINE_DATA, /* used for inline data*/ > FI_INLINE_DENTRY, /* used for inline dentry */
| |