Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
On 05/08, Jaegeuk Kim wrote: > Is this v2? nvm. it seems the same version. > > On 05/08, Daeho Jeong wrote: > > From: Daeho Jeong > > > > Now, if writing pages and releasing compress blocks occur > > simultaneously, and releasing cblocks is executed more than one time > > to a file, then total block count of filesystem and block count of the > > file could be incorrect and damaged. > > > > We have to execute releasing compress blocks only one time for a file > > without being interfered by writepages path. > > > > Signed-off-by: Daeho Jeong > > Reviewed-by: Chao Yu > > --- > > fs/f2fs/file.c | 34 -- > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > > index 4aab4b42d8ba..f7de2a1da528 100644 > > --- a/fs/f2fs/file.c > > +++ b/fs/f2fs/file.c > > @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file > > *filp, unsigned long arg) > > pgoff_t page_idx = 0, last_idx; > > unsigned int released_blocks = 0; > > int ret; > > + int writecount; > > > > if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) > > return -EOPNOTSUPP; > > @@ -3502,20 +3503,33 @@ static int f2fs_release_compress_blocks(struct file > > *filp, unsigned long arg) > > if (ret) > > return ret; > > > > - if (!F2FS_I(inode)->i_compr_blocks) > > - goto out; > > - > > f2fs_balance_fs(F2FS_I_SB(inode), true); > > > > inode_lock(inode); > > > > - if (!IS_IMMUTABLE(inode)) { > > - F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; > > - f2fs_set_inode_flags(inode); > > - inode->i_ctime = current_time(inode); > > - f2fs_mark_inode_dirty_sync(inode, true); > > + writecount = atomic_read(>i_writecount); > > + if ((filp->f_mode & FMODE_WRITE && writecount != 1) || writecount) { > > + ret = -EBUSY; > > + goto out; > > } > > > > + if (IS_IMMUTABLE(inode)) { > > + ret = -EINVAL; > > + goto out; > > + } > > + > > + ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); > > + if (ret) > > + goto out; > > + > > + if (!F2FS_I(inode)->i_compr_blocks) > > + goto out; > > + > > + F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; > > + f2fs_set_inode_flags(inode); > > + inode->i_ctime = current_time(inode); > > + f2fs_mark_inode_dirty_sync(inode, true); > > + > > down_write(_I(inode)->i_gc_rwsem[WRITE]); > > down_write(_I(inode)->i_mmap_sem); > > > > @@ -3554,9 +3568,9 @@ static int f2fs_release_compress_blocks(struct file > > *filp, unsigned long arg) > > > > up_write(_I(inode)->i_gc_rwsem[WRITE]); > > up_write(_I(inode)->i_mmap_sem); > > - > > - inode_unlock(inode); > > out: > > + inode_unlock(inode); > > + > > mnt_drop_write_file(filp); > > > > if (ret >= 0) { > > -- > > 2.26.2.645.ge9eca65c58-goog > > > ___ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
Is this v2? On 05/08, Daeho Jeong wrote: > From: Daeho Jeong > > Now, if writing pages and releasing compress blocks occur > simultaneously, and releasing cblocks is executed more than one time > to a file, then total block count of filesystem and block count of the > file could be incorrect and damaged. > > We have to execute releasing compress blocks only one time for a file > without being interfered by writepages path. > > Signed-off-by: Daeho Jeong > Reviewed-by: Chao Yu > --- > fs/f2fs/file.c | 34 -- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 4aab4b42d8ba..f7de2a1da528 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file > *filp, unsigned long arg) > pgoff_t page_idx = 0, last_idx; > unsigned int released_blocks = 0; > int ret; > + int writecount; > > if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) > return -EOPNOTSUPP; > @@ -3502,20 +3503,33 @@ static int f2fs_release_compress_blocks(struct file > *filp, unsigned long arg) > if (ret) > return ret; > > - if (!F2FS_I(inode)->i_compr_blocks) > - goto out; > - > f2fs_balance_fs(F2FS_I_SB(inode), true); > > inode_lock(inode); > > - if (!IS_IMMUTABLE(inode)) { > - F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; > - f2fs_set_inode_flags(inode); > - inode->i_ctime = current_time(inode); > - f2fs_mark_inode_dirty_sync(inode, true); > + writecount = atomic_read(>i_writecount); > + if ((filp->f_mode & FMODE_WRITE && writecount != 1) || writecount) { > + ret = -EBUSY; > + goto out; > } > > + if (IS_IMMUTABLE(inode)) { > + ret = -EINVAL; > + goto out; > + } > + > + ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); > + if (ret) > + goto out; > + > + if (!F2FS_I(inode)->i_compr_blocks) > + goto out; > + > + F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; > + f2fs_set_inode_flags(inode); > + inode->i_ctime = current_time(inode); > + f2fs_mark_inode_dirty_sync(inode, true); > + > down_write(_I(inode)->i_gc_rwsem[WRITE]); > down_write(_I(inode)->i_mmap_sem); > > @@ -3554,9 +3568,9 @@ static int f2fs_release_compress_blocks(struct file > *filp, unsigned long arg) > > up_write(_I(inode)->i_gc_rwsem[WRITE]); > up_write(_I(inode)->i_mmap_sem); > - > - inode_unlock(inode); > out: > + inode_unlock(inode); > + > mnt_drop_write_file(filp); > > if (ret >= 0) { > -- > 2.26.2.645.ge9eca65c58-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
Hi Daeho, Please let me integrate this patch into the original patch since it is still in the dev branch. Thanks, On 05/08, Daeho Jeong wrote: > From: Daeho Jeong > > Now, if writing pages and releasing compress blocks occur > simultaneously, and releasing cblocks is executed more than one time > to a file, then total block count of filesystem and block count of the > file could be incorrect and damaged. > > We have to execute releasing compress blocks only one time for a file > without being interfered by writepages path. > > Signed-off-by: Daeho Jeong > --- > fs/f2fs/file.c | 34 -- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 4aab4b42d8ba..f7de2a1da528 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file > *filp, unsigned long arg) > pgoff_t page_idx = 0, last_idx; > unsigned int released_blocks = 0; > int ret; > + int writecount; > > if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) > return -EOPNOTSUPP; > @@ -3502,20 +3503,33 @@ static int f2fs_release_compress_blocks(struct file > *filp, unsigned long arg) > if (ret) > return ret; > > - if (!F2FS_I(inode)->i_compr_blocks) > - goto out; > - > f2fs_balance_fs(F2FS_I_SB(inode), true); > > inode_lock(inode); > > - if (!IS_IMMUTABLE(inode)) { > - F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; > - f2fs_set_inode_flags(inode); > - inode->i_ctime = current_time(inode); > - f2fs_mark_inode_dirty_sync(inode, true); > + writecount = atomic_read(>i_writecount); > + if ((filp->f_mode & FMODE_WRITE && writecount != 1) || writecount) { > + ret = -EBUSY; > + goto out; > } > > + if (IS_IMMUTABLE(inode)) { > + ret = -EINVAL; > + goto out; > + } > + > + ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); > + if (ret) > + goto out; > + > + if (!F2FS_I(inode)->i_compr_blocks) > + goto out; > + > + F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; > + f2fs_set_inode_flags(inode); > + inode->i_ctime = current_time(inode); > + f2fs_mark_inode_dirty_sync(inode, true); > + > down_write(_I(inode)->i_gc_rwsem[WRITE]); > down_write(_I(inode)->i_mmap_sem); > > @@ -3554,9 +3568,9 @@ static int f2fs_release_compress_blocks(struct file > *filp, unsigned long arg) > > up_write(_I(inode)->i_gc_rwsem[WRITE]); > up_write(_I(inode)->i_mmap_sem); > - > - inode_unlock(inode); > out: > + inode_unlock(inode); > + > mnt_drop_write_file(filp); > > if (ret >= 0) { > -- > 2.26.2.645.ge9eca65c58-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
From: Daeho Jeong Now, if writing pages and releasing compress blocks occur simultaneously, and releasing cblocks is executed more than one time to a file, then total block count of filesystem and block count of the file could be incorrect and damaged. We have to execute releasing compress blocks only one time for a file without being interfered by writepages path. Signed-off-by: Daeho Jeong Reviewed-by: Chao Yu --- fs/f2fs/file.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 4aab4b42d8ba..f7de2a1da528 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) pgoff_t page_idx = 0, last_idx; unsigned int released_blocks = 0; int ret; + int writecount; if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) return -EOPNOTSUPP; @@ -3502,20 +3503,33 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) if (ret) return ret; - if (!F2FS_I(inode)->i_compr_blocks) - goto out; - f2fs_balance_fs(F2FS_I_SB(inode), true); inode_lock(inode); - if (!IS_IMMUTABLE(inode)) { - F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; - f2fs_set_inode_flags(inode); - inode->i_ctime = current_time(inode); - f2fs_mark_inode_dirty_sync(inode, true); + writecount = atomic_read(>i_writecount); + if ((filp->f_mode & FMODE_WRITE && writecount != 1) || writecount) { + ret = -EBUSY; + goto out; } + if (IS_IMMUTABLE(inode)) { + ret = -EINVAL; + goto out; + } + + ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); + if (ret) + goto out; + + if (!F2FS_I(inode)->i_compr_blocks) + goto out; + + F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; + f2fs_set_inode_flags(inode); + inode->i_ctime = current_time(inode); + f2fs_mark_inode_dirty_sync(inode, true); + down_write(_I(inode)->i_gc_rwsem[WRITE]); down_write(_I(inode)->i_mmap_sem); @@ -3554,9 +3568,9 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) up_write(_I(inode)->i_gc_rwsem[WRITE]); up_write(_I(inode)->i_mmap_sem); - - inode_unlock(inode); out: + inode_unlock(inode); + mnt_drop_write_file(filp); if (ret >= 0) { -- 2.26.2.645.ge9eca65c58-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
On 2020/5/8 17:29, Daeho Jeong wrote: > From: Daeho Jeong > > Now, if writing pages and releasing compress blocks occur > simultaneously, and releasing cblocks is executed more than one time > to a file, then total block count of filesystem and block count of the > file could be incorrect and damaged. > > We have to execute releasing compress blocks only one time for a file > without being interfered by writepages path. > > Signed-off-by: Daeho Jeong Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
From: Daeho Jeong Now, if writing pages and releasing compress blocks occur simultaneously, and releasing cblocks is executed more than one time to a file, then total block count of filesystem and block count of the file could be incorrect and damaged. We have to execute releasing compress blocks only one time for a file without being interfered by writepages path. Signed-off-by: Daeho Jeong --- fs/f2fs/file.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 4aab4b42d8ba..f7de2a1da528 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) pgoff_t page_idx = 0, last_idx; unsigned int released_blocks = 0; int ret; + int writecount; if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) return -EOPNOTSUPP; @@ -3502,20 +3503,33 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) if (ret) return ret; - if (!F2FS_I(inode)->i_compr_blocks) - goto out; - f2fs_balance_fs(F2FS_I_SB(inode), true); inode_lock(inode); - if (!IS_IMMUTABLE(inode)) { - F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; - f2fs_set_inode_flags(inode); - inode->i_ctime = current_time(inode); - f2fs_mark_inode_dirty_sync(inode, true); + writecount = atomic_read(>i_writecount); + if ((filp->f_mode & FMODE_WRITE && writecount != 1) || writecount) { + ret = -EBUSY; + goto out; } + if (IS_IMMUTABLE(inode)) { + ret = -EINVAL; + goto out; + } + + ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); + if (ret) + goto out; + + if (!F2FS_I(inode)->i_compr_blocks) + goto out; + + F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; + f2fs_set_inode_flags(inode); + inode->i_ctime = current_time(inode); + f2fs_mark_inode_dirty_sync(inode, true); + down_write(_I(inode)->i_gc_rwsem[WRITE]); down_write(_I(inode)->i_mmap_sem); @@ -3554,9 +3568,9 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) up_write(_I(inode)->i_gc_rwsem[WRITE]); up_write(_I(inode)->i_mmap_sem); - - inode_unlock(inode); out: + inode_unlock(inode); + mnt_drop_write_file(filp); if (ret >= 0) { -- 2.26.2.645.ge9eca65c58-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
Oops, I will re-check it. Thanks, 2020년 5월 8일 (금) 오후 4:09, Chao Yu 님이 작성: > > On 2020/5/8 14:58, Daeho Jeong wrote: > > I moved checking i_compr_blocks phrase after calling inode_lock() > > already, because we should check this after writing pages. > > > > So, if it fails to check i_compr_blocks, we need to call inode_unlock(). > > > > Am I missing something? > > After applying this patch, I get this: > > ret = mnt_want_write_file(filp); > if (ret) > return ret; > > if (!F2FS_I(inode)->i_compr_blocks) > goto out; > > f2fs_balance_fs(F2FS_I_SB(inode), true); > > inode_lock(inode); > > > > > 2020년 5월 8일 (금) 오후 3:50, Chao Yu 님이 작성: > >> > >> On 2020/5/8 12:25, Daeho Jeong wrote: > >>> From: Daeho Jeong > >>> > >>> Now, if writing pages and releasing compress blocks occur > >>> simultaneously, and releasing cblocks is executed more than one time > >>> to a file, then total block count of filesystem and block count of the > >>> file could be incorrect and damaged. > >>> > >>> We have to execute releasing compress blocks only one time for a file > >>> without being interfered by writepages path. > >>> > >>> Signed-off-by: Daeho Jeong > >>> --- > >>> fs/f2fs/file.c | 31 --- > >>> 1 file changed, 24 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > >>> index 4aab4b42d8ba..a92bc51b9b28 100644 > >>> --- a/fs/f2fs/file.c > >>> +++ b/fs/f2fs/file.c > >>> @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file > >>> *filp, unsigned long arg) > >>> pgoff_t page_idx = 0, last_idx; > >>> unsigned int released_blocks = 0; > >>> int ret; > >>> + int writecount; > >>> > >>> if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) > >>> return -EOPNOTSUPP; > >> > >> Before inode_lock(), there is one case we may jump to out label, in > >> this case, we may unlock inode incorrectly. > >> > >> if (!F2FS_I(inode)->i_compr_blocks) > >> goto out; > >> > >>> - > >>> - inode_unlock(inode); > >>> out: > >>> + inode_unlock(inode); > >>> + > >>> mnt_drop_write_file(filp); > >> > >> Thanks, > > . > > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
On 2020/5/8 14:58, Daeho Jeong wrote: > I moved checking i_compr_blocks phrase after calling inode_lock() > already, because we should check this after writing pages. > > So, if it fails to check i_compr_blocks, we need to call inode_unlock(). > > Am I missing something? After applying this patch, I get this: ret = mnt_want_write_file(filp); if (ret) return ret; if (!F2FS_I(inode)->i_compr_blocks) goto out; f2fs_balance_fs(F2FS_I_SB(inode), true); inode_lock(inode); > > 2020년 5월 8일 (금) 오후 3:50, Chao Yu 님이 작성: >> >> On 2020/5/8 12:25, Daeho Jeong wrote: >>> From: Daeho Jeong >>> >>> Now, if writing pages and releasing compress blocks occur >>> simultaneously, and releasing cblocks is executed more than one time >>> to a file, then total block count of filesystem and block count of the >>> file could be incorrect and damaged. >>> >>> We have to execute releasing compress blocks only one time for a file >>> without being interfered by writepages path. >>> >>> Signed-off-by: Daeho Jeong >>> --- >>> fs/f2fs/file.c | 31 --- >>> 1 file changed, 24 insertions(+), 7 deletions(-) >>> >>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>> index 4aab4b42d8ba..a92bc51b9b28 100644 >>> --- a/fs/f2fs/file.c >>> +++ b/fs/f2fs/file.c >>> @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file >>> *filp, unsigned long arg) >>> pgoff_t page_idx = 0, last_idx; >>> unsigned int released_blocks = 0; >>> int ret; >>> + int writecount; >>> >>> if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) >>> return -EOPNOTSUPP; >> >> Before inode_lock(), there is one case we may jump to out label, in >> this case, we may unlock inode incorrectly. >> >> if (!F2FS_I(inode)->i_compr_blocks) >> goto out; >> >>> - >>> - inode_unlock(inode); >>> out: >>> + inode_unlock(inode); >>> + >>> mnt_drop_write_file(filp); >> >> Thanks, > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
I moved checking i_compr_blocks phrase after calling inode_lock() already, because we should check this after writing pages. So, if it fails to check i_compr_blocks, we need to call inode_unlock(). Am I missing something? 2020년 5월 8일 (금) 오후 3:50, Chao Yu 님이 작성: > > On 2020/5/8 12:25, Daeho Jeong wrote: > > From: Daeho Jeong > > > > Now, if writing pages and releasing compress blocks occur > > simultaneously, and releasing cblocks is executed more than one time > > to a file, then total block count of filesystem and block count of the > > file could be incorrect and damaged. > > > > We have to execute releasing compress blocks only one time for a file > > without being interfered by writepages path. > > > > Signed-off-by: Daeho Jeong > > --- > > fs/f2fs/file.c | 31 --- > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > > index 4aab4b42d8ba..a92bc51b9b28 100644 > > --- a/fs/f2fs/file.c > > +++ b/fs/f2fs/file.c > > @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file > > *filp, unsigned long arg) > > pgoff_t page_idx = 0, last_idx; > > unsigned int released_blocks = 0; > > int ret; > > + int writecount; > > > > if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) > > return -EOPNOTSUPP; > > Before inode_lock(), there is one case we may jump to out label, in > this case, we may unlock inode incorrectly. > > if (!F2FS_I(inode)->i_compr_blocks) > goto out; > > > - > > - inode_unlock(inode); > > out: > > + inode_unlock(inode); > > + > > mnt_drop_write_file(filp); > > Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
On 2020/5/8 12:25, Daeho Jeong wrote: > From: Daeho Jeong > > Now, if writing pages and releasing compress blocks occur > simultaneously, and releasing cblocks is executed more than one time > to a file, then total block count of filesystem and block count of the > file could be incorrect and damaged. > > We have to execute releasing compress blocks only one time for a file > without being interfered by writepages path. > > Signed-off-by: Daeho Jeong > --- > fs/f2fs/file.c | 31 --- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 4aab4b42d8ba..a92bc51b9b28 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file > *filp, unsigned long arg) > pgoff_t page_idx = 0, last_idx; > unsigned int released_blocks = 0; > int ret; > + int writecount; > > if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) > return -EOPNOTSUPP; Before inode_lock(), there is one case we may jump to out label, in this case, we may unlock inode incorrectly. if (!F2FS_I(inode)->i_compr_blocks) goto out; > - > - inode_unlock(inode); > out: > + inode_unlock(inode); > + > mnt_drop_write_file(filp); Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: remove race condition in releasing cblocks
From: Daeho Jeong Now, if writing pages and releasing compress blocks occur simultaneously, and releasing cblocks is executed more than one time to a file, then total block count of filesystem and block count of the file could be incorrect and damaged. We have to execute releasing compress blocks only one time for a file without being interfered by writepages path. Signed-off-by: Daeho Jeong --- fs/f2fs/file.c | 31 --- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 4aab4b42d8ba..a92bc51b9b28 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3488,6 +3488,7 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) pgoff_t page_idx = 0, last_idx; unsigned int released_blocks = 0; int ret; + int writecount; if (!f2fs_sb_has_compression(F2FS_I_SB(inode))) return -EOPNOTSUPP; @@ -3509,13 +3510,29 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) inode_lock(inode); - if (!IS_IMMUTABLE(inode)) { - F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; - f2fs_set_inode_flags(inode); - inode->i_ctime = current_time(inode); - f2fs_mark_inode_dirty_sync(inode, true); + writecount = atomic_read(>i_writecount); + if ((filp->f_mode & FMODE_WRITE && writecount != 1) || writecount) { + ret = -EBUSY; + goto out; } + if (IS_IMMUTABLE(inode)) { + ret = -EINVAL; + goto out; + } + + ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); + if (ret) + goto out; + + if (!F2FS_I(inode)->i_compr_blocks) + goto out; + + F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; + f2fs_set_inode_flags(inode); + inode->i_ctime = current_time(inode); + f2fs_mark_inode_dirty_sync(inode, true); + down_write(_I(inode)->i_gc_rwsem[WRITE]); down_write(_I(inode)->i_mmap_sem); @@ -3554,9 +3571,9 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) up_write(_I(inode)->i_gc_rwsem[WRITE]); up_write(_I(inode)->i_mmap_sem); - - inode_unlock(inode); out: + inode_unlock(inode); + mnt_drop_write_file(filp); if (ret >= 0) { -- 2.26.2.526.g744177e7f7-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel