diff mbox series

[f2fs-dev] f2fs: compress: don't redirty sparse cluster during {, de}compress

Message ID 20240813061104.1052-1-youngjin.gil@samsung.com (mailing list archive)
State Superseded
Headers show
Series [f2fs-dev] f2fs: compress: don't redirty sparse cluster during {, de}compress | expand

Commit Message

Yeongjin Gil Aug. 13, 2024, 6:11 a.m. UTC
In f2fs_do_write_data_page, when the data block is NULL_ADDR, it skips
writepage considering that it has been already truncated.
This results in an infinite loop as the PAGECACHE_TAG_TOWRITE tag is not
cleared during the writeback process for a compressed file including
NULL_ADDR in compress_mode=user.

This is the reproduction process:

1. dd if=/dev/zero bs=4096 count=1024 seek=1024 of=testfile
2. f2fs_io compress testfile
3. dd if=/dev/zero bs=4096 count=1 conv=notrunc of=testfile
4. f2fs_io decompress testfile

To prevent the problem, let's check whether the cluster is fully
allocated before redirty its pages.

Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
Reviewed-by: Sunmin Jeong <s_min.jeong@samsung.com>
Tested-by: Jaewook Kim <jw5454.kim@samsung.com>
Signed-off-by: Yeongjin Gil <youngjin.gil@samsung.com>
---
 fs/f2fs/compress.c | 34 ++++++++++++++++++++++++++--------
 fs/f2fs/f2fs.h     | 12 ++++++++++++
 fs/f2fs/file.c     | 39 +++++++++++++++++++++------------------
 3 files changed, 59 insertions(+), 26 deletions(-)

Comments

Chao Yu Aug. 15, 2024, 10:12 a.m. UTC | #1
On 2024/8/13 14:11, Yeongjin Gil wrote:
> In f2fs_do_write_data_page, when the data block is NULL_ADDR, it skips
> writepage considering that it has been already truncated.
> This results in an infinite loop as the PAGECACHE_TAG_TOWRITE tag is not
> cleared during the writeback process for a compressed file including
> NULL_ADDR in compress_mode=user.
> 
> This is the reproduction process:
> 
> 1. dd if=/dev/zero bs=4096 count=1024 seek=1024 of=testfile
> 2. f2fs_io compress testfile
> 3. dd if=/dev/zero bs=4096 count=1 conv=notrunc of=testfile
> 4. f2fs_io decompress testfile

Good catch!

> 
> To prevent the problem, let's check whether the cluster is fully
> allocated before redirty its pages.
> 
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
> Reviewed-by: Sunmin Jeong <s_min.jeong@samsung.com>
> Tested-by: Jaewook Kim <jw5454.kim@samsung.com>
> Signed-off-by: Yeongjin Gil <youngjin.gil@samsung.com>

Need a fixes line?

> ---
>   fs/f2fs/compress.c | 34 ++++++++++++++++++++++++++--------
>   fs/f2fs/f2fs.h     | 12 ++++++++++++
>   fs/f2fs/file.c     | 39 +++++++++++++++++++++------------------
>   3 files changed, 59 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 990b93689b46..09b91d725807 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -945,7 +945,7 @@ static int __f2fs_get_cluster_blocks(struct inode *inode,
>   	unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
>   	int count, i;
>   
> -	for (i = 1, count = 1; i < cluster_size; i++) {
> +	for (i = 0, count = 0; i < cluster_size; i++) {
>   		block_t blkaddr = data_blkaddr(dn->inode, dn->node_page,
>   							dn->ofs_in_node + i);
>   
> @@ -956,8 +956,8 @@ static int __f2fs_get_cluster_blocks(struct inode *inode,
>   	return count;
>   }
>   
> -static int __f2fs_cluster_blocks(struct inode *inode,
> -				unsigned int cluster_idx, bool compr_blks)
> +static int __f2fs_cluster_blocks(struct inode *inode, unsigned int cluster_idx,
> +				enum cluster_check_type type)
>   {
>   	struct dnode_of_data dn;
>   	unsigned int start_idx = cluster_idx <<
> @@ -978,10 +978,12 @@ static int __f2fs_cluster_blocks(struct inode *inode,
>   	}
>   
>   	if (dn.data_blkaddr == COMPRESS_ADDR) {
> -		if (compr_blks)
> -			ret = __f2fs_get_cluster_blocks(inode, &dn);
> -		else
> +		if (type == COMPR_BLKS)
> +			ret = 1 + __f2fs_get_cluster_blocks(inode, &dn);
> +		else if (type == COMPR_CLUSTER)
>   			ret = 1;
> +	} else if (type == DECOMPR_BLKS) {
> +		ret = __f2fs_get_cluster_blocks(inode, &dn);
>   	}
>   fail:
>   	f2fs_put_dnode(&dn);
> @@ -991,7 +993,14 @@ static int __f2fs_cluster_blocks(struct inode *inode,
>   /* return # of compressed blocks in compressed cluster */
>   static int f2fs_compressed_blocks(struct compress_ctx *cc)
>   {
> -	return __f2fs_cluster_blocks(cc->inode, cc->cluster_idx, true);
> +	return __f2fs_cluster_blocks(cc->inode, cc->cluster_idx, COMPR_BLKS);
> +}
> +
> +/* return # of raw blocks in non-compressed cluster */
> +static int f2fs_decompressed_blocks(struct inode *inode,
> +				unsigned int cluster_idx)
> +{
> +	return __f2fs_cluster_blocks(inode, cluster_idx, DECOMPR_BLKS);
>   }
>   
>   /* return whether cluster is compressed one or not */
> @@ -999,7 +1008,16 @@ int f2fs_is_compressed_cluster(struct inode *inode, pgoff_t index)
>   {
>   	return __f2fs_cluster_blocks(inode,
>   		index >> F2FS_I(inode)->i_log_cluster_size,
> -		false);
> +		COMPR_CLUSTER);
> +}
> +
> +/* return whether the cluster is filled with raw blocks */
> +bool f2fs_is_non_sparse_cluster(struct inode *inode, pgoff_t index)

f2fs_is_sparse_cluster()?

> +{
> +	unsigned int cluster_idx = index >> F2FS_I(inode)->i_log_cluster_size;
> +
> +	return f2fs_decompressed_blocks(inode, cluster_idx) ==
> +		F2FS_I(inode)->i_cluster_size;
>   }
>   
>   static bool cluster_may_compress(struct compress_ctx *cc)
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 51fd5063a69c..6f26c8cb8857 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4302,6 +4302,11 @@ static inline bool f2fs_meta_inode_gc_required(struct inode *inode)
>    * compress.c
>    */
>   #ifdef CONFIG_F2FS_FS_COMPRESSION
> +enum cluster_check_type {
> +	COMPR_CLUSTER,
> +	COMPR_BLKS,
> +	DECOMPR_BLKS

How about?

CLUSTER_IS_COMPR,
CLUSTER_COMPR_BLKS,
CLUSTER_RAW_BLKS

> +};

Can you please add some comments for these enums?

>   bool f2fs_is_compressed_page(struct page *page);
>   struct page *f2fs_compress_control_page(struct page *page);
>   int f2fs_prepare_compress_overwrite(struct inode *inode,
> @@ -4328,6 +4333,7 @@ int f2fs_write_multi_pages(struct compress_ctx *cc,
>   						struct writeback_control *wbc,
>   						enum iostat_type io_type);
>   int f2fs_is_compressed_cluster(struct inode *inode, pgoff_t index);
> +bool f2fs_is_non_sparse_cluster(struct inode *inode, pgoff_t index);
>   void f2fs_update_read_extent_tree_range_compressed(struct inode *inode,
>   				pgoff_t fofs, block_t blkaddr,
>   				unsigned int llen, unsigned int c_len);
> @@ -4414,6 +4420,12 @@ static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>   static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
>   							nid_t ino) { }
>   #define inc_compr_inode_stat(inode)		do { } while (0)
> +static inline int f2fs_is_compressed_cluster(
> +				struct inode *inode,
> +				pgoff_t index) { return 0; }
> +static inline bool f2fs_is_non_sparse_cluster(
> +				struct inode *inode,
> +				pgoff_t index) { return false; }
>   static inline void f2fs_update_read_extent_tree_range_compressed(
>   				struct inode *inode,
>   				pgoff_t fofs, block_t blkaddr,
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 270c32e3385f..fba8b5f216f9 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -4220,9 +4220,8 @@ static int f2fs_ioc_decompress_file(struct file *filp)
>   	struct inode *inode = file_inode(filp);
>   	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>   	struct f2fs_inode_info *fi = F2FS_I(inode);
> -	pgoff_t page_idx = 0, last_idx;
> -	int cluster_size = fi->i_cluster_size;
> -	int count, ret;
> +	pgoff_t page_idx = 0, last_idx, cluster_idx;
> +	int ret;
>   
>   	if (!f2fs_sb_has_compression(sbi) ||
>   			F2FS_OPTION(sbi).compress_mode != COMPR_MODE_USER)
> @@ -4257,10 +4256,15 @@ static int f2fs_ioc_decompress_file(struct file *filp)
>   		goto out;
>   
>   	last_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
> +	last_idx >>= fi->i_log_cluster_size;
> +
> +	for (cluster_idx = 0; cluster_idx < last_idx; cluster_idx++) {
> +		page_idx = cluster_idx << fi->i_log_cluster_size;
> +
> +		if (!f2fs_is_compressed_cluster(inode, page_idx))
> +			continue;
>   
> -	count = last_idx - page_idx;
> -	while (count && count >= cluster_size) {
> -		ret = redirty_blocks(inode, page_idx, cluster_size);
> +		ret = redirty_blocks(inode, page_idx, fi->i_cluster_size);
>   		if (ret < 0)
>   			break;
>   
> @@ -4270,9 +4274,6 @@ static int f2fs_ioc_decompress_file(struct file *filp)
>   				break;
>   		}
>   
> -		count -= cluster_size;
> -		page_idx += cluster_size;
> -
>   		cond_resched();
>   		if (fatal_signal_pending(current)) {
>   			ret = -EINTR;
> @@ -4299,9 +4300,9 @@ static int f2fs_ioc_compress_file(struct file *filp)
>   {
>   	struct inode *inode = file_inode(filp);
>   	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> -	pgoff_t page_idx = 0, last_idx;
> -	int cluster_size = F2FS_I(inode)->i_cluster_size;
> -	int count, ret;
> +	struct f2fs_inode_info *fi = F2FS_I(inode);
> +	pgoff_t page_idx = 0, last_idx, cluster_idx;
> +	int ret;
>   
>   	if (!f2fs_sb_has_compression(sbi) ||
>   			F2FS_OPTION(sbi).compress_mode != COMPR_MODE_USER)
> @@ -4335,10 +4336,15 @@ static int f2fs_ioc_compress_file(struct file *filp)
>   	set_inode_flag(inode, FI_ENABLE_COMPRESS);
>   
>   	last_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
> +	last_idx >>= fi->i_log_cluster_size;
>   
> -	count = last_idx - page_idx;
> -	while (count && count >= cluster_size) {
> -		ret = redirty_blocks(inode, page_idx, cluster_size);
> +	for (cluster_idx = 0; cluster_idx < last_idx; cluster_idx++) {
> +		page_idx = cluster_idx << fi->i_log_cluster_size;
> +
> +		if (!f2fs_is_non_sparse_cluster(inode, page_idx))

if (f2fs_is_sparse_cluster())
	continue;

Thanks,

> +			continue;
> +
> +		ret = redirty_blocks(inode, page_idx, fi->i_cluster_size);
>   		if (ret < 0)
>   			break;
>   
> @@ -4348,9 +4354,6 @@ static int f2fs_ioc_compress_file(struct file *filp)
>   				break;
>   		}
>   
> -		count -= cluster_size;
> -		page_idx += cluster_size;
> -
>   		cond_resched();
>   		if (fatal_signal_pending(current)) {
>   			ret = -EINTR;
Yeongjin Gil Aug. 15, 2024, 11:10 a.m. UTC | #2
Thanks for your review. I will send patch v2.

>On 2024/8/13 14:11, Yeongjin Gil wrote:
>> In f2fs_do_write_data_page, when the data block is NULL_ADDR, it skips
>> writepage considering that it has been already truncated.
>> This results in an infinite loop as the PAGECACHE_TAG_TOWRITE tag is
>> not cleared during the writeback process for a compressed file
>> including NULL_ADDR in compress_mode=user.
>>
>> This is the reproduction process:
>>
>> 1. dd if=/dev/zero bs=4096 count=1024 seek=1024 of=testfile 2. f2fs_io
>> compress testfile 3. dd if=/dev/zero bs=4096 count=1 conv=notrunc
>> of=testfile 4. f2fs_io decompress testfile
>
>Good catch!
>
>>
>> To prevent the problem, let's check whether the cluster is fully
>> allocated before redirty its pages.
>>
>> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
>> Reviewed-by: Sunmin Jeong <s_min.jeong@samsung.com>
>> Tested-by: Jaewook Kim <jw5454.kim@samsung.com>
>> Signed-off-by: Yeongjin Gil <youngjin.gil@samsung.com>
>
>Need a fixes line?
>
>> ---
>>   fs/f2fs/compress.c | 34 ++++++++++++++++++++++++++--------
>>   fs/f2fs/f2fs.h     | 12 ++++++++++++
>>   fs/f2fs/file.c     | 39 +++++++++++++++++++++------------------
>>   3 files changed, 59 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index
>> 990b93689b46..09b91d725807 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -945,7 +945,7 @@ static int __f2fs_get_cluster_blocks(struct inode
>*inode,
>>   	unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
>>   	int count, i;
>>
>> -	for (i = 1, count = 1; i < cluster_size; i++) {
>> +	for (i = 0, count = 0; i < cluster_size; i++) {
>>   		block_t blkaddr = data_blkaddr(dn->inode, dn->node_page,
>>   							dn->ofs_in_node + i);
>>
>> @@ -956,8 +956,8 @@ static int __f2fs_get_cluster_blocks(struct inode
>*inode,
>>   	return count;
>>   }
>>
>> -static int __f2fs_cluster_blocks(struct inode *inode,
>> -				unsigned int cluster_idx, bool compr_blks)
>> +static int __f2fs_cluster_blocks(struct inode *inode, unsigned int
>cluster_idx,
>> +				enum cluster_check_type type)
>>   {
>>   	struct dnode_of_data dn;
>>   	unsigned int start_idx = cluster_idx << @@ -978,10 +978,12 @@
>> static int __f2fs_cluster_blocks(struct inode *inode,
>>   	}
>>
>>   	if (dn.data_blkaddr == COMPRESS_ADDR) {
>> -		if (compr_blks)
>> -			ret = __f2fs_get_cluster_blocks(inode, &dn);
>> -		else
>> +		if (type == COMPR_BLKS)
>> +			ret = 1 + __f2fs_get_cluster_blocks(inode, &dn);
>> +		else if (type == COMPR_CLUSTER)
>>   			ret = 1;
>> +	} else if (type == DECOMPR_BLKS) {
>> +		ret = __f2fs_get_cluster_blocks(inode, &dn);
>>   	}
>>   fail:
>>   	f2fs_put_dnode(&dn);
>> @@ -991,7 +993,14 @@ static int __f2fs_cluster_blocks(struct inode *inode,
>>   /* return # of compressed blocks in compressed cluster */
>>   static int f2fs_compressed_blocks(struct compress_ctx *cc)
>>   {
>> -	return __f2fs_cluster_blocks(cc->inode, cc->cluster_idx, true);
>> +	return __f2fs_cluster_blocks(cc->inode, cc->cluster_idx,
>> +COMPR_BLKS); }
>> +
>> +/* return # of raw blocks in non-compressed cluster */ static int
>> +f2fs_decompressed_blocks(struct inode *inode,
>> +				unsigned int cluster_idx)
>> +{
>> +	return __f2fs_cluster_blocks(inode, cluster_idx, DECOMPR_BLKS);
>>   }
>>
>>   /* return whether cluster is compressed one or not */ @@ -999,7
>> +1008,16 @@ int f2fs_is_compressed_cluster(struct inode *inode, pgoff_t
>index)
>>   {
>>   	return __f2fs_cluster_blocks(inode,
>>   		index >> F2FS_I(inode)->i_log_cluster_size,
>> -		false);
>> +		COMPR_CLUSTER);
>> +}
>> +
>> +/* return whether the cluster is filled with raw blocks */ bool
>> +f2fs_is_non_sparse_cluster(struct inode *inode, pgoff_t index)
>
>f2fs_is_sparse_cluster()?
>
>> +{
>> +	unsigned int cluster_idx = index >>
>> +F2FS_I(inode)->i_log_cluster_size;
>> +
>> +	return f2fs_decompressed_blocks(inode, cluster_idx) ==
>> +		F2FS_I(inode)->i_cluster_size;
>>   }
>>
>>   static bool cluster_may_compress(struct compress_ctx *cc) diff --git
>> a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 51fd5063a69c..6f26c8cb8857
>> 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -4302,6 +4302,11 @@ static inline bool
>f2fs_meta_inode_gc_required(struct inode *inode)
>>    * compress.c
>>    */
>>   #ifdef CONFIG_F2FS_FS_COMPRESSION
>> +enum cluster_check_type {
>> +	COMPR_CLUSTER,
>> +	COMPR_BLKS,
>> +	DECOMPR_BLKS
>
>How about?
>
>CLUSTER_IS_COMPR,
>CLUSTER_COMPR_BLKS,
>CLUSTER_RAW_BLKS
>
>> +};
>
>Can you please add some comments for these enums?
>
>>   bool f2fs_is_compressed_page(struct page *page);
>>   struct page *f2fs_compress_control_page(struct page *page);
>>   int f2fs_prepare_compress_overwrite(struct inode *inode, @@ -4328,6
>> +4333,7 @@ int f2fs_write_multi_pages(struct compress_ctx *cc,
>>   						struct writeback_control *wbc,
>>   						enum iostat_type io_type);
>>   int f2fs_is_compressed_cluster(struct inode *inode, pgoff_t index);
>> +bool f2fs_is_non_sparse_cluster(struct inode *inode, pgoff_t index);
>>   void f2fs_update_read_extent_tree_range_compressed(struct inode *inode,
>>   				pgoff_t fofs, block_t blkaddr,
>>   				unsigned int llen, unsigned int c_len); @@ -
>4414,6 +4420,12 @@
>> static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>>   static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info
>*sbi,
>>   							nid_t ino) { }
>>   #define inc_compr_inode_stat(inode)		do { } while (0)
>> +static inline int f2fs_is_compressed_cluster(
>> +				struct inode *inode,
>> +				pgoff_t index) { return 0; }
>> +static inline bool f2fs_is_non_sparse_cluster(
>> +				struct inode *inode,
>> +				pgoff_t index) { return false; }
>>   static inline void f2fs_update_read_extent_tree_range_compressed(
>>   				struct inode *inode,
>>   				pgoff_t fofs, block_t blkaddr,
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index
>> 270c32e3385f..fba8b5f216f9 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -4220,9 +4220,8 @@ static int f2fs_ioc_decompress_file(struct file
>*filp)
>>   	struct inode *inode = file_inode(filp);
>>   	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>>   	struct f2fs_inode_info *fi = F2FS_I(inode);
>> -	pgoff_t page_idx = 0, last_idx;
>> -	int cluster_size = fi->i_cluster_size;
>> -	int count, ret;
>> +	pgoff_t page_idx = 0, last_idx, cluster_idx;
>> +	int ret;
>>
>>   	if (!f2fs_sb_has_compression(sbi) ||
>>   			F2FS_OPTION(sbi).compress_mode != COMPR_MODE_USER) @@ -
>4257,10
>> +4256,15 @@ static int f2fs_ioc_decompress_file(struct file *filp)
>>   		goto out;
>>
>>   	last_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
>> +	last_idx >>= fi->i_log_cluster_size;
>> +
>> +	for (cluster_idx = 0; cluster_idx < last_idx; cluster_idx++) {
>> +		page_idx = cluster_idx << fi->i_log_cluster_size;
>> +
>> +		if (!f2fs_is_compressed_cluster(inode, page_idx))
>> +			continue;
>>
>> -	count = last_idx - page_idx;
>> -	while (count && count >= cluster_size) {
>> -		ret = redirty_blocks(inode, page_idx, cluster_size);
>> +		ret = redirty_blocks(inode, page_idx, fi->i_cluster_size);
>>   		if (ret < 0)
>>   			break;
>>
>> @@ -4270,9 +4274,6 @@ static int f2fs_ioc_decompress_file(struct file
>*filp)
>>   				break;
>>   		}
>>
>> -		count -= cluster_size;
>> -		page_idx += cluster_size;
>> -
>>   		cond_resched();
>>   		if (fatal_signal_pending(current)) {
>>   			ret = -EINTR;
>> @@ -4299,9 +4300,9 @@ static int f2fs_ioc_compress_file(struct file *filp)
>>   {
>>   	struct inode *inode = file_inode(filp);
>>   	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> -	pgoff_t page_idx = 0, last_idx;
>> -	int cluster_size = F2FS_I(inode)->i_cluster_size;
>> -	int count, ret;
>> +	struct f2fs_inode_info *fi = F2FS_I(inode);
>> +	pgoff_t page_idx = 0, last_idx, cluster_idx;
>> +	int ret;
>>
>>   	if (!f2fs_sb_has_compression(sbi) ||
>>   			F2FS_OPTION(sbi).compress_mode != COMPR_MODE_USER) @@ -
>4335,10
>> +4336,15 @@ static int f2fs_ioc_compress_file(struct file *filp)
>>   	set_inode_flag(inode, FI_ENABLE_COMPRESS);
>>
>>   	last_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
>> +	last_idx >>= fi->i_log_cluster_size;
>>
>> -	count = last_idx - page_idx;
>> -	while (count && count >= cluster_size) {
>> -		ret = redirty_blocks(inode, page_idx, cluster_size);
>> +	for (cluster_idx = 0; cluster_idx < last_idx; cluster_idx++) {
>> +		page_idx = cluster_idx << fi->i_log_cluster_size;
>> +
>> +		if (!f2fs_is_non_sparse_cluster(inode, page_idx))
>
>if (f2fs_is_sparse_cluster())
>	continue;
>
>Thanks,
>
>> +			continue;
>> +
>> +		ret = redirty_blocks(inode, page_idx, fi->i_cluster_size);
>>   		if (ret < 0)
>>   			break;
>>
>> @@ -4348,9 +4354,6 @@ static int f2fs_ioc_compress_file(struct file *filp)
>>   				break;
>>   		}
>>
>> -		count -= cluster_size;
>> -		page_idx += cluster_size;
>> -
>>   		cond_resched();
>>   		if (fatal_signal_pending(current)) {
>>   			ret = -EINTR;
diff mbox series

Patch

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 990b93689b46..09b91d725807 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -945,7 +945,7 @@  static int __f2fs_get_cluster_blocks(struct inode *inode,
 	unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
 	int count, i;
 
-	for (i = 1, count = 1; i < cluster_size; i++) {
+	for (i = 0, count = 0; i < cluster_size; i++) {
 		block_t blkaddr = data_blkaddr(dn->inode, dn->node_page,
 							dn->ofs_in_node + i);
 
@@ -956,8 +956,8 @@  static int __f2fs_get_cluster_blocks(struct inode *inode,
 	return count;
 }
 
-static int __f2fs_cluster_blocks(struct inode *inode,
-				unsigned int cluster_idx, bool compr_blks)
+static int __f2fs_cluster_blocks(struct inode *inode, unsigned int cluster_idx,
+				enum cluster_check_type type)
 {
 	struct dnode_of_data dn;
 	unsigned int start_idx = cluster_idx <<
@@ -978,10 +978,12 @@  static int __f2fs_cluster_blocks(struct inode *inode,
 	}
 
 	if (dn.data_blkaddr == COMPRESS_ADDR) {
-		if (compr_blks)
-			ret = __f2fs_get_cluster_blocks(inode, &dn);
-		else
+		if (type == COMPR_BLKS)
+			ret = 1 + __f2fs_get_cluster_blocks(inode, &dn);
+		else if (type == COMPR_CLUSTER)
 			ret = 1;
+	} else if (type == DECOMPR_BLKS) {
+		ret = __f2fs_get_cluster_blocks(inode, &dn);
 	}
 fail:
 	f2fs_put_dnode(&dn);
@@ -991,7 +993,14 @@  static int __f2fs_cluster_blocks(struct inode *inode,
 /* return # of compressed blocks in compressed cluster */
 static int f2fs_compressed_blocks(struct compress_ctx *cc)
 {
-	return __f2fs_cluster_blocks(cc->inode, cc->cluster_idx, true);
+	return __f2fs_cluster_blocks(cc->inode, cc->cluster_idx, COMPR_BLKS);
+}
+
+/* return # of raw blocks in non-compressed cluster */
+static int f2fs_decompressed_blocks(struct inode *inode,
+				unsigned int cluster_idx)
+{
+	return __f2fs_cluster_blocks(inode, cluster_idx, DECOMPR_BLKS);
 }
 
 /* return whether cluster is compressed one or not */
@@ -999,7 +1008,16 @@  int f2fs_is_compressed_cluster(struct inode *inode, pgoff_t index)
 {
 	return __f2fs_cluster_blocks(inode,
 		index >> F2FS_I(inode)->i_log_cluster_size,
-		false);
+		COMPR_CLUSTER);
+}
+
+/* return whether the cluster is filled with raw blocks */
+bool f2fs_is_non_sparse_cluster(struct inode *inode, pgoff_t index)
+{
+	unsigned int cluster_idx = index >> F2FS_I(inode)->i_log_cluster_size;
+
+	return f2fs_decompressed_blocks(inode, cluster_idx) ==
+		F2FS_I(inode)->i_cluster_size;
 }
 
 static bool cluster_may_compress(struct compress_ctx *cc)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 51fd5063a69c..6f26c8cb8857 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4302,6 +4302,11 @@  static inline bool f2fs_meta_inode_gc_required(struct inode *inode)
  * compress.c
  */
 #ifdef CONFIG_F2FS_FS_COMPRESSION
+enum cluster_check_type {
+	COMPR_CLUSTER,
+	COMPR_BLKS,
+	DECOMPR_BLKS
+};
 bool f2fs_is_compressed_page(struct page *page);
 struct page *f2fs_compress_control_page(struct page *page);
 int f2fs_prepare_compress_overwrite(struct inode *inode,
@@ -4328,6 +4333,7 @@  int f2fs_write_multi_pages(struct compress_ctx *cc,
 						struct writeback_control *wbc,
 						enum iostat_type io_type);
 int f2fs_is_compressed_cluster(struct inode *inode, pgoff_t index);
+bool f2fs_is_non_sparse_cluster(struct inode *inode, pgoff_t index);
 void f2fs_update_read_extent_tree_range_compressed(struct inode *inode,
 				pgoff_t fofs, block_t blkaddr,
 				unsigned int llen, unsigned int c_len);
@@ -4414,6 +4420,12 @@  static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
 static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
 							nid_t ino) { }
 #define inc_compr_inode_stat(inode)		do { } while (0)
+static inline int f2fs_is_compressed_cluster(
+				struct inode *inode,
+				pgoff_t index) { return 0; }
+static inline bool f2fs_is_non_sparse_cluster(
+				struct inode *inode,
+				pgoff_t index) { return false; }
 static inline void f2fs_update_read_extent_tree_range_compressed(
 				struct inode *inode,
 				pgoff_t fofs, block_t blkaddr,
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 270c32e3385f..fba8b5f216f9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -4220,9 +4220,8 @@  static int f2fs_ioc_decompress_file(struct file *filp)
 	struct inode *inode = file_inode(filp);
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct f2fs_inode_info *fi = F2FS_I(inode);
-	pgoff_t page_idx = 0, last_idx;
-	int cluster_size = fi->i_cluster_size;
-	int count, ret;
+	pgoff_t page_idx = 0, last_idx, cluster_idx;
+	int ret;
 
 	if (!f2fs_sb_has_compression(sbi) ||
 			F2FS_OPTION(sbi).compress_mode != COMPR_MODE_USER)
@@ -4257,10 +4256,15 @@  static int f2fs_ioc_decompress_file(struct file *filp)
 		goto out;
 
 	last_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
+	last_idx >>= fi->i_log_cluster_size;
+
+	for (cluster_idx = 0; cluster_idx < last_idx; cluster_idx++) {
+		page_idx = cluster_idx << fi->i_log_cluster_size;
+
+		if (!f2fs_is_compressed_cluster(inode, page_idx))
+			continue;
 
-	count = last_idx - page_idx;
-	while (count && count >= cluster_size) {
-		ret = redirty_blocks(inode, page_idx, cluster_size);
+		ret = redirty_blocks(inode, page_idx, fi->i_cluster_size);
 		if (ret < 0)
 			break;
 
@@ -4270,9 +4274,6 @@  static int f2fs_ioc_decompress_file(struct file *filp)
 				break;
 		}
 
-		count -= cluster_size;
-		page_idx += cluster_size;
-
 		cond_resched();
 		if (fatal_signal_pending(current)) {
 			ret = -EINTR;
@@ -4299,9 +4300,9 @@  static int f2fs_ioc_compress_file(struct file *filp)
 {
 	struct inode *inode = file_inode(filp);
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	pgoff_t page_idx = 0, last_idx;
-	int cluster_size = F2FS_I(inode)->i_cluster_size;
-	int count, ret;
+	struct f2fs_inode_info *fi = F2FS_I(inode);
+	pgoff_t page_idx = 0, last_idx, cluster_idx;
+	int ret;
 
 	if (!f2fs_sb_has_compression(sbi) ||
 			F2FS_OPTION(sbi).compress_mode != COMPR_MODE_USER)
@@ -4335,10 +4336,15 @@  static int f2fs_ioc_compress_file(struct file *filp)
 	set_inode_flag(inode, FI_ENABLE_COMPRESS);
 
 	last_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
+	last_idx >>= fi->i_log_cluster_size;
 
-	count = last_idx - page_idx;
-	while (count && count >= cluster_size) {
-		ret = redirty_blocks(inode, page_idx, cluster_size);
+	for (cluster_idx = 0; cluster_idx < last_idx; cluster_idx++) {
+		page_idx = cluster_idx << fi->i_log_cluster_size;
+
+		if (!f2fs_is_non_sparse_cluster(inode, page_idx))
+			continue;
+
+		ret = redirty_blocks(inode, page_idx, fi->i_cluster_size);
 		if (ret < 0)
 			break;
 
@@ -4348,9 +4354,6 @@  static int f2fs_ioc_compress_file(struct file *filp)
 				break;
 		}
 
-		count -= cluster_size;
-		page_idx += cluster_size;
-
 		cond_resched();
 		if (fatal_signal_pending(current)) {
 			ret = -EINTR;