linux-kernel - Re: [f2fs-dev] [PATCH] ENOSPC returned but there still many free segments

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <26f11617-92e4-1dc5-38fd-5048e92e059e@huawei.com>
Date:   Mon, 23 Mar 2020 09:42:17 +0800
From:   Chao Yu <yuchao0@...wei.com>
To:     chenying <chen.ying153@....com.cn>, <jaegeuk@...nel.org>
CC:     <wang.yi59@....com.cn>, <xue.zhihong@....com.cn>,
        <linux-kernel@...r.kernel.org>,
        <linux-f2fs-devel@...ts.sourceforge.net>, <jiang.xuexin@....com.cn>
Subject: Re: [f2fs-dev] [PATCH] ENOSPC returned but there still many free
 segments

On 2020/3/21 9:31, chenying wrote:
> I write data to a compressed file when the disk space is almost full
> and it return -ENOSPC error, but cat /sys/kernel/debug/f2fs/status
> shows that there still many free segments.

free segments include reserved segments, so its value should never be zero,
otherwise there should be a bug.

BTW, could you check result in 'stat -f <your_mountpoint>' rather than f2fs
status in debugfs?

> 
> Signed-off-by: chenying <chen.ying153@....com.cn>
> ---
>  fs/f2fs/compress.c | 5 ++++-
>  fs/f2fs/file.c     | 4 ++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index d8a64be..6ca058b 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -854,6 +854,8 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>  				fio.compr_blocks++;
>  			if (__is_valid_data_blkaddr(blkaddr))
>  				f2fs_invalidate_blocks(sbi, blkaddr);
> +			else if (blkaddr != NULL_ADDR)
> +				dec_valid_block_count(sbi, dn.inode, 1);
>  			f2fs_update_data_blkaddr(&dn, COMPRESS_ADDR);
>  			goto unlock_continue;
>  		}
> @@ -865,7 +867,8 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>  			if (__is_valid_data_blkaddr(blkaddr)) {
>  				f2fs_invalidate_blocks(sbi, blkaddr);
>  				f2fs_update_data_blkaddr(&dn, NEW_ADDR);
> -			}
> +			} else if (blkaddr != NULL_ADDR)
> +				dec_valid_block_count(sbi, dn.inode, 1);

I don't think this is correct, you could check message in git pull of 5.6:

Quoted:

"f2fs-for-5.6

In this series, we've implemented transparent compression experimentally. It
supports LZO and LZ4, but will add more later as we investigate in the field
more. At this point, the feature doesn't expose compressed space to user
directly in order to guarantee potential data updates later to the space.
Instead, the main goal is to reduce data writes to flash disk as much as
possible, resulting in extending disk life time as well as relaxing IO
congestion. Alternatively, we're also considering to add ioctl() to reclaim
compressed space and show it to user after putting the immutable bit.
"

That means we will keep reserved blocks in compressed cluster until user
release them via ioctl.

Thanks,

>  			goto unlock_continue;
>  		}
>  
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 0d4da64..f07c9e2 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -589,6 +589,10 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  			clear_inode_flag(dn->inode, FI_FIRST_BLOCK_WRITTEN);
>  
>  		f2fs_invalidate_blocks(sbi, blkaddr);
> +		if (compressed_cluster &&
> +			(blkaddr == NEW_ADDR || blkaddr == COMPRESS_ADDR))
> +			continue;
> +
>  		nr_free++;
>  	}
>  
>