linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] f2fs: separate NOCoW and pinfile semantics
@ 2022-05-12  8:21 Chao Yu
  2022-05-12 21:57 ` Jaegeuk Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2022-05-12  8:21 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, Chao Yu

Pinning a file is heavy, because skipping pinned files make GC
running with heavy load or no effect.

So that this patch proposes to separate nocow and pinfile semantics:
- NOCoW flag can only be set on regular file.
- NOCoW file will only trigger IPU at common writeback/flush.
- NOCow file will do OPU during GC.

This flag can satisfying the demand of:
1) avoiding fragment of file's physical block
2) userspace doesn't want to pin file's physical address

Signed-off-by: Chao Yu <chao.yu@oppo.com>
---
 fs/f2fs/data.c |  3 ++-
 fs/f2fs/f2fs.h |  1 +
 fs/f2fs/file.c | 25 ++++++++++++++++++++++++-
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 54a7a8ad994d..c8eab78f7d89 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2495,7 +2495,8 @@ bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio)
 	if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
 		return false;
 
-	if (f2fs_is_pinned_file(inode))
+	if (f2fs_is_pinned_file(inode) ||
+			F2FS_I(inode)->i_flags & F2FS_NOCOW_FL)
 		return true;
 
 	/* if this is cold file, we should overwrite to avoid fragmentation */
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 492af5b96de1..e91ece55f5e8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2916,6 +2916,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
 #define F2FS_NOCOMP_FL			0x00000400 /* Don't compress */
 #define F2FS_INDEX_FL			0x00001000 /* hash-indexed directory */
 #define F2FS_DIRSYNC_FL			0x00010000 /* dirsync behaviour (directories only) */
+#define F2FS_NOCOW_FL			0x00800000 /* Do not cow file */
 #define F2FS_PROJINHERIT_FL		0x20000000 /* Create with parents projid */
 #define F2FS_CASEFOLD_FL		0x40000000 /* Casefolded file */
 
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 09287876dbb7..7f92a3a157f7 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1851,6 +1851,20 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
 	if (IS_NOQUOTA(inode))
 		return -EPERM;
 
+	if ((iflags ^ masked_flags) & F2FS_NOCOW_FL) {
+		int ret;
+
+		if (!S_ISREG(inode->i_mode))
+			return -EINVAL;
+		if (f2fs_should_update_outplace(inode, NULL))
+			return -EINVAL;
+		if (f2fs_is_pinned_file(inode))
+			return -EINVAL;
+		ret = f2fs_convert_inline_inode(inode);
+		if (ret)
+			return ret;
+	}
+
 	if ((iflags ^ masked_flags) & F2FS_CASEFOLD_FL) {
 		if (!f2fs_sb_has_casefold(F2FS_I_SB(inode)))
 			return -EOPNOTSUPP;
@@ -1926,6 +1940,7 @@ static const struct {
 	{ F2FS_NOCOMP_FL,	FS_NOCOMP_FL },
 	{ F2FS_INDEX_FL,	FS_INDEX_FL },
 	{ F2FS_DIRSYNC_FL,	FS_DIRSYNC_FL },
+	{ F2FS_NOCOW_FL,	FS_NOCOW_FL },
 	{ F2FS_PROJINHERIT_FL,	FS_PROJINHERIT_FL },
 	{ F2FS_CASEFOLD_FL,	FS_CASEFOLD_FL },
 };
@@ -1957,7 +1972,8 @@ static const struct {
 		FS_NOCOMP_FL |		\
 		FS_DIRSYNC_FL |		\
 		FS_PROJINHERIT_FL |	\
-		FS_CASEFOLD_FL)
+		FS_CASEFOLD_FL |	\
+		FS_NOCOW_FL)
 
 /* Convert f2fs on-disk i_flags to FS_IOC_{GET,SET}FLAGS flags */
 static inline u32 f2fs_iflags_to_fsflags(u32 iflags)
@@ -3081,6 +3097,13 @@ static int f2fs_ioc_set_pin_file(struct file *filp, unsigned long arg)
 
 	inode_lock(inode);
 
+	if (F2FS_I(inode)->i_flags & F2FS_NOCOW_FL) {
+		f2fs_info(F2FS_I_SB(inode), "inode (%lu) is already NOCOW one",
+			inode->i_ino);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	if (!pin) {
 		clear_inode_flag(inode, FI_PIN_FILE);
 		f2fs_i_gc_failures_write(inode, 0);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] f2fs: separate NOCoW and pinfile semantics
  2022-05-12  8:21 [PATCH] f2fs: separate NOCoW and pinfile semantics Chao Yu
@ 2022-05-12 21:57 ` Jaegeuk Kim
  2022-05-13  3:20   ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Jaegeuk Kim @ 2022-05-12 21:57 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

On 05/12, Chao Yu wrote:
> Pinning a file is heavy, because skipping pinned files make GC
> running with heavy load or no effect.
> 
> So that this patch proposes to separate nocow and pinfile semantics:
> - NOCoW flag can only be set on regular file.
> - NOCoW file will only trigger IPU at common writeback/flush.
> - NOCow file will do OPU during GC.
> 
> This flag can satisfying the demand of:
> 1) avoiding fragment of file's physical block
> 2) userspace doesn't want to pin file's physical address
> 
> Signed-off-by: Chao Yu <chao.yu@oppo.com>
> ---
>  fs/f2fs/data.c |  3 ++-
>  fs/f2fs/f2fs.h |  1 +
>  fs/f2fs/file.c | 25 ++++++++++++++++++++++++-
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 54a7a8ad994d..c8eab78f7d89 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2495,7 +2495,8 @@ bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio)
>  	if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
>  		return false;
>  
> -	if (f2fs_is_pinned_file(inode))
> +	if (f2fs_is_pinned_file(inode) ||
> +			F2FS_I(inode)->i_flags & F2FS_NOCOW_FL)
>  		return true;
>  
>  	/* if this is cold file, we should overwrite to avoid fragmentation */
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 492af5b96de1..e91ece55f5e8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2916,6 +2916,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
>  #define F2FS_NOCOMP_FL			0x00000400 /* Don't compress */
>  #define F2FS_INDEX_FL			0x00001000 /* hash-indexed directory */
>  #define F2FS_DIRSYNC_FL			0x00010000 /* dirsync behaviour (directories only) */
> +#define F2FS_NOCOW_FL			0x00800000 /* Do not cow file */
>  #define F2FS_PROJINHERIT_FL		0x20000000 /* Create with parents projid */
>  #define F2FS_CASEFOLD_FL		0x40000000 /* Casefolded file */
>  
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 09287876dbb7..7f92a3a157f7 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1851,6 +1851,20 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>  	if (IS_NOQUOTA(inode))
>  		return -EPERM;
>  
> +	if ((iflags ^ masked_flags) & F2FS_NOCOW_FL) {
> +		int ret;
> +
> +		if (!S_ISREG(inode->i_mode))
> +			return -EINVAL;
> +		if (f2fs_should_update_outplace(inode, NULL))
> +			return -EINVAL;
> +		if (f2fs_is_pinned_file(inode))
> +			return -EINVAL;
> +		ret = f2fs_convert_inline_inode(inode);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if ((iflags ^ masked_flags) & F2FS_CASEFOLD_FL) {
>  		if (!f2fs_sb_has_casefold(F2FS_I_SB(inode)))
>  			return -EOPNOTSUPP;
> @@ -1926,6 +1940,7 @@ static const struct {
>  	{ F2FS_NOCOMP_FL,	FS_NOCOMP_FL },
>  	{ F2FS_INDEX_FL,	FS_INDEX_FL },
>  	{ F2FS_DIRSYNC_FL,	FS_DIRSYNC_FL },
> +	{ F2FS_NOCOW_FL,	FS_NOCOW_FL },
>  	{ F2FS_PROJINHERIT_FL,	FS_PROJINHERIT_FL },
>  	{ F2FS_CASEFOLD_FL,	FS_CASEFOLD_FL },
>  };
> @@ -1957,7 +1972,8 @@ static const struct {
>  		FS_NOCOMP_FL |		\
>  		FS_DIRSYNC_FL |		\
>  		FS_PROJINHERIT_FL |	\
> -		FS_CASEFOLD_FL)
> +		FS_CASEFOLD_FL |	\
> +		FS_NOCOW_FL)
>  
>  /* Convert f2fs on-disk i_flags to FS_IOC_{GET,SET}FLAGS flags */
>  static inline u32 f2fs_iflags_to_fsflags(u32 iflags)
> @@ -3081,6 +3097,13 @@ static int f2fs_ioc_set_pin_file(struct file *filp, unsigned long arg)
>  
>  	inode_lock(inode);
>  
> +	if (F2FS_I(inode)->i_flags & F2FS_NOCOW_FL) {
> +		f2fs_info(F2FS_I_SB(inode), "inode (%lu) is already NOCOW one",
> +			inode->i_ino);
> +		ret = -EINVAL;

Why rejecting this? We can pin the file to get 2MB-aligned allocation on the
NOCOW file.

> +		goto out;
> +	}
> +
>  	if (!pin) {
>  		clear_inode_flag(inode, FI_PIN_FILE);
>  		f2fs_i_gc_failures_write(inode, 0);
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] f2fs: separate NOCoW and pinfile semantics
  2022-05-12 21:57 ` Jaegeuk Kim
@ 2022-05-13  3:20   ` Chao Yu
  2022-05-13 17:58     ` Jaegeuk Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2022-05-13  3:20 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

On 2022/5/13 5:57, Jaegeuk Kim wrote:
> On 05/12, Chao Yu wrote:
>> Pinning a file is heavy, because skipping pinned files make GC
>> running with heavy load or no effect.
>>
>> So that this patch proposes to separate nocow and pinfile semantics:
>> - NOCoW flag can only be set on regular file.
>> - NOCoW file will only trigger IPU at common writeback/flush.
>> - NOCow file will do OPU during GC.
>>
>> This flag can satisfying the demand of:
>> 1) avoiding fragment of file's physical block
>> 2) userspace doesn't want to pin file's physical address
>>
>> Signed-off-by: Chao Yu <chao.yu@oppo.com>
>> ---
>>   fs/f2fs/data.c |  3 ++-
>>   fs/f2fs/f2fs.h |  1 +
>>   fs/f2fs/file.c | 25 ++++++++++++++++++++++++-
>>   3 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 54a7a8ad994d..c8eab78f7d89 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -2495,7 +2495,8 @@ bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio)
>>   	if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
>>   		return false;
>>   
>> -	if (f2fs_is_pinned_file(inode))
>> +	if (f2fs_is_pinned_file(inode) ||
>> +			F2FS_I(inode)->i_flags & F2FS_NOCOW_FL)
>>   		return true;
>>   
>>   	/* if this is cold file, we should overwrite to avoid fragmentation */
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 492af5b96de1..e91ece55f5e8 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -2916,6 +2916,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
>>   #define F2FS_NOCOMP_FL			0x00000400 /* Don't compress */
>>   #define F2FS_INDEX_FL			0x00001000 /* hash-indexed directory */
>>   #define F2FS_DIRSYNC_FL			0x00010000 /* dirsync behaviour (directories only) */
>> +#define F2FS_NOCOW_FL			0x00800000 /* Do not cow file */
>>   #define F2FS_PROJINHERIT_FL		0x20000000 /* Create with parents projid */
>>   #define F2FS_CASEFOLD_FL		0x40000000 /* Casefolded file */
>>   
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 09287876dbb7..7f92a3a157f7 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -1851,6 +1851,20 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>>   	if (IS_NOQUOTA(inode))
>>   		return -EPERM;
>>   
>> +	if ((iflags ^ masked_flags) & F2FS_NOCOW_FL) {
>> +		int ret;
>> +
>> +		if (!S_ISREG(inode->i_mode))
>> +			return -EINVAL;
>> +		if (f2fs_should_update_outplace(inode, NULL))
>> +			return -EINVAL;
>> +		if (f2fs_is_pinned_file(inode))
>> +			return -EINVAL;
>> +		ret = f2fs_convert_inline_inode(inode);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>>   	if ((iflags ^ masked_flags) & F2FS_CASEFOLD_FL) {
>>   		if (!f2fs_sb_has_casefold(F2FS_I_SB(inode)))
>>   			return -EOPNOTSUPP;
>> @@ -1926,6 +1940,7 @@ static const struct {
>>   	{ F2FS_NOCOMP_FL,	FS_NOCOMP_FL },
>>   	{ F2FS_INDEX_FL,	FS_INDEX_FL },
>>   	{ F2FS_DIRSYNC_FL,	FS_DIRSYNC_FL },
>> +	{ F2FS_NOCOW_FL,	FS_NOCOW_FL },
>>   	{ F2FS_PROJINHERIT_FL,	FS_PROJINHERIT_FL },
>>   	{ F2FS_CASEFOLD_FL,	FS_CASEFOLD_FL },
>>   };
>> @@ -1957,7 +1972,8 @@ static const struct {
>>   		FS_NOCOMP_FL |		\
>>   		FS_DIRSYNC_FL |		\
>>   		FS_PROJINHERIT_FL |	\
>> -		FS_CASEFOLD_FL)
>> +		FS_CASEFOLD_FL |	\
>> +		FS_NOCOW_FL)
>>   
>>   /* Convert f2fs on-disk i_flags to FS_IOC_{GET,SET}FLAGS flags */
>>   static inline u32 f2fs_iflags_to_fsflags(u32 iflags)
>> @@ -3081,6 +3097,13 @@ static int f2fs_ioc_set_pin_file(struct file *filp, unsigned long arg)
>>   
>>   	inode_lock(inode);
>>   
>> +	if (F2FS_I(inode)->i_flags & F2FS_NOCOW_FL) {
>> +		f2fs_info(F2FS_I_SB(inode), "inode (%lu) is already NOCOW one",
>> +			inode->i_ino);
>> +		ret = -EINVAL;
> 
> Why rejecting this? We can pin the file to get 2MB-aligned allocation on the
> NOCOW file.

I try to separate these two flag completely, but, seems it can't, because after
commit 5d539245cb18 ("f2fs: export FS_NOCOW_FL flag to user"), these two flags
have already been twined closely....

@@ -1651,6 +1651,8 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned long arg)
  		flags |= F2FS_ENCRYPT_FL;
  	if (f2fs_has_inline_data(inode) || f2fs_has_inline_dentry(inode))
  		flags |= F2FS_INLINE_DATA_FL;
+	if (is_inode_flag_set(inode, FI_PIN_FILE))
+		flags |= F2FS_NOCOW_FL;

How about:

f2fs_ioc_set_pin_file/f2fs_fileattr_set logic:
		pinfile			nocow
set		set pinfile | nocow	set nocow
clear		clear pinfile | nocow	clear nocow

Behaviors:
w/ pinfile, w/ nocow:		use pinfile semantics
w/ pinfile, w/o nocow:		use pinfile semantics
w/o pinfile, w/ nocow:		use nocow semantics
w/o pinfile, w/o nocow:		no pinfile or nocow semantics

Thanks,

> 
>> +		goto out;
>> +	}
>> +
>>   	if (!pin) {
>>   		clear_inode_flag(inode, FI_PIN_FILE);
>>   		f2fs_i_gc_failures_write(inode, 0);
>> -- 
>> 2.25.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] f2fs: separate NOCoW and pinfile semantics
  2022-05-13  3:20   ` Chao Yu
@ 2022-05-13 17:58     ` Jaegeuk Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Jaegeuk Kim @ 2022-05-13 17:58 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

On 05/13, Chao Yu wrote:
> On 2022/5/13 5:57, Jaegeuk Kim wrote:
> > On 05/12, Chao Yu wrote:
> > > Pinning a file is heavy, because skipping pinned files make GC
> > > running with heavy load or no effect.
> > > 
> > > So that this patch proposes to separate nocow and pinfile semantics:
> > > - NOCoW flag can only be set on regular file.
> > > - NOCoW file will only trigger IPU at common writeback/flush.
> > > - NOCow file will do OPU during GC.
> > > 
> > > This flag can satisfying the demand of:
> > > 1) avoiding fragment of file's physical block
> > > 2) userspace doesn't want to pin file's physical address
> > > 
> > > Signed-off-by: Chao Yu <chao.yu@oppo.com>
> > > ---
> > >   fs/f2fs/data.c |  3 ++-
> > >   fs/f2fs/f2fs.h |  1 +
> > >   fs/f2fs/file.c | 25 ++++++++++++++++++++++++-
> > >   3 files changed, 27 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index 54a7a8ad994d..c8eab78f7d89 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -2495,7 +2495,8 @@ bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio)
> > >   	if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
> > >   		return false;
> > > -	if (f2fs_is_pinned_file(inode))
> > > +	if (f2fs_is_pinned_file(inode) ||
> > > +			F2FS_I(inode)->i_flags & F2FS_NOCOW_FL)
> > >   		return true;
> > >   	/* if this is cold file, we should overwrite to avoid fragmentation */
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 492af5b96de1..e91ece55f5e8 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -2916,6 +2916,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
> > >   #define F2FS_NOCOMP_FL			0x00000400 /* Don't compress */
> > >   #define F2FS_INDEX_FL			0x00001000 /* hash-indexed directory */
> > >   #define F2FS_DIRSYNC_FL			0x00010000 /* dirsync behaviour (directories only) */
> > > +#define F2FS_NOCOW_FL			0x00800000 /* Do not cow file */
> > >   #define F2FS_PROJINHERIT_FL		0x20000000 /* Create with parents projid */
> > >   #define F2FS_CASEFOLD_FL		0x40000000 /* Casefolded file */
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index 09287876dbb7..7f92a3a157f7 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -1851,6 +1851,20 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
> > >   	if (IS_NOQUOTA(inode))
> > >   		return -EPERM;
> > > +	if ((iflags ^ masked_flags) & F2FS_NOCOW_FL) {
> > > +		int ret;
> > > +
> > > +		if (!S_ISREG(inode->i_mode))
> > > +			return -EINVAL;
> > > +		if (f2fs_should_update_outplace(inode, NULL))
> > > +			return -EINVAL;
> > > +		if (f2fs_is_pinned_file(inode))
> > > +			return -EINVAL;
> > > +		ret = f2fs_convert_inline_inode(inode);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > >   	if ((iflags ^ masked_flags) & F2FS_CASEFOLD_FL) {
> > >   		if (!f2fs_sb_has_casefold(F2FS_I_SB(inode)))
> > >   			return -EOPNOTSUPP;
> > > @@ -1926,6 +1940,7 @@ static const struct {
> > >   	{ F2FS_NOCOMP_FL,	FS_NOCOMP_FL },
> > >   	{ F2FS_INDEX_FL,	FS_INDEX_FL },
> > >   	{ F2FS_DIRSYNC_FL,	FS_DIRSYNC_FL },
> > > +	{ F2FS_NOCOW_FL,	FS_NOCOW_FL },
> > >   	{ F2FS_PROJINHERIT_FL,	FS_PROJINHERIT_FL },
> > >   	{ F2FS_CASEFOLD_FL,	FS_CASEFOLD_FL },
> > >   };
> > > @@ -1957,7 +1972,8 @@ static const struct {
> > >   		FS_NOCOMP_FL |		\
> > >   		FS_DIRSYNC_FL |		\
> > >   		FS_PROJINHERIT_FL |	\
> > > -		FS_CASEFOLD_FL)
> > > +		FS_CASEFOLD_FL |	\
> > > +		FS_NOCOW_FL)
> > >   /* Convert f2fs on-disk i_flags to FS_IOC_{GET,SET}FLAGS flags */
> > >   static inline u32 f2fs_iflags_to_fsflags(u32 iflags)
> > > @@ -3081,6 +3097,13 @@ static int f2fs_ioc_set_pin_file(struct file *filp, unsigned long arg)
> > >   	inode_lock(inode);
> > > +	if (F2FS_I(inode)->i_flags & F2FS_NOCOW_FL) {
> > > +		f2fs_info(F2FS_I_SB(inode), "inode (%lu) is already NOCOW one",
> > > +			inode->i_ino);
> > > +		ret = -EINVAL;
> > 
> > Why rejecting this? We can pin the file to get 2MB-aligned allocation on the
> > NOCOW file.
> 
> I try to separate these two flag completely, but, seems it can't, because after
> commit 5d539245cb18 ("f2fs: export FS_NOCOW_FL flag to user"), these two flags
> have already been twined closely....
> 
> @@ -1651,6 +1651,8 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned long arg)
>  		flags |= F2FS_ENCRYPT_FL;
>  	if (f2fs_has_inline_data(inode) || f2fs_has_inline_dentry(inode))
>  		flags |= F2FS_INLINE_DATA_FL;
> +	if (is_inode_flag_set(inode, FI_PIN_FILE))
> +		flags |= F2FS_NOCOW_FL;
> 
> How about:
> 
> f2fs_ioc_set_pin_file/f2fs_fileattr_set logic:
> 		pinfile			nocow
> set		set pinfile | nocow	set nocow
> clear		clear pinfile | nocow	clear nocow
> 
> Behaviors:
> w/ pinfile, w/ nocow:		use pinfile semantics
> w/ pinfile, w/o nocow:		use pinfile semantics
> w/o pinfile, w/ nocow:		use nocow semantics
> w/o pinfile, w/o nocow:		no pinfile or nocow semantics

This looks good to me. Thanks,

> 
> Thanks,
> 
> > 
> > > +		goto out;
> > > +	}
> > > +
> > >   	if (!pin) {
> > >   		clear_inode_flag(inode, FI_PIN_FILE);
> > >   		f2fs_i_gc_failures_write(inode, 0);
> > > -- 
> > > 2.25.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] f2fs: separate NOCoW and pinfile semantics
@ 2019-07-18 10:24 Chao Yu
  0 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2019-07-18 10:24 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Pinning a file is heavy, because skipping pinned files make GC
running with heavy load or no effect.

So that this patch propose to separate nocow and pinfile semantics:
- NOCoW flag can only be set on regular file.
- NOCoW file will only trigger IPU at common writeback/flush.
- NOCow file will do OPU during GC.

For the demand of 1) avoid fragment of file's physical block and
2) userspace don't care about file's specific physical address,
tagging file as NOCoW will be cheaper than pinned one.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/data.c |  3 ++-
 fs/f2fs/f2fs.h |  1 +
 fs/f2fs/file.c | 20 +++++++++++++++++---
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a2a28bb269bf..15fb8954c363 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1884,7 +1884,8 @@ static inline bool check_inplace_update_policy(struct inode *inode,
 
 bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio)
 {
-	if (f2fs_is_pinned_file(inode))
+	if (f2fs_is_pinned_file(inode) ||
+			F2FS_I(inode)->i_flags & F2FS_NOCOW_FL)
 		return true;
 
 	/* if this is cold file, we should overwrite to avoid fragmentation */
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 596ab3e1dd7b..f6c5a3d2e659 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2374,6 +2374,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
 #define F2FS_NOATIME_FL			0x00000080 /* do not update atime */
 #define F2FS_INDEX_FL			0x00001000 /* hash-indexed directory */
 #define F2FS_DIRSYNC_FL			0x00010000 /* dirsync behaviour (directories only) */
+#define F2FS_NOCOW_FL			0x00800000 /* Do not cow file */
 #define F2FS_PROJINHERIT_FL		0x20000000 /* Create with parents projid */
 
 /* Flags that should be inherited by new inodes from their parent. */
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 7ca545874060..ed6556f4cba7 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1659,6 +1659,20 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
 	if (IS_NOQUOTA(inode))
 		return -EPERM;
 
+	if ((iflags ^ oldflags) & F2FS_NOCOW_FL) {
+		int err;
+
+		if (!S_ISREG(inode->i_mode))
+			return -EINVAL;
+
+		if (f2fs_should_update_outplace(inode, NULL))
+			return -EINVAL;
+
+		err = f2fs_convert_inline_inode(inode);
+		if (err)
+			return err;
+	}
+
 	fi->i_flags = iflags | (fi->i_flags & ~mask);
 
 	if (fi->i_flags & F2FS_PROJINHERIT_FL)
@@ -1692,6 +1706,7 @@ static const struct {
 	{ F2FS_NOATIME_FL,	FS_NOATIME_FL },
 	{ F2FS_INDEX_FL,	FS_INDEX_FL },
 	{ F2FS_DIRSYNC_FL,	FS_DIRSYNC_FL },
+	{ F2FS_NOCOW_FL,	FS_NOCOW_FL },
 	{ F2FS_PROJINHERIT_FL,	FS_PROJINHERIT_FL },
 };
 
@@ -1715,7 +1730,8 @@ static const struct {
 		FS_NODUMP_FL |		\
 		FS_NOATIME_FL |		\
 		FS_DIRSYNC_FL |		\
-		FS_PROJINHERIT_FL)
+		FS_PROJINHERIT_FL |	\
+		FS_NOCOW_FL)
 
 /* Convert f2fs on-disk i_flags to FS_IOC_{GET,SET}FLAGS flags */
 static inline u32 f2fs_iflags_to_fsflags(u32 iflags)
@@ -1753,8 +1769,6 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned long arg)
 		fsflags |= FS_ENCRYPT_FL;
 	if (f2fs_has_inline_data(inode) || f2fs_has_inline_dentry(inode))
 		fsflags |= FS_INLINE_DATA_FL;
-	if (is_inode_flag_set(inode, FI_PIN_FILE))
-		fsflags |= FS_NOCOW_FL;
 
 	fsflags &= F2FS_GETTABLE_FS_FL;
 
-- 
2.18.0.rc1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-05-13 17:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12  8:21 [PATCH] f2fs: separate NOCoW and pinfile semantics Chao Yu
2022-05-12 21:57 ` Jaegeuk Kim
2022-05-13  3:20   ` Chao Yu
2022-05-13 17:58     ` Jaegeuk Kim
  -- strict thread matches above, loose matches on Subject: below --
2019-07-18 10:24 Chao Yu

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).