linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chao Yu <chao@kernel.org>
To: Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <yuchao0@huawei.com>
Cc: Eric Biggers <ebiggers@kernel.org>,
	linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: support data compression
Date: Thu, 12 Dec 2019 23:07:32 +0800	[thread overview]
Message-ID: <8b1eccb2-d791-c349-d6c2-ec2c5b33eb1d@kernel.org> (raw)
In-Reply-To: <20191211012723.GA57416@jaegeuk-macbookpro.roam.corp.google.com>

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,
>

  reply	other threads:[~2019-12-12 15:07 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-22 17:16 [PATCH 1/2] f2fs: support aligned pinned file Jaegeuk Kim
2019-10-22 17:16 ` [PATCH 2/2] f2fs: support data compression Jaegeuk Kim
2019-10-23  5:24   ` Eric Biggers
2019-10-23 17:28     ` Jaegeuk Kim
2019-10-25  9:07     ` [f2fs-dev] " Chao Yu
2019-10-27 22:50   ` Eric Biggers
2019-10-28  2:33     ` [f2fs-dev] " Chao Yu
2019-10-29  8:33     ` Chao Yu
2019-10-30  2:55       ` Eric Biggers
2019-10-30  8:43         ` Chao Yu
2019-10-30 16:50           ` Eric Biggers
2019-10-30 17:22             ` [f2fs-dev] " Gao Xiang
2019-10-30 17:47             ` Jaegeuk Kim
2019-10-31  2:16             ` Chao Yu
2019-10-31 15:35               ` Jaegeuk Kim
2019-11-01 10:02                 ` Chao Yu
2019-10-30 17:02           ` Eric Biggers
2019-10-31  2:21             ` Chao Yu
2019-11-13 13:10             ` Chao Yu
2019-11-18 16:11               ` Jaegeuk Kim
2019-11-18 20:58                 ` Jaegeuk Kim
2019-11-25 17:42                   ` [f2fs-dev] " Jaegeuk Kim
2019-12-11  1:27                     ` Jaegeuk Kim
2019-12-12 15:07                       ` Chao Yu [this message]
2019-10-24  8:21 ` [f2fs-dev] [PATCH 1/2] f2fs: support aligned pinned file Chao Yu
2019-10-25 18:18   ` Jaegeuk Kim
2019-10-26  1:31     ` Chao Yu
2019-10-30 16:09       ` Jaegeuk Kim
2019-10-31  2:27         ` Chao Yu
2019-10-31 15:29           ` Jaegeuk Kim
2019-11-05  3:39             ` Chao Yu
2019-11-07 19:14 ` [PATCH 1/2 v2] " Jaegeuk Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b1eccb2-d791-c349-d6c2-ec2c5b33eb1d@kernel.org \
    --to=chao@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yuchao0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).