[PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

In this patch, we add a new sysfs interface, we can use it to gradually achieve
the reserved_blocks finally, even when reserved_blocks is initially set over
user_block_count - total_valid_block_count. This is very useful, especially when
we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
With this patch, f2fs can try its best to reserve space and get close to the
reserved_blocks, and the current value of achieved reserved_blocks can be shown
in real time.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
 fs/f2fs/f2fs.h                          |  9 +++++++--
 fs/f2fs/super.c                         |  4 +++-
 fs/f2fs/sysfs.c                         | 15 ++++++++++-----
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 11b7f4e..bdbb9f3 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -151,3 +151,9 @@ Date:		August 2017
 Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
 Description:
 		 Controls sleep time of GC urgent mode
+
+What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
+Date:		August 2017
+Contact:	"Yunlong Song" <yunlong.song@huawei.com>
+Description:
+		 Shows current reserved blocks in system.
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index cea329f..3b7056f 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1040,6 +1040,7 @@ struct f2fs_sb_info {
 	block_t discard_blks;			/* discard command candidats */
 	block_t last_valid_block_count;		/* for recovery */
 	block_t reserved_blocks;		/* configurable reserved blocks */
+	block_t cur_reserved_blocks;		/* current reserved blocks */
 
 	u32 s_next_generation;			/* for NFS support */
 
@@ -1514,7 +1515,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
 
 	spin_lock(&sbi->stat_lock);
 	sbi->total_valid_block_count += (block_t)(*count);
-	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
+	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
 	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
 		diff = sbi->total_valid_block_count - avail_user_block_count;
 		*count -= diff;
@@ -1548,6 +1549,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
 	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
 	f2fs_bug_on(sbi, inode->i_blocks < sectors);
 	sbi->total_valid_block_count -= (block_t)count;
+	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
+									sbi->cur_reserved_blocks + count);
 	spin_unlock(&sbi->stat_lock);
 	f2fs_i_blocks_write(inode, count, false, true);
 }
@@ -1694,7 +1697,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
 	spin_lock(&sbi->stat_lock);
 
 	valid_block_count = sbi->total_valid_block_count + 1;
-	if (unlikely(valid_block_count + sbi->reserved_blocks >
+	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
 						sbi->user_block_count)) {
 		spin_unlock(&sbi->stat_lock);
 		goto enospc;
@@ -1737,6 +1740,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
 
 	sbi->total_valid_node_count--;
 	sbi->total_valid_block_count--;
+	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
+									sbi->cur_reserved_blocks + 1);
 
 	spin_unlock(&sbi->stat_lock);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4c1bdcb..2934aa2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_blocks = total_count - start_count;
 	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
 	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
-						sbi->reserved_blocks;
+						sbi->cur_reserved_blocks;
 
 	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
 
@@ -2411,6 +2411,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 				le64_to_cpu(sbi->ckpt->valid_block_count);
 	sbi->last_valid_block_count = sbi->total_valid_block_count;
 	sbi->reserved_blocks = 0;
+	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
+						sbi->user_block_count - sbi->total_valid_block_count);
 
 	for (i = 0; i < NR_INODE_TYPE; i++) {
 		INIT_LIST_HEAD(&sbi->inode_list[i]);
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index b769a3d..405c13f 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
 			BD_PART_WRITTEN(sbi)));
 }
 
+static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
+		struct f2fs_sb_info *sbi, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
+}
+
 static ssize_t features_show(struct f2fs_attr *a,
 		struct f2fs_sb_info *sbi, char *buf)
 {
@@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 #endif
 	if (a->struct_type == RESERVED_BLOCKS) {
 		spin_lock(&sbi->stat_lock);
-		if ((unsigned long)sbi->total_valid_block_count + t >
-				(unsigned long)sbi->user_block_count) {
-			spin_unlock(&sbi->stat_lock);
-			return -EINVAL;
-		}
 		*ui = t;
+		sbi->cur_reserved_blocks = min(t, sbi->user_block_count -
+										sbi->total_valid_block_count);
 		spin_unlock(&sbi->stat_lock);
 		return count;
 	}
@@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 #endif
 F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
 F2FS_GENERAL_RO_ATTR(features);
+F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
 
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
@@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 	ATTR_LIST(lifetime_write_kbytes),
 	ATTR_LIST(features),
 	ATTR_LIST(reserved_blocks),
+	ATTR_LIST(cur_reserved_blocks),
 	NULL,
 };
 
-- 
1.8.5.2

Re: [PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[Chao Yu]

On 2017/8/8 21:43, Yunlong Song wrote:
> In this patch, we add a new sysfs interface, we can use it to gradually achieve
> the reserved_blocks finally, even when reserved_blocks is initially set over
> user_block_count - total_valid_block_count. This is very useful, especially when
> we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
> user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
> With this patch, f2fs can try its best to reserve space and get close to the
> reserved_blocks, and the current value of achieved reserved_blocks can be shown
> in real time.

Oh, this looks like a soft limitation in quota system, but original
reserved_blocks implementation likes a hard one, so this patch changes the
semantics of reserved_blocks.

Actually, I doubt that it would be hard to reserve all left free space in real
user scenario now, since system user's activation may depend on free space of
data partition due to file creation requirement, so w/o supporting feature of
uid/gid reserved block, soft reservation will block any activation of system
user, such as android.

Thanks,

> 
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> ---
>  Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>  fs/f2fs/f2fs.h                          |  9 +++++++--
>  fs/f2fs/super.c                         |  4 +++-
>  fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>  4 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 11b7f4e..bdbb9f3 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -151,3 +151,9 @@ Date:		August 2017
>  Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
>  Description:
>  		 Controls sleep time of GC urgent mode
> +
> +What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
> +Date:		August 2017
> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
> +Description:
> +		 Shows current reserved blocks in system.
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index cea329f..3b7056f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1040,6 +1040,7 @@ struct f2fs_sb_info {
>  	block_t discard_blks;			/* discard command candidats */
>  	block_t last_valid_block_count;		/* for recovery */
>  	block_t reserved_blocks;		/* configurable reserved blocks */
> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>  
>  	u32 s_next_generation;			/* for NFS support */
>  
> @@ -1514,7 +1515,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>  
>  	spin_lock(&sbi->stat_lock);
>  	sbi->total_valid_block_count += (block_t)(*count);
> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> +	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
>  	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>  		diff = sbi->total_valid_block_count - avail_user_block_count;
>  		*count -= diff;
> @@ -1548,6 +1549,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>  	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>  	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>  	sbi->total_valid_block_count -= (block_t)count;
> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
> +									sbi->cur_reserved_blocks + count);
>  	spin_unlock(&sbi->stat_lock);
>  	f2fs_i_blocks_write(inode, count, false, true);
>  }
> @@ -1694,7 +1697,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>  	spin_lock(&sbi->stat_lock);
>  
>  	valid_block_count = sbi->total_valid_block_count + 1;
> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>  						sbi->user_block_count)) {
>  		spin_unlock(&sbi->stat_lock);
>  		goto enospc;
> @@ -1737,6 +1740,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>  
>  	sbi->total_valid_node_count--;
>  	sbi->total_valid_block_count--;
> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
> +									sbi->cur_reserved_blocks + 1);
>  
>  	spin_unlock(&sbi->stat_lock);
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 4c1bdcb..2934aa2 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	buf->f_blocks = total_count - start_count;
>  	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>  	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> -						sbi->reserved_blocks;
> +						sbi->cur_reserved_blocks;
>  
>  	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>  
> @@ -2411,6 +2411,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  				le64_to_cpu(sbi->ckpt->valid_block_count);
>  	sbi->last_valid_block_count = sbi->total_valid_block_count;
>  	sbi->reserved_blocks = 0;
> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
> +						sbi->user_block_count - sbi->total_valid_block_count);
>  
>  	for (i = 0; i < NR_INODE_TYPE; i++) {
>  		INIT_LIST_HEAD(&sbi->inode_list[i]);
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index b769a3d..405c13f 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
>  			BD_PART_WRITTEN(sbi)));
>  }
>  
> +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
> +		struct f2fs_sb_info *sbi, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
> +}
> +
>  static ssize_t features_show(struct f2fs_attr *a,
>  		struct f2fs_sb_info *sbi, char *buf)
>  {
> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>  #endif
>  	if (a->struct_type == RESERVED_BLOCKS) {
>  		spin_lock(&sbi->stat_lock);
> -		if ((unsigned long)sbi->total_valid_block_count + t >
> -				(unsigned long)sbi->user_block_count) {
> -			spin_unlock(&sbi->stat_lock);
> -			return -EINVAL;
> -		}
>  		*ui = t;
> +		sbi->cur_reserved_blocks = min(t, sbi->user_block_count -
> +										sbi->total_valid_block_count);
>  		spin_unlock(&sbi->stat_lock);
>  		return count;
>  	}
> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>  #endif
>  F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>  F2FS_GENERAL_RO_ATTR(features);
> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>  
>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
>  F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>  	ATTR_LIST(lifetime_write_kbytes),
>  	ATTR_LIST(features),
>  	ATTR_LIST(reserved_blocks),
> +	ATTR_LIST(cur_reserved_blocks),
>  	NULL,
>  };
>  
> 

Re: [PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

I think the aim of reserved_blocks function is to leave space for f2fs 
and FTL, so I change it to a
soft version so that it can be used to fit to the data image which does 
not satisfy the hard version,
especially for backward compatibility when updated kernel with new 
default reserved_blocks set
(currently it is initially set 0 as default, but can be set any value 
with my new patch).

As for the uid/gid, does current f2fs space management design consider 
this issue? IMO, I think we
can just ensure the reserved space for file system no matter user/system 
type. Whether the value of
reserved_blocks is OK or not, should not be filesystem's issue. 
Filesystem just provide this interface,
and the upper layer, such as android vold should take care of the value 
of reserved_blocks and make
sure its value is appropriate and will not block any activation of 
system user, if it really happens, android
should change the value dynamically, it is fine, since we make 
reserved_blocks to a soft version.

On 2017/8/10 11:15, Chao Yu wrote:
> On 2017/8/8 21:43, Yunlong Song wrote:
>> In this patch, we add a new sysfs interface, we can use it to gradually achieve
>> the reserved_blocks finally, even when reserved_blocks is initially set over
>> user_block_count - total_valid_block_count. This is very useful, especially when
>> we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
>> user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
>> With this patch, f2fs can try its best to reserve space and get close to the
>> reserved_blocks, and the current value of achieved reserved_blocks can be shown
>> in real time.
> Oh, this looks like a soft limitation in quota system, but original
> reserved_blocks implementation likes a hard one, so this patch changes the
> semantics of reserved_blocks.
>
> Actually, I doubt that it would be hard to reserve all left free space in real
> user scenario now, since system user's activation may depend on free space of
> data partition due to file creation requirement, so w/o supporting feature of
> uid/gid reserved block, soft reservation will block any activation of system
> user, such as android.
>
> Thanks,
>
>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>> ---
>>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>>   fs/f2fs/f2fs.h                          |  9 +++++++--
>>   fs/f2fs/super.c                         |  4 +++-
>>   fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>>   4 files changed, 26 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>> index 11b7f4e..bdbb9f3 100644
>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>> @@ -151,3 +151,9 @@ Date:		August 2017
>>   Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
>>   Description:
>>   		 Controls sleep time of GC urgent mode
>> +
>> +What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
>> +Date:		August 2017
>> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
>> +Description:
>> +		 Shows current reserved blocks in system.
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index cea329f..3b7056f 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1040,6 +1040,7 @@ struct f2fs_sb_info {
>>   	block_t discard_blks;			/* discard command candidats */
>>   	block_t last_valid_block_count;		/* for recovery */
>>   	block_t reserved_blocks;		/* configurable reserved blocks */
>> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>>   
>>   	u32 s_next_generation;			/* for NFS support */
>>   
>> @@ -1514,7 +1515,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>   
>>   	spin_lock(&sbi->stat_lock);
>>   	sbi->total_valid_block_count += (block_t)(*count);
>> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>> +	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
>>   	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>   		diff = sbi->total_valid_block_count - avail_user_block_count;
>>   		*count -= diff;
>> @@ -1548,6 +1549,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>   	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>   	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>   	sbi->total_valid_block_count -= (block_t)count;
>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>> +									sbi->cur_reserved_blocks + count);
>>   	spin_unlock(&sbi->stat_lock);
>>   	f2fs_i_blocks_write(inode, count, false, true);
>>   }
>> @@ -1694,7 +1697,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>   	spin_lock(&sbi->stat_lock);
>>   
>>   	valid_block_count = sbi->total_valid_block_count + 1;
>> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
>> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>   						sbi->user_block_count)) {
>>   		spin_unlock(&sbi->stat_lock);
>>   		goto enospc;
>> @@ -1737,6 +1740,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>   
>>   	sbi->total_valid_node_count--;
>>   	sbi->total_valid_block_count--;
>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>> +									sbi->cur_reserved_blocks + 1);
>>   
>>   	spin_unlock(&sbi->stat_lock);
>>   
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 4c1bdcb..2934aa2 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>   	buf->f_blocks = total_count - start_count;
>>   	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>   	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>> -						sbi->reserved_blocks;
>> +						sbi->cur_reserved_blocks;
>>   
>>   	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>   
>> @@ -2411,6 +2411,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>   				le64_to_cpu(sbi->ckpt->valid_block_count);
>>   	sbi->last_valid_block_count = sbi->total_valid_block_count;
>>   	sbi->reserved_blocks = 0;
>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>> +						sbi->user_block_count - sbi->total_valid_block_count);
>>   
>>   	for (i = 0; i < NR_INODE_TYPE; i++) {
>>   		INIT_LIST_HEAD(&sbi->inode_list[i]);
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index b769a3d..405c13f 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
>>   			BD_PART_WRITTEN(sbi)));
>>   }
>>   
>> +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
>> +		struct f2fs_sb_info *sbi, char *buf)
>> +{
>> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
>> +}
>> +
>>   static ssize_t features_show(struct f2fs_attr *a,
>>   		struct f2fs_sb_info *sbi, char *buf)
>>   {
>> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>   #endif
>>   	if (a->struct_type == RESERVED_BLOCKS) {
>>   		spin_lock(&sbi->stat_lock);
>> -		if ((unsigned long)sbi->total_valid_block_count + t >
>> -				(unsigned long)sbi->user_block_count) {
>> -			spin_unlock(&sbi->stat_lock);
>> -			return -EINVAL;
>> -		}
>>   		*ui = t;
>> +		sbi->cur_reserved_blocks = min(t, sbi->user_block_count -
>> +										sbi->total_valid_block_count);
>>   		spin_unlock(&sbi->stat_lock);
>>   		return count;
>>   	}
>> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>   #endif
>>   F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>>   F2FS_GENERAL_RO_ATTR(features);
>> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>>   
>>   #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>   F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
>> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>   	ATTR_LIST(lifetime_write_kbytes),
>>   	ATTR_LIST(features),
>>   	ATTR_LIST(reserved_blocks),
>> +	ATTR_LIST(cur_reserved_blocks),
>>   	NULL,
>>   };
>>   
>>
>
> .
>

-- 
Thanks,
Yunlong Song

Re: [PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[Chao Yu]

On 2017/8/10 11:58, Yunlong Song wrote:
> I think the aim of reserved_blocks function is to leave space for f2fs 
> and FTL, so I change it to a
> soft version so that it can be used to fit to the data image which does 
> not satisfy the hard version,
> especially for backward compatibility when updated kernel with new 
> default reserved_blocks set
> (currently it is initially set 0 as default, but can be set any value 
> with my new patch).
> 
> As for the uid/gid, does current f2fs space management design consider 
> this issue? IMO, I think we

Upper layer application has considered this issue, as when free space touchs the
threshold, system will block any operation and give a hint to clean up user's
data. So w/o uid/gid reserved block, it will be OK. But for your requirement,
user free space will drop to zero, and before current reserved block number
touch soft reserved block limitation, user can only do deletion, so in this
time, if any system/service flow depends on file creation, could result in
blocking the system.

> can just ensure the reserved space for file system no matter user/system 
> type. Whether the value of
> reserved_blocks is OK or not, should not be filesystem's issue. 
> Filesystem just provide this interface,
> and the upper layer, such as android vold should take care of the value 
> of reserved_blocks and make
> sure its value is appropriate and will not block any activation of 
> system user, if it really happens, android
> should change the value dynamically, it is fine, since we make 
> reserved_blocks to a soft version.

We can not make sure in strategy changing flow, there is no file creation
dependence. Even if we can, if the reserved_blocks is not appropriate, why not
setting it correctly before?

Thanks,

> 
> On 2017/8/10 11:15, Chao Yu wrote:
>> On 2017/8/8 21:43, Yunlong Song wrote:
>>> In this patch, we add a new sysfs interface, we can use it to gradually achieve
>>> the reserved_blocks finally, even when reserved_blocks is initially set over
>>> user_block_count - total_valid_block_count. This is very useful, especially when
>>> we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
>>> user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
>>> With this patch, f2fs can try its best to reserve space and get close to the
>>> reserved_blocks, and the current value of achieved reserved_blocks can be shown
>>> in real time.
>> Oh, this looks like a soft limitation in quota system, but original
>> reserved_blocks implementation likes a hard one, so this patch changes the
>> semantics of reserved_blocks.
>>
>> Actually, I doubt that it would be hard to reserve all left free space in real
>> user scenario now, since system user's activation may depend on free space of
>> data partition due to file creation requirement, so w/o supporting feature of
>> uid/gid reserved block, soft reservation will block any activation of system
>> user, such as android.
>>
>> Thanks,
>>
>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>> ---
>>>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>>>   fs/f2fs/f2fs.h                          |  9 +++++++--
>>>   fs/f2fs/super.c                         |  4 +++-
>>>   fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>>>   4 files changed, 26 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>> index 11b7f4e..bdbb9f3 100644
>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>> @@ -151,3 +151,9 @@ Date:		August 2017
>>>   Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
>>>   Description:
>>>   		 Controls sleep time of GC urgent mode
>>> +
>>> +What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
>>> +Date:		August 2017
>>> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
>>> +Description:
>>> +		 Shows current reserved blocks in system.
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index cea329f..3b7056f 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1040,6 +1040,7 @@ struct f2fs_sb_info {
>>>   	block_t discard_blks;			/* discard command candidats */
>>>   	block_t last_valid_block_count;		/* for recovery */
>>>   	block_t reserved_blocks;		/* configurable reserved blocks */
>>> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>>>   
>>>   	u32 s_next_generation;			/* for NFS support */
>>>   
>>> @@ -1514,7 +1515,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>   
>>>   	spin_lock(&sbi->stat_lock);
>>>   	sbi->total_valid_block_count += (block_t)(*count);
>>> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>> +	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
>>>   	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>   		diff = sbi->total_valid_block_count - avail_user_block_count;
>>>   		*count -= diff;
>>> @@ -1548,6 +1549,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>   	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>   	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>   	sbi->total_valid_block_count -= (block_t)count;
>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>> +									sbi->cur_reserved_blocks + count);
>>>   	spin_unlock(&sbi->stat_lock);
>>>   	f2fs_i_blocks_write(inode, count, false, true);
>>>   }
>>> @@ -1694,7 +1697,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>   	spin_lock(&sbi->stat_lock);
>>>   
>>>   	valid_block_count = sbi->total_valid_block_count + 1;
>>> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
>>> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>   						sbi->user_block_count)) {
>>>   		spin_unlock(&sbi->stat_lock);
>>>   		goto enospc;
>>> @@ -1737,6 +1740,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>   
>>>   	sbi->total_valid_node_count--;
>>>   	sbi->total_valid_block_count--;
>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>> +									sbi->cur_reserved_blocks + 1);
>>>   
>>>   	spin_unlock(&sbi->stat_lock);
>>>   
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 4c1bdcb..2934aa2 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>   	buf->f_blocks = total_count - start_count;
>>>   	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>   	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>> -						sbi->reserved_blocks;
>>> +						sbi->cur_reserved_blocks;
>>>   
>>>   	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>   
>>> @@ -2411,6 +2411,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>   				le64_to_cpu(sbi->ckpt->valid_block_count);
>>>   	sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>   	sbi->reserved_blocks = 0;
>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>> +						sbi->user_block_count - sbi->total_valid_block_count);
>>>   
>>>   	for (i = 0; i < NR_INODE_TYPE; i++) {
>>>   		INIT_LIST_HEAD(&sbi->inode_list[i]);
>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>> index b769a3d..405c13f 100644
>>> --- a/fs/f2fs/sysfs.c
>>> +++ b/fs/f2fs/sysfs.c
>>> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
>>>   			BD_PART_WRITTEN(sbi)));
>>>   }
>>>   
>>> +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
>>> +		struct f2fs_sb_info *sbi, char *buf)
>>> +{
>>> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
>>> +}
>>> +
>>>   static ssize_t features_show(struct f2fs_attr *a,
>>>   		struct f2fs_sb_info *sbi, char *buf)
>>>   {
>>> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>   #endif
>>>   	if (a->struct_type == RESERVED_BLOCKS) {
>>>   		spin_lock(&sbi->stat_lock);
>>> -		if ((unsigned long)sbi->total_valid_block_count + t >
>>> -				(unsigned long)sbi->user_block_count) {
>>> -			spin_unlock(&sbi->stat_lock);
>>> -			return -EINVAL;
>>> -		}
>>>   		*ui = t;
>>> +		sbi->cur_reserved_blocks = min(t, sbi->user_block_count -
>>> +										sbi->total_valid_block_count);
>>>   		spin_unlock(&sbi->stat_lock);
>>>   		return count;
>>>   	}
>>> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>   #endif
>>>   F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>>>   F2FS_GENERAL_RO_ATTR(features);
>>> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>>>   
>>>   #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>>   F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
>>> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>   	ATTR_LIST(lifetime_write_kbytes),
>>>   	ATTR_LIST(features),
>>>   	ATTR_LIST(reserved_blocks),
>>> +	ATTR_LIST(cur_reserved_blocks),
>>>   	NULL,
>>>   };
>>>   
>>>
>>
>> .
>>
> 

Re: [PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

It may block the system in such case, but so does the hard version 
design. In the hard version,
when reserve_blocks is set to the value = user_block_count - 
total_valid_block_count, then
the user free space can also drop to zero and has the same problem. IMO, 
kernel does not need
to take this responsibility, it's android's responsibility.

On 2017/8/10 19:26, Chao Yu wrote:
> On 2017/8/10 11:58, Yunlong Song wrote:
>> I think the aim of reserved_blocks function is to leave space for f2fs
>> and FTL, so I change it to a
>> soft version so that it can be used to fit to the data image which does
>> not satisfy the hard version,
>> especially for backward compatibility when updated kernel with new
>> default reserved_blocks set
>> (currently it is initially set 0 as default, but can be set any value
>> with my new patch).
>>
>> As for the uid/gid, does current f2fs space management design consider
>> this issue? IMO, I think we
> Upper layer application has considered this issue, as when free space touchs the
> threshold, system will block any operation and give a hint to clean up user's
> data. So w/o uid/gid reserved block, it will be OK. But for your requirement,
> user free space will drop to zero, and before current reserved block number
> touch soft reserved block limitation, user can only do deletion, so in this
> time, if any system/service flow depends on file creation, could result in
> blocking the system.
>
>> can just ensure the reserved space for file system no matter user/system
>> type. Whether the value of
>> reserved_blocks is OK or not, should not be filesystem's issue.
>> Filesystem just provide this interface,
>> and the upper layer, such as android vold should take care of the value
>> of reserved_blocks and make
>> sure its value is appropriate and will not block any activation of
>> system user, if it really happens, android
>> should change the value dynamically, it is fine, since we make
>> reserved_blocks to a soft version.
> We can not make sure in strategy changing flow, there is no file creation
> dependence. Even if we can, if the reserved_blocks is not appropriate, why not
> setting it correctly before?
>
> Thanks,
>
>> On 2017/8/10 11:15, Chao Yu wrote:
>>> On 2017/8/8 21:43, Yunlong Song wrote:
>>>> In this patch, we add a new sysfs interface, we can use it to gradually achieve
>>>> the reserved_blocks finally, even when reserved_blocks is initially set over
>>>> user_block_count - total_valid_block_count. This is very useful, especially when
>>>> we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
>>>> user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
>>>> With this patch, f2fs can try its best to reserve space and get close to the
>>>> reserved_blocks, and the current value of achieved reserved_blocks can be shown
>>>> in real time.
>>> Oh, this looks like a soft limitation in quota system, but original
>>> reserved_blocks implementation likes a hard one, so this patch changes the
>>> semantics of reserved_blocks.
>>>
>>> Actually, I doubt that it would be hard to reserve all left free space in real
>>> user scenario now, since system user's activation may depend on free space of
>>> data partition due to file creation requirement, so w/o supporting feature of
>>> uid/gid reserved block, soft reservation will block any activation of system
>>> user, such as android.
>>>
>>> Thanks,
>>>
>>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>>> ---
>>>>    Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>>>>    fs/f2fs/f2fs.h                          |  9 +++++++--
>>>>    fs/f2fs/super.c                         |  4 +++-
>>>>    fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>>>>    4 files changed, 26 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> index 11b7f4e..bdbb9f3 100644
>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> @@ -151,3 +151,9 @@ Date:		August 2017
>>>>    Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
>>>>    Description:
>>>>    		 Controls sleep time of GC urgent mode
>>>> +
>>>> +What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
>>>> +Date:		August 2017
>>>> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
>>>> +Description:
>>>> +		 Shows current reserved blocks in system.
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index cea329f..3b7056f 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -1040,6 +1040,7 @@ struct f2fs_sb_info {
>>>>    	block_t discard_blks;			/* discard command candidats */
>>>>    	block_t last_valid_block_count;		/* for recovery */
>>>>    	block_t reserved_blocks;		/* configurable reserved blocks */
>>>> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>>>>    
>>>>    	u32 s_next_generation;			/* for NFS support */
>>>>    
>>>> @@ -1514,7 +1515,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>>    
>>>>    	spin_lock(&sbi->stat_lock);
>>>>    	sbi->total_valid_block_count += (block_t)(*count);
>>>> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>>> +	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
>>>>    	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>>    		diff = sbi->total_valid_block_count - avail_user_block_count;
>>>>    		*count -= diff;
>>>> @@ -1548,6 +1549,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>>    	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>>    	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>>    	sbi->total_valid_block_count -= (block_t)count;
>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>> +									sbi->cur_reserved_blocks + count);
>>>>    	spin_unlock(&sbi->stat_lock);
>>>>    	f2fs_i_blocks_write(inode, count, false, true);
>>>>    }
>>>> @@ -1694,7 +1697,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>>    	spin_lock(&sbi->stat_lock);
>>>>    
>>>>    	valid_block_count = sbi->total_valid_block_count + 1;
>>>> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
>>>> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>>    						sbi->user_block_count)) {
>>>>    		spin_unlock(&sbi->stat_lock);
>>>>    		goto enospc;
>>>> @@ -1737,6 +1740,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>>    
>>>>    	sbi->total_valid_node_count--;
>>>>    	sbi->total_valid_block_count--;
>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>> +									sbi->cur_reserved_blocks + 1);
>>>>    
>>>>    	spin_unlock(&sbi->stat_lock);
>>>>    
>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>> index 4c1bdcb..2934aa2 100644
>>>> --- a/fs/f2fs/super.c
>>>> +++ b/fs/f2fs/super.c
>>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>>    	buf->f_blocks = total_count - start_count;
>>>>    	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>>    	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>>> -						sbi->reserved_blocks;
>>>> +						sbi->cur_reserved_blocks;
>>>>    
>>>>    	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>>    
>>>> @@ -2411,6 +2411,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>    				le64_to_cpu(sbi->ckpt->valid_block_count);
>>>>    	sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>>    	sbi->reserved_blocks = 0;
>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>> +						sbi->user_block_count - sbi->total_valid_block_count);
>>>>    
>>>>    	for (i = 0; i < NR_INODE_TYPE; i++) {
>>>>    		INIT_LIST_HEAD(&sbi->inode_list[i]);
>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>> index b769a3d..405c13f 100644
>>>> --- a/fs/f2fs/sysfs.c
>>>> +++ b/fs/f2fs/sysfs.c
>>>> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
>>>>    			BD_PART_WRITTEN(sbi)));
>>>>    }
>>>>    
>>>> +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
>>>> +		struct f2fs_sb_info *sbi, char *buf)
>>>> +{
>>>> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
>>>> +}
>>>> +
>>>>    static ssize_t features_show(struct f2fs_attr *a,
>>>>    		struct f2fs_sb_info *sbi, char *buf)
>>>>    {
>>>> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>>    #endif
>>>>    	if (a->struct_type == RESERVED_BLOCKS) {
>>>>    		spin_lock(&sbi->stat_lock);
>>>> -		if ((unsigned long)sbi->total_valid_block_count + t >
>>>> -				(unsigned long)sbi->user_block_count) {
>>>> -			spin_unlock(&sbi->stat_lock);
>>>> -			return -EINVAL;
>>>> -		}
>>>>    		*ui = t;
>>>> +		sbi->cur_reserved_blocks = min(t, sbi->user_block_count -
>>>> +										sbi->total_valid_block_count);
>>>>    		spin_unlock(&sbi->stat_lock);
>>>>    		return count;
>>>>    	}
>>>> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>>    #endif
>>>>    F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>>>>    F2FS_GENERAL_RO_ATTR(features);
>>>> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>>>>    
>>>>    #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>>>    F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
>>>> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>>    	ATTR_LIST(lifetime_write_kbytes),
>>>>    	ATTR_LIST(features),
>>>>    	ATTR_LIST(reserved_blocks),
>>>> +	ATTR_LIST(cur_reserved_blocks),
>>>>    	NULL,
>>>>    };
>>>>    
>>>>
>>> .
>>>
>
> .
>

-- 
Thanks,
Yunlong Song

Re: [PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[Chao Yu]

On 2017/8/10 19:41, Yunlong Song wrote:
> It may block the system in such case, but so does the hard version 
> design. In the hard version,

We'd better not let soft/hard reserved_blocks to eat all left free space, at
least needs to leave enough space for supporting system's activation.

Thanks,

> when reserve_blocks is set to the value = user_block_count - 
> total_valid_block_count, then
> the user free space can also drop to zero and has the same problem. IMO, 
> kernel does not need
> to take this responsibility, it's android's responsibility.
> 
> On 2017/8/10 19:26, Chao Yu wrote:
>> On 2017/8/10 11:58, Yunlong Song wrote:
>>> I think the aim of reserved_blocks function is to leave space for f2fs
>>> and FTL, so I change it to a
>>> soft version so that it can be used to fit to the data image which does
>>> not satisfy the hard version,
>>> especially for backward compatibility when updated kernel with new
>>> default reserved_blocks set
>>> (currently it is initially set 0 as default, but can be set any value
>>> with my new patch).
>>>
>>> As for the uid/gid, does current f2fs space management design consider
>>> this issue? IMO, I think we
>> Upper layer application has considered this issue, as when free space touchs the
>> threshold, system will block any operation and give a hint to clean up user's
>> data. So w/o uid/gid reserved block, it will be OK. But for your requirement,
>> user free space will drop to zero, and before current reserved block number
>> touch soft reserved block limitation, user can only do deletion, so in this
>> time, if any system/service flow depends on file creation, could result in
>> blocking the system.
>>
>>> can just ensure the reserved space for file system no matter user/system
>>> type. Whether the value of
>>> reserved_blocks is OK or not, should not be filesystem's issue.
>>> Filesystem just provide this interface,
>>> and the upper layer, such as android vold should take care of the value
>>> of reserved_blocks and make
>>> sure its value is appropriate and will not block any activation of
>>> system user, if it really happens, android
>>> should change the value dynamically, it is fine, since we make
>>> reserved_blocks to a soft version.
>> We can not make sure in strategy changing flow, there is no file creation
>> dependence. Even if we can, if the reserved_blocks is not appropriate, why not
>> setting it correctly before?
>>
>> Thanks,
>>
>>> On 2017/8/10 11:15, Chao Yu wrote:
>>>> On 2017/8/8 21:43, Yunlong Song wrote:
>>>>> In this patch, we add a new sysfs interface, we can use it to gradually achieve
>>>>> the reserved_blocks finally, even when reserved_blocks is initially set over
>>>>> user_block_count - total_valid_block_count. This is very useful, especially when
>>>>> we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
>>>>> user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
>>>>> With this patch, f2fs can try its best to reserve space and get close to the
>>>>> reserved_blocks, and the current value of achieved reserved_blocks can be shown
>>>>> in real time.
>>>> Oh, this looks like a soft limitation in quota system, but original
>>>> reserved_blocks implementation likes a hard one, so this patch changes the
>>>> semantics of reserved_blocks.
>>>>
>>>> Actually, I doubt that it would be hard to reserve all left free space in real
>>>> user scenario now, since system user's activation may depend on free space of
>>>> data partition due to file creation requirement, so w/o supporting feature of
>>>> uid/gid reserved block, soft reservation will block any activation of system
>>>> user, such as android.
>>>>
>>>> Thanks,
>>>>
>>>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>>>> ---
>>>>>    Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>>>>>    fs/f2fs/f2fs.h                          |  9 +++++++--
>>>>>    fs/f2fs/super.c                         |  4 +++-
>>>>>    fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>>>>>    4 files changed, 26 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>> index 11b7f4e..bdbb9f3 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>> @@ -151,3 +151,9 @@ Date:		August 2017
>>>>>    Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
>>>>>    Description:
>>>>>    		 Controls sleep time of GC urgent mode
>>>>> +
>>>>> +What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
>>>>> +Date:		August 2017
>>>>> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
>>>>> +Description:
>>>>> +		 Shows current reserved blocks in system.
>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>> index cea329f..3b7056f 100644
>>>>> --- a/fs/f2fs/f2fs.h
>>>>> +++ b/fs/f2fs/f2fs.h
>>>>> @@ -1040,6 +1040,7 @@ struct f2fs_sb_info {
>>>>>    	block_t discard_blks;			/* discard command candidats */
>>>>>    	block_t last_valid_block_count;		/* for recovery */
>>>>>    	block_t reserved_blocks;		/* configurable reserved blocks */
>>>>> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>>>>>    
>>>>>    	u32 s_next_generation;			/* for NFS support */
>>>>>    
>>>>> @@ -1514,7 +1515,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>    
>>>>>    	spin_lock(&sbi->stat_lock);
>>>>>    	sbi->total_valid_block_count += (block_t)(*count);
>>>>> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>>>> +	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
>>>>>    	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>>>    		diff = sbi->total_valid_block_count - avail_user_block_count;
>>>>>    		*count -= diff;
>>>>> @@ -1548,6 +1549,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>    	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>>>    	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>>>    	sbi->total_valid_block_count -= (block_t)count;
>>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>> +									sbi->cur_reserved_blocks + count);
>>>>>    	spin_unlock(&sbi->stat_lock);
>>>>>    	f2fs_i_blocks_write(inode, count, false, true);
>>>>>    }
>>>>> @@ -1694,7 +1697,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>    	spin_lock(&sbi->stat_lock);
>>>>>    
>>>>>    	valid_block_count = sbi->total_valid_block_count + 1;
>>>>> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
>>>>> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>>>    						sbi->user_block_count)) {
>>>>>    		spin_unlock(&sbi->stat_lock);
>>>>>    		goto enospc;
>>>>> @@ -1737,6 +1740,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>    
>>>>>    	sbi->total_valid_node_count--;
>>>>>    	sbi->total_valid_block_count--;
>>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>> +									sbi->cur_reserved_blocks + 1);
>>>>>    
>>>>>    	spin_unlock(&sbi->stat_lock);
>>>>>    
>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>> index 4c1bdcb..2934aa2 100644
>>>>> --- a/fs/f2fs/super.c
>>>>> +++ b/fs/f2fs/super.c
>>>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>>>    	buf->f_blocks = total_count - start_count;
>>>>>    	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>>>    	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>>>> -						sbi->reserved_blocks;
>>>>> +						sbi->cur_reserved_blocks;
>>>>>    
>>>>>    	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>>>    
>>>>> @@ -2411,6 +2411,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>>    				le64_to_cpu(sbi->ckpt->valid_block_count);
>>>>>    	sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>>>    	sbi->reserved_blocks = 0;
>>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>> +						sbi->user_block_count - sbi->total_valid_block_count);
>>>>>    
>>>>>    	for (i = 0; i < NR_INODE_TYPE; i++) {
>>>>>    		INIT_LIST_HEAD(&sbi->inode_list[i]);
>>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>>> index b769a3d..405c13f 100644
>>>>> --- a/fs/f2fs/sysfs.c
>>>>> +++ b/fs/f2fs/sysfs.c
>>>>> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
>>>>>    			BD_PART_WRITTEN(sbi)));
>>>>>    }
>>>>>    
>>>>> +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
>>>>> +		struct f2fs_sb_info *sbi, char *buf)
>>>>> +{
>>>>> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
>>>>> +}
>>>>> +
>>>>>    static ssize_t features_show(struct f2fs_attr *a,
>>>>>    		struct f2fs_sb_info *sbi, char *buf)
>>>>>    {
>>>>> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>>>    #endif
>>>>>    	if (a->struct_type == RESERVED_BLOCKS) {
>>>>>    		spin_lock(&sbi->stat_lock);
>>>>> -		if ((unsigned long)sbi->total_valid_block_count + t >
>>>>> -				(unsigned long)sbi->user_block_count) {
>>>>> -			spin_unlock(&sbi->stat_lock);
>>>>> -			return -EINVAL;
>>>>> -		}
>>>>>    		*ui = t;
>>>>> +		sbi->cur_reserved_blocks = min(t, sbi->user_block_count -
>>>>> +										sbi->total_valid_block_count);
>>>>>    		spin_unlock(&sbi->stat_lock);
>>>>>    		return count;
>>>>>    	}
>>>>> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>>>    #endif
>>>>>    F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>>>>>    F2FS_GENERAL_RO_ATTR(features);
>>>>> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>>>>>    
>>>>>    #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>>>>    F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
>>>>> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>>>    	ATTR_LIST(lifetime_write_kbytes),
>>>>>    	ATTR_LIST(features),
>>>>>    	ATTR_LIST(reserved_blocks),
>>>>> +	ATTR_LIST(cur_reserved_blocks),
>>>>>    	NULL,
>>>>>    };
>>>>>    
>>>>>
>>>> .
>>>>
>>
>> .
>>
> 

[PATCH v2] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

In this patch, we add a new sysfs interface, we can use it to gradually achieve
the reserved_blocks finally, even when reserved_blocks is initially set over
user_block_count - total_valid_block_count. This is very useful, especially when
we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
With this patch, f2fs can try its best to reserve space and get close to the
reserved_blocks, and the current value of achieved reserved_blocks can be shown
in real time.

To ensure there is enough space for supporting system's activation, we safely
increase cur_reserved_blocks in dev_valid_block(,node)_count to only take up the
blocks which are just obsoleted.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
 fs/f2fs/f2fs.h                          |  9 +++++++--
 fs/f2fs/super.c                         |  3 ++-
 fs/f2fs/sysfs.c                         | 15 ++++++++++-----
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 11b7f4e..bdbb9f3 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -151,3 +151,9 @@ Date:		August 2017
 Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
 Description:
 		 Controls sleep time of GC urgent mode
+
+What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
+Date:		August 2017
+Contact:	"Yunlong Song" <yunlong.song@huawei.com>
+Description:
+		 Shows current reserved blocks in system.
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 2f20b6b..62d7343 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
 	block_t discard_blks;			/* discard command candidats */
 	block_t last_valid_block_count;		/* for recovery */
 	block_t reserved_blocks;		/* configurable reserved blocks */
+	block_t cur_reserved_blocks;		/* current reserved blocks */
 
 	u32 s_next_generation;			/* for NFS support */
 
@@ -1515,7 +1516,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
 
 	spin_lock(&sbi->stat_lock);
 	sbi->total_valid_block_count += (block_t)(*count);
-	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
+	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
 	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
 		diff = sbi->total_valid_block_count - avail_user_block_count;
 		*count -= diff;
@@ -1549,6 +1550,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
 	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
 	f2fs_bug_on(sbi, inode->i_blocks < sectors);
 	sbi->total_valid_block_count -= (block_t)count;
+	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
+									sbi->cur_reserved_blocks + count);
 	spin_unlock(&sbi->stat_lock);
 	f2fs_i_blocks_write(inode, count, false, true);
 }
@@ -1695,7 +1698,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
 	spin_lock(&sbi->stat_lock);
 
 	valid_block_count = sbi->total_valid_block_count + 1;
-	if (unlikely(valid_block_count + sbi->reserved_blocks >
+	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
 						sbi->user_block_count)) {
 		spin_unlock(&sbi->stat_lock);
 		goto enospc;
@@ -1738,6 +1741,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
 
 	sbi->total_valid_node_count--;
 	sbi->total_valid_block_count--;
+	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
+									sbi->cur_reserved_blocks + 1);
 
 	spin_unlock(&sbi->stat_lock);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4c1bdcb..16a805f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_blocks = total_count - start_count;
 	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
 	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
-						sbi->reserved_blocks;
+						sbi->cur_reserved_blocks;
 
 	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
 
@@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 				le64_to_cpu(sbi->ckpt->valid_block_count);
 	sbi->last_valid_block_count = sbi->total_valid_block_count;
 	sbi->reserved_blocks = 0;
+	sbi->cur_reserved_blocks = 0;
 
 	for (i = 0; i < NR_INODE_TYPE; i++) {
 		INIT_LIST_HEAD(&sbi->inode_list[i]);
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index a1be5ac..03f9d3d 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
 			BD_PART_WRITTEN(sbi)));
 }
 
+static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
+		struct f2fs_sb_info *sbi, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
+}
+
 static ssize_t features_show(struct f2fs_attr *a,
 		struct f2fs_sb_info *sbi, char *buf)
 {
@@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 #endif
 	if (a->struct_type == RESERVED_BLOCKS) {
 		spin_lock(&sbi->stat_lock);
-		if ((unsigned long)sbi->total_valid_block_count + t >
-				(unsigned long)sbi->user_block_count) {
-			spin_unlock(&sbi->stat_lock);
-			return -EINVAL;
-		}
 		*ui = t;
+		if (t < sbi->cur_reserved_blocks)
+			sbi->cur_reserved_blocks = t;
 		spin_unlock(&sbi->stat_lock);
 		return count;
 	}
@@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 #endif
 F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
 F2FS_GENERAL_RO_ATTR(features);
+F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
 
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
@@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 	ATTR_LIST(lifetime_write_kbytes),
 	ATTR_LIST(features),
 	ATTR_LIST(reserved_blocks),
+	ATTR_LIST(cur_reserved_blocks),
 	NULL,
 };
 
-- 
1.8.5.2

Re: [PATCH] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

OK, I change the initial value of cur_reserved_blocks to 0 and increase 
its value only
in the dec_valid_block(,node)_count process, to only take up the blocks 
which are
just obsoleted. With this change, if it is OK for system activation 
before, it is OK after
as well. Please review the patch v2 I resent.

On 2017/8/11 18:18, Chao Yu wrote:
> On 2017/8/10 19:41, Yunlong Song wrote:
>> It may block the system in such case, but so does the hard version
>> design. In the hard version,
> We'd better not let soft/hard reserved_blocks to eat all left free space, at
> least needs to leave enough space for supporting system's activation.
>
> Thanks,
>
>> when reserve_blocks is set to the value = user_block_count -
>> total_valid_block_count, then
>> the user free space can also drop to zero and has the same problem. IMO,
>> kernel does not need
>> to take this responsibility, it's android's responsibility.
>>
>> On 2017/8/10 19:26, Chao Yu wrote:
>>> On 2017/8/10 11:58, Yunlong Song wrote:
>>>> I think the aim of reserved_blocks function is to leave space for f2fs
>>>> and FTL, so I change it to a
>>>> soft version so that it can be used to fit to the data image which does
>>>> not satisfy the hard version,
>>>> especially for backward compatibility when updated kernel with new
>>>> default reserved_blocks set
>>>> (currently it is initially set 0 as default, but can be set any value
>>>> with my new patch).
>>>>
>>>> As for the uid/gid, does current f2fs space management design consider
>>>> this issue? IMO, I think we
>>> Upper layer application has considered this issue, as when free space touchs the
>>> threshold, system will block any operation and give a hint to clean up user's
>>> data. So w/o uid/gid reserved block, it will be OK. But for your requirement,
>>> user free space will drop to zero, and before current reserved block number
>>> touch soft reserved block limitation, user can only do deletion, so in this
>>> time, if any system/service flow depends on file creation, could result in
>>> blocking the system.
>>>
>>>> can just ensure the reserved space for file system no matter user/system
>>>> type. Whether the value of
>>>> reserved_blocks is OK or not, should not be filesystem's issue.
>>>> Filesystem just provide this interface,
>>>> and the upper layer, such as android vold should take care of the value
>>>> of reserved_blocks and make
>>>> sure its value is appropriate and will not block any activation of
>>>> system user, if it really happens, android
>>>> should change the value dynamically, it is fine, since we make
>>>> reserved_blocks to a soft version.
>>> We can not make sure in strategy changing flow, there is no file creation
>>> dependence. Even if we can, if the reserved_blocks is not appropriate, why not
>>> setting it correctly before?
>>>
>>> Thanks,
>>>
>>>> On 2017/8/10 11:15, Chao Yu wrote:
>>>>> On 2017/8/8 21:43, Yunlong Song wrote:
>>>>>> In this patch, we add a new sysfs interface, we can use it to gradually achieve
>>>>>> the reserved_blocks finally, even when reserved_blocks is initially set over
>>>>>> user_block_count - total_valid_block_count. This is very useful, especially when
>>>>>> we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
>>>>>> user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
>>>>>> With this patch, f2fs can try its best to reserve space and get close to the
>>>>>> reserved_blocks, and the current value of achieved reserved_blocks can be shown
>>>>>> in real time.
>>>>> Oh, this looks like a soft limitation in quota system, but original
>>>>> reserved_blocks implementation likes a hard one, so this patch changes the
>>>>> semantics of reserved_blocks.
>>>>>
>>>>> Actually, I doubt that it would be hard to reserve all left free space in real
>>>>> user scenario now, since system user's activation may depend on free space of
>>>>> data partition due to file creation requirement, so w/o supporting feature of
>>>>> uid/gid reserved block, soft reservation will block any activation of system
>>>>> user, such as android.
>>>>>
>>>>> Thanks,
>>>>>
>>>>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>>>>> ---
>>>>>>     Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>>>>>>     fs/f2fs/f2fs.h                          |  9 +++++++--
>>>>>>     fs/f2fs/super.c                         |  4 +++-
>>>>>>     fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>>>>>>     4 files changed, 26 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>> index 11b7f4e..bdbb9f3 100644
>>>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>> @@ -151,3 +151,9 @@ Date:		August 2017
>>>>>>     Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
>>>>>>     Description:
>>>>>>     		 Controls sleep time of GC urgent mode
>>>>>> +
>>>>>> +What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
>>>>>> +Date:		August 2017
>>>>>> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
>>>>>> +Description:
>>>>>> +		 Shows current reserved blocks in system.
>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>> index cea329f..3b7056f 100644
>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>> @@ -1040,6 +1040,7 @@ struct f2fs_sb_info {
>>>>>>     	block_t discard_blks;			/* discard command candidats */
>>>>>>     	block_t last_valid_block_count;		/* for recovery */
>>>>>>     	block_t reserved_blocks;		/* configurable reserved blocks */
>>>>>> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>>>>>>     
>>>>>>     	u32 s_next_generation;			/* for NFS support */
>>>>>>     
>>>>>> @@ -1514,7 +1515,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>>     
>>>>>>     	spin_lock(&sbi->stat_lock);
>>>>>>     	sbi->total_valid_block_count += (block_t)(*count);
>>>>>> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>>>>> +	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
>>>>>>     	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>>>>     		diff = sbi->total_valid_block_count - avail_user_block_count;
>>>>>>     		*count -= diff;
>>>>>> @@ -1548,6 +1549,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>>     	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>>>>     	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>>>>     	sbi->total_valid_block_count -= (block_t)count;
>>>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>>> +									sbi->cur_reserved_blocks + count);
>>>>>>     	spin_unlock(&sbi->stat_lock);
>>>>>>     	f2fs_i_blocks_write(inode, count, false, true);
>>>>>>     }
>>>>>> @@ -1694,7 +1697,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>>     	spin_lock(&sbi->stat_lock);
>>>>>>     
>>>>>>     	valid_block_count = sbi->total_valid_block_count + 1;
>>>>>> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
>>>>>> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>>>>     						sbi->user_block_count)) {
>>>>>>     		spin_unlock(&sbi->stat_lock);
>>>>>>     		goto enospc;
>>>>>> @@ -1737,6 +1740,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>>     
>>>>>>     	sbi->total_valid_node_count--;
>>>>>>     	sbi->total_valid_block_count--;
>>>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>>> +									sbi->cur_reserved_blocks + 1);
>>>>>>     
>>>>>>     	spin_unlock(&sbi->stat_lock);
>>>>>>     
>>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>>> index 4c1bdcb..2934aa2 100644
>>>>>> --- a/fs/f2fs/super.c
>>>>>> +++ b/fs/f2fs/super.c
>>>>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>>>>     	buf->f_blocks = total_count - start_count;
>>>>>>     	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>>>>     	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>>>>> -						sbi->reserved_blocks;
>>>>>> +						sbi->cur_reserved_blocks;
>>>>>>     
>>>>>>     	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>>>>     
>>>>>> @@ -2411,6 +2411,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>>>     				le64_to_cpu(sbi->ckpt->valid_block_count);
>>>>>>     	sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>>>>     	sbi->reserved_blocks = 0;
>>>>>> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>>> +						sbi->user_block_count - sbi->total_valid_block_count);
>>>>>>     
>>>>>>     	for (i = 0; i < NR_INODE_TYPE; i++) {
>>>>>>     		INIT_LIST_HEAD(&sbi->inode_list[i]);
>>>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>>>> index b769a3d..405c13f 100644
>>>>>> --- a/fs/f2fs/sysfs.c
>>>>>> +++ b/fs/f2fs/sysfs.c
>>>>>> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
>>>>>>     			BD_PART_WRITTEN(sbi)));
>>>>>>     }
>>>>>>     
>>>>>> +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
>>>>>> +		struct f2fs_sb_info *sbi, char *buf)
>>>>>> +{
>>>>>> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
>>>>>> +}
>>>>>> +
>>>>>>     static ssize_t features_show(struct f2fs_attr *a,
>>>>>>     		struct f2fs_sb_info *sbi, char *buf)
>>>>>>     {
>>>>>> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>>>>     #endif
>>>>>>     	if (a->struct_type == RESERVED_BLOCKS) {
>>>>>>     		spin_lock(&sbi->stat_lock);
>>>>>> -		if ((unsigned long)sbi->total_valid_block_count + t >
>>>>>> -				(unsigned long)sbi->user_block_count) {
>>>>>> -			spin_unlock(&sbi->stat_lock);
>>>>>> -			return -EINVAL;
>>>>>> -		}
>>>>>>     		*ui = t;
>>>>>> +		sbi->cur_reserved_blocks = min(t, sbi->user_block_count -
>>>>>> +										sbi->total_valid_block_count);
>>>>>>     		spin_unlock(&sbi->stat_lock);
>>>>>>     		return count;
>>>>>>     	}
>>>>>> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>>>>     #endif
>>>>>>     F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>>>>>>     F2FS_GENERAL_RO_ATTR(features);
>>>>>> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>>>>>>     
>>>>>>     #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>>>>>     F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
>>>>>> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>>>>     	ATTR_LIST(lifetime_write_kbytes),
>>>>>>     	ATTR_LIST(features),
>>>>>>     	ATTR_LIST(reserved_blocks),
>>>>>> +	ATTR_LIST(cur_reserved_blocks),
>>>>>>     	NULL,
>>>>>>     };
>>>>>>     
>>>>>>
>>>>> .
>>>>>
>>> .
>>>
>
> .
>

-- 
Thanks,
Yunlong Song

Re: [PATCH v2] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

ping...

On 2017/8/11 19:43, Yunlong Song wrote:
> In this patch, we add a new sysfs interface, we can use it to gradually achieve
> the reserved_blocks finally, even when reserved_blocks is initially set over
> user_block_count - total_valid_block_count. This is very useful, especially when
> we upgrade kernel with new reserved_blocks value, but old disk image unluckily has
> user_block_count - total_valid_block_count smaller than the desired reserved_blocks.
> With this patch, f2fs can try its best to reserve space and get close to the
> reserved_blocks, and the current value of achieved reserved_blocks can be shown
> in real time.
>
> To ensure there is enough space for supporting system's activation, we safely
> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take up the
> blocks which are just obsoleted.
>
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> ---
>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>   fs/f2fs/f2fs.h                          |  9 +++++++--
>   fs/f2fs/super.c                         |  3 ++-
>   fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>   4 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 11b7f4e..bdbb9f3 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -151,3 +151,9 @@ Date:		August 2017
>   Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
>   Description:
>   		 Controls sleep time of GC urgent mode
> +
> +What:		/sys/fs/f2fs/<disk>/cur_reserved_blocks
> +Date:		August 2017
> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
> +Description:
> +		 Shows current reserved blocks in system.
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 2f20b6b..62d7343 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>   	block_t discard_blks;			/* discard command candidats */
>   	block_t last_valid_block_count;		/* for recovery */
>   	block_t reserved_blocks;		/* configurable reserved blocks */
> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>   
>   	u32 s_next_generation;			/* for NFS support */
>   
> @@ -1515,7 +1516,7 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>   
>   	spin_lock(&sbi->stat_lock);
>   	sbi->total_valid_block_count += (block_t)(*count);
> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> +	avail_user_block_count = sbi->user_block_count - sbi->cur_reserved_blocks;
>   	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>   		diff = sbi->total_valid_block_count - avail_user_block_count;
>   		*count -= diff;
> @@ -1549,6 +1550,8 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>   	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>   	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>   	sbi->total_valid_block_count -= (block_t)count;
> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
> +									sbi->cur_reserved_blocks + count);
>   	spin_unlock(&sbi->stat_lock);
>   	f2fs_i_blocks_write(inode, count, false, true);
>   }
> @@ -1695,7 +1698,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>   	spin_lock(&sbi->stat_lock);
>   
>   	valid_block_count = sbi->total_valid_block_count + 1;
> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>   						sbi->user_block_count)) {
>   		spin_unlock(&sbi->stat_lock);
>   		goto enospc;
> @@ -1738,6 +1741,8 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>   
>   	sbi->total_valid_node_count--;
>   	sbi->total_valid_block_count--;
> +	sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
> +									sbi->cur_reserved_blocks + 1);
>   
>   	spin_unlock(&sbi->stat_lock);
>   
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 4c1bdcb..16a805f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>   	buf->f_blocks = total_count - start_count;
>   	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>   	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> -						sbi->reserved_blocks;
> +						sbi->cur_reserved_blocks;
>   
>   	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>   
> @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>   				le64_to_cpu(sbi->ckpt->valid_block_count);
>   	sbi->last_valid_block_count = sbi->total_valid_block_count;
>   	sbi->reserved_blocks = 0;
> +	sbi->cur_reserved_blocks = 0;
>   
>   	for (i = 0; i < NR_INODE_TYPE; i++) {
>   		INIT_LIST_HEAD(&sbi->inode_list[i]);
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index a1be5ac..03f9d3d 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct f2fs_attr *a,
>   			BD_PART_WRITTEN(sbi)));
>   }
>   
> +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
> +		struct f2fs_sb_info *sbi, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
> +}
> +
>   static ssize_t features_show(struct f2fs_attr *a,
>   		struct f2fs_sb_info *sbi, char *buf)
>   {
> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>   #endif
>   	if (a->struct_type == RESERVED_BLOCKS) {
>   		spin_lock(&sbi->stat_lock);
> -		if ((unsigned long)sbi->total_valid_block_count + t >
> -				(unsigned long)sbi->user_block_count) {
> -			spin_unlock(&sbi->stat_lock);
> -			return -EINVAL;
> -		}
>   		*ui = t;
> +		if (t < sbi->cur_reserved_blocks)
> +			sbi->cur_reserved_blocks = t;
>   		spin_unlock(&sbi->stat_lock);
>   		return count;
>   	}
> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>   #endif
>   F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>   F2FS_GENERAL_RO_ATTR(features);
> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>   
>   #ifdef CONFIG_F2FS_FS_ENCRYPTION
>   F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>   	ATTR_LIST(lifetime_write_kbytes),
>   	ATTR_LIST(features),
>   	ATTR_LIST(reserved_blocks),
> +	ATTR_LIST(cur_reserved_blocks),
>   	NULL,
>   };
>   

-- 
Thanks,
Yunlong Song

Re: [PATCH v2] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

ping...

On 2017/8/15 12:08, Yunlong Song wrote:
> ping...
>
> On 2017/8/11 19:43, Yunlong Song wrote:
>> In this patch, we add a new sysfs interface, we can use it to 
>> gradually achieve
>> the reserved_blocks finally, even when reserved_blocks is initially 
>> set over
>> user_block_count - total_valid_block_count. This is very useful, 
>> especially when
>> we upgrade kernel with new reserved_blocks value, but old disk image 
>> unluckily has
>> user_block_count - total_valid_block_count smaller than the desired 
>> reserved_blocks.
>> With this patch, f2fs can try its best to reserve space and get close 
>> to the
>> reserved_blocks, and the current value of achieved reserved_blocks 
>> can be shown
>> in real time.
>>
>> To ensure there is enough space for supporting system's activation, 
>> we safely
>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only 
>> take up the
>> blocks which are just obsoleted.
>>
>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>> ---
>>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>>   fs/f2fs/f2fs.h                          |  9 +++++++--
>>   fs/f2fs/super.c                         |  3 ++-
>>   fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>>   4 files changed, 25 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs 
>> b/Documentation/ABI/testing/sysfs-fs-f2fs
>> index 11b7f4e..bdbb9f3 100644
>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>> @@ -151,3 +151,9 @@ Date:        August 2017
>>   Contact:    "Jaegeuk Kim" <jaegeuk@kernel.org>
>>   Description:
>>            Controls sleep time of GC urgent mode
>> +
>> +What:        /sys/fs/f2fs/<disk>/cur_reserved_blocks
>> +Date:        August 2017
>> +Contact:    "Yunlong Song" <yunlong.song@huawei.com>
>> +Description:
>> +         Shows current reserved blocks in system.
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 2f20b6b..62d7343 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>       block_t discard_blks;            /* discard command candidats */
>>       block_t last_valid_block_count;        /* for recovery */
>>       block_t reserved_blocks;        /* configurable reserved blocks */
>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>         u32 s_next_generation;            /* for NFS support */
>>   @@ -1515,7 +1516,7 @@ static inline int 
>> inc_valid_block_count(struct f2fs_sb_info *sbi,
>>         spin_lock(&sbi->stat_lock);
>>       sbi->total_valid_block_count += (block_t)(*count);
>> -    avail_user_block_count = sbi->user_block_count - 
>> sbi->reserved_blocks;
>> +    avail_user_block_count = sbi->user_block_count - 
>> sbi->cur_reserved_blocks;
>>       if (unlikely(sbi->total_valid_block_count > 
>> avail_user_block_count)) {
>>           diff = sbi->total_valid_block_count - avail_user_block_count;
>>           *count -= diff;
>> @@ -1549,6 +1550,8 @@ static inline void dec_valid_block_count(struct 
>> f2fs_sb_info *sbi,
>>       f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>       f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>       sbi->total_valid_block_count -= (block_t)count;
>> +    sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>> +                                    sbi->cur_reserved_blocks + count);
>>       spin_unlock(&sbi->stat_lock);
>>       f2fs_i_blocks_write(inode, count, false, true);
>>   }
>> @@ -1695,7 +1698,7 @@ static inline int inc_valid_node_count(struct 
>> f2fs_sb_info *sbi,
>>       spin_lock(&sbi->stat_lock);
>>         valid_block_count = sbi->total_valid_block_count + 1;
>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>                           sbi->user_block_count)) {
>>           spin_unlock(&sbi->stat_lock);
>>           goto enospc;
>> @@ -1738,6 +1741,8 @@ static inline void dec_valid_node_count(struct 
>> f2fs_sb_info *sbi,
>>         sbi->total_valid_node_count--;
>>       sbi->total_valid_block_count--;
>> +    sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>> +                                    sbi->cur_reserved_blocks + 1);
>>         spin_unlock(&sbi->stat_lock);
>>   diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 4c1bdcb..16a805f 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, 
>> struct kstatfs *buf)
>>       buf->f_blocks = total_count - start_count;
>>       buf->f_bfree = user_block_count - valid_user_blocks(sbi) + 
>> ovp_count;
>>       buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>> -                        sbi->reserved_blocks;
>> +                        sbi->cur_reserved_blocks;
>>         avail_node_count = sbi->total_node_count - 
>> F2FS_RESERVED_NODE_NUM;
>>   @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block 
>> *sb, void *data, int silent)
>> le64_to_cpu(sbi->ckpt->valid_block_count);
>>       sbi->last_valid_block_count = sbi->total_valid_block_count;
>>       sbi->reserved_blocks = 0;
>> +    sbi->cur_reserved_blocks = 0;
>>         for (i = 0; i < NR_INODE_TYPE; i++) {
>>           INIT_LIST_HEAD(&sbi->inode_list[i]);
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index a1be5ac..03f9d3d 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct 
>> f2fs_attr *a,
>>               BD_PART_WRITTEN(sbi)));
>>   }
>>   +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
>> +        struct f2fs_sb_info *sbi, char *buf)
>> +{
>> +    return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
>> +}
>> +
>>   static ssize_t features_show(struct f2fs_attr *a,
>>           struct f2fs_sb_info *sbi, char *buf)
>>   {
>> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>   #endif
>>       if (a->struct_type == RESERVED_BLOCKS) {
>>           spin_lock(&sbi->stat_lock);
>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>> -                (unsigned long)sbi->user_block_count) {
>> -            spin_unlock(&sbi->stat_lock);
>> -            return -EINVAL;
>> -        }
>>           *ui = t;
>> +        if (t < sbi->cur_reserved_blocks)
>> +            sbi->cur_reserved_blocks = t;
>>           spin_unlock(&sbi->stat_lock);
>>           return count;
>>       }
>> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr 
>> *a,
>>   #endif
>>   F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>>   F2FS_GENERAL_RO_ATTR(features);
>> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>>     #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>   F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
>> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr 
>> *a,
>>       ATTR_LIST(lifetime_write_kbytes),
>>       ATTR_LIST(features),
>>       ATTR_LIST(reserved_blocks),
>> +    ATTR_LIST(cur_reserved_blocks),
>>       NULL,
>>   };
>

-- 
Thanks,
Yunlong Song

Re: [PATCH v2] f2fs: introduce cur_reserved_blocks in sysfs

[Chao Yu]

Hi Yunlong,

IMO, we don't need additional sysfs entry, how about changing a bit as below?

>From 3fc8206871fe457859f1537c9dc8918b45f14601 Mon Sep 17 00:00:00 2001
From: Yunlong Song <yunlong.song@huawei.com>
Date: Wed, 16 Aug 2017 23:01:56 +0800
Subject: [PATCH] f2fs: support soft block reservation

It supports to extend reserved_blocks sysfs interface to be soft
threshold, which allows user configure it exceeding current available
user space.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
 fs/f2fs/f2fs.h                          | 12 ++++++++++--
 fs/f2fs/super.c                         |  3 ++-
 fs/f2fs/sysfs.c                         | 16 ++++++++++++++--
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 11b7f4ebea7c..45c3d92f77c8 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -138,7 +138,8 @@ What:		/sys/fs/f2fs/<disk>/reserved_blocks
 Date:		June 2017
 Contact:	"Chao Yu" <yuchao0@huawei.com>
 Description:
-		 Controls current reserved blocks in system.
+		 Controls current reserved blocks in system, the threshold
+		 is soft, it could exceed current avaible user space.

 What:		/sys/fs/f2fs/<disk>/gc_urgent
 Date:		August 2017
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 336021b9b93e..a7b2d257e8ee 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1046,6 +1046,7 @@ struct f2fs_sb_info {
 	block_t discard_blks;			/* discard command candidats */
 	block_t last_valid_block_count;		/* for recovery */
 	block_t reserved_blocks;		/* configurable reserved blocks */
+	block_t current_reserved_blocks;	/* current reserved blocks */

 	u32 s_next_generation;			/* for NFS support */

@@ -1520,7 +1521,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,

 	spin_lock(&sbi->stat_lock);
 	sbi->total_valid_block_count += (block_t)(*count);
-	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
+	avail_user_block_count = sbi->user_block_count -
+					sbi->current_reserved_blocks;
 	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
 		diff = sbi->total_valid_block_count - avail_user_block_count;
 		*count -= diff;
@@ -1554,6 +1556,9 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
 	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
 	f2fs_bug_on(sbi, inode->i_blocks < sectors);
 	sbi->total_valid_block_count -= (block_t)count;
+	if (sbi->reserved_blocks)
+		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+					sbi->current_reserved_blocks + count);
 	spin_unlock(&sbi->stat_lock);
 	f2fs_i_blocks_write(inode, count, false, true);
 }
@@ -1700,7 +1705,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
 	spin_lock(&sbi->stat_lock);

 	valid_block_count = sbi->total_valid_block_count + 1;
-	if (unlikely(valid_block_count + sbi->reserved_blocks >
+	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
 						sbi->user_block_count)) {
 		spin_unlock(&sbi->stat_lock);
 		goto enospc;
@@ -1743,6 +1748,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,

 	sbi->total_valid_node_count--;
 	sbi->total_valid_block_count--;
+	if (sbi->reserved_blocks)
+		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+					sbi->current_reserved_blocks + 1);

 	spin_unlock(&sbi->stat_lock);

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4c1bdcb94133..036f083bbf56 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_blocks = total_count - start_count;
 	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
 	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
-						sbi->reserved_blocks;
+						sbi->current_reserved_blocks;

 	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;

@@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 				le64_to_cpu(sbi->ckpt->valid_block_count);
 	sbi->last_valid_block_count = sbi->total_valid_block_count;
 	sbi->reserved_blocks = 0;
+	sbi->current_reserved_blocks = 0;

 	for (i = 0; i < NR_INODE_TYPE; i++) {
 		INIT_LIST_HEAD(&sbi->inode_list[i]);
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 4bcaa9059026..3173a272fe7c 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -104,12 +104,23 @@ static ssize_t features_show(struct f2fs_attr *a,
 	return len;
 }

+static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
+					struct f2fs_sb_info *sbi, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE,
+			"expected:	%u\ncurrent:	%u\n",
+			sbi->reserved_blocks, sbi->current_reserved_blocks);
+}
+
 static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
 			struct f2fs_sb_info *sbi, char *buf)
 {
 	unsigned char *ptr = NULL;
 	unsigned int *ui;

+	if (a->struct_type == RESERVED_BLOCKS)
+		return f2fs_reserved_blocks_show(a, sbi, buf);
+
 	ptr = __struct_ptr(sbi, a->struct_type);
 	if (!ptr)
 		return -EINVAL;
@@ -143,12 +154,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 #endif
 	if (a->struct_type == RESERVED_BLOCKS) {
 		spin_lock(&sbi->stat_lock);
-		if ((unsigned long)sbi->total_valid_block_count + t >
-				(unsigned long)sbi->user_block_count) {
+		if (t > (unsigned long)sbi->user_block_count) {
 			spin_unlock(&sbi->stat_lock);
 			return -EINVAL;
 		}
 		*ui = t;
+		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+				sbi->user_block_count - valid_user_blocks(sbi));
 		spin_unlock(&sbi->stat_lock);
 		return count;
 	}
-- 
2.13.1.388.g69e6b9b4f4a9



On 2017/8/18 18:05, Yunlong Song wrote:
> ping...
> 
> On 2017/8/15 12:08, Yunlong Song wrote:
>> ping...
>>
>> On 2017/8/11 19:43, Yunlong Song wrote:
>>> In this patch, we add a new sysfs interface, we can use it to 
>>> gradually achieve
>>> the reserved_blocks finally, even when reserved_blocks is initially 
>>> set over
>>> user_block_count - total_valid_block_count. This is very useful, 
>>> especially when
>>> we upgrade kernel with new reserved_blocks value, but old disk image 
>>> unluckily has
>>> user_block_count - total_valid_block_count smaller than the desired 
>>> reserved_blocks.
>>> With this patch, f2fs can try its best to reserve space and get close 
>>> to the
>>> reserved_blocks, and the current value of achieved reserved_blocks 
>>> can be shown
>>> in real time.
>>>
>>> To ensure there is enough space for supporting system's activation, 
>>> we safely
>>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only 
>>> take up the
>>> blocks which are just obsoleted.
>>>
>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>> ---
>>>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>>>   fs/f2fs/f2fs.h                          |  9 +++++++--
>>>   fs/f2fs/super.c                         |  3 ++-
>>>   fs/f2fs/sysfs.c                         | 15 ++++++++++-----
>>>   4 files changed, 25 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs 
>>> b/Documentation/ABI/testing/sysfs-fs-f2fs
>>> index 11b7f4e..bdbb9f3 100644
>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>> @@ -151,3 +151,9 @@ Date:        August 2017
>>>   Contact:    "Jaegeuk Kim" <jaegeuk@kernel.org>
>>>   Description:
>>>            Controls sleep time of GC urgent mode
>>> +
>>> +What:        /sys/fs/f2fs/<disk>/cur_reserved_blocks
>>> +Date:        August 2017
>>> +Contact:    "Yunlong Song" <yunlong.song@huawei.com>
>>> +Description:
>>> +         Shows current reserved blocks in system.
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 2f20b6b..62d7343 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>>       block_t discard_blks;            /* discard command candidats */
>>>       block_t last_valid_block_count;        /* for recovery */
>>>       block_t reserved_blocks;        /* configurable reserved blocks */
>>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>>         u32 s_next_generation;            /* for NFS support */
>>>   @@ -1515,7 +1516,7 @@ static inline int 
>>> inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>         spin_lock(&sbi->stat_lock);
>>>       sbi->total_valid_block_count += (block_t)(*count);
>>> -    avail_user_block_count = sbi->user_block_count - 
>>> sbi->reserved_blocks;
>>> +    avail_user_block_count = sbi->user_block_count - 
>>> sbi->cur_reserved_blocks;
>>>       if (unlikely(sbi->total_valid_block_count > 
>>> avail_user_block_count)) {
>>>           diff = sbi->total_valid_block_count - avail_user_block_count;
>>>           *count -= diff;
>>> @@ -1549,6 +1550,8 @@ static inline void dec_valid_block_count(struct 
>>> f2fs_sb_info *sbi,
>>>       f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>       f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>       sbi->total_valid_block_count -= (block_t)count;
>>> +    sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>> +                                    sbi->cur_reserved_blocks + count);
>>>       spin_unlock(&sbi->stat_lock);
>>>       f2fs_i_blocks_write(inode, count, false, true);
>>>   }
>>> @@ -1695,7 +1698,7 @@ static inline int inc_valid_node_count(struct 
>>> f2fs_sb_info *sbi,
>>>       spin_lock(&sbi->stat_lock);
>>>         valid_block_count = sbi->total_valid_block_count + 1;
>>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>                           sbi->user_block_count)) {
>>>           spin_unlock(&sbi->stat_lock);
>>>           goto enospc;
>>> @@ -1738,6 +1741,8 @@ static inline void dec_valid_node_count(struct 
>>> f2fs_sb_info *sbi,
>>>         sbi->total_valid_node_count--;
>>>       sbi->total_valid_block_count--;
>>> +    sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>> +                                    sbi->cur_reserved_blocks + 1);
>>>         spin_unlock(&sbi->stat_lock);
>>>   diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 4c1bdcb..16a805f 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, 
>>> struct kstatfs *buf)
>>>       buf->f_blocks = total_count - start_count;
>>>       buf->f_bfree = user_block_count - valid_user_blocks(sbi) + 
>>> ovp_count;
>>>       buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>> -                        sbi->reserved_blocks;
>>> +                        sbi->cur_reserved_blocks;
>>>         avail_node_count = sbi->total_node_count - 
>>> F2FS_RESERVED_NODE_NUM;
>>>   @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block 
>>> *sb, void *data, int silent)
>>> le64_to_cpu(sbi->ckpt->valid_block_count);
>>>       sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>       sbi->reserved_blocks = 0;
>>> +    sbi->cur_reserved_blocks = 0;
>>>         for (i = 0; i < NR_INODE_TYPE; i++) {
>>>           INIT_LIST_HEAD(&sbi->inode_list[i]);
>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>> index a1be5ac..03f9d3d 100644
>>> --- a/fs/f2fs/sysfs.c
>>> +++ b/fs/f2fs/sysfs.c
>>> @@ -76,6 +76,12 @@ static ssize_t lifetime_write_kbytes_show(struct 
>>> f2fs_attr *a,
>>>               BD_PART_WRITTEN(sbi)));
>>>   }
>>>   +static ssize_t cur_reserved_blocks_show(struct f2fs_attr *a,
>>> +        struct f2fs_sb_info *sbi, char *buf)
>>> +{
>>> +    return snprintf(buf, PAGE_SIZE, "%u\n", sbi->cur_reserved_blocks);
>>> +}
>>> +
>>>   static ssize_t features_show(struct f2fs_attr *a,
>>>           struct f2fs_sb_info *sbi, char *buf)
>>>   {
>>> @@ -143,12 +149,9 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>   #endif
>>>       if (a->struct_type == RESERVED_BLOCKS) {
>>>           spin_lock(&sbi->stat_lock);
>>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>>> -                (unsigned long)sbi->user_block_count) {
>>> -            spin_unlock(&sbi->stat_lock);
>>> -            return -EINVAL;
>>> -        }
>>>           *ui = t;
>>> +        if (t < sbi->cur_reserved_blocks)
>>> +            sbi->cur_reserved_blocks = t;
>>>           spin_unlock(&sbi->stat_lock);
>>>           return count;
>>>       }
>>> @@ -274,6 +277,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr 
>>> *a,
>>>   #endif
>>>   F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>>>   F2FS_GENERAL_RO_ATTR(features);
>>> +F2FS_GENERAL_RO_ATTR(cur_reserved_blocks);
>>>     #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>>   F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
>>> @@ -317,6 +321,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr 
>>> *a,
>>>       ATTR_LIST(lifetime_write_kbytes),
>>>       ATTR_LIST(features),
>>>       ATTR_LIST(reserved_blocks),
>>> +    ATTR_LIST(cur_reserved_blocks),
>>>       NULL,
>>>   };
>>
> 

[PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Yunlong Song]

This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
interface to be soft threshold, which allows user configure it exceeding
current available user space. To ensure there is enough space for
supporting system's activation, this patch does not set the reserved space
to the configured reserved_blocks value at once, instead, it safely
increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
up the blocks which are just obsoleted.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
 fs/f2fs/f2fs.h                          | 13 +++++++++++--
 fs/f2fs/super.c                         |  3 ++-
 fs/f2fs/sysfs.c                         | 15 +++++++++++++--
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 11b7f4e..ba282ca 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -138,7 +138,8 @@ What:		/sys/fs/f2fs/<disk>/reserved_blocks
 Date:		June 2017
 Contact:	"Chao Yu" <yuchao0@huawei.com>
 Description:
-		 Controls current reserved blocks in system.
+		 Controls current reserved blocks in system, the threshold
+		 is soft, it could exceed current available user space.
 
 What:		/sys/fs/f2fs/<disk>/gc_urgent
 Date:		August 2017
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 2f20b6b..84ccbdc 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
 	block_t discard_blks;			/* discard command candidats */
 	block_t last_valid_block_count;		/* for recovery */
 	block_t reserved_blocks;		/* configurable reserved blocks */
+	block_t cur_reserved_blocks;		/* current reserved blocks */
 
 	u32 s_next_generation;			/* for NFS support */
 
@@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
 
 	spin_lock(&sbi->stat_lock);
 	sbi->total_valid_block_count += (block_t)(*count);
-	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
+	avail_user_block_count = sbi->user_block_count -
+						sbi->cur_reserved_blocks;
 	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
 		diff = sbi->total_valid_block_count - avail_user_block_count;
 		*count -= diff;
@@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
 	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
 	f2fs_bug_on(sbi, inode->i_blocks < sectors);
 	sbi->total_valid_block_count -= (block_t)count;
+	if (sbi->reserved_blocks &&
+		sbi->reserved_blocks != sbi->cur_reserved_blocks)
+		sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
+					sbi->cur_reserved_blocks + count);
 	spin_unlock(&sbi->stat_lock);
 	f2fs_i_blocks_write(inode, count, false, true);
 }
@@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
 	spin_lock(&sbi->stat_lock);
 
 	valid_block_count = sbi->total_valid_block_count + 1;
-	if (unlikely(valid_block_count + sbi->reserved_blocks >
+	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
 						sbi->user_block_count)) {
 		spin_unlock(&sbi->stat_lock);
 		goto enospc;
@@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
 
 	sbi->total_valid_node_count--;
 	sbi->total_valid_block_count--;
+	if (sbi->reserved_blocks &&
+		sbi->reserved_blocks != sbi->cur_reserved_blocks)
+		sbi->cur_reserved_blocks++;
 
 	spin_unlock(&sbi->stat_lock);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4c1bdcb..16a805f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_blocks = total_count - start_count;
 	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
 	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
-						sbi->reserved_blocks;
+						sbi->cur_reserved_blocks;
 
 	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
 
@@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 				le64_to_cpu(sbi->ckpt->valid_block_count);
 	sbi->last_valid_block_count = sbi->total_valid_block_count;
 	sbi->reserved_blocks = 0;
+	sbi->cur_reserved_blocks = 0;
 
 	for (i = 0; i < NR_INODE_TYPE; i++) {
 		INIT_LIST_HEAD(&sbi->inode_list[i]);
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index a1be5ac..75c37bb 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
 	return len;
 }
 
+static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
+		struct f2fs_sb_info *sbi, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
+			sbi->reserved_blocks, sbi->cur_reserved_blocks);
+}
+
 static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
 			struct f2fs_sb_info *sbi, char *buf)
 {
 	unsigned char *ptr = NULL;
 	unsigned int *ui;
 
+	if (a->struct_type == RESERVED_BLOCKS)
+		return f2fs_reserved_blocks_show(a, sbi, buf);
+
 	ptr = __struct_ptr(sbi, a->struct_type);
 	if (!ptr)
 		return -EINVAL;
@@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 #endif
 	if (a->struct_type == RESERVED_BLOCKS) {
 		spin_lock(&sbi->stat_lock);
-		if ((unsigned long)sbi->total_valid_block_count + t >
-				(unsigned long)sbi->user_block_count) {
+		if (t > (unsigned long)sbi->user_block_count) {
 			spin_unlock(&sbi->stat_lock);
 			return -EINVAL;
 		}
 		*ui = t;
+		if (t < (unsigned long)sbi->cur_reserved_blocks)
+			sbi->cur_reserved_blocks = t;
 		spin_unlock(&sbi->stat_lock);
 		return count;
 	}
-- 
1.8.5.2

Re: [PATCH v2] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

I send a v3 patch, please review, and the main difference is shown below.

On 2017/8/18 18:20, Chao Yu wrote:
> Hi Yunlong,
>
> IMO, we don't need additional sysfs entry, how about changing a bit as below?
>
> >From 3fc8206871fe457859f1537c9dc8918b45f14601 Mon Sep 17 00:00:00 2001
> From: Yunlong Song <yunlong.song@huawei.com>
> Date: Wed, 16 Aug 2017 23:01:56 +0800
> Subject: [PATCH] f2fs: support soft block reservation
>
> It supports to extend reserved_blocks sysfs interface to be soft
> threshold, which allows user configure it exceeding current available
> user space.
>
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>   Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>   fs/f2fs/f2fs.h                          | 12 ++++++++++--
>   fs/f2fs/super.c                         |  3 ++-
>   fs/f2fs/sysfs.c                         | 16 ++++++++++++++--
>   4 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 11b7f4ebea7c..45c3d92f77c8 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -138,7 +138,8 @@ What:		/sys/fs/f2fs/<disk>/reserved_blocks
>   Date:		June 2017
>   Contact:	"Chao Yu" <yuchao0@huawei.com>
>   Description:
> -		 Controls current reserved blocks in system.
> +		 Controls current reserved blocks in system, the threshold
> +		 is soft, it could exceed current avaible user space.
>
>   What:		/sys/fs/f2fs/<disk>/gc_urgent
>   Date:		August 2017
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 336021b9b93e..a7b2d257e8ee 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1046,6 +1046,7 @@ struct f2fs_sb_info {
>   	block_t discard_blks;			/* discard command candidats */
>   	block_t last_valid_block_count;		/* for recovery */
>   	block_t reserved_blocks;		/* configurable reserved blocks */
> +	block_t current_reserved_blocks;	/* current reserved blocks */
>
>   	u32 s_next_generation;			/* for NFS support */
>
> @@ -1520,7 +1521,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>
>   	spin_lock(&sbi->stat_lock);
>   	sbi->total_valid_block_count += (block_t)(*count);
> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> +	avail_user_block_count = sbi->user_block_count -
> +					sbi->current_reserved_blocks;
>   	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>   		diff = sbi->total_valid_block_count - avail_user_block_count;
>   		*count -= diff;
> @@ -1554,6 +1556,9 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>   	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>   	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>   	sbi->total_valid_block_count -= (block_t)count;
> +	if (sbi->reserved_blocks)
Add sbi->reserved_blocks != sbi->cur_reserved_blocks check
> +		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
> +					sbi->current_reserved_blocks + count);
>   	spin_unlock(&sbi->stat_lock);
>   	f2fs_i_blocks_write(inode, count, false, true);
>   }
> @@ -1700,7 +1705,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>   	spin_lock(&sbi->stat_lock);
>
>   	valid_block_count = sbi->total_valid_block_count + 1;
> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> +	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
>   						sbi->user_block_count)) {
>   		spin_unlock(&sbi->stat_lock);
>   		goto enospc;
> @@ -1743,6 +1748,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>
>   	sbi->total_valid_node_count--;
>   	sbi->total_valid_block_count--;
> +	if (sbi->reserved_blocks)
> +		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
> +					sbi->current_reserved_blocks + 1);
change to sbi->cur_reserved_blocks++;
>
>   	spin_unlock(&sbi->stat_lock);
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 4c1bdcb94133..036f083bbf56 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>   	buf->f_blocks = total_count - start_count;
>   	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>   	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> -						sbi->reserved_blocks;
> +						sbi->current_reserved_blocks;
>
>   	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>
> @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>   				le64_to_cpu(sbi->ckpt->valid_block_count);
>   	sbi->last_valid_block_count = sbi->total_valid_block_count;
>   	sbi->reserved_blocks = 0;
> +	sbi->current_reserved_blocks = 0;
>
>   	for (i = 0; i < NR_INODE_TYPE; i++) {
>   		INIT_LIST_HEAD(&sbi->inode_list[i]);
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index 4bcaa9059026..3173a272fe7c 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -104,12 +104,23 @@ static ssize_t features_show(struct f2fs_attr *a,
>   	return len;
>   }
>
> +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
> +					struct f2fs_sb_info *sbi, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE,
> +			"expected:	%u\ncurrent:	%u\n",
> +			sbi->reserved_blocks, sbi->current_reserved_blocks);
> +}
> +
>   static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>   			struct f2fs_sb_info *sbi, char *buf)
>   {
>   	unsigned char *ptr = NULL;
>   	unsigned int *ui;
>
> +	if (a->struct_type == RESERVED_BLOCKS)
> +		return f2fs_reserved_blocks_show(a, sbi, buf);
> +
>   	ptr = __struct_ptr(sbi, a->struct_type);
>   	if (!ptr)
>   		return -EINVAL;
> @@ -143,12 +154,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>   #endif
>   	if (a->struct_type == RESERVED_BLOCKS) {
>   		spin_lock(&sbi->stat_lock);
> -		if ((unsigned long)sbi->total_valid_block_count + t >
> -				(unsigned long)sbi->user_block_count) {
> +		if (t > (unsigned long)sbi->user_block_count) {
>   			spin_unlock(&sbi->stat_lock);
>   			return -EINVAL;
>   		}
>   		*ui = t;
> +		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
> +				sbi->user_block_count - valid_user_blocks(sbi));
This is not safe for support system's activation, since the user 
available space can be suddenly zero!
So I change this to only admit t < sbi->cur_reserved_blocks then change 
its value.
>   		spin_unlock(&sbi->stat_lock);
>   		return count;
>   	}

-- 
Thanks,
Yunlong Song

Re: [f2fs-dev] [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Chao Yu]

On 2017/8/18 23:09, Yunlong Song wrote:
> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
> interface to be soft threshold, which allows user configure it exceeding
> current available user space. To ensure there is enough space for
> supporting system's activation, this patch does not set the reserved space

I still don't think soft block reservation could be used in android scenario,
which has users on f2fs data partition, like system or apps, who are sensitive
with free space. IMO, hard block reservation is enough.

Instead, I expect soft block reservation could be used as a method of global
disk quota controlling or enforced over-provision ratio alteration that is
operated by administrator, most possibly in distributed storage scenario, in
where single disk state (free space) changing does affect whole system.

Thanks,

> to the configured reserved_blocks value at once, instead, it safely
> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
> up the blocks which are just obsoleted.
> 
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>  fs/f2fs/f2fs.h                          | 13 +++++++++++--
>  fs/f2fs/super.c                         |  3 ++-
>  fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>  4 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 11b7f4e..ba282ca 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -138,7 +138,8 @@ What:		/sys/fs/f2fs/<disk>/reserved_blocks
>  Date:		June 2017
>  Contact:	"Chao Yu" <yuchao0@huawei.com>
>  Description:
> -		 Controls current reserved blocks in system.
> +		 Controls current reserved blocks in system, the threshold
> +		 is soft, it could exceed current available user space.
>  
>  What:		/sys/fs/f2fs/<disk>/gc_urgent
>  Date:		August 2017
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 2f20b6b..84ccbdc 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>  	block_t discard_blks;			/* discard command candidats */
>  	block_t last_valid_block_count;		/* for recovery */
>  	block_t reserved_blocks;		/* configurable reserved blocks */
> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>  
>  	u32 s_next_generation;			/* for NFS support */
>  
> @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>  
>  	spin_lock(&sbi->stat_lock);
>  	sbi->total_valid_block_count += (block_t)(*count);
> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> +	avail_user_block_count = sbi->user_block_count -
> +						sbi->cur_reserved_blocks;
>  	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>  		diff = sbi->total_valid_block_count - avail_user_block_count;
>  		*count -= diff;
> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>  	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>  	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>  	sbi->total_valid_block_count -= (block_t)count;
> +	if (sbi->reserved_blocks &&
> +		sbi->reserved_blocks != sbi->cur_reserved_blocks)
> +		sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
> +					sbi->cur_reserved_blocks + count);
>  	spin_unlock(&sbi->stat_lock);
>  	f2fs_i_blocks_write(inode, count, false, true);
>  }
> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>  	spin_lock(&sbi->stat_lock);
>  
>  	valid_block_count = sbi->total_valid_block_count + 1;
> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>  						sbi->user_block_count)) {
>  		spin_unlock(&sbi->stat_lock);
>  		goto enospc;
> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>  
>  	sbi->total_valid_node_count--;
>  	sbi->total_valid_block_count--;
> +	if (sbi->reserved_blocks &&
> +		sbi->reserved_blocks != sbi->cur_reserved_blocks)
> +		sbi->cur_reserved_blocks++;
>  
>  	spin_unlock(&sbi->stat_lock);
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 4c1bdcb..16a805f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	buf->f_blocks = total_count - start_count;
>  	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>  	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> -						sbi->reserved_blocks;
> +						sbi->cur_reserved_blocks;
>  
>  	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>  
> @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  				le64_to_cpu(sbi->ckpt->valid_block_count);
>  	sbi->last_valid_block_count = sbi->total_valid_block_count;
>  	sbi->reserved_blocks = 0;
> +	sbi->cur_reserved_blocks = 0;
>  
>  	for (i = 0; i < NR_INODE_TYPE; i++) {
>  		INIT_LIST_HEAD(&sbi->inode_list[i]);
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index a1be5ac..75c37bb 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>  	return len;
>  }
>  
> +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
> +		struct f2fs_sb_info *sbi, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
> +			sbi->reserved_blocks, sbi->cur_reserved_blocks);
> +}
> +
>  static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>  			struct f2fs_sb_info *sbi, char *buf)
>  {
>  	unsigned char *ptr = NULL;
>  	unsigned int *ui;
>  
> +	if (a->struct_type == RESERVED_BLOCKS)
> +		return f2fs_reserved_blocks_show(a, sbi, buf);
> +
>  	ptr = __struct_ptr(sbi, a->struct_type);
>  	if (!ptr)
>  		return -EINVAL;
> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>  #endif
>  	if (a->struct_type == RESERVED_BLOCKS) {
>  		spin_lock(&sbi->stat_lock);
> -		if ((unsigned long)sbi->total_valid_block_count + t >
> -				(unsigned long)sbi->user_block_count) {
> +		if (t > (unsigned long)sbi->user_block_count) {
>  			spin_unlock(&sbi->stat_lock);
>  			return -EINVAL;
>  		}
>  		*ui = t;
> +		if (t < (unsigned long)sbi->cur_reserved_blocks)
> +			sbi->cur_reserved_blocks = t;>  		spin_unlock(&sbi->stat_lock);
>  		return count;
>  	}
> 

Re: [PATCH v2] f2fs: introduce cur_reserved_blocks in sysfs

[Yunlong Song]

Hi, Chao,

ping...

Please see the review comment below. And the change is already in the v3 
patch.

On 2017/8/18 23:16, Yunlong Song wrote:
> I send a v3 patch, please review, and the main difference is shown below.
>
> On 2017/8/18 18:20, Chao Yu wrote:
>> Hi Yunlong,
>>
>> IMO, we don't need additional sysfs entry, how about changing a bit 
>> as below?
>>
>> >From 3fc8206871fe457859f1537c9dc8918b45f14601 Mon Sep 17 00:00:00 2001
>> From: Yunlong Song <yunlong.song@huawei.com>
>> Date: Wed, 16 Aug 2017 23:01:56 +0800
>> Subject: [PATCH] f2fs: support soft block reservation
>>
>> It supports to extend reserved_blocks sysfs interface to be soft
>> threshold, which allows user configure it exceeding current available
>> user space.
>>
>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>   Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>>   fs/f2fs/f2fs.h                          | 12 ++++++++++--
>>   fs/f2fs/super.c                         |  3 ++-
>>   fs/f2fs/sysfs.c                         | 16 ++++++++++++++--
>>   4 files changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs 
>> b/Documentation/ABI/testing/sysfs-fs-f2fs
>> index 11b7f4ebea7c..45c3d92f77c8 100644
>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>> @@ -138,7 +138,8 @@ What: /sys/fs/f2fs/<disk>/reserved_blocks
>>   Date:        June 2017
>>   Contact:    "Chao Yu" <yuchao0@huawei.com>
>>   Description:
>> -         Controls current reserved blocks in system.
>> +         Controls current reserved blocks in system, the threshold
>> +         is soft, it could exceed current avaible user space.
>>
>>   What:        /sys/fs/f2fs/<disk>/gc_urgent
>>   Date:        August 2017
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 336021b9b93e..a7b2d257e8ee 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1046,6 +1046,7 @@ struct f2fs_sb_info {
>>       block_t discard_blks;            /* discard command candidats */
>>       block_t last_valid_block_count;        /* for recovery */
>>       block_t reserved_blocks;        /* configurable reserved blocks */
>> +    block_t current_reserved_blocks;    /* current reserved blocks */
>>
>>       u32 s_next_generation;            /* for NFS support */
>>
>> @@ -1520,7 +1521,8 @@ static inline int inc_valid_block_count(struct 
>> f2fs_sb_info *sbi,
>>
>>       spin_lock(&sbi->stat_lock);
>>       sbi->total_valid_block_count += (block_t)(*count);
>> -    avail_user_block_count = sbi->user_block_count - 
>> sbi->reserved_blocks;
>> +    avail_user_block_count = sbi->user_block_count -
>> +                    sbi->current_reserved_blocks;
>>       if (unlikely(sbi->total_valid_block_count > 
>> avail_user_block_count)) {
>>           diff = sbi->total_valid_block_count - avail_user_block_count;
>>           *count -= diff;
>> @@ -1554,6 +1556,9 @@ static inline void dec_valid_block_count(struct 
>> f2fs_sb_info *sbi,
>>       f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>       f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>       sbi->total_valid_block_count -= (block_t)count;
>> +    if (sbi->reserved_blocks)
> Add sbi->reserved_blocks != sbi->cur_reserved_blocks check
>> +        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
>> +                    sbi->current_reserved_blocks + count);
>>       spin_unlock(&sbi->stat_lock);
>>       f2fs_i_blocks_write(inode, count, false, true);
>>   }
>> @@ -1700,7 +1705,7 @@ static inline int inc_valid_node_count(struct 
>> f2fs_sb_info *sbi,
>>       spin_lock(&sbi->stat_lock);
>>
>>       valid_block_count = sbi->total_valid_block_count + 1;
>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>> +    if (unlikely(valid_block_count + sbi->current_reserved_blocks >
>>                           sbi->user_block_count)) {
>>           spin_unlock(&sbi->stat_lock);
>>           goto enospc;
>> @@ -1743,6 +1748,9 @@ static inline void dec_valid_node_count(struct 
>> f2fs_sb_info *sbi,
>>
>>       sbi->total_valid_node_count--;
>>       sbi->total_valid_block_count--;
>> +    if (sbi->reserved_blocks)
>> +        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
>> +                    sbi->current_reserved_blocks + 1);
> change to sbi->cur_reserved_blocks++;
>>
>>       spin_unlock(&sbi->stat_lock);
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 4c1bdcb94133..036f083bbf56 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, 
>> struct kstatfs *buf)
>>       buf->f_blocks = total_count - start_count;
>>       buf->f_bfree = user_block_count - valid_user_blocks(sbi) + 
>> ovp_count;
>>       buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>> -                        sbi->reserved_blocks;
>> +                        sbi->current_reserved_blocks;
>>
>>       avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>
>> @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block 
>> *sb, void *data, int silent)
>> le64_to_cpu(sbi->ckpt->valid_block_count);
>>       sbi->last_valid_block_count = sbi->total_valid_block_count;
>>       sbi->reserved_blocks = 0;
>> +    sbi->current_reserved_blocks = 0;
>>
>>       for (i = 0; i < NR_INODE_TYPE; i++) {
>>           INIT_LIST_HEAD(&sbi->inode_list[i]);
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index 4bcaa9059026..3173a272fe7c 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -104,12 +104,23 @@ static ssize_t features_show(struct f2fs_attr *a,
>>       return len;
>>   }
>>
>> +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
>> +                    struct f2fs_sb_info *sbi, char *buf)
>> +{
>> +    return snprintf(buf, PAGE_SIZE,
>> +            "expected:    %u\ncurrent:    %u\n",
>> +            sbi->reserved_blocks, sbi->current_reserved_blocks);
>> +}
>> +
>>   static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>               struct f2fs_sb_info *sbi, char *buf)
>>   {
>>       unsigned char *ptr = NULL;
>>       unsigned int *ui;
>>
>> +    if (a->struct_type == RESERVED_BLOCKS)
>> +        return f2fs_reserved_blocks_show(a, sbi, buf);
>> +
>>       ptr = __struct_ptr(sbi, a->struct_type);
>>       if (!ptr)
>>           return -EINVAL;
>> @@ -143,12 +154,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>   #endif
>>       if (a->struct_type == RESERVED_BLOCKS) {
>>           spin_lock(&sbi->stat_lock);
>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>> -                (unsigned long)sbi->user_block_count) {
>> +        if (t > (unsigned long)sbi->user_block_count) {
>>               spin_unlock(&sbi->stat_lock);
>>               return -EINVAL;
>>           }
>>           *ui = t;
>> +        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
>> +                sbi->user_block_count - valid_user_blocks(sbi));
> This is not safe for support system's activation, since the user 
> available space can be suddenly zero!
> So I change this to only admit t < sbi->cur_reserved_blocks then 
> change its value.
>> spin_unlock(&sbi->stat_lock);
>>           return count;
>>       }
>

-- 
Thanks,
Yunlong Song

Re: [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Yunlong Song]

ping...

On 2017/8/18 23:09, Yunlong Song wrote:
> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
> interface to be soft threshold, which allows user configure it exceeding
> current available user space. To ensure there is enough space for
> supporting system's activation, this patch does not set the reserved space
> to the configured reserved_blocks value at once, instead, it safely
> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
> up the blocks which are just obsoleted.
>
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>   Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>   fs/f2fs/f2fs.h                          | 13 +++++++++++--
>   fs/f2fs/super.c                         |  3 ++-
>   fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>   4 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 11b7f4e..ba282ca 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -138,7 +138,8 @@ What:		/sys/fs/f2fs/<disk>/reserved_blocks
>   Date:		June 2017
>   Contact:	"Chao Yu" <yuchao0@huawei.com>
>   Description:
> -		 Controls current reserved blocks in system.
> +		 Controls current reserved blocks in system, the threshold
> +		 is soft, it could exceed current available user space.
>   
>   What:		/sys/fs/f2fs/<disk>/gc_urgent
>   Date:		August 2017
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 2f20b6b..84ccbdc 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>   	block_t discard_blks;			/* discard command candidats */
>   	block_t last_valid_block_count;		/* for recovery */
>   	block_t reserved_blocks;		/* configurable reserved blocks */
> +	block_t cur_reserved_blocks;		/* current reserved blocks */
>   
>   	u32 s_next_generation;			/* for NFS support */
>   
> @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>   
>   	spin_lock(&sbi->stat_lock);
>   	sbi->total_valid_block_count += (block_t)(*count);
> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> +	avail_user_block_count = sbi->user_block_count -
> +						sbi->cur_reserved_blocks;
>   	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>   		diff = sbi->total_valid_block_count - avail_user_block_count;
>   		*count -= diff;
> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>   	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>   	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>   	sbi->total_valid_block_count -= (block_t)count;
> +	if (sbi->reserved_blocks &&
> +		sbi->reserved_blocks != sbi->cur_reserved_blocks)
> +		sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
> +					sbi->cur_reserved_blocks + count);
>   	spin_unlock(&sbi->stat_lock);
>   	f2fs_i_blocks_write(inode, count, false, true);
>   }
> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>   	spin_lock(&sbi->stat_lock);
>   
>   	valid_block_count = sbi->total_valid_block_count + 1;
> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> +	if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>   						sbi->user_block_count)) {
>   		spin_unlock(&sbi->stat_lock);
>   		goto enospc;
> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>   
>   	sbi->total_valid_node_count--;
>   	sbi->total_valid_block_count--;
> +	if (sbi->reserved_blocks &&
> +		sbi->reserved_blocks != sbi->cur_reserved_blocks)
> +		sbi->cur_reserved_blocks++;
>   
>   	spin_unlock(&sbi->stat_lock);
>   
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 4c1bdcb..16a805f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>   	buf->f_blocks = total_count - start_count;
>   	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>   	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> -						sbi->reserved_blocks;
> +						sbi->cur_reserved_blocks;
>   
>   	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>   
> @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>   				le64_to_cpu(sbi->ckpt->valid_block_count);
>   	sbi->last_valid_block_count = sbi->total_valid_block_count;
>   	sbi->reserved_blocks = 0;
> +	sbi->cur_reserved_blocks = 0;
>   
>   	for (i = 0; i < NR_INODE_TYPE; i++) {
>   		INIT_LIST_HEAD(&sbi->inode_list[i]);
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index a1be5ac..75c37bb 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>   	return len;
>   }
>   
> +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
> +		struct f2fs_sb_info *sbi, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
> +			sbi->reserved_blocks, sbi->cur_reserved_blocks);
> +}
> +
>   static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>   			struct f2fs_sb_info *sbi, char *buf)
>   {
>   	unsigned char *ptr = NULL;
>   	unsigned int *ui;
>   
> +	if (a->struct_type == RESERVED_BLOCKS)
> +		return f2fs_reserved_blocks_show(a, sbi, buf);
> +
>   	ptr = __struct_ptr(sbi, a->struct_type);
>   	if (!ptr)
>   		return -EINVAL;
> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>   #endif
>   	if (a->struct_type == RESERVED_BLOCKS) {
>   		spin_lock(&sbi->stat_lock);
> -		if ((unsigned long)sbi->total_valid_block_count + t >
> -				(unsigned long)sbi->user_block_count) {
> +		if (t > (unsigned long)sbi->user_block_count) {
>   			spin_unlock(&sbi->stat_lock);
>   			return -EINVAL;
>   		}
>   		*ui = t;
> +		if (t < (unsigned long)sbi->cur_reserved_blocks)
> +			sbi->cur_reserved_blocks = t;
>   		spin_unlock(&sbi->stat_lock);
>   		return count;
>   	}

-- 
Thanks,
Yunlong Song

Re: [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Chao Yu]

On 2017/10/25 18:02, Yunlong Song wrote:
> ping...

I've replied in this thread, check your email list please, or you can check the
comments in below link:

https://patchwork.kernel.org/patch/9909407/

Anyway, see comments below.

> 
> On 2017/8/18 23:09, Yunlong Song wrote:
>> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
>> interface to be soft threshold, which allows user configure it exceeding
>> current available user space. To ensure there is enough space for
>> supporting system's activation, this patch does not set the reserved space
>> to the configured reserved_blocks value at once, instead, it safely
>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
>> up the blocks which are just obsoleted.
>>
>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>   Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>>   fs/f2fs/f2fs.h                          | 13 +++++++++++--
>>   fs/f2fs/super.c                         |  3 ++-
>>   fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>>   4 files changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>> index 11b7f4e..ba282ca 100644
>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>> @@ -138,7 +138,8 @@ What:        /sys/fs/f2fs/<disk>/reserved_blocks
>>   Date:        June 2017
>>   Contact:    "Chao Yu" <yuchao0@huawei.com>
>>   Description:
>> -         Controls current reserved blocks in system.
>> +         Controls current reserved blocks in system, the threshold
>> +         is soft, it could exceed current available user space.
>>     What:        /sys/fs/f2fs/<disk>/gc_urgent
>>   Date:        August 2017
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 2f20b6b..84ccbdc 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>       block_t discard_blks;            /* discard command candidats */
>>       block_t last_valid_block_count;        /* for recovery */
>>       block_t reserved_blocks;        /* configurable reserved blocks */
>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>         u32 s_next_generation;            /* for NFS support */
>>   @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>         spin_lock(&sbi->stat_lock);
>>       sbi->total_valid_block_count += (block_t)(*count);
>> -    avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>> +    avail_user_block_count = sbi->user_block_count -
>> +                        sbi->cur_reserved_blocks;
>>       if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>           diff = sbi->total_valid_block_count - avail_user_block_count;
>>           *count -= diff;
>> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>       f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>       f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>       sbi->total_valid_block_count -= (block_t)count;
>> +    if (sbi->reserved_blocks &&
>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)

It's redundent check here...

>> +        sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>> +                    sbi->cur_reserved_blocks + count);
>>       spin_unlock(&sbi->stat_lock);
>>       f2fs_i_blocks_write(inode, count, false, true);
>>   }
>> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>       spin_lock(&sbi->stat_lock);
>>         valid_block_count = sbi->total_valid_block_count + 1;
>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>                           sbi->user_block_count)) {
>>           spin_unlock(&sbi->stat_lock);
>>           goto enospc;
>> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>         sbi->total_valid_node_count--;
>>       sbi->total_valid_block_count--;
>> +    if (sbi->reserved_blocks &&
>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)

Checking low boundary is more safe here.

>> +        sbi->cur_reserved_blocks++;
>>         spin_unlock(&sbi->stat_lock);
>>   diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 4c1bdcb..16a805f 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>       buf->f_blocks = total_count - start_count;
>>       buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>       buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>> -                        sbi->reserved_blocks;
>> +                        sbi->cur_reserved_blocks;
>>         avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>   @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>                   le64_to_cpu(sbi->ckpt->valid_block_count);
>>       sbi->last_valid_block_count = sbi->total_valid_block_count;
>>       sbi->reserved_blocks = 0;
>> +    sbi->cur_reserved_blocks = 0;
>>         for (i = 0; i < NR_INODE_TYPE; i++) {
>>           INIT_LIST_HEAD(&sbi->inode_list[i]);
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index a1be5ac..75c37bb 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>>       return len;
>>   }
>>   +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
>> +        struct f2fs_sb_info *sbi, char *buf)
>> +{
>> +    return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
>> +            sbi->reserved_blocks, sbi->cur_reserved_blocks);
>> +}
>> +
>>   static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>               struct f2fs_sb_info *sbi, char *buf)
>>   {
>>       unsigned char *ptr = NULL;
>>       unsigned int *ui;
>>   +    if (a->struct_type == RESERVED_BLOCKS)
>> +        return f2fs_reserved_blocks_show(a, sbi, buf);
>> +
>>       ptr = __struct_ptr(sbi, a->struct_type);
>>       if (!ptr)
>>           return -EINVAL;
>> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>   #endif
>>       if (a->struct_type == RESERVED_BLOCKS) {
>>           spin_lock(&sbi->stat_lock);
>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>> -                (unsigned long)sbi->user_block_count) {
>> +        if (t > (unsigned long)sbi->user_block_count) {
>>               spin_unlock(&sbi->stat_lock);
>>               return -EINVAL;
>>           }
>>           *ui = t;
>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>> +            sbi->cur_reserved_blocks = t;

No, for 't < cur_reserved_blocks' case, cur_reserved_blocks will out of update
even if there is enough free space. You know, for soft block resevation, we need
to reserve blocks as many as possible, making free space being zero suddenly is
possible.

Thanks,

>>           spin_unlock(&sbi->stat_lock);
>>           return count;
>>       }
> 

Re: [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Yunlong Song]

Hi, Chao,
     Please see my comments below.

On 2017/10/25 20:26, Chao Yu wrote:
> On 2017/10/25 18:02, Yunlong Song wrote:
>> ping...
> I've replied in this thread, check your email list please, or you can check the
> comments in below link:
>
> https://patchwork.kernel.org/patch/9909407/
>
> Anyway, see comments below.
>
>> On 2017/8/18 23:09, Yunlong Song wrote:
>>> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
>>> interface to be soft threshold, which allows user configure it exceeding
>>> current available user space. To ensure there is enough space for
>>> supporting system's activation, this patch does not set the reserved space
>>> to the configured reserved_blocks value at once, instead, it safely
>>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
>>> up the blocks which are just obsoleted.
>>>
>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>>    Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>>>    fs/f2fs/f2fs.h                          | 13 +++++++++++--
>>>    fs/f2fs/super.c                         |  3 ++-
>>>    fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>>>    4 files changed, 28 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>> index 11b7f4e..ba282ca 100644
>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>> @@ -138,7 +138,8 @@ What:        /sys/fs/f2fs/<disk>/reserved_blocks
>>>    Date:        June 2017
>>>    Contact:    "Chao Yu" <yuchao0@huawei.com>
>>>    Description:
>>> -         Controls current reserved blocks in system.
>>> +         Controls current reserved blocks in system, the threshold
>>> +         is soft, it could exceed current available user space.
>>>      What:        /sys/fs/f2fs/<disk>/gc_urgent
>>>    Date:        August 2017
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 2f20b6b..84ccbdc 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>>        block_t discard_blks;            /* discard command candidats */
>>>        block_t last_valid_block_count;        /* for recovery */
>>>        block_t reserved_blocks;        /* configurable reserved blocks */
>>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>>          u32 s_next_generation;            /* for NFS support */
>>>    @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>          spin_lock(&sbi->stat_lock);
>>>        sbi->total_valid_block_count += (block_t)(*count);
>>> -    avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>> +    avail_user_block_count = sbi->user_block_count -
>>> +                        sbi->cur_reserved_blocks;
>>>        if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>            diff = sbi->total_valid_block_count - avail_user_block_count;
>>>            *count -= diff;
>>> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>        f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>        f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>        sbi->total_valid_block_count -= (block_t)count;
>>> +    if (sbi->reserved_blocks &&
>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
> It's redundent check here...
I think in most cases, cur_reserved_blocks is equal to reserved_blocks, 
so we do not need to calculate min any more, otherwise,
if reserved_blocks is not 0, it will calculate min and set 
current_reserved_blocks each time.
>
>>> +        sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>> +                    sbi->cur_reserved_blocks + count);
>>>        spin_unlock(&sbi->stat_lock);
>>>        f2fs_i_blocks_write(inode, count, false, true);
>>>    }
>>> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>        spin_lock(&sbi->stat_lock);
>>>          valid_block_count = sbi->total_valid_block_count + 1;
>>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>                            sbi->user_block_count)) {
>>>            spin_unlock(&sbi->stat_lock);
>>>            goto enospc;
>>> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>          sbi->total_valid_node_count--;
>>>        sbi->total_valid_block_count--;
>>> +    if (sbi->reserved_blocks &&
>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
> Checking low boundary is more safe here.
I think cur_reserved_blocks can never be larger than reserved_blocks in 
any case. so min(reserved_blocks,
cur_reserved_blocks +1) is same to cur_reserved_blocks++ when 
reserved_blocks != cur_reserved_blocks
(which means reserved_blocks > cur_reserved_block )
>
>>> +        sbi->cur_reserved_blocks++;
>>>          spin_unlock(&sbi->stat_lock);
>>>    diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 4c1bdcb..16a805f 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>        buf->f_blocks = total_count - start_count;
>>>        buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>        buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>> -                        sbi->reserved_blocks;
>>> +                        sbi->cur_reserved_blocks;
>>>          avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>    @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>                    le64_to_cpu(sbi->ckpt->valid_block_count);
>>>        sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>        sbi->reserved_blocks = 0;
>>> +    sbi->cur_reserved_blocks = 0;
>>>          for (i = 0; i < NR_INODE_TYPE; i++) {
>>>            INIT_LIST_HEAD(&sbi->inode_list[i]);
>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>> index a1be5ac..75c37bb 100644
>>> --- a/fs/f2fs/sysfs.c
>>> +++ b/fs/f2fs/sysfs.c
>>> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>>>        return len;
>>>    }
>>>    +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
>>> +        struct f2fs_sb_info *sbi, char *buf)
>>> +{
>>> +    return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
>>> +            sbi->reserved_blocks, sbi->cur_reserved_blocks);
>>> +}
>>> +
>>>    static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>>                struct f2fs_sb_info *sbi, char *buf)
>>>    {
>>>        unsigned char *ptr = NULL;
>>>        unsigned int *ui;
>>>    +    if (a->struct_type == RESERVED_BLOCKS)
>>> +        return f2fs_reserved_blocks_show(a, sbi, buf);
>>> +
>>>        ptr = __struct_ptr(sbi, a->struct_type);
>>>        if (!ptr)
>>>            return -EINVAL;
>>> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>    #endif
>>>        if (a->struct_type == RESERVED_BLOCKS) {
>>>            spin_lock(&sbi->stat_lock);
>>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>>> -                (unsigned long)sbi->user_block_count) {
>>> +        if (t > (unsigned long)sbi->user_block_count) {
>>>                spin_unlock(&sbi->stat_lock);
>>>                return -EINVAL;
>>>            }
>>>            *ui = t;
>>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>>> +            sbi->cur_reserved_blocks = t;
> No, for 't < cur_reserved_blocks' case, cur_reserved_blocks will out of update
> even if there is enough free space. You know, for soft block resevation, we need
> to reserve blocks as many as possible, making free space being zero suddenly is
> possible.
I do not understand why it is not safe to decrease cur_reserved_blocks, 
for example,
if current cur_reserved_blocks is 100, now decrease it to 80, is there 
any problem?
If 80 will make free space zero, how does 100 exist?
And I do not think it is safe as following:
          *ui = t;
+        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+                sbi->user_block_count - valid_user_blocks(sbi));

If user_block_count = 200, valid_user_blocks=150， reserved_blocks = 100,
then current_reserved_block = min(100,200-50) = 50, in this case, free space
is suddenly becoming zero.
To avoid this, I change the code to:

+        if (t < (unsigned long)sbi->cur_reserved_blocks)
+            sbi->cur_reserved_blocks = t;

I think it is only safe to decrease the value of cur_reserved_blocks, 
and leave increase operation to
dec_valid_block(,node)_count, it is safe to increase cur_reserved_blocks 
there.
>
> Thanks,
>
>>>            spin_unlock(&sbi->stat_lock);
>>>            return count;
>>>        }
> .
>

-- 
Thanks,
Yunlong Song

Re: [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Chao Yu]

On 2017/10/25 22:06, Yunlong Song wrote:
> Hi, Chao,
>     Please see my comments below.
> 
> On 2017/10/25 20:26, Chao Yu wrote:
>> On 2017/10/25 18:02, Yunlong Song wrote:
>>> ping...
>> I've replied in this thread, check your email list please, or you can check the
>> comments in below link:
>>
>> https://patchwork.kernel.org/patch/9909407/
>>
>> Anyway, see comments below.
>>
>>> On 2017/8/18 23:09, Yunlong Song wrote:
>>>> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
>>>> interface to be soft threshold, which allows user configure it exceeding
>>>> current available user space. To ensure there is enough space for
>>>> supporting system's activation, this patch does not set the reserved space
>>>> to the configured reserved_blocks value at once, instead, it safely
>>>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
>>>> up the blocks which are just obsoleted.
>>>>
>>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>>    Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>>>>    fs/f2fs/f2fs.h                          | 13 +++++++++++--
>>>>    fs/f2fs/super.c                         |  3 ++-
>>>>    fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>>>>    4 files changed, 28 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> index 11b7f4e..ba282ca 100644
>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> @@ -138,7 +138,8 @@ What:        /sys/fs/f2fs/<disk>/reserved_blocks
>>>>    Date:        June 2017
>>>>    Contact:    "Chao Yu" <yuchao0@huawei.com>
>>>>    Description:
>>>> -         Controls current reserved blocks in system.
>>>> +         Controls current reserved blocks in system, the threshold
>>>> +         is soft, it could exceed current available user space.
>>>>      What:        /sys/fs/f2fs/<disk>/gc_urgent
>>>>    Date:        August 2017
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 2f20b6b..84ccbdc 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>>>        block_t discard_blks;            /* discard command candidats */
>>>>        block_t last_valid_block_count;        /* for recovery */
>>>>        block_t reserved_blocks;        /* configurable reserved blocks */
>>>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>>>          u32 s_next_generation;            /* for NFS support */
>>>>    @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>>          spin_lock(&sbi->stat_lock);
>>>>        sbi->total_valid_block_count += (block_t)(*count);
>>>> -    avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>>> +    avail_user_block_count = sbi->user_block_count -
>>>> +                        sbi->cur_reserved_blocks;
>>>>        if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>>            diff = sbi->total_valid_block_count - avail_user_block_count;
>>>>            *count -= diff;
>>>> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>>        f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>>        f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>>        sbi->total_valid_block_count -= (block_t)count;
>>>> +    if (sbi->reserved_blocks &&
>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>> It's redundent check here...
> I think in most cases, cur_reserved_blocks is equal to reserved_blocks, so we do not need to calculate min any more, otherwise,
> if reserved_blocks is not 0, it will calculate min and set current_reserved_blocks each time.

OK, IMO, in some condition, we can save dirtying cache line to decrease cache
line missing with that check.

>>
>>>> +        sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>> +                    sbi->cur_reserved_blocks + count);
>>>>        spin_unlock(&sbi->stat_lock);
>>>>        f2fs_i_blocks_write(inode, count, false, true);
>>>>    }
>>>> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>>        spin_lock(&sbi->stat_lock);
>>>>          valid_block_count = sbi->total_valid_block_count + 1;
>>>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>>>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>>                            sbi->user_block_count)) {
>>>>            spin_unlock(&sbi->stat_lock);
>>>>            goto enospc;
>>>> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>>          sbi->total_valid_node_count--;
>>>>        sbi->total_valid_block_count--;
>>>> +    if (sbi->reserved_blocks &&
>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>> Checking low boundary is more safe here.
> I think cur_reserved_blocks can never be larger than reserved_blocks in any case. so min(reserved_blocks,
> cur_reserved_blocks +1) is same to cur_reserved_blocks++ when reserved_blocks != cur_reserved_blocks
> (which means reserved_blocks > cur_reserved_block )

Ditto.

>>
>>>> +        sbi->cur_reserved_blocks++;
>>>>          spin_unlock(&sbi->stat_lock);
>>>>    diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>> index 4c1bdcb..16a805f 100644
>>>> --- a/fs/f2fs/super.c
>>>> +++ b/fs/f2fs/super.c
>>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>>        buf->f_blocks = total_count - start_count;
>>>>        buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>>        buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>>> -                        sbi->reserved_blocks;
>>>> +                        sbi->cur_reserved_blocks;
>>>>          avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>>    @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>                    le64_to_cpu(sbi->ckpt->valid_block_count);
>>>>        sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>>        sbi->reserved_blocks = 0;
>>>> +    sbi->cur_reserved_blocks = 0;
>>>>          for (i = 0; i < NR_INODE_TYPE; i++) {
>>>>            INIT_LIST_HEAD(&sbi->inode_list[i]);
>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>> index a1be5ac..75c37bb 100644
>>>> --- a/fs/f2fs/sysfs.c
>>>> +++ b/fs/f2fs/sysfs.c
>>>> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>>>>        return len;
>>>>    }
>>>>    +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
>>>> +        struct f2fs_sb_info *sbi, char *buf)
>>>> +{
>>>> +    return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
>>>> +            sbi->reserved_blocks, sbi->cur_reserved_blocks);
>>>> +}
>>>> +
>>>>    static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>>>                struct f2fs_sb_info *sbi, char *buf)
>>>>    {
>>>>        unsigned char *ptr = NULL;
>>>>        unsigned int *ui;
>>>>    +    if (a->struct_type == RESERVED_BLOCKS)
>>>> +        return f2fs_reserved_blocks_show(a, sbi, buf);
>>>> +
>>>>        ptr = __struct_ptr(sbi, a->struct_type);
>>>>        if (!ptr)
>>>>            return -EINVAL;
>>>> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>>    #endif
>>>>        if (a->struct_type == RESERVED_BLOCKS) {
>>>>            spin_lock(&sbi->stat_lock);
>>>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>>>> -                (unsigned long)sbi->user_block_count) {
>>>> +        if (t > (unsigned long)sbi->user_block_count) {
>>>>                spin_unlock(&sbi->stat_lock);
>>>>                return -EINVAL;
>>>>            }
>>>>            *ui = t;
>>>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>>>> +            sbi->cur_reserved_blocks = t;
>> No, for 't < cur_reserved_blocks' case, cur_reserved_blocks will out of update
>> even if there is enough free space. You know, for soft block resevation, we need
>> to reserve blocks as many as possible, making free space being zero suddenly is
>> possible.
> I do not understand why it is not safe to decrease cur_reserved_blocks, for example,
> if current cur_reserved_blocks is 100, now decrease it to 80, is there any problem?
> If 80 will make free space zero, how does 100 exist?
> And I do not think it is safe as following:
>          *ui = t;
> +        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
> +                sbi->user_block_count - valid_user_blocks(sbi));
> 
> If user_block_count = 200, valid_user_blocks=150， reserved_blocks = 100,
> then current_reserved_block = min(100,200-50) = 50, in this case, free space
> is suddenly becoming zero.
Free space becomes zero suddenly is safe, as I said before, I don't expect this
feature can be used in android, instead, it may be used in distributed storage
scenario, in where, once we configure soft_reserved_block making one server out
of free space, it's not critical issue to this system since we can move current
copy to another server which has enough free space.

Secondly, as an global configuration, it's due to usage of administrator with
it, if there is critical application which is sensitive with free space,
administrator should make sure our reservation should not overload consuming free
space, which means soft reservation is not suitable.

> To avoid this, I change the code to:
> 
> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
> +            sbi->cur_reserved_blocks = t;
> 
> I think it is only safe to decrease the value of cur_reserved_blocks, and leave increase operation to
> dec_valid_block(,node)_count, it is safe to increase cur_reserved_blocks there.

For initialization of reserved_blocks, cur_reserved_blocks will always be zero
due to this check, and will be updated to close to reserved_blocks after block
allocation and deallocation of user, IMO, it's not looks reasonable to user.

Anyway, it's due to how you define semantics of soft reservation, so what is your
understanding of it?

Thanks,

>>
>> Thanks,
>>
>>>>            spin_unlock(&sbi->stat_lock);
>>>>            return count;
>>>>        }
>> .
>>
> 

Re: [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Yunlong Song]

Yes, I agree with the soft semantic you introduce, it is too slow to 
increase cur_reserved_blocks only in
dec_valid_block(,node)_count, e.g. if users want to set 
cur_reserved_blocks to 10G.

Then how about fix the initialization of cur_reserved_blocks in 
fs/f2fs/super.c as following:
sbi->current_reserved_blocks = 0
change to
sbi->current_reserved_blocks = min(sbi->reserved_blocks, 
sbi->user_block_count - valid_user_blocks(sbi));

On 2017/10/25 23:46, Chao Yu wrote:
> On 2017/10/25 22:06, Yunlong Song wrote:
>> Hi, Chao,
>>      Please see my comments below.
>>
>> On 2017/10/25 20:26, Chao Yu wrote:
>>> On 2017/10/25 18:02, Yunlong Song wrote:
>>>> ping...
>>> I've replied in this thread, check your email list please, or you can check the
>>> comments in below link:
>>>
>>> https://patchwork.kernel.org/patch/9909407/
>>>
>>> Anyway, see comments below.
>>>
>>>> On 2017/8/18 23:09, Yunlong Song wrote:
>>>>> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
>>>>> interface to be soft threshold, which allows user configure it exceeding
>>>>> current available user space. To ensure there is enough space for
>>>>> supporting system's activation, this patch does not set the reserved space
>>>>> to the configured reserved_blocks value at once, instead, it safely
>>>>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
>>>>> up the blocks which are just obsoleted.
>>>>>
>>>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>> ---
>>>>>     Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>>>>>     fs/f2fs/f2fs.h                          | 13 +++++++++++--
>>>>>     fs/f2fs/super.c                         |  3 ++-
>>>>>     fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>>>>>     4 files changed, 28 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>> index 11b7f4e..ba282ca 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>> @@ -138,7 +138,8 @@ What:        /sys/fs/f2fs/<disk>/reserved_blocks
>>>>>     Date:        June 2017
>>>>>     Contact:    "Chao Yu" <yuchao0@huawei.com>
>>>>>     Description:
>>>>> -         Controls current reserved blocks in system.
>>>>> +         Controls current reserved blocks in system, the threshold
>>>>> +         is soft, it could exceed current available user space.
>>>>>       What:        /sys/fs/f2fs/<disk>/gc_urgent
>>>>>     Date:        August 2017
>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>> index 2f20b6b..84ccbdc 100644
>>>>> --- a/fs/f2fs/f2fs.h
>>>>> +++ b/fs/f2fs/f2fs.h
>>>>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>>>>         block_t discard_blks;            /* discard command candidats */
>>>>>         block_t last_valid_block_count;        /* for recovery */
>>>>>         block_t reserved_blocks;        /* configurable reserved blocks */
>>>>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>>>>           u32 s_next_generation;            /* for NFS support */
>>>>>     @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>           spin_lock(&sbi->stat_lock);
>>>>>         sbi->total_valid_block_count += (block_t)(*count);
>>>>> -    avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>>>> +    avail_user_block_count = sbi->user_block_count -
>>>>> +                        sbi->cur_reserved_blocks;
>>>>>         if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>>>             diff = sbi->total_valid_block_count - avail_user_block_count;
>>>>>             *count -= diff;
>>>>> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>         f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>>>         f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>>>         sbi->total_valid_block_count -= (block_t)count;
>>>>> +    if (sbi->reserved_blocks &&
>>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>>> It's redundent check here...
>> I think in most cases, cur_reserved_blocks is equal to reserved_blocks, so we do not need to calculate min any more, otherwise,
>> if reserved_blocks is not 0, it will calculate min and set current_reserved_blocks each time.
> OK, IMO, in some condition, we can save dirtying cache line to decrease cache
> line missing with that check.
>
>>>>> +        sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>> +                    sbi->cur_reserved_blocks + count);
>>>>>         spin_unlock(&sbi->stat_lock);
>>>>>         f2fs_i_blocks_write(inode, count, false, true);
>>>>>     }
>>>>> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>         spin_lock(&sbi->stat_lock);
>>>>>           valid_block_count = sbi->total_valid_block_count + 1;
>>>>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>>>>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>>>                             sbi->user_block_count)) {
>>>>>             spin_unlock(&sbi->stat_lock);
>>>>>             goto enospc;
>>>>> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>           sbi->total_valid_node_count--;
>>>>>         sbi->total_valid_block_count--;
>>>>> +    if (sbi->reserved_blocks &&
>>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>>> Checking low boundary is more safe here.
>> I think cur_reserved_blocks can never be larger than reserved_blocks in any case. so min(reserved_blocks,
>> cur_reserved_blocks +1) is same to cur_reserved_blocks++ when reserved_blocks != cur_reserved_blocks
>> (which means reserved_blocks > cur_reserved_block )
> Ditto.
>
>>>>> +        sbi->cur_reserved_blocks++;
>>>>>           spin_unlock(&sbi->stat_lock);
>>>>>     diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>> index 4c1bdcb..16a805f 100644
>>>>> --- a/fs/f2fs/super.c
>>>>> +++ b/fs/f2fs/super.c
>>>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>>>         buf->f_blocks = total_count - start_count;
>>>>>         buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>>>         buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>>>> -                        sbi->reserved_blocks;
>>>>> +                        sbi->cur_reserved_blocks;
>>>>>           avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>>>     @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>>                     le64_to_cpu(sbi->ckpt->valid_block_count);
>>>>>         sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>>>         sbi->reserved_blocks = 0;
>>>>> +    sbi->cur_reserved_blocks = 0;
>>>>>           for (i = 0; i < NR_INODE_TYPE; i++) {
>>>>>             INIT_LIST_HEAD(&sbi->inode_list[i]);
>>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>>> index a1be5ac..75c37bb 100644
>>>>> --- a/fs/f2fs/sysfs.c
>>>>> +++ b/fs/f2fs/sysfs.c
>>>>> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>>>>>         return len;
>>>>>     }
>>>>>     +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
>>>>> +        struct f2fs_sb_info *sbi, char *buf)
>>>>> +{
>>>>> +    return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
>>>>> +            sbi->reserved_blocks, sbi->cur_reserved_blocks);
>>>>> +}
>>>>> +
>>>>>     static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>>>>                 struct f2fs_sb_info *sbi, char *buf)
>>>>>     {
>>>>>         unsigned char *ptr = NULL;
>>>>>         unsigned int *ui;
>>>>>     +    if (a->struct_type == RESERVED_BLOCKS)
>>>>> +        return f2fs_reserved_blocks_show(a, sbi, buf);
>>>>> +
>>>>>         ptr = __struct_ptr(sbi, a->struct_type);
>>>>>         if (!ptr)
>>>>>             return -EINVAL;
>>>>> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>>>     #endif
>>>>>         if (a->struct_type == RESERVED_BLOCKS) {
>>>>>             spin_lock(&sbi->stat_lock);
>>>>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>>>>> -                (unsigned long)sbi->user_block_count) {
>>>>> +        if (t > (unsigned long)sbi->user_block_count) {
>>>>>                 spin_unlock(&sbi->stat_lock);
>>>>>                 return -EINVAL;
>>>>>             }
>>>>>             *ui = t;
>>>>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>>>>> +            sbi->cur_reserved_blocks = t;
>>> No, for 't < cur_reserved_blocks' case, cur_reserved_blocks will out of update
>>> even if there is enough free space. You know, for soft block resevation, we need
>>> to reserve blocks as many as possible, making free space being zero suddenly is
>>> possible.
>> I do not understand why it is not safe to decrease cur_reserved_blocks, for example,
>> if current cur_reserved_blocks is 100, now decrease it to 80, is there any problem?
>> If 80 will make free space zero, how does 100 exist?
>> And I do not think it is safe as following:
>>           *ui = t;
>> +        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
>> +                sbi->user_block_count - valid_user_blocks(sbi));
>>
>> If user_block_count = 200, valid_user_blocks=150， reserved_blocks = 100,
>> then current_reserved_block = min(100,200-50) = 50, in this case, free space
>> is suddenly becoming zero.
> Free space becomes zero suddenly is safe, as I said before, I don't expect this
> feature can be used in android, instead, it may be used in distributed storage
> scenario, in where, once we configure soft_reserved_block making one server out
> of free space, it's not critical issue to this system since we can move current
> copy to another server which has enough free space.
>
> Secondly, as an global configuration, it's due to usage of administrator with
> it, if there is critical application which is sensitive with free space,
> administrator should make sure our reservation should not overload consuming free
> space, which means soft reservation is not suitable.
>
>> To avoid this, I change the code to:
>>
>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>> +            sbi->cur_reserved_blocks = t;
>>
>> I think it is only safe to decrease the value of cur_reserved_blocks, and leave increase operation to
>> dec_valid_block(,node)_count, it is safe to increase cur_reserved_blocks there.
> For initialization of reserved_blocks, cur_reserved_blocks will always be zero
> due to this check, and will be updated to close to reserved_blocks after block
> allocation and deallocation of user, IMO, it's not looks reasonable to user.
>
> Anyway, it's due to how you define semantics of soft reservation, so what is your
> understanding of it?
>
> Thanks,
>
>>> Thanks,
>>>
>>>>>             spin_unlock(&sbi->stat_lock);
>>>>>             return count;
>>>>>         }
>>> .
>>>
> .
>

-- 
Thanks,
Yunlong Song

Re: [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Chao Yu]

On 2017/10/26 11:07, Yunlong Song wrote:
> Yes, I agree with the soft semantic you introduce, it is too slow to 
> increase cur_reserved_blocks only in
> dec_valid_block(,node)_count, e.g. if users want to set 
> cur_reserved_blocks to 10G.

Yup,

> 
> Then how about fix the initialization of cur_reserved_blocks in 
> fs/f2fs/super.c as following:
> sbi->current_reserved_blocks = 0
> change to
> sbi->current_reserved_blocks = min(sbi->reserved_blocks, 
> sbi->user_block_count - valid_user_blocks(sbi));

It will be necessary only if reserved_blocks is initialized with a non-zero value,
but now it has value of zero, so it's redundant...

What about adding this check if we initialize reserved_blocks with a non-zero default
value or value which may be configured with mount_option?

Thanks,

> 
> On 2017/10/25 23:46, Chao Yu wrote:
>> On 2017/10/25 22:06, Yunlong Song wrote:
>>> Hi, Chao,
>>>      Please see my comments below.
>>>
>>> On 2017/10/25 20:26, Chao Yu wrote:
>>>> On 2017/10/25 18:02, Yunlong Song wrote:
>>>>> ping...
>>>> I've replied in this thread, check your email list please, or you can check the
>>>> comments in below link:
>>>>
>>>> https://patchwork.kernel.org/patch/9909407/
>>>>
>>>> Anyway, see comments below.
>>>>
>>>>> On 2017/8/18 23:09, Yunlong Song wrote:
>>>>>> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
>>>>>> interface to be soft threshold, which allows user configure it exceeding
>>>>>> current available user space. To ensure there is enough space for
>>>>>> supporting system's activation, this patch does not set the reserved space
>>>>>> to the configured reserved_blocks value at once, instead, it safely
>>>>>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
>>>>>> up the blocks which are just obsoleted.
>>>>>>
>>>>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>>> ---
>>>>>>     Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>>>>>>     fs/f2fs/f2fs.h                          | 13 +++++++++++--
>>>>>>     fs/f2fs/super.c                         |  3 ++-
>>>>>>     fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>>>>>>     4 files changed, 28 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>> index 11b7f4e..ba282ca 100644
>>>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>> @@ -138,7 +138,8 @@ What:        /sys/fs/f2fs/<disk>/reserved_blocks
>>>>>>     Date:        June 2017
>>>>>>     Contact:    "Chao Yu" <yuchao0@huawei.com>
>>>>>>     Description:
>>>>>> -         Controls current reserved blocks in system.
>>>>>> +         Controls current reserved blocks in system, the threshold
>>>>>> +         is soft, it could exceed current available user space.
>>>>>>       What:        /sys/fs/f2fs/<disk>/gc_urgent
>>>>>>     Date:        August 2017
>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>> index 2f20b6b..84ccbdc 100644
>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>>>>>         block_t discard_blks;            /* discard command candidats */
>>>>>>         block_t last_valid_block_count;        /* for recovery */
>>>>>>         block_t reserved_blocks;        /* configurable reserved blocks */
>>>>>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>>>>>           u32 s_next_generation;            /* for NFS support */
>>>>>>     @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>>           spin_lock(&sbi->stat_lock);
>>>>>>         sbi->total_valid_block_count += (block_t)(*count);
>>>>>> -    avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>>>>> +    avail_user_block_count = sbi->user_block_count -
>>>>>> +                        sbi->cur_reserved_blocks;
>>>>>>         if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>>>>             diff = sbi->total_valid_block_count - avail_user_block_count;
>>>>>>             *count -= diff;
>>>>>> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>>         f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>>>>         f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>>>>         sbi->total_valid_block_count -= (block_t)count;
>>>>>> +    if (sbi->reserved_blocks &&
>>>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>>>> It's redundent check here...
>>> I think in most cases, cur_reserved_blocks is equal to reserved_blocks, so we do not need to calculate min any more, otherwise,
>>> if reserved_blocks is not 0, it will calculate min and set current_reserved_blocks each time.
>> OK, IMO, in some condition, we can save dirtying cache line to decrease cache
>> line missing with that check.
>>
>>>>>> +        sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>>> +                    sbi->cur_reserved_blocks + count);
>>>>>>         spin_unlock(&sbi->stat_lock);
>>>>>>         f2fs_i_blocks_write(inode, count, false, true);
>>>>>>     }
>>>>>> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>>         spin_lock(&sbi->stat_lock);
>>>>>>           valid_block_count = sbi->total_valid_block_count + 1;
>>>>>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>>>>>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>>>>                             sbi->user_block_count)) {
>>>>>>             spin_unlock(&sbi->stat_lock);
>>>>>>             goto enospc;
>>>>>> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>>           sbi->total_valid_node_count--;
>>>>>>         sbi->total_valid_block_count--;
>>>>>> +    if (sbi->reserved_blocks &&
>>>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>>>> Checking low boundary is more safe here.
>>> I think cur_reserved_blocks can never be larger than reserved_blocks in any case. so min(reserved_blocks,
>>> cur_reserved_blocks +1) is same to cur_reserved_blocks++ when reserved_blocks != cur_reserved_blocks
>>> (which means reserved_blocks > cur_reserved_block )
>> Ditto.
>>
>>>>>> +        sbi->cur_reserved_blocks++;
>>>>>>           spin_unlock(&sbi->stat_lock);
>>>>>>     diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>>> index 4c1bdcb..16a805f 100644
>>>>>> --- a/fs/f2fs/super.c
>>>>>> +++ b/fs/f2fs/super.c
>>>>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>>>>         buf->f_blocks = total_count - start_count;
>>>>>>         buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>>>>         buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>>>>> -                        sbi->reserved_blocks;
>>>>>> +                        sbi->cur_reserved_blocks;
>>>>>>           avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>>>>     @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>>>                     le64_to_cpu(sbi->ckpt->valid_block_count);
>>>>>>         sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>>>>         sbi->reserved_blocks = 0;
>>>>>> +    sbi->cur_reserved_blocks = 0;
>>>>>>           for (i = 0; i < NR_INODE_TYPE; i++) {
>>>>>>             INIT_LIST_HEAD(&sbi->inode_list[i]);
>>>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>>>> index a1be5ac..75c37bb 100644
>>>>>> --- a/fs/f2fs/sysfs.c
>>>>>> +++ b/fs/f2fs/sysfs.c
>>>>>> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>>>>>>         return len;
>>>>>>     }
>>>>>>     +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
>>>>>> +        struct f2fs_sb_info *sbi, char *buf)
>>>>>> +{
>>>>>> +    return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
>>>>>> +            sbi->reserved_blocks, sbi->cur_reserved_blocks);
>>>>>> +}
>>>>>> +
>>>>>>     static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>>>>>                 struct f2fs_sb_info *sbi, char *buf)
>>>>>>     {
>>>>>>         unsigned char *ptr = NULL;
>>>>>>         unsigned int *ui;
>>>>>>     +    if (a->struct_type == RESERVED_BLOCKS)
>>>>>> +        return f2fs_reserved_blocks_show(a, sbi, buf);
>>>>>> +
>>>>>>         ptr = __struct_ptr(sbi, a->struct_type);
>>>>>>         if (!ptr)
>>>>>>             return -EINVAL;
>>>>>> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>>>>     #endif
>>>>>>         if (a->struct_type == RESERVED_BLOCKS) {
>>>>>>             spin_lock(&sbi->stat_lock);
>>>>>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>>>>>> -                (unsigned long)sbi->user_block_count) {
>>>>>> +        if (t > (unsigned long)sbi->user_block_count) {
>>>>>>                 spin_unlock(&sbi->stat_lock);
>>>>>>                 return -EINVAL;
>>>>>>             }
>>>>>>             *ui = t;
>>>>>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>>>>>> +            sbi->cur_reserved_blocks = t;
>>>> No, for 't < cur_reserved_blocks' case, cur_reserved_blocks will out of update
>>>> even if there is enough free space. You know, for soft block resevation, we need
>>>> to reserve blocks as many as possible, making free space being zero suddenly is
>>>> possible.
>>> I do not understand why it is not safe to decrease cur_reserved_blocks, for example,
>>> if current cur_reserved_blocks is 100, now decrease it to 80, is there any problem?
>>> If 80 will make free space zero, how does 100 exist?
>>> And I do not think it is safe as following:
>>>           *ui = t;
>>> +        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
>>> +                sbi->user_block_count - valid_user_blocks(sbi));
>>>
>>> If user_block_count = 200, valid_user_blocks=150， reserved_blocks = 100,
>>> then current_reserved_block = min(100,200-50) = 50, in this case, free space
>>> is suddenly becoming zero.
>> Free space becomes zero suddenly is safe, as I said before, I don't expect this
>> feature can be used in android, instead, it may be used in distributed storage
>> scenario, in where, once we configure soft_reserved_block making one server out
>> of free space, it's not critical issue to this system since we can move current
>> copy to another server which has enough free space.
>>
>> Secondly, as an global configuration, it's due to usage of administrator with
>> it, if there is critical application which is sensitive with free space,
>> administrator should make sure our reservation should not overload consuming free
>> space, which means soft reservation is not suitable.
>>
>>> To avoid this, I change the code to:
>>>
>>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>>> +            sbi->cur_reserved_blocks = t;
>>>
>>> I think it is only safe to decrease the value of cur_reserved_blocks, and leave increase operation to
>>> dec_valid_block(,node)_count, it is safe to increase cur_reserved_blocks there.
>> For initialization of reserved_blocks, cur_reserved_blocks will always be zero
>> due to this check, and will be updated to close to reserved_blocks after block
>> allocation and deallocation of user, IMO, it's not looks reasonable to user.
>>
>> Anyway, it's due to how you define semantics of soft reservation, so what is your
>> understanding of it?
>>
>> Thanks,
>>
>>>> Thanks,
>>>>
>>>>>>             spin_unlock(&sbi->stat_lock);
>>>>>>             return count;
>>>>>>         }
>>>> .
>>>>
>> .
>>
> 

Re: [PATCH v3] f2fs: add cur_reserved_blocks to support soft block reservation

[Yunlong Song]

Agree

On 2017/10/26 11:26, Chao Yu wrote:
> On 2017/10/26 11:07, Yunlong Song wrote:
>> Yes, I agree with the soft semantic you introduce, it is too slow to
>> increase cur_reserved_blocks only in
>> dec_valid_block(,node)_count, e.g. if users want to set
>> cur_reserved_blocks to 10G.
> Yup,
>
>> Then how about fix the initialization of cur_reserved_blocks in
>> fs/f2fs/super.c as following:
>> sbi->current_reserved_blocks = 0
>> change to
>> sbi->current_reserved_blocks = min(sbi->reserved_blocks,
>> sbi->user_block_count - valid_user_blocks(sbi));
> It will be necessary only if reserved_blocks is initialized with a non-zero value,
> but now it has value of zero, so it's redundant...
>
> What about adding this check if we initialize reserved_blocks with a non-zero default
> value or value which may be configured with mount_option?
>
> Thanks,
>
>> On 2017/10/25 23:46, Chao Yu wrote:
>>> On 2017/10/25 22:06, Yunlong Song wrote:
>>>> Hi, Chao,
>>>>       Please see my comments below.
>>>>
>>>> On 2017/10/25 20:26, Chao Yu wrote:
>>>>> On 2017/10/25 18:02, Yunlong Song wrote:
>>>>>> ping...
>>>>> I've replied in this thread, check your email list please, or you can check the
>>>>> comments in below link:
>>>>>
>>>>> https://patchwork.kernel.org/patch/9909407/
>>>>>
>>>>> Anyway, see comments below.
>>>>>
>>>>>> On 2017/8/18 23:09, Yunlong Song wrote:
>>>>>>> This patch adds cur_reserved_blocks to extend reserved_blocks sysfs
>>>>>>> interface to be soft threshold, which allows user configure it exceeding
>>>>>>> current available user space. To ensure there is enough space for
>>>>>>> supporting system's activation, this patch does not set the reserved space
>>>>>>> to the configured reserved_blocks value at once, instead, it safely
>>>>>>> increase cur_reserved_blocks in dev_valid_block(,node)_count to only take
>>>>>>> up the blocks which are just obsoleted.
>>>>>>>
>>>>>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>>>> ---
>>>>>>>      Documentation/ABI/testing/sysfs-fs-f2fs |  3 ++-
>>>>>>>      fs/f2fs/f2fs.h                          | 13 +++++++++++--
>>>>>>>      fs/f2fs/super.c                         |  3 ++-
>>>>>>>      fs/f2fs/sysfs.c                         | 15 +++++++++++++--
>>>>>>>      4 files changed, 28 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>>> index 11b7f4e..ba282ca 100644
>>>>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>>>>> @@ -138,7 +138,8 @@ What:        /sys/fs/f2fs/<disk>/reserved_blocks
>>>>>>>      Date:        June 2017
>>>>>>>      Contact:    "Chao Yu" <yuchao0@huawei.com>
>>>>>>>      Description:
>>>>>>> -         Controls current reserved blocks in system.
>>>>>>> +         Controls current reserved blocks in system, the threshold
>>>>>>> +         is soft, it could exceed current available user space.
>>>>>>>        What:        /sys/fs/f2fs/<disk>/gc_urgent
>>>>>>>      Date:        August 2017
>>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>>> index 2f20b6b..84ccbdc 100644
>>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>>> @@ -1041,6 +1041,7 @@ struct f2fs_sb_info {
>>>>>>>          block_t discard_blks;            /* discard command candidats */
>>>>>>>          block_t last_valid_block_count;        /* for recovery */
>>>>>>>          block_t reserved_blocks;        /* configurable reserved blocks */
>>>>>>> +    block_t cur_reserved_blocks;        /* current reserved blocks */
>>>>>>>            u32 s_next_generation;            /* for NFS support */
>>>>>>>      @@ -1515,7 +1516,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>>>            spin_lock(&sbi->stat_lock);
>>>>>>>          sbi->total_valid_block_count += (block_t)(*count);
>>>>>>> -    avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
>>>>>>> +    avail_user_block_count = sbi->user_block_count -
>>>>>>> +                        sbi->cur_reserved_blocks;
>>>>>>>          if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>>>>>>>              diff = sbi->total_valid_block_count - avail_user_block_count;
>>>>>>>              *count -= diff;
>>>>>>> @@ -1549,6 +1551,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>>>>>>>          f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>>>>>>>          f2fs_bug_on(sbi, inode->i_blocks < sectors);
>>>>>>>          sbi->total_valid_block_count -= (block_t)count;
>>>>>>> +    if (sbi->reserved_blocks &&
>>>>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>>>>> It's redundent check here...
>>>> I think in most cases, cur_reserved_blocks is equal to reserved_blocks, so we do not need to calculate min any more, otherwise,
>>>> if reserved_blocks is not 0, it will calculate min and set current_reserved_blocks each time.
>>> OK, IMO, in some condition, we can save dirtying cache line to decrease cache
>>> line missing with that check.
>>>
>>>>>>> +        sbi->cur_reserved_blocks = min(sbi->reserved_blocks,
>>>>>>> +                    sbi->cur_reserved_blocks + count);
>>>>>>>          spin_unlock(&sbi->stat_lock);
>>>>>>>          f2fs_i_blocks_write(inode, count, false, true);
>>>>>>>      }
>>>>>>> @@ -1695,7 +1701,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>>>          spin_lock(&sbi->stat_lock);
>>>>>>>            valid_block_count = sbi->total_valid_block_count + 1;
>>>>>>> -    if (unlikely(valid_block_count + sbi->reserved_blocks >
>>>>>>> +    if (unlikely(valid_block_count + sbi->cur_reserved_blocks >
>>>>>>>                              sbi->user_block_count)) {
>>>>>>>              spin_unlock(&sbi->stat_lock);
>>>>>>>              goto enospc;
>>>>>>> @@ -1738,6 +1744,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>>>>>>>            sbi->total_valid_node_count--;
>>>>>>>          sbi->total_valid_block_count--;
>>>>>>> +    if (sbi->reserved_blocks &&
>>>>>>> +        sbi->reserved_blocks != sbi->cur_reserved_blocks)
>>>>> Checking low boundary is more safe here.
>>>> I think cur_reserved_blocks can never be larger than reserved_blocks in any case. so min(reserved_blocks,
>>>> cur_reserved_blocks +1) is same to cur_reserved_blocks++ when reserved_blocks != cur_reserved_blocks
>>>> (which means reserved_blocks > cur_reserved_block )
>>> Ditto.
>>>
>>>>>>> +        sbi->cur_reserved_blocks++;
>>>>>>>            spin_unlock(&sbi->stat_lock);
>>>>>>>      diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>>>> index 4c1bdcb..16a805f 100644
>>>>>>> --- a/fs/f2fs/super.c
>>>>>>> +++ b/fs/f2fs/super.c
>>>>>>> @@ -957,7 +957,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>>>>>>>          buf->f_blocks = total_count - start_count;
>>>>>>>          buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>>>>>>>          buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
>>>>>>> -                        sbi->reserved_blocks;
>>>>>>> +                        sbi->cur_reserved_blocks;
>>>>>>>            avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>>>>>>>      @@ -2411,6 +2411,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>>>>                      le64_to_cpu(sbi->ckpt->valid_block_count);
>>>>>>>          sbi->last_valid_block_count = sbi->total_valid_block_count;
>>>>>>>          sbi->reserved_blocks = 0;
>>>>>>> +    sbi->cur_reserved_blocks = 0;
>>>>>>>            for (i = 0; i < NR_INODE_TYPE; i++) {
>>>>>>>              INIT_LIST_HEAD(&sbi->inode_list[i]);
>>>>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>>>>> index a1be5ac..75c37bb 100644
>>>>>>> --- a/fs/f2fs/sysfs.c
>>>>>>> +++ b/fs/f2fs/sysfs.c
>>>>>>> @@ -104,12 +104,22 @@ static ssize_t features_show(struct f2fs_attr *a,
>>>>>>>          return len;
>>>>>>>      }
>>>>>>>      +static ssize_t f2fs_reserved_blocks_show(struct f2fs_attr *a,
>>>>>>> +        struct f2fs_sb_info *sbi, char *buf)
>>>>>>> +{
>>>>>>> +    return snprintf(buf, PAGE_SIZE, "expected: %u\ncurrent: %u\n",
>>>>>>> +            sbi->reserved_blocks, sbi->cur_reserved_blocks);
>>>>>>> +}
>>>>>>> +
>>>>>>>      static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>>>>>>                  struct f2fs_sb_info *sbi, char *buf)
>>>>>>>      {
>>>>>>>          unsigned char *ptr = NULL;
>>>>>>>          unsigned int *ui;
>>>>>>>      +    if (a->struct_type == RESERVED_BLOCKS)
>>>>>>> +        return f2fs_reserved_blocks_show(a, sbi, buf);
>>>>>>> +
>>>>>>>          ptr = __struct_ptr(sbi, a->struct_type);
>>>>>>>          if (!ptr)
>>>>>>>              return -EINVAL;
>>>>>>> @@ -143,12 +153,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>>>>>      #endif
>>>>>>>          if (a->struct_type == RESERVED_BLOCKS) {
>>>>>>>              spin_lock(&sbi->stat_lock);
>>>>>>> -        if ((unsigned long)sbi->total_valid_block_count + t >
>>>>>>> -                (unsigned long)sbi->user_block_count) {
>>>>>>> +        if (t > (unsigned long)sbi->user_block_count) {
>>>>>>>                  spin_unlock(&sbi->stat_lock);
>>>>>>>                  return -EINVAL;
>>>>>>>              }
>>>>>>>              *ui = t;
>>>>>>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>>>>>>> +            sbi->cur_reserved_blocks = t;
>>>>> No, for 't < cur_reserved_blocks' case, cur_reserved_blocks will out of update
>>>>> even if there is enough free space. You know, for soft block resevation, we need
>>>>> to reserve blocks as many as possible, making free space being zero suddenly is
>>>>> possible.
>>>> I do not understand why it is not safe to decrease cur_reserved_blocks, for example,
>>>> if current cur_reserved_blocks is 100, now decrease it to 80, is there any problem?
>>>> If 80 will make free space zero, how does 100 exist?
>>>> And I do not think it is safe as following:
>>>>            *ui = t;
>>>> +        sbi->current_reserved_blocks = min(sbi->reserved_blocks,
>>>> +                sbi->user_block_count - valid_user_blocks(sbi));
>>>>
>>>> If user_block_count = 200, valid_user_blocks=150， reserved_blocks = 100,
>>>> then current_reserved_block = min(100,200-50) = 50, in this case, free space
>>>> is suddenly becoming zero.
>>> Free space becomes zero suddenly is safe, as I said before, I don't expect this
>>> feature can be used in android, instead, it may be used in distributed storage
>>> scenario, in where, once we configure soft_reserved_block making one server out
>>> of free space, it's not critical issue to this system since we can move current
>>> copy to another server which has enough free space.
>>>
>>> Secondly, as an global configuration, it's due to usage of administrator with
>>> it, if there is critical application which is sensitive with free space,
>>> administrator should make sure our reservation should not overload consuming free
>>> space, which means soft reservation is not suitable.
>>>
>>>> To avoid this, I change the code to:
>>>>
>>>> +        if (t < (unsigned long)sbi->cur_reserved_blocks)
>>>> +            sbi->cur_reserved_blocks = t;
>>>>
>>>> I think it is only safe to decrease the value of cur_reserved_blocks, and leave increase operation to
>>>> dec_valid_block(,node)_count, it is safe to increase cur_reserved_blocks there.
>>> For initialization of reserved_blocks, cur_reserved_blocks will always be zero
>>> due to this check, and will be updated to close to reserved_blocks after block
>>> allocation and deallocation of user, IMO, it's not looks reasonable to user.
>>>
>>> Anyway, it's due to how you define semantics of soft reservation, so what is your
>>> understanding of it?
>>>
>>> Thanks,
>>>
>>>>> Thanks,
>>>>>
>>>>>>>              spin_unlock(&sbi->stat_lock);
>>>>>>>              return count;
>>>>>>>          }
>>>>> .
>>>>>
>>> .
>>>
>
> .
>

-- 
Thanks,
Yunlong Song

[PATCH v4] f2fs: support soft block reservation

[Yunlong Song]

It renames reserved_blocks sysfs interface to target_reserved_blocks,
and supports to extend it to be soft threshold, which allows user
configure it exceeding current available user space. So this patch
also introduces a new sysfs interface called current_reserved_blocks to
show the current blocks which have already been reserved.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs | 15 +++++++++++++--
 fs/f2fs/f2fs.h                          | 15 ++++++++++++---
 fs/f2fs/super.c                         |  5 +++--
 fs/f2fs/sysfs.c                         | 20 +++++++++++++++-----
 4 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 11b7f4e..482d78b 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -134,11 +134,22 @@ Contact:	"Sheng Yong" <shengyong1@huawei.com>
 Description:
 		 Controls the injection type.
 
-What:		/sys/fs/f2fs/<disk>/reserved_blocks
+What:		/sys/fs/f2fs/<disk>/target_reserved_blocks
 Date:		June 2017
 Contact:	"Chao Yu" <yuchao0@huawei.com>
 Description:
-		 Controls current reserved blocks in system.
+		 Controls target reserved blocks in system, the threshold
+		 is soft, it could exceed current available user space.
+
+What:		/sys/fs/f2fs/<disk>/current_reserved_blocks
+Date:		October 2017
+Contact:	"Yunlong Song" <yunlong.song@huawei.com>
+Contact:	"Chao Yu" <yuchao0@huawei.com>
+Description:
+		 Shows current reserved blocks in system, it may be temporarily
+		 smaller than target_reserved_blocks, but will gradually
+		 increase to target_reserved_blocks when more free blocks are
+		 freed by user later.
 
 What:		/sys/fs/f2fs/<disk>/gc_urgent
 Date:		August 2017
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 13a96b8..c26fccb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1081,7 +1081,8 @@ struct f2fs_sb_info {
 	block_t total_valid_block_count;	/* # of valid blocks */
 	block_t discard_blks;			/* discard command candidats */
 	block_t last_valid_block_count;		/* for recovery */
-	block_t reserved_blocks;		/* configurable reserved blocks */
+	block_t target_reserved_blocks;		/* configurable reserved blocks */
+	block_t current_reserved_blocks;	/* current reserved blocks */
 
 	u32 s_next_generation;			/* for NFS support */
 
@@ -1557,7 +1558,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
 
 	spin_lock(&sbi->stat_lock);
 	sbi->total_valid_block_count += (block_t)(*count);
-	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
+	avail_user_block_count = sbi->user_block_count -
+					sbi->current_reserved_blocks;
 	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
 		diff = sbi->total_valid_block_count - avail_user_block_count;
 		*count -= diff;
@@ -1591,6 +1593,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
 	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
 	f2fs_bug_on(sbi, inode->i_blocks < sectors);
 	sbi->total_valid_block_count -= (block_t)count;
+	if (sbi->target_reserved_blocks &&
+		sbi->current_reserved_blocks < sbi->target_reserved_blocks)
+		sbi->current_reserved_blocks = min(sbi->target_reserved_blocks,
+					sbi->current_reserved_blocks + count);
 	spin_unlock(&sbi->stat_lock);
 	f2fs_i_blocks_write(inode, count, false, true);
 }
@@ -1737,7 +1743,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
 	spin_lock(&sbi->stat_lock);
 
 	valid_block_count = sbi->total_valid_block_count + 1;
-	if (unlikely(valid_block_count + sbi->reserved_blocks >
+	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
 						sbi->user_block_count)) {
 		spin_unlock(&sbi->stat_lock);
 		goto enospc;
@@ -1780,6 +1786,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
 
 	sbi->total_valid_node_count--;
 	sbi->total_valid_block_count--;
+	if (sbi->target_reserved_blocks &&
+		sbi->current_reserved_blocks < sbi->target_reserved_blocks)
+		sbi->current_reserved_blocks++;
 
 	spin_unlock(&sbi->stat_lock);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 213d2c1..c682978 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -987,7 +987,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_blocks = total_count - start_count;
 	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
 	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
-						sbi->reserved_blocks;
+						sbi->current_reserved_blocks;
 
 	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
 
@@ -2457,7 +2457,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->total_valid_block_count =
 				le64_to_cpu(sbi->ckpt->valid_block_count);
 	sbi->last_valid_block_count = sbi->total_valid_block_count;
-	sbi->reserved_blocks = 0;
+	sbi->target_reserved_blocks = 0;
+	sbi->current_reserved_blocks = 0;
 
 	for (i = 0; i < NR_INODE_TYPE; i++) {
 		INIT_LIST_HEAD(&sbi->inode_list[i]);
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index e09e59c..568e32f 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -30,7 +30,7 @@ enum {
 	FAULT_INFO_RATE,	/* struct f2fs_fault_info */
 	FAULT_INFO_TYPE,	/* struct f2fs_fault_info */
 #endif
-	RESERVED_BLOCKS,
+	RESERVED_BLOCKS,	/* struct f2fs_sb_info */
 };
 
 struct f2fs_attr {
@@ -114,6 +114,12 @@ static ssize_t features_show(struct f2fs_attr *a,
 	return len;
 }
 
+static ssize_t current_reserved_blocks_show(struct f2fs_attr *a,
+					struct f2fs_sb_info *sbi, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->current_reserved_blocks);
+}
+
 static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
 			struct f2fs_sb_info *sbi, char *buf)
 {
@@ -153,12 +159,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 #endif
 	if (a->struct_type == RESERVED_BLOCKS) {
 		spin_lock(&sbi->stat_lock);
-		if ((unsigned long)sbi->total_valid_block_count + t >
-				(unsigned long)sbi->user_block_count) {
+		if (t > (unsigned long)sbi->user_block_count) {
 			spin_unlock(&sbi->stat_lock);
 			return -EINVAL;
 		}
 		*ui = t;
+		sbi->current_reserved_blocks = min(sbi->target_reserved_blocks,
+				sbi->user_block_count - valid_user_blocks(sbi));
 		spin_unlock(&sbi->stat_lock);
 		return count;
 	}
@@ -272,7 +279,8 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, reclaim_segments, rec_prefree_segments);
 F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, max_small_discards, max_discards);
 F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, discard_granularity, discard_granularity);
-F2FS_RW_ATTR(RESERVED_BLOCKS, f2fs_sb_info, reserved_blocks, reserved_blocks);
+F2FS_RW_ATTR(RESERVED_BLOCKS, f2fs_sb_info, target_reserved_blocks,
+							target_reserved_blocks);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, batched_trim_sections, trim_sections);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, ipu_policy, ipu_policy);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_ipu_util, min_ipu_util);
@@ -293,6 +301,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 F2FS_GENERAL_RO_ATTR(dirty_segments);
 F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
 F2FS_GENERAL_RO_ATTR(features);
+F2FS_GENERAL_RO_ATTR(current_reserved_blocks);
 
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
@@ -337,7 +346,8 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 	ATTR_LIST(dirty_segments),
 	ATTR_LIST(lifetime_write_kbytes),
 	ATTR_LIST(features),
-	ATTR_LIST(reserved_blocks),
+	ATTR_LIST(target_reserved_blocks),
+	ATTR_LIST(current_reserved_blocks),
 	NULL,
 };
 
-- 
1.8.5.2

Re: [PATCH v4] f2fs: support soft block reservation

[Chao Yu]

On 2017/10/27 11:11, Yunlong Song wrote:
> It renames reserved_blocks sysfs interface to target_reserved_blocks,
> and supports to extend it to be soft threshold, which allows user
> configure it exceeding current available user space. So this patch
> also introduces a new sysfs interface called current_reserved_blocks to
> show the current blocks which have already been reserved.
> 
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  Documentation/ABI/testing/sysfs-fs-f2fs | 15 +++++++++++++--
>  fs/f2fs/f2fs.h                          | 15 ++++++++++++---
>  fs/f2fs/super.c                         |  5 +++--
>  fs/f2fs/sysfs.c                         | 20 +++++++++++++++-----
>  4 files changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 11b7f4e..482d78b 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -134,11 +134,22 @@ Contact:	"Sheng Yong" <shengyong1@huawei.com>
>  Description:
>  		 Controls the injection type.
>  
> -What:		/sys/fs/f2fs/<disk>/reserved_blocks
> +What:		/sys/fs/f2fs/<disk>/target_reserved_blocks

For backward compatibility, how about keep this interface name, instead
just update its description?

Thanks,

>  Date:		June 2017
>  Contact:	"Chao Yu" <yuchao0@huawei.com>
>  Description:
> -		 Controls current reserved blocks in system.
> +		 Controls target reserved blocks in system, the threshold
> +		 is soft, it could exceed current available user space.
> +
> +What:		/sys/fs/f2fs/<disk>/current_reserved_blocks
> +Date:		October 2017
> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
> +Contact:	"Chao Yu" <yuchao0@huawei.com>
> +Description:
> +		 Shows current reserved blocks in system, it may be temporarily
> +		 smaller than target_reserved_blocks, but will gradually
> +		 increase to target_reserved_blocks when more free blocks are
> +		 freed by user later.
>  
>  What:		/sys/fs/f2fs/<disk>/gc_urgent
>  Date:		August 2017
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 13a96b8..c26fccb 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1081,7 +1081,8 @@ struct f2fs_sb_info {
>  	block_t total_valid_block_count;	/* # of valid blocks */
>  	block_t discard_blks;			/* discard command candidats */
>  	block_t last_valid_block_count;		/* for recovery */
> -	block_t reserved_blocks;		/* configurable reserved blocks */
> +	block_t target_reserved_blocks;		/* configurable reserved blocks */
> +	block_t current_reserved_blocks;	/* current reserved blocks */
>  
>  	u32 s_next_generation;			/* for NFS support */
>  
> @@ -1557,7 +1558,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>  
>  	spin_lock(&sbi->stat_lock);
>  	sbi->total_valid_block_count += (block_t)(*count);
> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> +	avail_user_block_count = sbi->user_block_count -
> +					sbi->current_reserved_blocks;
>  	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>  		diff = sbi->total_valid_block_count - avail_user_block_count;
>  		*count -= diff;
> @@ -1591,6 +1593,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>  	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>  	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>  	sbi->total_valid_block_count -= (block_t)count;
> +	if (sbi->target_reserved_blocks &&
> +		sbi->current_reserved_blocks < sbi->target_reserved_blocks)
> +		sbi->current_reserved_blocks = min(sbi->target_reserved_blocks,
> +					sbi->current_reserved_blocks + count);
>  	spin_unlock(&sbi->stat_lock);
>  	f2fs_i_blocks_write(inode, count, false, true);
>  }
> @@ -1737,7 +1743,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>  	spin_lock(&sbi->stat_lock);
>  
>  	valid_block_count = sbi->total_valid_block_count + 1;
> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> +	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
>  						sbi->user_block_count)) {
>  		spin_unlock(&sbi->stat_lock);
>  		goto enospc;
> @@ -1780,6 +1786,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>  
>  	sbi->total_valid_node_count--;
>  	sbi->total_valid_block_count--;
> +	if (sbi->target_reserved_blocks &&
> +		sbi->current_reserved_blocks < sbi->target_reserved_blocks)
> +		sbi->current_reserved_blocks++;
>  
>  	spin_unlock(&sbi->stat_lock);
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 213d2c1..c682978 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -987,7 +987,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	buf->f_blocks = total_count - start_count;
>  	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>  	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> -						sbi->reserved_blocks;
> +						sbi->current_reserved_blocks;
>  
>  	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>  
> @@ -2457,7 +2457,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  	sbi->total_valid_block_count =
>  				le64_to_cpu(sbi->ckpt->valid_block_count);
>  	sbi->last_valid_block_count = sbi->total_valid_block_count;
> -	sbi->reserved_blocks = 0;
> +	sbi->target_reserved_blocks = 0;
> +	sbi->current_reserved_blocks = 0;
>  
>  	for (i = 0; i < NR_INODE_TYPE; i++) {
>  		INIT_LIST_HEAD(&sbi->inode_list[i]);
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index e09e59c..568e32f 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -30,7 +30,7 @@ enum {
>  	FAULT_INFO_RATE,	/* struct f2fs_fault_info */
>  	FAULT_INFO_TYPE,	/* struct f2fs_fault_info */
>  #endif
> -	RESERVED_BLOCKS,
> +	RESERVED_BLOCKS,	/* struct f2fs_sb_info */
>  };
>  
>  struct f2fs_attr {
> @@ -114,6 +114,12 @@ static ssize_t features_show(struct f2fs_attr *a,
>  	return len;
>  }
>  
> +static ssize_t current_reserved_blocks_show(struct f2fs_attr *a,
> +					struct f2fs_sb_info *sbi, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->current_reserved_blocks);
> +}
> +
>  static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>  			struct f2fs_sb_info *sbi, char *buf)
>  {
> @@ -153,12 +159,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>  #endif
>  	if (a->struct_type == RESERVED_BLOCKS) {
>  		spin_lock(&sbi->stat_lock);
> -		if ((unsigned long)sbi->total_valid_block_count + t >
> -				(unsigned long)sbi->user_block_count) {
> +		if (t > (unsigned long)sbi->user_block_count) {
>  			spin_unlock(&sbi->stat_lock);
>  			return -EINVAL;
>  		}
>  		*ui = t;
> +		sbi->current_reserved_blocks = min(sbi->target_reserved_blocks,
> +				sbi->user_block_count - valid_user_blocks(sbi));
>  		spin_unlock(&sbi->stat_lock);
>  		return count;
>  	}
> @@ -272,7 +279,8 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, reclaim_segments, rec_prefree_segments);
>  F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, max_small_discards, max_discards);
>  F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, discard_granularity, discard_granularity);
> -F2FS_RW_ATTR(RESERVED_BLOCKS, f2fs_sb_info, reserved_blocks, reserved_blocks);
> +F2FS_RW_ATTR(RESERVED_BLOCKS, f2fs_sb_info, target_reserved_blocks,
> +							target_reserved_blocks);
>  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, batched_trim_sections, trim_sections);
>  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, ipu_policy, ipu_policy);
>  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_ipu_util, min_ipu_util);
> @@ -293,6 +301,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>  F2FS_GENERAL_RO_ATTR(dirty_segments);
>  F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>  F2FS_GENERAL_RO_ATTR(features);
> +F2FS_GENERAL_RO_ATTR(current_reserved_blocks);
>  
>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
>  F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
> @@ -337,7 +346,8 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>  	ATTR_LIST(dirty_segments),
>  	ATTR_LIST(lifetime_write_kbytes),
>  	ATTR_LIST(features),
> -	ATTR_LIST(reserved_blocks),
> +	ATTR_LIST(target_reserved_blocks),
> +	ATTR_LIST(current_reserved_blocks),
>  	NULL,
>  };
>  
> 

Re: [PATCH v4] f2fs: support soft block reservation

[Jaegeuk Kim]

On 10/27, Chao Yu wrote:
> On 2017/10/27 11:11, Yunlong Song wrote:
> > It renames reserved_blocks sysfs interface to target_reserved_blocks,
> > and supports to extend it to be soft threshold, which allows user
> > configure it exceeding current available user space. So this patch
> > also introduces a new sysfs interface called current_reserved_blocks to
> > show the current blocks which have already been reserved.
> > 
> > Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> >  Documentation/ABI/testing/sysfs-fs-f2fs | 15 +++++++++++++--
> >  fs/f2fs/f2fs.h                          | 15 ++++++++++++---
> >  fs/f2fs/super.c                         |  5 +++--
> >  fs/f2fs/sysfs.c                         | 20 +++++++++++++++-----
> >  4 files changed, 43 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> > index 11b7f4e..482d78b 100644
> > --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> > +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> > @@ -134,11 +134,22 @@ Contact:	"Sheng Yong" <shengyong1@huawei.com>
> >  Description:
> >  		 Controls the injection type.
> >  
> > -What:		/sys/fs/f2fs/<disk>/reserved_blocks
> > +What:		/sys/fs/f2fs/<disk>/target_reserved_blocks
> 
> For backward compatibility, how about keep this interface name, instead
> just update its description?

Good catch. It makes sense to keep the entry name. :)

Thanks,

> 
> Thanks,
> 
> >  Date:		June 2017
> >  Contact:	"Chao Yu" <yuchao0@huawei.com>
> >  Description:
> > -		 Controls current reserved blocks in system.
> > +		 Controls target reserved blocks in system, the threshold
> > +		 is soft, it could exceed current available user space.
> > +
> > +What:		/sys/fs/f2fs/<disk>/current_reserved_blocks
> > +Date:		October 2017
> > +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
> > +Contact:	"Chao Yu" <yuchao0@huawei.com>
> > +Description:
> > +		 Shows current reserved blocks in system, it may be temporarily
> > +		 smaller than target_reserved_blocks, but will gradually
> > +		 increase to target_reserved_blocks when more free blocks are
> > +		 freed by user later.
> >  
> >  What:		/sys/fs/f2fs/<disk>/gc_urgent
> >  Date:		August 2017
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 13a96b8..c26fccb 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1081,7 +1081,8 @@ struct f2fs_sb_info {
> >  	block_t total_valid_block_count;	/* # of valid blocks */
> >  	block_t discard_blks;			/* discard command candidats */
> >  	block_t last_valid_block_count;		/* for recovery */
> > -	block_t reserved_blocks;		/* configurable reserved blocks */
> > +	block_t target_reserved_blocks;		/* configurable reserved blocks */
> > +	block_t current_reserved_blocks;	/* current reserved blocks */
> >  
> >  	u32 s_next_generation;			/* for NFS support */
> >  
> > @@ -1557,7 +1558,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
> >  
> >  	spin_lock(&sbi->stat_lock);
> >  	sbi->total_valid_block_count += (block_t)(*count);
> > -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> > +	avail_user_block_count = sbi->user_block_count -
> > +					sbi->current_reserved_blocks;
> >  	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
> >  		diff = sbi->total_valid_block_count - avail_user_block_count;
> >  		*count -= diff;
> > @@ -1591,6 +1593,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
> >  	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
> >  	f2fs_bug_on(sbi, inode->i_blocks < sectors);
> >  	sbi->total_valid_block_count -= (block_t)count;
> > +	if (sbi->target_reserved_blocks &&
> > +		sbi->current_reserved_blocks < sbi->target_reserved_blocks)
> > +		sbi->current_reserved_blocks = min(sbi->target_reserved_blocks,
> > +					sbi->current_reserved_blocks + count);
> >  	spin_unlock(&sbi->stat_lock);
> >  	f2fs_i_blocks_write(inode, count, false, true);
> >  }
> > @@ -1737,7 +1743,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
> >  	spin_lock(&sbi->stat_lock);
> >  
> >  	valid_block_count = sbi->total_valid_block_count + 1;
> > -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> > +	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
> >  						sbi->user_block_count)) {
> >  		spin_unlock(&sbi->stat_lock);
> >  		goto enospc;
> > @@ -1780,6 +1786,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
> >  
> >  	sbi->total_valid_node_count--;
> >  	sbi->total_valid_block_count--;
> > +	if (sbi->target_reserved_blocks &&
> > +		sbi->current_reserved_blocks < sbi->target_reserved_blocks)
> > +		sbi->current_reserved_blocks++;
> >  
> >  	spin_unlock(&sbi->stat_lock);
> >  
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 213d2c1..c682978 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -987,7 +987,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
> >  	buf->f_blocks = total_count - start_count;
> >  	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
> >  	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> > -						sbi->reserved_blocks;
> > +						sbi->current_reserved_blocks;
> >  
> >  	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
> >  
> > @@ -2457,7 +2457,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> >  	sbi->total_valid_block_count =
> >  				le64_to_cpu(sbi->ckpt->valid_block_count);
> >  	sbi->last_valid_block_count = sbi->total_valid_block_count;
> > -	sbi->reserved_blocks = 0;
> > +	sbi->target_reserved_blocks = 0;
> > +	sbi->current_reserved_blocks = 0;
> >  
> >  	for (i = 0; i < NR_INODE_TYPE; i++) {
> >  		INIT_LIST_HEAD(&sbi->inode_list[i]);
> > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> > index e09e59c..568e32f 100644
> > --- a/fs/f2fs/sysfs.c
> > +++ b/fs/f2fs/sysfs.c
> > @@ -30,7 +30,7 @@ enum {
> >  	FAULT_INFO_RATE,	/* struct f2fs_fault_info */
> >  	FAULT_INFO_TYPE,	/* struct f2fs_fault_info */
> >  #endif
> > -	RESERVED_BLOCKS,
> > +	RESERVED_BLOCKS,	/* struct f2fs_sb_info */
> >  };
> >  
> >  struct f2fs_attr {
> > @@ -114,6 +114,12 @@ static ssize_t features_show(struct f2fs_attr *a,
> >  	return len;
> >  }
> >  
> > +static ssize_t current_reserved_blocks_show(struct f2fs_attr *a,
> > +					struct f2fs_sb_info *sbi, char *buf)
> > +{
> > +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->current_reserved_blocks);
> > +}
> > +
> >  static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
> >  			struct f2fs_sb_info *sbi, char *buf)
> >  {
> > @@ -153,12 +159,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
> >  #endif
> >  	if (a->struct_type == RESERVED_BLOCKS) {
> >  		spin_lock(&sbi->stat_lock);
> > -		if ((unsigned long)sbi->total_valid_block_count + t >
> > -				(unsigned long)sbi->user_block_count) {
> > +		if (t > (unsigned long)sbi->user_block_count) {
> >  			spin_unlock(&sbi->stat_lock);
> >  			return -EINVAL;
> >  		}
> >  		*ui = t;
> > +		sbi->current_reserved_blocks = min(sbi->target_reserved_blocks,
> > +				sbi->user_block_count - valid_user_blocks(sbi));
> >  		spin_unlock(&sbi->stat_lock);
> >  		return count;
> >  	}
> > @@ -272,7 +279,8 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> >  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, reclaim_segments, rec_prefree_segments);
> >  F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, max_small_discards, max_discards);
> >  F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, discard_granularity, discard_granularity);
> > -F2FS_RW_ATTR(RESERVED_BLOCKS, f2fs_sb_info, reserved_blocks, reserved_blocks);
> > +F2FS_RW_ATTR(RESERVED_BLOCKS, f2fs_sb_info, target_reserved_blocks,
> > +							target_reserved_blocks);
> >  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, batched_trim_sections, trim_sections);
> >  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, ipu_policy, ipu_policy);
> >  F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_ipu_util, min_ipu_util);
> > @@ -293,6 +301,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> >  F2FS_GENERAL_RO_ATTR(dirty_segments);
> >  F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
> >  F2FS_GENERAL_RO_ATTR(features);
> > +F2FS_GENERAL_RO_ATTR(current_reserved_blocks);
> >  
> >  #ifdef CONFIG_F2FS_FS_ENCRYPTION
> >  F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
> > @@ -337,7 +346,8 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> >  	ATTR_LIST(dirty_segments),
> >  	ATTR_LIST(lifetime_write_kbytes),
> >  	ATTR_LIST(features),
> > -	ATTR_LIST(reserved_blocks),
> > +	ATTR_LIST(target_reserved_blocks),
> > +	ATTR_LIST(current_reserved_blocks),
> >  	NULL,
> >  };
> >  
> > 

[PATCH v5] f2fs: support soft block reservation

[Yunlong Song]

It supports to extend reserved_blocks sysfs interface to be soft
threshold, which allows user configure it exceeding current available
user space. This patch also introduces a new sysfs interface called
current_reserved_blocks, which shows the current blocks that have
already been reserved.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs | 15 +++++++++++++--
 fs/f2fs/f2fs.h                          | 13 +++++++++++--
 fs/f2fs/super.c                         |  3 ++-
 fs/f2fs/sysfs.c                         | 15 ++++++++++++---
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 11b7f4e..482d78b 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -134,11 +134,22 @@ Contact:	"Sheng Yong" <shengyong1@huawei.com>
 Description:
 		 Controls the injection type.
 
-What:		/sys/fs/f2fs/<disk>/reserved_blocks
+What:		/sys/fs/f2fs/<disk>/target_reserved_blocks
 Date:		June 2017
 Contact:	"Chao Yu" <yuchao0@huawei.com>
 Description:
-		 Controls current reserved blocks in system.
+		 Controls target reserved blocks in system, the threshold
+		 is soft, it could exceed current available user space.
+
+What:		/sys/fs/f2fs/<disk>/current_reserved_blocks
+Date:		October 2017
+Contact:	"Yunlong Song" <yunlong.song@huawei.com>
+Contact:	"Chao Yu" <yuchao0@huawei.com>
+Description:
+		 Shows current reserved blocks in system, it may be temporarily
+		 smaller than target_reserved_blocks, but will gradually
+		 increase to target_reserved_blocks when more free blocks are
+		 freed by user later.
 
 What:		/sys/fs/f2fs/<disk>/gc_urgent
 Date:		August 2017
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 13a96b8..5ed40fb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1082,6 +1082,7 @@ struct f2fs_sb_info {
 	block_t discard_blks;			/* discard command candidats */
 	block_t last_valid_block_count;		/* for recovery */
 	block_t reserved_blocks;		/* configurable reserved blocks */
+	block_t current_reserved_blocks;	/* current reserved blocks */
 
 	u32 s_next_generation;			/* for NFS support */
 
@@ -1557,7 +1558,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
 
 	spin_lock(&sbi->stat_lock);
 	sbi->total_valid_block_count += (block_t)(*count);
-	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
+	avail_user_block_count = sbi->user_block_count -
+					sbi->current_reserved_blocks;
 	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
 		diff = sbi->total_valid_block_count - avail_user_block_count;
 		*count -= diff;
@@ -1591,6 +1593,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
 	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
 	f2fs_bug_on(sbi, inode->i_blocks < sectors);
 	sbi->total_valid_block_count -= (block_t)count;
+	if (sbi->reserved_blocks &&
+		sbi->current_reserved_blocks < sbi->reserved_blocks)
+		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+					sbi->current_reserved_blocks + count);
 	spin_unlock(&sbi->stat_lock);
 	f2fs_i_blocks_write(inode, count, false, true);
 }
@@ -1737,7 +1743,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
 	spin_lock(&sbi->stat_lock);
 
 	valid_block_count = sbi->total_valid_block_count + 1;
-	if (unlikely(valid_block_count + sbi->reserved_blocks >
+	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
 						sbi->user_block_count)) {
 		spin_unlock(&sbi->stat_lock);
 		goto enospc;
@@ -1780,6 +1786,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
 
 	sbi->total_valid_node_count--;
 	sbi->total_valid_block_count--;
+	if (sbi->reserved_blocks &&
+		sbi->current_reserved_blocks < sbi->reserved_blocks)
+		sbi->current_reserved_blocks++;
 
 	spin_unlock(&sbi->stat_lock);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 213d2c1..dab1fed 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -987,7 +987,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_blocks = total_count - start_count;
 	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
 	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
-						sbi->reserved_blocks;
+						sbi->current_reserved_blocks;
 
 	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
 
@@ -2458,6 +2458,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 				le64_to_cpu(sbi->ckpt->valid_block_count);
 	sbi->last_valid_block_count = sbi->total_valid_block_count;
 	sbi->reserved_blocks = 0;
+	sbi->current_reserved_blocks = 0;
 
 	for (i = 0; i < NR_INODE_TYPE; i++) {
 		INIT_LIST_HEAD(&sbi->inode_list[i]);
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index e09e59c..4166ac7 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -30,7 +30,7 @@ enum {
 	FAULT_INFO_RATE,	/* struct f2fs_fault_info */
 	FAULT_INFO_TYPE,	/* struct f2fs_fault_info */
 #endif
-	RESERVED_BLOCKS,
+	RESERVED_BLOCKS,	/* struct f2fs_sb_info */
 };
 
 struct f2fs_attr {
@@ -114,6 +114,12 @@ static ssize_t features_show(struct f2fs_attr *a,
 	return len;
 }
 
+static ssize_t current_reserved_blocks_show(struct f2fs_attr *a,
+					struct f2fs_sb_info *sbi, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->current_reserved_blocks);
+}
+
 static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
 			struct f2fs_sb_info *sbi, char *buf)
 {
@@ -153,12 +159,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 #endif
 	if (a->struct_type == RESERVED_BLOCKS) {
 		spin_lock(&sbi->stat_lock);
-		if ((unsigned long)sbi->total_valid_block_count + t >
-				(unsigned long)sbi->user_block_count) {
+		if (t > (unsigned long)sbi->user_block_count) {
 			spin_unlock(&sbi->stat_lock);
 			return -EINVAL;
 		}
 		*ui = t;
+		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+				sbi->user_block_count - valid_user_blocks(sbi));
 		spin_unlock(&sbi->stat_lock);
 		return count;
 	}
@@ -293,6 +300,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 F2FS_GENERAL_RO_ATTR(dirty_segments);
 F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
 F2FS_GENERAL_RO_ATTR(features);
+F2FS_GENERAL_RO_ATTR(current_reserved_blocks);
 
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
@@ -338,6 +346,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 	ATTR_LIST(lifetime_write_kbytes),
 	ATTR_LIST(features),
 	ATTR_LIST(reserved_blocks),
+	ATTR_LIST(current_reserved_blocks),
 	NULL,
 };
 
-- 
1.8.5.2

Re: [f2fs-dev] [PATCH v5] f2fs: support soft block reservation

[Chao Yu]

On 2017/10/27 19:47, Yunlong Song wrote:
> It supports to extend reserved_blocks sysfs interface to be soft
> threshold, which allows user configure it exceeding current available
> user space. This patch also introduces a new sysfs interface called
> current_reserved_blocks, which shows the current blocks that have
> already been reserved.
> 
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  Documentation/ABI/testing/sysfs-fs-f2fs | 15 +++++++++++++--
>  fs/f2fs/f2fs.h                          | 13 +++++++++++--
>  fs/f2fs/super.c                         |  3 ++-
>  fs/f2fs/sysfs.c                         | 15 ++++++++++++---
>  4 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 11b7f4e..482d78b 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -134,11 +134,22 @@ Contact:	"Sheng Yong" <shengyong1@huawei.com>
>  Description:
>  		 Controls the injection type.
>  
> -What:		/sys/fs/f2fs/<disk>/reserved_blocks
> +What:		/sys/fs/f2fs/<disk>/target_reserved_blocks

It needs to keep old sysfs interface description as you did in code.

Thanks,

>  Date:		June 2017
>  Contact:	"Chao Yu" <yuchao0@huawei.com>
>  Description:
> -		 Controls current reserved blocks in system.
> +		 Controls target reserved blocks in system, the threshold
> +		 is soft, it could exceed current available user space.
> +
> +What:		/sys/fs/f2fs/<disk>/current_reserved_blocks
> +Date:		October 2017
> +Contact:	"Yunlong Song" <yunlong.song@huawei.com>
> +Contact:	"Chao Yu" <yuchao0@huawei.com>
> +Description:
> +		 Shows current reserved blocks in system, it may be temporarily
> +		 smaller than target_reserved_blocks, but will gradually
> +		 increase to target_reserved_blocks when more free blocks are
> +		 freed by user later.
>  
>  What:		/sys/fs/f2fs/<disk>/gc_urgent
>  Date:		August 2017
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 13a96b8..5ed40fb 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1082,6 +1082,7 @@ struct f2fs_sb_info {
>  	block_t discard_blks;			/* discard command candidats */
>  	block_t last_valid_block_count;		/* for recovery */
>  	block_t reserved_blocks;		/* configurable reserved blocks */
> +	block_t current_reserved_blocks;	/* current reserved blocks */
>  
>  	u32 s_next_generation;			/* for NFS support */
>  
> @@ -1557,7 +1558,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>  
>  	spin_lock(&sbi->stat_lock);
>  	sbi->total_valid_block_count += (block_t)(*count);
> -	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
> +	avail_user_block_count = sbi->user_block_count -
> +					sbi->current_reserved_blocks;
>  	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
>  		diff = sbi->total_valid_block_count - avail_user_block_count;
>  		*count -= diff;
> @@ -1591,6 +1593,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
>  	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
>  	f2fs_bug_on(sbi, inode->i_blocks < sectors);
>  	sbi->total_valid_block_count -= (block_t)count;
> +	if (sbi->reserved_blocks &&
> +		sbi->current_reserved_blocks < sbi->reserved_blocks)
> +		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
> +					sbi->current_reserved_blocks + count);
>  	spin_unlock(&sbi->stat_lock);
>  	f2fs_i_blocks_write(inode, count, false, true);
>  }
> @@ -1737,7 +1743,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
>  	spin_lock(&sbi->stat_lock);
>  
>  	valid_block_count = sbi->total_valid_block_count + 1;
> -	if (unlikely(valid_block_count + sbi->reserved_blocks >
> +	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
>  						sbi->user_block_count)) {
>  		spin_unlock(&sbi->stat_lock);
>  		goto enospc;
> @@ -1780,6 +1786,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>  
>  	sbi->total_valid_node_count--;
>  	sbi->total_valid_block_count--;
> +	if (sbi->reserved_blocks &&
> +		sbi->current_reserved_blocks < sbi->reserved_blocks)
> +		sbi->current_reserved_blocks++;
>  
>  	spin_unlock(&sbi->stat_lock);
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 213d2c1..dab1fed 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -987,7 +987,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	buf->f_blocks = total_count - start_count;
>  	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
>  	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
> -						sbi->reserved_blocks;
> +						sbi->current_reserved_blocks;
>  
>  	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
>  
> @@ -2458,6 +2458,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  				le64_to_cpu(sbi->ckpt->valid_block_count);
>  	sbi->last_valid_block_count = sbi->total_valid_block_count;
>  	sbi->reserved_blocks = 0;
> +	sbi->current_reserved_blocks = 0;
>  
>  	for (i = 0; i < NR_INODE_TYPE; i++) {
>  		INIT_LIST_HEAD(&sbi->inode_list[i]);
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index e09e59c..4166ac7 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -30,7 +30,7 @@ enum {
>  	FAULT_INFO_RATE,	/* struct f2fs_fault_info */
>  	FAULT_INFO_TYPE,	/* struct f2fs_fault_info */
>  #endif
> -	RESERVED_BLOCKS,
> +	RESERVED_BLOCKS,	/* struct f2fs_sb_info */
>  };
>  
>  struct f2fs_attr {
> @@ -114,6 +114,12 @@ static ssize_t features_show(struct f2fs_attr *a,
>  	return len;
>  }
>  
> +static ssize_t current_reserved_blocks_show(struct f2fs_attr *a,
> +					struct f2fs_sb_info *sbi, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->current_reserved_blocks);
> +}
> +
>  static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>  			struct f2fs_sb_info *sbi, char *buf)
>  {
> @@ -153,12 +159,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>  #endif
>  	if (a->struct_type == RESERVED_BLOCKS) {
>  		spin_lock(&sbi->stat_lock);
> -		if ((unsigned long)sbi->total_valid_block_count + t >
> -				(unsigned long)sbi->user_block_count) {
> +		if (t > (unsigned long)sbi->user_block_count) {
>  			spin_unlock(&sbi->stat_lock);
>  			return -EINVAL;
>  		}
>  		*ui = t;
> +		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
> +				sbi->user_block_count - valid_user_blocks(sbi));
>  		spin_unlock(&sbi->stat_lock);
>  		return count;
>  	}
> @@ -293,6 +300,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>  F2FS_GENERAL_RO_ATTR(dirty_segments);
>  F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
>  F2FS_GENERAL_RO_ATTR(features);
> +F2FS_GENERAL_RO_ATTR(current_reserved_blocks);
>  
>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
>  F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
> @@ -338,6 +346,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>  	ATTR_LIST(lifetime_write_kbytes),
>  	ATTR_LIST(features),
>  	ATTR_LIST(reserved_blocks),
> +	ATTR_LIST(current_reserved_blocks),
>  	NULL,
>  };
>  
> 

[PATCH v5 RESEND] f2fs: support soft block reservation

[Yunlong Song]

It supports to extend reserved_blocks sysfs interface to be soft
threshold, which allows user configure it exceeding current available
user space. This patch also introduces a new sysfs interface called
current_reserved_blocks, which shows the current blocks that have
already been reserved.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs | 13 ++++++++++++-
 fs/f2fs/f2fs.h                          | 13 +++++++++++--
 fs/f2fs/super.c                         |  3 ++-
 fs/f2fs/sysfs.c                         | 15 ++++++++++++---
 4 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 11b7f4e..7562b0d 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -138,7 +138,18 @@ What:		/sys/fs/f2fs/<disk>/reserved_blocks
 Date:		June 2017
 Contact:	"Chao Yu" <yuchao0@huawei.com>
 Description:
-		 Controls current reserved blocks in system.
+		 Controls target reserved blocks in system, the threshold
+		 is soft, it could exceed current available user space.
+
+What:		/sys/fs/f2fs/<disk>/current_reserved_blocks
+Date:		October 2017
+Contact:	"Yunlong Song" <yunlong.song@huawei.com>
+Contact:	"Chao Yu" <yuchao0@huawei.com>
+Description:
+		 Shows current reserved blocks in system, it may be temporarily
+		 smaller than target_reserved_blocks, but will gradually
+		 increase to target_reserved_blocks when more free blocks are
+		 freed by user later.
 
 What:		/sys/fs/f2fs/<disk>/gc_urgent
 Date:		August 2017
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 13a96b8..5ed40fb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1082,6 +1082,7 @@ struct f2fs_sb_info {
 	block_t discard_blks;			/* discard command candidats */
 	block_t last_valid_block_count;		/* for recovery */
 	block_t reserved_blocks;		/* configurable reserved blocks */
+	block_t current_reserved_blocks;	/* current reserved blocks */
 
 	u32 s_next_generation;			/* for NFS support */
 
@@ -1557,7 +1558,8 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
 
 	spin_lock(&sbi->stat_lock);
 	sbi->total_valid_block_count += (block_t)(*count);
-	avail_user_block_count = sbi->user_block_count - sbi->reserved_blocks;
+	avail_user_block_count = sbi->user_block_count -
+					sbi->current_reserved_blocks;
 	if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
 		diff = sbi->total_valid_block_count - avail_user_block_count;
 		*count -= diff;
@@ -1591,6 +1593,10 @@ static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
 	f2fs_bug_on(sbi, sbi->total_valid_block_count < (block_t) count);
 	f2fs_bug_on(sbi, inode->i_blocks < sectors);
 	sbi->total_valid_block_count -= (block_t)count;
+	if (sbi->reserved_blocks &&
+		sbi->current_reserved_blocks < sbi->reserved_blocks)
+		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+					sbi->current_reserved_blocks + count);
 	spin_unlock(&sbi->stat_lock);
 	f2fs_i_blocks_write(inode, count, false, true);
 }
@@ -1737,7 +1743,7 @@ static inline int inc_valid_node_count(struct f2fs_sb_info *sbi,
 	spin_lock(&sbi->stat_lock);
 
 	valid_block_count = sbi->total_valid_block_count + 1;
-	if (unlikely(valid_block_count + sbi->reserved_blocks >
+	if (unlikely(valid_block_count + sbi->current_reserved_blocks >
 						sbi->user_block_count)) {
 		spin_unlock(&sbi->stat_lock);
 		goto enospc;
@@ -1780,6 +1786,9 @@ static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
 
 	sbi->total_valid_node_count--;
 	sbi->total_valid_block_count--;
+	if (sbi->reserved_blocks &&
+		sbi->current_reserved_blocks < sbi->reserved_blocks)
+		sbi->current_reserved_blocks++;
 
 	spin_unlock(&sbi->stat_lock);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 213d2c1..dab1fed 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -987,7 +987,7 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_blocks = total_count - start_count;
 	buf->f_bfree = user_block_count - valid_user_blocks(sbi) + ovp_count;
 	buf->f_bavail = user_block_count - valid_user_blocks(sbi) -
-						sbi->reserved_blocks;
+						sbi->current_reserved_blocks;
 
 	avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
 
@@ -2458,6 +2458,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 				le64_to_cpu(sbi->ckpt->valid_block_count);
 	sbi->last_valid_block_count = sbi->total_valid_block_count;
 	sbi->reserved_blocks = 0;
+	sbi->current_reserved_blocks = 0;
 
 	for (i = 0; i < NR_INODE_TYPE; i++) {
 		INIT_LIST_HEAD(&sbi->inode_list[i]);
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index e09e59c..4166ac7 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -30,7 +30,7 @@ enum {
 	FAULT_INFO_RATE,	/* struct f2fs_fault_info */
 	FAULT_INFO_TYPE,	/* struct f2fs_fault_info */
 #endif
-	RESERVED_BLOCKS,
+	RESERVED_BLOCKS,	/* struct f2fs_sb_info */
 };
 
 struct f2fs_attr {
@@ -114,6 +114,12 @@ static ssize_t features_show(struct f2fs_attr *a,
 	return len;
 }
 
+static ssize_t current_reserved_blocks_show(struct f2fs_attr *a,
+					struct f2fs_sb_info *sbi, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", sbi->current_reserved_blocks);
+}
+
 static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
 			struct f2fs_sb_info *sbi, char *buf)
 {
@@ -153,12 +159,13 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 #endif
 	if (a->struct_type == RESERVED_BLOCKS) {
 		spin_lock(&sbi->stat_lock);
-		if ((unsigned long)sbi->total_valid_block_count + t >
-				(unsigned long)sbi->user_block_count) {
+		if (t > (unsigned long)sbi->user_block_count) {
 			spin_unlock(&sbi->stat_lock);
 			return -EINVAL;
 		}
 		*ui = t;
+		sbi->current_reserved_blocks = min(sbi->reserved_blocks,
+				sbi->user_block_count - valid_user_blocks(sbi));
 		spin_unlock(&sbi->stat_lock);
 		return count;
 	}
@@ -293,6 +300,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 F2FS_GENERAL_RO_ATTR(dirty_segments);
 F2FS_GENERAL_RO_ATTR(lifetime_write_kbytes);
 F2FS_GENERAL_RO_ATTR(features);
+F2FS_GENERAL_RO_ATTR(current_reserved_blocks);
 
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
@@ -338,6 +346,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 	ATTR_LIST(lifetime_write_kbytes),
 	ATTR_LIST(features),
 	ATTR_LIST(reserved_blocks),
+	ATTR_LIST(current_reserved_blocks),
 	NULL,
 };
 
-- 
1.8.5.2