Messages in this thread | | | Subject | Re: [PATCH] f2fs: compress: remove unneed check condition | From | Chao Yu <> | Date | Thu, 22 Apr 2021 14:51:04 +0800 |
| |
On 2021/4/22 12:04, Jaegeuk Kim wrote: > On 04/21, Chao Yu wrote: >> In only call path of __cluster_may_compress(), __f2fs_write_data_pages() >> has checked SBI_POR_DOING condition, and also cluster_may_compress() >> has checked CP_ERROR_FLAG condition, so remove redundant check condition >> in __cluster_may_compress() for cleanup. > > I think cp_error can get any time without synchronization. Is it safe to say > it's redundant?
Yes,
But no matter how late we check cp_error, cp_error can happen after our check points, it won't cause regression if we remove cp_error check there, because for compress write, it uses OPU, it won't overwrite any existed data in device.
Seems it will be more appropriate to check cp_error in f2fs_write_compressed_pages() like we did in f2fs_write_single_data_page() rather than in __cluster_may_compress().
BTW, shouldn't we rename __cluster_may_compress() to cluster_beyond_filesize() for better readability?
Thanks,
> >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> fs/f2fs/compress.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c >> index 3c9d797dbdd6..532c311e3a89 100644 >> --- a/fs/f2fs/compress.c >> +++ b/fs/f2fs/compress.c >> @@ -906,11 +906,6 @@ static bool __cluster_may_compress(struct compress_ctx *cc) >> >> f2fs_bug_on(sbi, !page); >> >> - if (unlikely(f2fs_cp_error(sbi))) >> - return false; >> - if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) >> - return false; >> - >> /* beyond EOF */ >> if (page->index >= nr_pages) >> return false; >> -- >> 2.29.2 > . >
| |