[v2] f2fs: fix to set noatime and immutable flag for quota file

Message ID 20230525082508.2320763-1-chao@kernel.org
State New
Headers
Series [v2] f2fs: fix to set noatime and immutable flag for quota file |

Commit Message

Chao Yu May 25, 2023, 8:25 a.m. UTC
  We should set noatime bit for quota files, since no one cares about
atime of quota file, and we should set immutalbe bit as well, due to
nobody should write to the file through exported interfaces.

Meanwhile this patch use inode_lock to avoid race condition during
inode->i_flags, f2fs_inode->i_flags update.

Signed-off-by: Chao Yu <chao@kernel.org>
---
v2:
- fix to detect i_flags status correctly.
 fs/f2fs/super.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Jaegeuk Kim May 26, 2023, 5:05 p.m. UTC | #1
On 05/25, Chao Yu wrote:
> We should set noatime bit for quota files, since no one cares about
> atime of quota file, and we should set immutalbe bit as well, due to
> nobody should write to the file through exported interfaces.
> 
> Meanwhile this patch use inode_lock to avoid race condition during
> inode->i_flags, f2fs_inode->i_flags update.
> 
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> v2:
> - fix to detect i_flags status correctly.
>  fs/f2fs/super.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 51812f459581..1cf84c993d7c 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2763,7 +2763,17 @@ static int f2fs_quota_enable(struct super_block *sb, int type, int format_id,
>  	}
>  
>  	/* Don't account quota for quota files to avoid recursion */
> +	inode_lock(qf_inode);
>  	qf_inode->i_flags |= S_NOQUOTA;
> +
> +	if (!(F2FS_I(qf_inode)->i_flags & F2FS_NOATIME_FL) ||
> +		!(F2FS_I(qf_inode)->i_flags & F2FS_IMMUTABLE_FL)) {

What about this?

if ((F2FS_I(qf_inode)->i_flags & qf_flag) != qf_flag)

> +		F2FS_I(qf_inode)->i_flags |=
> +				F2FS_NOATIME_FL | F2FS_IMMUTABLE_FL;
> +		f2fs_set_inode_flags(qf_inode);
> +	}
> +	inode_unlock(qf_inode);
> +
>  	err = dquot_load_quota_inode(qf_inode, type, format_id, flags);
>  	iput(qf_inode);
>  	return err;
> -- 
> 2.40.1
  
Chao Yu May 27, 2023, 12:16 a.m. UTC | #2
On 2023/5/27 1:05, Jaegeuk Kim wrote:
> On 05/25, Chao Yu wrote:
>> We should set noatime bit for quota files, since no one cares about
>> atime of quota file, and we should set immutalbe bit as well, due to
>> nobody should write to the file through exported interfaces.
>>
>> Meanwhile this patch use inode_lock to avoid race condition during
>> inode->i_flags, f2fs_inode->i_flags update.
>>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> v2:
>> - fix to detect i_flags status correctly.
>>   fs/f2fs/super.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 51812f459581..1cf84c993d7c 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -2763,7 +2763,17 @@ static int f2fs_quota_enable(struct super_block *sb, int type, int format_id,
>>   	}
>>   
>>   	/* Don't account quota for quota files to avoid recursion */
>> +	inode_lock(qf_inode);
>>   	qf_inode->i_flags |= S_NOQUOTA;
>> +
>> +	if (!(F2FS_I(qf_inode)->i_flags & F2FS_NOATIME_FL) ||
>> +		!(F2FS_I(qf_inode)->i_flags & F2FS_IMMUTABLE_FL)) {
> 
> What about this?
> 
> if ((F2FS_I(qf_inode)->i_flags & qf_flag) != qf_flag)

Ah, better, and updated in v3.

Thanks,

> 
>> +		F2FS_I(qf_inode)->i_flags |=
>> +				F2FS_NOATIME_FL | F2FS_IMMUTABLE_FL;
>> +		f2fs_set_inode_flags(qf_inode);
>> +	}
>> +	inode_unlock(qf_inode);
>> +
>>   	err = dquot_load_quota_inode(qf_inode, type, format_id, flags);
>>   	iput(qf_inode);
>>   	return err;
>> -- 
>> 2.40.1
  

Patch

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 51812f459581..1cf84c993d7c 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2763,7 +2763,17 @@  static int f2fs_quota_enable(struct super_block *sb, int type, int format_id,
 	}
 
 	/* Don't account quota for quota files to avoid recursion */
+	inode_lock(qf_inode);
 	qf_inode->i_flags |= S_NOQUOTA;
+
+	if (!(F2FS_I(qf_inode)->i_flags & F2FS_NOATIME_FL) ||
+		!(F2FS_I(qf_inode)->i_flags & F2FS_IMMUTABLE_FL)) {
+		F2FS_I(qf_inode)->i_flags |=
+				F2FS_NOATIME_FL | F2FS_IMMUTABLE_FL;
+		f2fs_set_inode_flags(qf_inode);
+	}
+	inode_unlock(qf_inode);
+
 	err = dquot_load_quota_inode(qf_inode, type, format_id, flags);
 	iput(qf_inode);
 	return err;