Hi,
2013-01-15 (화), 10:57 +0800, Huajun Li:
> On Mon, Jan 14, 2013 at 8:08 PM, majianpeng <majianpeng@xxxxxxxxx> wrote:
> > There is an race between umount f2fs and read f2fs/status.
> > It will case oops.
> > Fox example:
> > Thread A Thread B
> > umount f2fs cat f2fs/status
> > f2fs_destroy_stats() { stat_show() {
> > list_for_each_entry_safe(&f2fs_stat_list)
> > list_del(&si->stat_list);
> > mutex_lock(&si->stat_lock);
> > si->sbi = NULL;
> > mutex_unlock(&si->stat_lock);
> > kfree(sbi->stat_info);
> > mutex_lock(&si->stat_lock)
> >
Nice catch.
Actually, &si->stat_lock was introduced to cope with this issue.
So, we need to remove the existing &si->stat_lock, and add your global
f2fs_stat_mutex.
Then, we're able to remove "si->sbi = NULL"-related stuffs too.
> > Signed-off-by: Jianpeng Ma <majianpeng@xxxxxxxxx>
> > ---
> > fs/f2fs/debug.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > index 0e0380a..ec6d004 100644
> > --- a/fs/f2fs/debug.c
> > +++ b/fs/f2fs/debug.c
> > @@ -26,6 +26,7 @@
> >
> > static LIST_HEAD(f2fs_stat_list);
> > static struct dentry *debugfs_root;
> > +static DEFINE_MUTEX(f2fs_stat_mutex);
> >
> > static void update_general_status(struct f2fs_sb_info *sbi)
> > {
> > @@ -180,6 +181,7 @@ static int stat_show(struct seq_file *s, void *v)
> > int i = 0;
> > int j;
> >
> > + mutex_lock(&f2fs_stat_mutex);
> > list_for_each_entry_safe(si, next, &f2fs_stat_list, stat_list) {
> >
> > mutex_lock(&si->stat_lock);
> > @@ -288,6 +290,7 @@ static int stat_show(struct seq_file *s, void *v)
> > si->base_mem >> 10, si->cache_mem >> 10);
> > mutex_unlock(&si->stat_lock);
> > }
> > + mutex_unlock(&f2fs_stat_mutex);
> > return 0;
> > }
> >
> > @@ -314,7 +317,10 @@ static int init_stats(struct f2fs_sb_info *sbi)
> >
> > si = sbi->stat_info;
> > mutex_init(&si->stat_lock);
> > +
> > + mutex_lock(&f2fs_stat_mutex);
> > list_add_tail(&si->stat_list, &f2fs_stat_list);
> > + mutex_unlock(&f2fs_stat_mutex);
> >
> > si->all_area_segs = le32_to_cpu(raw_super->segment_count);
> > si->sit_area_segs = le32_to_cpu(raw_super->segment_count_sit);
> > @@ -347,7 +353,10 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
> > {
> > struct f2fs_stat_info *si = sbi->stat_info;
> >
> > + mutex_lock(&f2fs_stat_mutex);
> > list_del(&si->stat_list);
> > + mutex_unlock(&f2fs_stat_mutex);
> > +
>
> Hi Jianpeng,
> Is it possible to fix the issue by holding si->stat_lock while
> executing list_del(&si->stat_list) ?
I think it cannot fix the problem.
Still the above errorneous scenario is able to be occurred.
Thank you,
--
Jaegeuk Kim
Samsung
Attachment:
signature.asc
Description: This is a digitally signed message part
