From: Chao Yu <chao@kernel.org>
To: Xiuhong Wang <xiuhong.wang@unisoc.com>,
jaegeuk@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org
Cc: ke.wang@unisoc.com, xiuhong.wang.cn@gmail.com, hongyu.jin.cn@gmail.com
Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: compress: fix reserve_cblocks counting error when out of space
Date: Wed, 6 Mar 2024 10:02:07 +0800 [thread overview]
Message-ID: <f4bc07b0-a32b-4074-a66e-9388cb1b4461@kernel.org> (raw)
In-Reply-To: <20240305084023.3686070-2-xiuhong.wang@unisoc.com>
On 2024/3/5 16:40, Xiuhong Wang wrote:
> When a file only needs one direct_node, performing the following
> operations will cause the file to be unrepairable:
>
> unisoc # ./f2fs_io compress test.apk
> unisoc #df -h | grep dm-48
> /dev/block/dm-48 112G 112G 1.2M 100% /data
>
> unisoc # ./f2fs_io release_cblocks test.apk
> 924
> unisoc # df -h | grep dm-48
> /dev/block/dm-48 112G 112G 4.8M 100% /data
>
> unisoc # dd if=/dev/random of=file4 bs=1M count=3
> 3145728 bytes (3.0 M) copied, 0.025 s, 120 M/s
> unisoc # df -h | grep dm-48
> /dev/block/dm-48 112G 112G 1.8M 100% /data
>
> unisoc # ./f2fs_io reserve_cblocks test.apk
> F2FS_IOC_RESERVE_COMPRESS_BLOCKS failed: No space left on device
>
> adb reboot
> unisoc # df -h | grep dm-48
> /dev/block/dm-48 112G 112G 11M 100% /data
> unisoc # ./f2fs_io reserve_cblocks test.apk
> 0
>
> This is because the file has only one direct_node. After returning
> to -ENOSPC, reserved_blocks += ret will not be executed. As a result,
> the reserved_blocks at this time is still 0, which is not the real
> number of reserved blocks. Therefore, fsck cannot be set to repair
> the file.
>
> After this patch, the fsck flag will be set to fix this problem.
>
> unisoc # df -h | grep dm-48
> /dev/block/dm-48 112G 112G 1.8M 100% /data
> unisoc # ./f2fs_io reserve_cblocks test.apk
> F2FS_IOC_RESERVE_COMPRESS_BLOCKS failed: No space left on device
>
> adb reboot then fsck will be executed
> unisoc # df -h | grep dm-48
> /dev/block/dm-48 112G 112G 11M 100% /data
> unisoc # ./f2fs_io reserve_cblocks test.apk
> 924
>
> Fixes: c75488fb4d82 ("f2fs: introduce F2FS_IOC_RESERVE_COMPRESS_BLOCKS")
> Signed-off-by: Xiuhong Wang <xiuhong.wang@unisoc.com>
> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> ---
> fs/f2fs/file.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 572d7bd4d161..97a7233c7ea7 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3624,10 +3624,10 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
> return ret;
> }
>
> -static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
> +static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count,
> + unsigned int *reserved_blocks)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> - unsigned int reserved_blocks = 0;
> int cluster_size = F2FS_I(dn->inode)->i_cluster_size;
> block_t blkaddr;
> int i;
> @@ -3691,12 +3691,12 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
>
> f2fs_i_compr_blocks_update(dn->inode, compr_blocks, true);
>
> - reserved_blocks += reserved;
> + *reserved_blocks += reserved;
> next:
> count -= cluster_size;
> }
>
> - return reserved_blocks;
> + return 0;
> }
>
> static int f2fs_reserve_compress_blocks(struct file *filp, unsigned long arg)
> @@ -3740,6 +3740,7 @@ static int f2fs_reserve_compress_blocks(struct file *filp, unsigned long arg)
> while (page_idx < last_idx) {
> struct dnode_of_data dn;
> pgoff_t end_offset, count;
> + unsigned int tmp_reserved_blocks;
>
> set_new_dnode(&dn, inode, NULL, NULL, 0);
> ret = f2fs_get_dnode_of_data(&dn, page_idx, LOOKUP_NODE);
> @@ -3757,7 +3758,8 @@ static int f2fs_reserve_compress_blocks(struct file *filp, unsigned long arg)
> count = min(end_offset - dn.ofs_in_node, last_idx - page_idx);
> count = round_up(count, F2FS_I(inode)->i_cluster_size);
>
> - ret = reserve_compress_blocks(&dn, count);
> + ret = reserve_compress_blocks(&dn, count, &tmp_reserved_blocks);
How about passing &reserved_blocks into reserve_compress_blocks()?
Thanks,
> + reserved_blocks += tmp_reserved_blocks;
>
> f2fs_put_dnode(&dn);
>
> @@ -3765,13 +3767,12 @@ static int f2fs_reserve_compress_blocks(struct file *filp, unsigned long arg)
> break;
>
> page_idx += count;
> - reserved_blocks += ret;
> }
>
> filemap_invalidate_unlock(inode->i_mapping);
> f2fs_up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>
> - if (ret >= 0) {
> + if (!ret) {
> clear_inode_flag(inode, FI_COMPRESS_RELEASED);
> inode_set_ctime_current(inode);
> f2fs_mark_inode_dirty_sync(inode, true);
> @@ -3780,7 +3781,7 @@ static int f2fs_reserve_compress_blocks(struct file *filp, unsigned long arg)
> inode_unlock(inode);
> mnt_drop_write_file(filp);
>
> - if (ret >= 0) {
> + if (!ret) {
> ret = put_user(reserved_blocks, (u64 __user *)arg);
> } else if (reserved_blocks &&
> atomic_read(&F2FS_I(inode)->i_compr_blocks)) {
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2024-03-06 2:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-05 8:40 [f2fs-dev] [PATCH 1/2] f2fs: compress: relocate some judgments in f2fs_reserve_compress_blocks Xiuhong Wang
2024-03-05 8:40 ` [f2fs-dev] [PATCH 2/2] f2fs: compress: fix reserve_cblocks counting error when out of space Xiuhong Wang
2024-03-06 2:02 ` Chao Yu [this message]
2024-03-06 1:51 ` [f2fs-dev] [PATCH 1/2] f2fs: compress: relocate some judgments in f2fs_reserve_compress_blocks Chao Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f4bc07b0-a32b-4074-a66e-9388cb1b4461@kernel.org \
--to=chao@kernel.org \
--cc=hongyu.jin.cn@gmail.com \
--cc=jaegeuk@kernel.org \
--cc=ke.wang@unisoc.com \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=xiuhong.wang.cn@gmail.com \
--cc=xiuhong.wang@unisoc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).