Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752820AbdHKKSm (ORCPT ); Fri, 11 Aug 2017 06:18:42 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:3066 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751873AbdHKKSk (ORCPT ); Fri, 11 Aug 2017 06:18:40 -0400 Subject: Re: [PATCH] f2fs: introduce cur_reserved_blocks in sysfs To: Yunlong Song , , , CC: , , , , References: <1502199825-15218-1-git-send-email-yunlong.song@huawei.com> <5952a72c-9b42-6f5a-ce03-410b42ada799@huawei.com> <5bd696d4-a79d-5e58-e4a1-ce9ecbe979c7@huawei.com> <8e96a71d-9b01-9fb4-d855-d08106ea2d60@huawei.com> From: Chao Yu Message-ID: <58a75537-d7ff-1e9b-8b4b-86b78935ae62@huawei.com> Date: Fri, 11 Aug 2017 18:18:25 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <8e96a71d-9b01-9fb4-d855-d08106ea2d60@huawei.com> Content-Type: text/plain; charset="windows-1252" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.134.22.195] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020202.598D8474.0101,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 00ef47ba489f6035b08b5984ab41273c Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10097 Lines: 221 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 >>>>> --- >>>>> 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" >>>>> Description: >>>>> Controls sleep time of GC urgent mode >>>>> + >>>>> +What: /sys/fs/f2fs//cur_reserved_blocks >>>>> +Date: August 2017 >>>>> +Contact: "Yunlong Song" >>>>> +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, >>>>> }; >>>>> >>>>> >>>> . >>>> >> >> . >> >