Re: [f2fs-dev] [PATCH 15/20] f2fs: support subpage blocks in atomic writes

[Chao Yu]

On 11/3/26 03:39, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@xxxxxxxxxx>
> 
> Writeback already writes dirty blocks to the COW inode per block. Make
> write_begin of atomic files reserve and read COW blocks per block as
> well, and remove the -EOPNOTSUPP checks. Skipping the read of the
> blocks that a write fully covers also applies to large folios when the
> block size equals PAGE_SIZE.

With this patch atomic files are allowed in subpage mode, so
f2fs_abort_atomic_write(inode, true) can extend i_size back to
original_i_size (e.g. after F2FS_IOC_START_ATOMIC_REPLACE set it to 0).
FI_ATOMIC_FILE is cleared before i_size is restored and abort doesn't hold
invalidate_lock, so a racing mmap fault can run ->page_mkwrite() with the
small i_size and map the folio writable; stores to its blocks between the
old EOF and original_i_size then don't fault and aren't marked dirty.
Should we call pagecache_isize_extended() after restoring i_size there, as
patch 08 does for the other i_size extension paths?

Thanks,

> 
> Signed-off-by: Daeho Jeong <daehojeong@xxxxxxxxxx>
> ---
>  fs/f2fs/data.c | 24 ++++++++++++++++--------
>  fs/f2fs/file.c |  5 +----
>  2 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index a23fd4b01a4c..4e4d1b0d6004 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -4624,7 +4624,7 @@ static int f2fs_write_cache_folios(struct address_space *mapping,
>  		}
>  
>  		if (f2fs_has_subpage_blocks(sbi) &&
> -		    (f2fs_compressed_file(inode) || f2fs_is_atomic_file(inode))) {
> +		    f2fs_compressed_file(inode)) {
>  			WARN_ON_ONCE(1);
>  			mapping_set_error(mapping, -EOPNOTSUPP);
>  			folio_clear_dirty_for_io(folio);
> @@ -5247,7 +5247,7 @@ static int prepare_large_folio_write_begin(struct inode *inode,
>  	int err = 0;
>  
>  	if (f2fs_has_subpage_blocks(sbi) &&
> -	    (f2fs_compressed_file(inode) || f2fs_is_atomic_file(inode)))
> +	    f2fs_compressed_file(inode))
>  		return -EOPNOTSUPP;
>  
>  	/*
> @@ -5359,13 +5359,16 @@ static int prepare_large_folio_atomic_write_begin(struct inode *inode,
>  	/* Inline data must have been converted before reaching here. */
>  	f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
>  
> -	start = folio->index + (ori_off >> PAGE_SHIFT);
> -	end = folio->index + ((ori_off + len - 1) >> PAGE_SHIFT);
> +	start = f2fs_folio_lblk(sbi, folio) + (ori_off >> sbi->log_blocksize);
> +	end = f2fs_folio_lblk(sbi, folio) +
> +		((ori_off + len - 1) >> sbi->log_blocksize);
>  
>  	for (index = start; index <= end; index++) {
> +		loff_t block_start = F2FS_BLK_TO_BYTES(sbi, index);
> +		loff_t block_end = block_start + F2FS_BLKSIZE(sbi);
>  		block_t blkaddr = NULL_ADDR;
>  		bool node_changed = false;
> -		size_t off = (index - folio->index) << PAGE_SHIFT;
> +		size_t off = f2fs_folio_byte_offset(sbi, folio, index);
>  
>  		err = prepare_atomic_write_begin(sbi, inode, index,
>  						&blkaddr, &node_changed,
> @@ -5373,12 +5376,17 @@ static int prepare_large_folio_atomic_write_begin(struct inode *inode,
>  		if (err)
>  			return err;
>  
> +		/* Skip read if the write covers the entire block. */
> +		if (pos <= block_start && pos + len >= block_end)
> +			goto balance;
> +
>  		if (f2fs_ffs_test_blk_uptodate(folio, index))
>  			goto balance;
>  
>  		if (blkaddr == NEW_ADDR) {
> -			folio_zero_segment(folio, off, off + PAGE_SIZE);
> -			f2fs_ffs_mark_subrange_uptodate(folio, off, PAGE_SIZE);
> +			folio_zero_segment(folio, off, off + F2FS_BLKSIZE(sbi));
> +			f2fs_ffs_mark_subrange_uptodate(folio, off,
> +							F2FS_BLKSIZE(sbi));
>  			goto balance;
>  		}
>  
> @@ -5436,7 +5444,7 @@ static int f2fs_write_begin(const struct kiocb *iocb,
>  	}
>  
>  	if (f2fs_has_subpage_blocks(sbi) &&
> -	    (f2fs_compressed_file(inode) || f2fs_is_atomic_file(inode))) {
> +	    f2fs_compressed_file(inode)) {
>  		err = -EOPNOTSUPP;
>  		goto fail;
>  	}
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 1c440b5fa9bd..91f792cf879f 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2299,7 +2299,7 @@ static long f2fs_fallocate(struct file *file, int mode,
>  	}
>  
>  	if (f2fs_has_subpage_blocks(F2FS_I_SB(inode))) {
> -		if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
> +		if (f2fs_compressed_file(inode)) {
>  			ret = -EOPNOTSUPP;
>  			goto out;
>  		}
> @@ -2553,9 +2553,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
>  	loff_t isize;
>  	int ret;
>  
> -	if (f2fs_has_subpage_blocks(sbi))
> -		return -EOPNOTSUPP;
> -
>  	if (!(filp->f_mode & FMODE_WRITE))
>  		return -EBADF;
>