From: Chao Yu <yuchao0@huawei.com> To: Sahitya Tummala <stummala@codeaurora.org>, Jaegeuk Kim <jaegeuk@kernel.org>, <linux-f2fs-devel@lists.sourceforge.net> Cc: <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 2/2] f2fs: fix memory leak of quota files extent tree and it's nodes Date: Thu, 22 Nov 2018 20:16:15 +0800 [thread overview] Message-ID: <47e3b4e7-c1a6-f130-5765-d212ac9de367@huawei.com> (raw) In-Reply-To: <1542884360-19470-2-git-send-email-stummala@codeaurora.org> On 2018/11/22 18:59, Sahitya Tummala wrote: > When there is a failure in f2fs_fill_super() after the quota is > enabled, then f2fs_quota_off_umount() is called in the error handling. > Then sbi is freed up and f2fs_fill_super() retries again. > But f2fs_quota_off_umount() doesn't guarantee that quota file's extent > tree/nodes are removed/freed. It will just add to sbi->zombie_list, > if those files are referenced. In the next retry, quota is enabled > again with the new extent tree and nodes, causing memory leak for the > previously allocated memory. > > Fix this by cleaning up the sbi->zombie_list before freeing sbi and > before the next retry. I guess we can fix this issue with the same way as the comment I add in your previous patch, how do you think? Thanks, > > Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> > --- > fs/f2fs/extent_cache.c | 21 +++++++++++++++++++++ > fs/f2fs/f2fs.h | 1 + > fs/f2fs/super.c | 5 +++++ > 3 files changed, 27 insertions(+) > > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c > index 763ba83..c2bcd88 100644 > --- a/fs/f2fs/extent_cache.c > +++ b/fs/f2fs/extent_cache.c > @@ -629,6 +629,27 @@ static void f2fs_update_extent_tree_range(struct inode *inode, > f2fs_mark_inode_dirty_sync(inode, true); > } > > +void f2fs_cleanup_zombie_list(struct f2fs_sb_info *sbi) > +{ > + struct extent_tree *et, *next; > + > + mutex_lock(&sbi->extent_tree_lock); > + list_for_each_entry_safe(et, next, &sbi->zombie_list, list) { > + if (atomic_read(&et->node_cnt)) { > + write_lock(&et->lock); > + __free_extent_tree(sbi, et); > + write_unlock(&et->lock); > + } > + f2fs_bug_on(sbi, atomic_read(&et->node_cnt)); > + list_del_init(&et->list); > + radix_tree_delete(&sbi->extent_tree_root, et->ino); > + kmem_cache_free(extent_tree_slab, et); > + atomic_dec(&sbi->total_ext_tree); > + atomic_dec(&sbi->total_zombie_tree); > + } > + mutex_unlock(&sbi->extent_tree_lock); > +} > + > unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink) > { > struct extent_tree *et, *next; > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index db8a919..6807815 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3419,6 +3419,7 @@ void f2fs_update_extent_cache_range(struct dnode_of_data *dn, > void f2fs_init_extent_cache_info(struct f2fs_sb_info *sbi); > int __init f2fs_create_extent_cache(void); > void f2fs_destroy_extent_cache(void); > +void f2fs_cleanup_zombie_list(struct f2fs_sb_info *sbi); > > /* > * sysfs.c > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index f41ac43..521fe3f 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -3023,6 +3023,11 @@ void f2fs_cleanup_extent_cache(struct f2fs_sb_info *sbi) > > list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) > f2fs_destroy_extent_tree(inode, true); > + > + f2fs_cleanup_zombie_list(sbi); > + > + f2fs_bug_on(sbi, !list_empty(&sbi->zombie_list)); > + f2fs_bug_on(sbi, !list_empty(&sbi->extent_list)); > } > > static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >
WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <yuchao0@huawei.com> To: Sahitya Tummala <stummala@codeaurora.org>, Jaegeuk Kim <jaegeuk@kernel.org>, linux-f2fs-devel@lists.sourceforge.net Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] f2fs: fix memory leak of quota files extent tree and it's nodes Date: Thu, 22 Nov 2018 20:16:15 +0800 [thread overview] Message-ID: <47e3b4e7-c1a6-f130-5765-d212ac9de367@huawei.com> (raw) In-Reply-To: <1542884360-19470-2-git-send-email-stummala@codeaurora.org> On 2018/11/22 18:59, Sahitya Tummala wrote: > When there is a failure in f2fs_fill_super() after the quota is > enabled, then f2fs_quota_off_umount() is called in the error handling. > Then sbi is freed up and f2fs_fill_super() retries again. > But f2fs_quota_off_umount() doesn't guarantee that quota file's extent > tree/nodes are removed/freed. It will just add to sbi->zombie_list, > if those files are referenced. In the next retry, quota is enabled > again with the new extent tree and nodes, causing memory leak for the > previously allocated memory. > > Fix this by cleaning up the sbi->zombie_list before freeing sbi and > before the next retry. I guess we can fix this issue with the same way as the comment I add in your previous patch, how do you think? Thanks, > > Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> > --- > fs/f2fs/extent_cache.c | 21 +++++++++++++++++++++ > fs/f2fs/f2fs.h | 1 + > fs/f2fs/super.c | 5 +++++ > 3 files changed, 27 insertions(+) > > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c > index 763ba83..c2bcd88 100644 > --- a/fs/f2fs/extent_cache.c > +++ b/fs/f2fs/extent_cache.c > @@ -629,6 +629,27 @@ static void f2fs_update_extent_tree_range(struct inode *inode, > f2fs_mark_inode_dirty_sync(inode, true); > } > > +void f2fs_cleanup_zombie_list(struct f2fs_sb_info *sbi) > +{ > + struct extent_tree *et, *next; > + > + mutex_lock(&sbi->extent_tree_lock); > + list_for_each_entry_safe(et, next, &sbi->zombie_list, list) { > + if (atomic_read(&et->node_cnt)) { > + write_lock(&et->lock); > + __free_extent_tree(sbi, et); > + write_unlock(&et->lock); > + } > + f2fs_bug_on(sbi, atomic_read(&et->node_cnt)); > + list_del_init(&et->list); > + radix_tree_delete(&sbi->extent_tree_root, et->ino); > + kmem_cache_free(extent_tree_slab, et); > + atomic_dec(&sbi->total_ext_tree); > + atomic_dec(&sbi->total_zombie_tree); > + } > + mutex_unlock(&sbi->extent_tree_lock); > +} > + > unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink) > { > struct extent_tree *et, *next; > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index db8a919..6807815 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3419,6 +3419,7 @@ void f2fs_update_extent_cache_range(struct dnode_of_data *dn, > void f2fs_init_extent_cache_info(struct f2fs_sb_info *sbi); > int __init f2fs_create_extent_cache(void); > void f2fs_destroy_extent_cache(void); > +void f2fs_cleanup_zombie_list(struct f2fs_sb_info *sbi); > > /* > * sysfs.c > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index f41ac43..521fe3f 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -3023,6 +3023,11 @@ void f2fs_cleanup_extent_cache(struct f2fs_sb_info *sbi) > > list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) > f2fs_destroy_extent_tree(inode, true); > + > + f2fs_cleanup_zombie_list(sbi); > + > + f2fs_bug_on(sbi, !list_empty(&sbi->zombie_list)); > + f2fs_bug_on(sbi, !list_empty(&sbi->extent_list)); > } > > static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >
next prev parent reply other threads:[~2018-11-22 12:16 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-22 10:59 [PATCH 1/2] f2fs: fix sbi->extent_list corruption issue Sahitya Tummala 2018-11-22 10:59 ` [PATCH 2/2] f2fs: fix memory leak of quota files extent tree and it's nodes Sahitya Tummala 2018-11-22 12:16 ` Chao Yu [this message] 2018-11-22 12:16 ` Chao Yu 2018-11-22 11:51 ` [PATCH 1/2] f2fs: fix sbi->extent_list corruption issue Chao Yu 2018-11-22 11:51 ` Chao Yu 2018-11-22 12:11 ` Jaegeuk Kim 2018-11-23 3:42 ` Sahitya Tummala 2018-11-23 3:42 ` Sahitya Tummala 2018-11-23 9:52 ` Chao Yu 2018-11-23 9:52 ` Chao Yu 2018-11-23 10:19 ` Sahitya Tummala 2018-11-24 9:36 ` Chao Yu 2018-11-24 9:36 ` Chao Yu 2018-11-26 4:28 ` Sahitya Tummala 2018-11-26 4:28 ` Sahitya Tummala
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=47e3b4e7-c1a6-f130-5765-d212ac9de367@huawei.com \ --to=yuchao0@huawei.com \ --cc=jaegeuk@kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-kernel@vger.kernel.org \ --cc=stummala@codeaurora.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.