lkml.org 
[lkml]   [2019]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [f2fs-dev] [PATCH 2/2] f2fs: support data compression
From
Date
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,
>

\
 
 \ /
  Last update: 2019-12-12 16:08    [W:0.063 / U:16.372 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site