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

[Daeho Jeong]

On Tue, Nov 10, 2026 at 3:50 AM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> 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,
>

Right, thanks for catching this.

There is one more problem in the same window. A read which instantiates
the folio there zero-fills the blocks beyond the small i_size and marks
them uptodate, while they have the original data on disk once i_size is
restored. pagecache_isize_extended() write-protects the folio, but it
does not drop these stale blocks.

So I'd rather close the window. In v2, the callers of
f2fs_abort_atomic_write() hold invalidate_lock across it, as
f2fs_ioc_start_atomic_write() does. Then neither ->page_mkwrite() nor a
read can see the page cache between dropping it and restoring i_size, and
no folio is left when the lock is released, so pagecache_isize_extended()
is not needed.

The eviction path is the only caller which doesn't take it, since no one
uses the page cache then.

> >
> > 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;
> >
>