diff mbox series

[5/6] f2fs: fix to restrict condition of compress inode conversion

Message ID 20231210113547.3412782-5-chao@kernel.org
State New
Headers
Series [1/6] f2fs: fix to tag gcing flag on page during block migration |

Commit Message

Chao Yu Dec. 10, 2023, 11:35 a.m. UTC 
  This patch adds i_size check during compress inode conversion in order
to avoid .page_mkwrite races w/ conversion.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/f2fs.h | 8 +++++++-
 fs/f2fs/file.c | 5 ++---
 2 files changed, 9 insertions(+), 4 deletions(-)

Comments

Jaegeuk Kim Dec. 11, 2023, 10:11 p.m. UTC | #1 
On 12/10, Chao Yu wrote:
> This patch adds i_size check during compress inode conversion in order
> to avoid .page_mkwrite races w/ conversion.

Which race condition do you see?

> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/f2fs.h | 8 +++++++-
>  fs/f2fs/file.c | 5 ++---
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 65294e3b0bef..c9b8a1953913 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4397,13 +4397,19 @@ static inline int set_compress_context(struct inode *inode)
>  #endif
>  }
>  
> +static inline bool inode_has_data(struct inode *inode)
> +{
> +	return (S_ISREG(inode->i_mode) &&
> +		(F2FS_HAS_BLOCKS(inode) || i_size_read(inode)));
> +}
> +
>  static inline bool f2fs_disable_compressed_file(struct inode *inode)
>  {
>  	struct f2fs_inode_info *fi = F2FS_I(inode);
>  
>  	if (!f2fs_compressed_file(inode))
>  		return true;
> -	if (S_ISREG(inode->i_mode) && F2FS_HAS_BLOCKS(inode))
> +	if (inode_has_data(inode))
>  		return false;
>  
>  	fi->i_flags &= ~F2FS_COMPR_FL;
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 1a3c29a9a6a0..8af4b29c3e1a 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1922,8 +1922,7 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>  
>  			f2fs_down_write(&F2FS_I(inode)->i_sem);
>  			if (!f2fs_may_compress(inode) ||
> -					(S_ISREG(inode->i_mode) &&
> -					F2FS_HAS_BLOCKS(inode))) {
> +					inode_has_data(inode)) {
>  				f2fs_up_write(&F2FS_I(inode)->i_sem);
>  				return -EINVAL;
>  			}
> @@ -3996,7 +3995,7 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
>  		goto out;
>  	}
>  
> -	if (F2FS_HAS_BLOCKS(inode)) {
> +	if (inode_has_data(inode)) {
>  		ret = -EFBIG;
>  		goto out;
>  	}
> -- 
> 2.40.1
Chao Yu Dec. 12, 2023, 1:05 a.m. UTC | #2 
On 2023/12/12 6:11, Jaegeuk Kim wrote:
> On 12/10, Chao Yu wrote:
>> This patch adds i_size check during compress inode conversion in order
>> to avoid .page_mkwrite races w/ conversion.
> 
> Which race condition do you see?

Something like:

- f2fs_setflags_common
  - check S_ISREG && F2FS_HAS_BLOCKS
					- mkwrite
					 - f2fs_get_block_locked
					  : update metadata in old inode's disk layout
  - set_compress_context

Thanks,

> 
>>
>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/f2fs.h | 8 +++++++-
>>   fs/f2fs/file.c | 5 ++---
>>   2 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 65294e3b0bef..c9b8a1953913 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -4397,13 +4397,19 @@ static inline int set_compress_context(struct inode *inode)
>>   #endif
>>   }
>>   
>> +static inline bool inode_has_data(struct inode *inode)
>> +{
>> +	return (S_ISREG(inode->i_mode) &&
>> +		(F2FS_HAS_BLOCKS(inode) || i_size_read(inode)));
>> +}
>> +
>>   static inline bool f2fs_disable_compressed_file(struct inode *inode)
>>   {
>>   	struct f2fs_inode_info *fi = F2FS_I(inode);
>>   
>>   	if (!f2fs_compressed_file(inode))
>>   		return true;
>> -	if (S_ISREG(inode->i_mode) && F2FS_HAS_BLOCKS(inode))
>> +	if (inode_has_data(inode))
>>   		return false;
>>   
>>   	fi->i_flags &= ~F2FS_COMPR_FL;
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 1a3c29a9a6a0..8af4b29c3e1a 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -1922,8 +1922,7 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>>   
>>   			f2fs_down_write(&F2FS_I(inode)->i_sem);
>>   			if (!f2fs_may_compress(inode) ||
>> -					(S_ISREG(inode->i_mode) &&
>> -					F2FS_HAS_BLOCKS(inode))) {
>> +					inode_has_data(inode)) {
>>   				f2fs_up_write(&F2FS_I(inode)->i_sem);
>>   				return -EINVAL;
>>   			}
>> @@ -3996,7 +3995,7 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
>>   		goto out;
>>   	}
>>   
>> -	if (F2FS_HAS_BLOCKS(inode)) {
>> +	if (inode_has_data(inode)) {
>>   		ret = -EFBIG;
>>   		goto out;
>>   	}
>> -- 
>> 2.40.1
Jaegeuk Kim Dec. 12, 2023, 10:21 p.m. UTC | #3 
On 12/12, Chao Yu wrote:
> On 2023/12/12 6:11, Jaegeuk Kim wrote:
> > On 12/10, Chao Yu wrote:
> > > This patch adds i_size check during compress inode conversion in order
> > > to avoid .page_mkwrite races w/ conversion.
> > 
> > Which race condition do you see?
> 
> Something like:
> 
> - f2fs_setflags_common
>  - check S_ISREG && F2FS_HAS_BLOCKS
> 					- mkwrite
> 					 - f2fs_get_block_locked
> 					  : update metadata in old inode's disk layout

Don't we need to prevent setting this for mmapped file?

>  - set_compress_context
> 
> Thanks,
> 
> > 
> > > 
> > > Fixes: 4c8ff7095bef ("f2fs: support data compression")
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > ---
> > >   fs/f2fs/f2fs.h | 8 +++++++-
> > >   fs/f2fs/file.c | 5 ++---
> > >   2 files changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 65294e3b0bef..c9b8a1953913 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -4397,13 +4397,19 @@ static inline int set_compress_context(struct inode *inode)
> > >   #endif
> > >   }
> > > +static inline bool inode_has_data(struct inode *inode)
> > > +{
> > > +	return (S_ISREG(inode->i_mode) &&
> > > +		(F2FS_HAS_BLOCKS(inode) || i_size_read(inode)));
> > > +}
> > > +
> > >   static inline bool f2fs_disable_compressed_file(struct inode *inode)
> > >   {
> > >   	struct f2fs_inode_info *fi = F2FS_I(inode);
> > >   	if (!f2fs_compressed_file(inode))
> > >   		return true;
> > > -	if (S_ISREG(inode->i_mode) && F2FS_HAS_BLOCKS(inode))
> > > +	if (inode_has_data(inode))
> > >   		return false;
> > >   	fi->i_flags &= ~F2FS_COMPR_FL;
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index 1a3c29a9a6a0..8af4b29c3e1a 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -1922,8 +1922,7 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
> > >   			f2fs_down_write(&F2FS_I(inode)->i_sem);
> > >   			if (!f2fs_may_compress(inode) ||
> > > -					(S_ISREG(inode->i_mode) &&
> > > -					F2FS_HAS_BLOCKS(inode))) {
> > > +					inode_has_data(inode)) {
> > >   				f2fs_up_write(&F2FS_I(inode)->i_sem);
> > >   				return -EINVAL;
> > >   			}
> > > @@ -3996,7 +3995,7 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
> > >   		goto out;
> > >   	}
> > > -	if (F2FS_HAS_BLOCKS(inode)) {
> > > +	if (inode_has_data(inode)) {
> > >   		ret = -EFBIG;
> > >   		goto out;
> > >   	}
> > > -- 
> > > 2.40.1
Chao Yu Dec. 28, 2023, 3:33 p.m. UTC | #4 
On 2023/12/13 6:21, Jaegeuk Kim wrote:
> On 12/12, Chao Yu wrote:
>> On 2023/12/12 6:11, Jaegeuk Kim wrote:
>>> On 12/10, Chao Yu wrote:
>>>> This patch adds i_size check during compress inode conversion in order
>>>> to avoid .page_mkwrite races w/ conversion.
>>>
>>> Which race condition do you see?
>>
>> Something like:
>>
>> - f2fs_setflags_common
>>   - check S_ISREG && F2FS_HAS_BLOCKS
>> 					- mkwrite
>> 					 - f2fs_get_block_locked
>> 					  : update metadata in old inode's disk layout
> 
> Don't we need to prevent setting this for mmapped file?

Oh, we have used i_sem lock to prevent such race case, however
we missed f2fs_disable_compressed_file():

- f2fs_disable_compressed_file
  - check inode_has_data
						- f2fs_file_mmap
						- mkwrite
						 - f2fs_get_block_locked
						 : update metadata in compressed
						   inode's disk layout
  - fi->i_flags &= ~F2FS_COMPR_FL
  - clear_inode_flag(inode, FI_COMPRESSED_FILE);

Thanks,

> 
>>   - set_compress_context
>>
>> Thanks,
>>
>>>
>>>>
>>>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>>    fs/f2fs/f2fs.h | 8 +++++++-
>>>>    fs/f2fs/file.c | 5 ++---
>>>>    2 files changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 65294e3b0bef..c9b8a1953913 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -4397,13 +4397,19 @@ static inline int set_compress_context(struct inode *inode)
>>>>    #endif
>>>>    }
>>>> +static inline bool inode_has_data(struct inode *inode)
>>>> +{
>>>> +	return (S_ISREG(inode->i_mode) &&
>>>> +		(F2FS_HAS_BLOCKS(inode) || i_size_read(inode)));
>>>> +}
>>>> +
>>>>    static inline bool f2fs_disable_compressed_file(struct inode *inode)
>>>>    {
>>>>    	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>    	if (!f2fs_compressed_file(inode))
>>>>    		return true;
>>>> -	if (S_ISREG(inode->i_mode) && F2FS_HAS_BLOCKS(inode))
>>>> +	if (inode_has_data(inode))
>>>>    		return false;
>>>>    	fi->i_flags &= ~F2FS_COMPR_FL;
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 1a3c29a9a6a0..8af4b29c3e1a 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -1922,8 +1922,7 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>>>>    			f2fs_down_write(&F2FS_I(inode)->i_sem);
>>>>    			if (!f2fs_may_compress(inode) ||
>>>> -					(S_ISREG(inode->i_mode) &&
>>>> -					F2FS_HAS_BLOCKS(inode))) {
>>>> +					inode_has_data(inode)) {
>>>>    				f2fs_up_write(&F2FS_I(inode)->i_sem);
>>>>    				return -EINVAL;
>>>>    			}
>>>> @@ -3996,7 +3995,7 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
>>>>    		goto out;
>>>>    	}
>>>> -	if (F2FS_HAS_BLOCKS(inode)) {
>>>> +	if (inode_has_data(inode)) {
>>>>    		ret = -EFBIG;
>>>>    		goto out;
>>>>    	}
>>>> -- 
>>>> 2.40.1
Jaegeuk Kim Jan. 2, 2024, 10:54 p.m. UTC | #5 
On 12/28, Chao Yu wrote:
> On 2023/12/13 6:21, Jaegeuk Kim wrote:
> > On 12/12, Chao Yu wrote:
> > > On 2023/12/12 6:11, Jaegeuk Kim wrote:
> > > > On 12/10, Chao Yu wrote:
> > > > > This patch adds i_size check during compress inode conversion in order
> > > > > to avoid .page_mkwrite races w/ conversion.
> > > > 
> > > > Which race condition do you see?
> > > 
> > > Something like:
> > > 
> > > - f2fs_setflags_common
> > >   - check S_ISREG && F2FS_HAS_BLOCKS
> > > 					- mkwrite
> > > 					 - f2fs_get_block_locked
> > > 					  : update metadata in old inode's disk layout
> > 
> > Don't we need to prevent setting this for mmapped file?
> 
> Oh, we have used i_sem lock to prevent such race case, however
> we missed f2fs_disable_compressed_file():
> 
> - f2fs_disable_compressed_file
>  - check inode_has_data
> 						- f2fs_file_mmap
> 						- mkwrite
> 						 - f2fs_get_block_locked
> 						 : update metadata in compressed
> 						   inode's disk layout
>  - fi->i_flags &= ~F2FS_COMPR_FL
>  - clear_inode_flag(inode, FI_COMPRESSED_FILE);

So, needing i_sem for disabling it on mmapped file? It seems i_size would not
be enough?

> 
> Thanks,
> 
> > 
> > >   - set_compress_context
> > > 
> > > Thanks,
> > > 
> > > > 
> > > > > 
> > > > > Fixes: 4c8ff7095bef ("f2fs: support data compression")
> > > > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > > > ---
> > > > >    fs/f2fs/f2fs.h | 8 +++++++-
> > > > >    fs/f2fs/file.c | 5 ++---
> > > > >    2 files changed, 9 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index 65294e3b0bef..c9b8a1953913 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -4397,13 +4397,19 @@ static inline int set_compress_context(struct inode *inode)
> > > > >    #endif
> > > > >    }
> > > > > +static inline bool inode_has_data(struct inode *inode)
> > > > > +{
> > > > > +	return (S_ISREG(inode->i_mode) &&
> > > > > +		(F2FS_HAS_BLOCKS(inode) || i_size_read(inode)));
> > > > > +}
> > > > > +
> > > > >    static inline bool f2fs_disable_compressed_file(struct inode *inode)
> > > > >    {
> > > > >    	struct f2fs_inode_info *fi = F2FS_I(inode);
> > > > >    	if (!f2fs_compressed_file(inode))
> > > > >    		return true;
> > > > > -	if (S_ISREG(inode->i_mode) && F2FS_HAS_BLOCKS(inode))
> > > > > +	if (inode_has_data(inode))
> > > > >    		return false;
> > > > >    	fi->i_flags &= ~F2FS_COMPR_FL;
> > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > > > index 1a3c29a9a6a0..8af4b29c3e1a 100644
> > > > > --- a/fs/f2fs/file.c
> > > > > +++ b/fs/f2fs/file.c
> > > > > @@ -1922,8 +1922,7 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
> > > > >    			f2fs_down_write(&F2FS_I(inode)->i_sem);
> > > > >    			if (!f2fs_may_compress(inode) ||
> > > > > -					(S_ISREG(inode->i_mode) &&
> > > > > -					F2FS_HAS_BLOCKS(inode))) {
> > > > > +					inode_has_data(inode)) {
> > > > >    				f2fs_up_write(&F2FS_I(inode)->i_sem);
> > > > >    				return -EINVAL;
> > > > >    			}
> > > > > @@ -3996,7 +3995,7 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
> > > > >    		goto out;
> > > > >    	}
> > > > > -	if (F2FS_HAS_BLOCKS(inode)) {
> > > > > +	if (inode_has_data(inode)) {
> > > > >    		ret = -EFBIG;
> > > > >    		goto out;
> > > > >    	}
> > > > > -- 
> > > > > 2.40.1
Chao Yu Jan. 11, 2024, 3:07 a.m. UTC | #6 
On 2024/1/3 6:54, Jaegeuk Kim wrote:
> On 12/28, Chao Yu wrote:
>> On 2023/12/13 6:21, Jaegeuk Kim wrote:
>>> On 12/12, Chao Yu wrote:
>>>> On 2023/12/12 6:11, Jaegeuk Kim wrote:
>>>>> On 12/10, Chao Yu wrote:
>>>>>> This patch adds i_size check during compress inode conversion in order
>>>>>> to avoid .page_mkwrite races w/ conversion.
>>>>>
>>>>> Which race condition do you see?
>>>>
>>>> Something like:
>>>>
>>>> - f2fs_setflags_common
>>>>    - check S_ISREG && F2FS_HAS_BLOCKS
>>>> 					- mkwrite
>>>> 					 - f2fs_get_block_locked
>>>> 					  : update metadata in old inode's disk layout
>>>
>>> Don't we need to prevent setting this for mmapped file?
>>
>> Oh, we have used i_sem lock to prevent such race case, however
>> we missed f2fs_disable_compressed_file():
>>
>> - f2fs_disable_compressed_file
>>   - check inode_has_data
>> 						- f2fs_file_mmap
>> 						- mkwrite
>> 						 - f2fs_get_block_locked
>> 						 : update metadata in compressed
>> 						   inode's disk layout
>>   - fi->i_flags &= ~F2FS_COMPR_FL
>>   - clear_inode_flag(inode, FI_COMPRESSED_FILE);
> 
> So, needing i_sem for disabling it on mmapped file? It seems i_size would not
> be enough?

Agreed, let me update the patch.

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>>>    - set_compress_context
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>>> ---
>>>>>>     fs/f2fs/f2fs.h | 8 +++++++-
>>>>>>     fs/f2fs/file.c | 5 ++---
>>>>>>     2 files changed, 9 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>> index 65294e3b0bef..c9b8a1953913 100644
>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>> @@ -4397,13 +4397,19 @@ static inline int set_compress_context(struct inode *inode)
>>>>>>     #endif
>>>>>>     }
>>>>>> +static inline bool inode_has_data(struct inode *inode)
>>>>>> +{
>>>>>> +	return (S_ISREG(inode->i_mode) &&
>>>>>> +		(F2FS_HAS_BLOCKS(inode) || i_size_read(inode)));
>>>>>> +}
>>>>>> +
>>>>>>     static inline bool f2fs_disable_compressed_file(struct inode *inode)
>>>>>>     {
>>>>>>     	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>>>     	if (!f2fs_compressed_file(inode))
>>>>>>     		return true;
>>>>>> -	if (S_ISREG(inode->i_mode) && F2FS_HAS_BLOCKS(inode))
>>>>>> +	if (inode_has_data(inode))
>>>>>>     		return false;
>>>>>>     	fi->i_flags &= ~F2FS_COMPR_FL;
>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>> index 1a3c29a9a6a0..8af4b29c3e1a 100644
>>>>>> --- a/fs/f2fs/file.c
>>>>>> +++ b/fs/f2fs/file.c
>>>>>> @@ -1922,8 +1922,7 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>>>>>>     			f2fs_down_write(&F2FS_I(inode)->i_sem);
>>>>>>     			if (!f2fs_may_compress(inode) ||
>>>>>> -					(S_ISREG(inode->i_mode) &&
>>>>>> -					F2FS_HAS_BLOCKS(inode))) {
>>>>>> +					inode_has_data(inode)) {
>>>>>>     				f2fs_up_write(&F2FS_I(inode)->i_sem);
>>>>>>     				return -EINVAL;
>>>>>>     			}
>>>>>> @@ -3996,7 +3995,7 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
>>>>>>     		goto out;
>>>>>>     	}
>>>>>> -	if (F2FS_HAS_BLOCKS(inode)) {
>>>>>> +	if (inode_has_data(inode)) {
>>>>>>     		ret = -EFBIG;
>>>>>>     		goto out;
>>>>>>     	}
>>>>>> -- 
>>>>>> 2.40.1
diff mbox series

Patch

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 65294e3b0bef..c9b8a1953913 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4397,13 +4397,19 @@  static inline int set_compress_context(struct inode *inode)
 #endif
 }
 
+static inline bool inode_has_data(struct inode *inode)
+{
+	return (S_ISREG(inode->i_mode) &&
+		(F2FS_HAS_BLOCKS(inode) || i_size_read(inode)));
+}
+
 static inline bool f2fs_disable_compressed_file(struct inode *inode)
 {
 	struct f2fs_inode_info *fi = F2FS_I(inode);
 
 	if (!f2fs_compressed_file(inode))
 		return true;
-	if (S_ISREG(inode->i_mode) && F2FS_HAS_BLOCKS(inode))
+	if (inode_has_data(inode))
 		return false;
 
 	fi->i_flags &= ~F2FS_COMPR_FL;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 1a3c29a9a6a0..8af4b29c3e1a 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1922,8 +1922,7 @@  static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
 
 			f2fs_down_write(&F2FS_I(inode)->i_sem);
 			if (!f2fs_may_compress(inode) ||
-					(S_ISREG(inode->i_mode) &&
-					F2FS_HAS_BLOCKS(inode))) {
+					inode_has_data(inode)) {
 				f2fs_up_write(&F2FS_I(inode)->i_sem);
 				return -EINVAL;
 			}
@@ -3996,7 +3995,7 @@  static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
 		goto out;
 	}
 
-	if (F2FS_HAS_BLOCKS(inode)) {
+	if (inode_has_data(inode)) {
 		ret = -EFBIG;
 		goto out;
 	}