Messages in this thread | | | Subject | Re: [f2fs-dev] [PATCH 2/2] f2fs: support data compression | From | Chao Yu <> | Date | Thu, 12 Dec 2019 23:07:32 +0800 |
| |
Hi Jaegeuk,
On 2019-12-11 9:27, Jaegeuk Kim wrote: > Hi Chao, > > Let me know, if it's okay to integrate compression patch all together. > I don't have a critical bug to fix w/ them now.
Cool, let me send a new RFC with below fix applied.
Thanks,
> > Another fix: > --- > fs/f2fs/compress.c | 101 ++++++++++++++++++++++++++++----------------- > fs/f2fs/data.c | 15 ++++--- > fs/f2fs/f2fs.h | 1 - > 3 files changed, 72 insertions(+), 45 deletions(-) > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > index 7ebd2bc018bd..af23ed6deffd 100644 > --- a/fs/f2fs/compress.c > +++ b/fs/f2fs/compress.c > @@ -73,6 +73,17 @@ static void f2fs_put_compressed_page(struct page *page) > put_page(page); > } > > +static void f2fs_put_rpages(struct compress_ctx *cc) > +{ > + unsigned int i; > + > + for (i = 0; i < cc->cluster_size; i++) { > + if (!cc->rpages[i]) > + continue; > + put_page(cc->rpages[i]); > + } > +} > + > struct page *f2fs_compress_control_page(struct page *page) > { > return ((struct compress_io_ctx *)page_private(page))->rpages[0]; > @@ -93,7 +104,10 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc) > void f2fs_destroy_compress_ctx(struct compress_ctx *cc) > { > kfree(cc->rpages); > - f2fs_reset_compress_ctx(cc); > + cc->rpages = NULL; > + cc->nr_rpages = 0; > + cc->nr_cpages = 0; > + cc->cluster_idx = NULL_CLUSTER; > } > > void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page) > @@ -536,14 +550,6 @@ static bool cluster_may_compress(struct compress_ctx *cc) > return __cluster_may_compress(cc); > } > > -void f2fs_reset_compress_ctx(struct compress_ctx *cc) > -{ > - cc->rpages = NULL; > - cc->nr_rpages = 0; > - cc->nr_cpages = 0; > - cc->cluster_idx = NULL_CLUSTER; > -} > - > static void set_cluster_writeback(struct compress_ctx *cc) > { > int i; > @@ -602,13 +608,13 @@ static int prepare_compress_overwrite(struct compress_ctx *cc, > ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size, > &last_block_in_bio, false); > if (ret) > - return ret; > + goto release_pages; > if (bio) > f2fs_submit_bio(sbi, bio, DATA); > > ret = f2fs_init_compress_ctx(cc); > if (ret) > - return ret; > + goto release_pages; > } > > for (i = 0; i < cc->cluster_size; i++) { > @@ -638,9 +644,11 @@ static int prepare_compress_overwrite(struct compress_ctx *cc, > > for (i = cc->cluster_size - 1; i > 0; i--) { > ret = f2fs_get_block(&dn, start_idx + i); > - if (ret) > + if (ret) { > /* TODO: release preallocate blocks */ > - goto release_pages; > + i = cc->cluster_size; > + goto unlock_pages; > + } > > if (dn.data_blkaddr != NEW_ADDR) > break; > @@ -769,7 +777,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc, > cic->magic = F2FS_COMPRESSED_PAGE_MAGIC; > cic->inode = inode; > refcount_set(&cic->ref, 1); > - cic->rpages = cc->rpages; > + cic->rpages = f2fs_kzalloc(sbi, sizeof(struct page *) << > + cc->log_cluster_size, GFP_NOFS); > + if (!cic->rpages) > + goto out_put_cic; > + > cic->nr_rpages = cc->cluster_size; > > for (i = 0; i < cc->nr_cpages; i++) { > @@ -793,7 +805,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc, > > blkaddr = datablock_addr(dn.inode, dn.node_page, > dn.ofs_in_node); > - fio.page = cc->rpages[i]; > + fio.page = cic->rpages[i] = cc->rpages[i]; > fio.old_blkaddr = blkaddr; > > /* cluster header */ > @@ -819,7 +831,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc, > > f2fs_bug_on(fio.sbi, blkaddr == NULL_ADDR); > > - > if (fio.encrypted) > fio.encrypted_page = cc->cpages[i - 1]; > else if (fio.compressed) > @@ -859,17 +870,22 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc, > fi->last_disk_size = psize; > up_write(&fi->i_sem); > } > - f2fs_reset_compress_ctx(cc); > + f2fs_put_rpages(cc); > + f2fs_destroy_compress_ctx(cc); > return 0; > > out_destroy_crypt: > - for (i -= 1; i >= 0; i--) > + kfree(cic->rpages); > + > + for (--i; i >= 0; i--) > fscrypt_finalize_bounce_page(&cc->cpages[i]); > for (i = 0; i < cc->nr_cpages; i++) { > if (!cc->cpages[i]) > continue; > f2fs_put_page(cc->cpages[i], 1); > } > +out_put_cic: > + kfree(cic); > out_put_dnode: > f2fs_put_dnode(&dn); > out_unlock_op: > @@ -963,37 +979,39 @@ int f2fs_write_multi_pages(struct compress_ctx *cc, > struct f2fs_inode_info *fi = F2FS_I(cc->inode); > const struct f2fs_compress_ops *cops = > f2fs_cops[fi->i_compress_algorithm]; > - int err = -EAGAIN; > + bool compressed = false; > + int err; > > *submitted = 0; > if (cluster_may_compress(cc)) { > err = f2fs_compress_pages(cc); > - if (err) { > - err = -EAGAIN; > + if (err == -EAGAIN) > goto write; > - } > + else if (err) > + goto put_out; > + > err = f2fs_write_compressed_pages(cc, submitted, > wbc, io_type); > cops->destroy_compress_ctx(cc); > + if (!err) > + return 0; > + f2fs_bug_on(F2FS_I_SB(cc->inode), err != -EAGAIN); > } > write: > - if (err == -EAGAIN) { > - bool compressed = false; > - > - f2fs_bug_on(F2FS_I_SB(cc->inode), *submitted); > + f2fs_bug_on(F2FS_I_SB(cc->inode), *submitted); > > - if (is_compressed_cluster(cc)) > - compressed = true; > + if (is_compressed_cluster(cc)) > + compressed = true; > > - err = f2fs_write_raw_pages(cc, submitted, wbc, > - io_type, compressed); > - if (compressed) { > - stat_sub_compr_blocks(cc->inode, *submitted); > - F2FS_I(cc->inode)->i_compressed_blocks -= *submitted; > - f2fs_mark_inode_dirty_sync(cc->inode, true); > - } > - f2fs_destroy_compress_ctx(cc); > + err = f2fs_write_raw_pages(cc, submitted, wbc, io_type, compressed); > + if (compressed) { > + stat_sub_compr_blocks(cc->inode, *submitted); > + F2FS_I(cc->inode)->i_compressed_blocks -= *submitted; > + f2fs_mark_inode_dirty_sync(cc->inode, true); > } > +put_out: > + f2fs_put_rpages(cc); > + f2fs_destroy_compress_ctx(cc); > return err; > } > > @@ -1055,7 +1073,13 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) > dic->tpages[i] = cc->rpages[i]; > } > > - dic->rpages = cc->rpages; > + dic->rpages = f2fs_kzalloc(sbi, sizeof(struct page *) << > + cc->log_cluster_size, GFP_NOFS); > + if (!dic->rpages) > + goto out_free; > + > + for (i = 0; i < dic->cluster_size; i++) > + dic->rpages[i] = cc->rpages[i]; > dic->nr_rpages = cc->cluster_size; > return dic; > > @@ -1072,8 +1096,7 @@ void f2fs_free_dic(struct decompress_io_ctx *dic) > for (i = 0; i < dic->cluster_size; i++) { > if (dic->rpages[i]) > continue; > - unlock_page(dic->tpages[i]); > - put_page(dic->tpages[i]); > + f2fs_put_page(dic->tpages[i], 1); > } > kfree(dic->tpages); > } > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 7046b222e8de..19cd03450066 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -2099,7 +2099,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret, > false); > f2fs_free_dic(dic); > f2fs_put_dnode(&dn); > - f2fs_reset_compress_ctx(cc); > + f2fs_destroy_compress_ctx(cc); > *bio_ret = bio; > return ret; > } > @@ -2117,7 +2117,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret, > > f2fs_put_dnode(&dn); > > - f2fs_reset_compress_ctx(cc); > + f2fs_destroy_compress_ctx(cc); > *bio_ret = bio; > return 0; > > @@ -2125,7 +2125,6 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret, > f2fs_put_dnode(&dn); > out: > f2fs_decompress_end_io(cc->rpages, cc->cluster_size, true, false); > - f2fs_destroy_compress_ctx(cc); > *bio_ret = bio; > return ret; > } > @@ -2192,8 +2191,10 @@ int f2fs_mpage_readpages(struct address_space *mapping, > max_nr_pages, > &last_block_in_bio, > is_readahead); > - if (ret) > + if (ret) { > + f2fs_destroy_compress_ctx(&cc); > goto set_error_page; > + } > } > ret = f2fs_is_compressed_cluster(inode, page->index); > if (ret < 0) > @@ -2229,11 +2230,14 @@ int f2fs_mpage_readpages(struct address_space *mapping, > #ifdef CONFIG_F2FS_FS_COMPRESSION > if (f2fs_compressed_file(inode)) { > /* last page */ > - if (nr_pages == 1 && !f2fs_cluster_is_empty(&cc)) > + if (nr_pages == 1 && !f2fs_cluster_is_empty(&cc)) { > ret = f2fs_read_multi_pages(&cc, &bio, > max_nr_pages, > &last_block_in_bio, > is_readahead); > + if (ret) > + f2fs_destroy_compress_ctx(&cc); > + } > } > #endif > } > @@ -2856,6 +2860,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping, > > #ifdef CONFIG_F2FS_FS_COMPRESSION > if (f2fs_compressed_file(inode)) { > + get_page(page); > f2fs_compress_ctx_add_page(&cc, page); > continue; > } > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 26a4cc1fd686..5d55cef66410 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3765,7 +3765,6 @@ static inline bool f2fs_post_read_required(struct inode *inode) > #ifdef CONFIG_F2FS_FS_COMPRESSION > bool f2fs_is_compressed_page(struct page *page); > struct page *f2fs_compress_control_page(struct page *page); > -void f2fs_reset_compress_ctx(struct compress_ctx *cc); > int f2fs_prepare_compress_overwrite(struct inode *inode, > struct page **pagep, pgoff_t index, void **fsdata); > bool f2fs_compress_write_end(struct inode *inode, void *fsdata, >
| |