Re: [f2fs-dev] [PATCH 07/14] f2fs: make GC migration large-folio aware

[Daeho Jeong]

On Wed, Aug 26, 2026 at 1:33 AM Nanzhe Zhao via Linux-f2fs-devel
<linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> GC can operate on a 4K block that is cached inside a large folio.
> The data lookup helpers therefore need to test and update uptodate
> state for the addressed subpage instead of rejecting large folios or
> treating the whole folio as the target block.
>
> Let f2fs_get_read_data_folio(), f2fs_find_data_folio(), and
> f2fs_get_lock_data_folio() to use subpage uptodate state. Submit
> single-block reads at the requested folio offset and zero only the
> addressed 4K range for NEW_ADDR.
>
> Also update `move_data_page` to mark, clear, and restore dirty
> state for the target subpage, and submit write I/O with the subpage
> offset recorded in f2fs_io_info.
>
> Signed-off-by: Nanzhe Zhao <zhaonanzhe@xxxxxxxxxx>
> ---
>  fs/f2fs/data.c | 96 ++++++++++++++++++++++++++++++++++----------------
>  fs/f2fs/f2fs.h |  6 ++--
>  fs/f2fs/gc.c   | 30 ++++++++++++++--
>  3 files changed, 97 insertions(+), 35 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 608d8ea8e607..0e54b1e25893 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1241,19 +1241,31 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode,
>
>  /* This can handle encryption stuffs */
>  static void f2fs_submit_page_read(struct inode *inode, struct fsverity_info *vi,
> -                                 struct folio *folio, block_t blkaddr,
> -                                 blk_opf_t op_flags, bool for_write)
> +                                 struct folio *folio, pgoff_t index,
> +                                 block_t blkaddr, blk_opf_t op_flags,
> +                                 bool for_write)
>  {
>         struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>         struct bio *bio;
> +       size_t offset = 0;
>
> -       bio = f2fs_grab_read_bio(inode, vi, blkaddr, 1, op_flags, folio->index,
> +       if (f2fs_folio_has_ffs(folio)) {
> +               struct f2fs_folio_state *ffs = folio->private;
> +               unsigned long flags;
> +
> +               offset = offset_in_folio(folio, (loff_t)index << PAGE_SHIFT);
> +               spin_lock_irqsave(&ffs->state_lock, flags);
> +               ffs->read_pages_pending++;
> +               spin_unlock_irqrestore(&ffs->state_lock, flags);
> +       }
> +
> +       bio = f2fs_grab_read_bio(inode, vi, blkaddr, 1, op_flags, index,
>                                  for_write);
>
>         /* wait for GCed page writeback via META_MAPPING */
>         f2fs_wait_on_block_writeback(inode, blkaddr);
>
> -       if (!bio_add_folio(bio, folio, PAGE_SIZE, 0))
> +       if (!bio_add_folio(bio, folio, PAGE_SIZE, offset))
>                 f2fs_bug_on(sbi, 1);
>
>         inc_page_count(sbi, F2FS_RD_DATA);
> @@ -1399,21 +1411,13 @@ struct folio *f2fs_get_read_data_folio(struct inode *inode, pgoff_t index,
>         struct dnode_of_data dn;
>         struct folio *folio;
>         int err;
> -retry:
> +
>         folio = f2fs_grab_cache_folio(mapping, index, for_write);
>         if (IS_ERR(folio))
>                 return folio;
>
> -       if (folio_test_large(folio)) {
> -               pgoff_t folio_index = mapping_align_index(mapping, index);
> -               unsigned long nr_pages = folio_nr_pages(folio);
> -
> -               f2fs_folio_put(folio, true);
> -               invalidate_inode_pages2_range(mapping, folio_index,
> -                               folio_index + nr_pages - 1);
> -               f2fs_schedule_timeout(DEFAULT_SCHEDULE_TIMEOUT);
> -               goto retry;
> -       }
> +       if (folio_test_large(folio))
> +               f2fs_ffs_find_or_alloc(folio);
>
>         if (f2fs_lookup_read_extent_cache_block(inode, index,
>                                                 &dn.data_blkaddr)) {
> @@ -1448,7 +1452,7 @@ struct folio *f2fs_get_read_data_folio(struct inode *inode, pgoff_t index,
>                 goto put_err;
>         }
>  got_it:
> -       if (folio_test_uptodate(folio)) {
> +       if (f2fs_ffs_test_blk_uptodate(folio, index)) {
>                 folio_unlock(folio);
>                 return folio;
>         }
> @@ -1461,15 +1465,17 @@ struct folio *f2fs_get_read_data_folio(struct inode *inode, pgoff_t index,
>          * f2fs_init_inode_metadata.
>          */
>         if (dn.data_blkaddr == NEW_ADDR) {
> -               folio_zero_segment(folio, 0, folio_size(folio));
> -               if (!folio_test_uptodate(folio))
> -                       folio_mark_uptodate(folio);
> +               size_t offset = offset_in_folio(folio,
> +                                               (loff_t)index << PAGE_SHIFT);
> +
> +               folio_zero_segment(folio, offset, offset + PAGE_SIZE);
> +               f2fs_ffs_mark_subrange_uptodate(folio, offset, PAGE_SIZE);
>                 folio_unlock(folio);
>                 return folio;
>         }
>
> -       f2fs_submit_page_read(inode, f2fs_need_verity(inode, folio->index),
> -                             folio, dn.data_blkaddr, op_flags, for_write);
> +       f2fs_submit_page_read(inode, f2fs_need_verity(inode, index),
> +                             folio, index, dn.data_blkaddr, op_flags, for_write);
>         return folio;
>
>  put_err:
> @@ -1486,7 +1492,7 @@ struct folio *f2fs_find_data_folio(struct inode *inode, pgoff_t index,
>         folio = f2fs_filemap_get_folio(mapping, index, FGP_ACCESSED, 0);
>         if (IS_ERR(folio))
>                 goto read;
> -       if (folio_test_uptodate(folio))
> +       if (f2fs_ffs_test_blk_uptodate(folio, index))
>                 return folio;
>         f2fs_folio_put(folio, false);
>
> @@ -1495,11 +1501,11 @@ struct folio *f2fs_find_data_folio(struct inode *inode, pgoff_t index,
>         if (IS_ERR(folio))
>                 return folio;
>
> -       if (folio_test_uptodate(folio))
> +       if (f2fs_ffs_test_blk_uptodate(folio, index))
>                 return folio;
>
>         folio_wait_locked(folio);
> -       if (unlikely(!folio_test_uptodate(folio))) {
> +       if (unlikely(!f2fs_ffs_test_blk_uptodate(folio, index))) {
>                 f2fs_folio_put(folio, false);
>                 return ERR_PTR(-EIO);
>         }
> @@ -1523,7 +1529,8 @@ struct folio *f2fs_get_lock_data_folio(struct inode *inode, pgoff_t index,
>
>         /* wait for read completion */
>         folio_lock(folio);
> -       if (unlikely(folio->mapping != mapping || !folio_test_uptodate(folio))) {
> +       if (unlikely(folio->mapping != mapping ||
> +                   !f2fs_ffs_test_blk_uptodate(folio, index))) {
>                 f2fs_folio_put(folio, true);
>                 return ERR_PTR(-EIO);
>         }
> @@ -2681,6 +2688,23 @@ static void f2fs_ffs_mark_subrange_uptodate(struct folio *folio, size_t offset,
>                 folio_mark_uptodate(folio);
>  }
>
> +bool f2fs_ffs_test_blk_dirty(const struct folio *folio, pgoff_t index)
> +{
> +       struct f2fs_folio_state *ffs;
> +       unsigned int idx, nr_subpages;
> +
> +       if (!f2fs_folio_has_ffs(folio))
> +               return folio_test_dirty(folio);
> +
> +       if (!folio_contains(folio, index))
> +               return false;
> +
> +       ffs = folio->private;
> +       nr_subpages = folio_nr_pages(folio);
> +       idx = index - folio->index;
> +       return test_bit(nr_subpages + idx, ffs->state);
> +}
> +
>  void f2fs_ffs_mark_subrange_dirty(struct folio *folio,
>                                     size_t offset, size_t len)
>  {
> @@ -2719,18 +2743,30 @@ static bool __ffs_clear_subrange_dirty(struct folio *folio,
>                         2 * nr_subpages;
>  }
>
> -void f2fs_ffs_clear_subrange_dirty(struct folio *folio, size_t offset, size_t len)
> +/**
> + * f2fs_ffs_clear_subrange_dirty - clear the dirty bits of a folio subrange
> + * @folio: the large folio
> + * @offset: start byte offset within the folio
> + * @len: byte length of the subrange
> + *
> + * Clear the dirty bits of the 4K subpages covered by [offset, offset + len),
> + * and return whether the folio still has any dirty subpage left.
> + */
> +bool f2fs_ffs_clear_subrange_dirty(struct folio *folio, size_t offset,
> +                                  size_t len)
>  {
>         struct f2fs_folio_state *ffs;
>         unsigned long flags;
> +       bool dirty;
>
>         if (!f2fs_folio_has_ffs(folio))
> -               return;
> +               return false;
>
> -       ffs = (struct f2fs_folio_state *)folio->private;
> +       ffs = folio->private;
>         spin_lock_irqsave(&ffs->state_lock, flags);
> -       __ffs_clear_subrange_dirty(folio, ffs, offset, len);
> +       dirty = __ffs_clear_subrange_dirty(folio, ffs, offset, len);
>         spin_unlock_irqrestore(&ffs->state_lock, flags);
> +       return dirty;
>  }
>
>  static unsigned int ffs_next_dirty_subpage(struct f2fs_folio_state *ffs,
> @@ -4853,7 +4889,7 @@ static int f2fs_write_begin(const struct kiocb *iocb,
>                  */
>                 f2fs_submit_page_read(inode,
>                                       NULL, /* can't write to fsverity files */
> -                                     folio, blkaddr, 0, true);
> +                                     folio, index, blkaddr, 0, true);
>
>                 folio_lock(folio);
>                 if (unlikely(folio->mapping != mapping)) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 71e6d7e34c7b..0ffbe2bd04c8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4344,10 +4344,10 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
>                                 int compr_blocks, bool allow_balance);
>  bool f2fs_ffs_test_blk_uptodate(const struct folio *folio, pgoff_t index);
>  struct f2fs_folio_state *f2fs_ffs_find_or_alloc(struct folio *folio);
> +bool f2fs_ffs_test_blk_dirty(const struct folio *folio, pgoff_t index);
>  void f2fs_ffs_mark_subrange_dirty(struct folio *folio, size_t offset, size_t len);
> -bool f2fs_ffs_clear_subrange_dirty_and_test(struct folio *folio, size_t offset,
> -                                       size_t len);
> -void f2fs_ffs_clear_subrange_dirty(struct folio *folio, size_t offset, size_t len);
> +bool f2fs_ffs_clear_subrange_dirty(struct folio *folio, size_t offset,
> +                                  size_t len);
>  void f2fs_write_failed(struct inode *inode, loff_t to);
>  void f2fs_invalidate_folio(struct folio *folio, size_t offset, size_t length);
>  bool f2fs_release_folio(struct folio *folio, gfp_t wait);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 0c17038fcfd7..42e6dfcbf98d 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1504,12 +1504,19 @@ static int move_data_page(struct inode *inode, block_t bidx, int gc_type,
>                                                 unsigned int segno, int off)
>  {
>         struct folio *folio;
> +       size_t foff = 0;
> +       bool large = false;
>         int err = 0;
>
>         folio = f2fs_get_lock_data_folio(inode, bidx, true);
>         if (IS_ERR(folio))
>                 return PTR_ERR(folio);
>
> +       if (f2fs_folio_has_ffs(folio)) {
> +               large = true;
> +               foff = offset_in_folio(folio, (loff_t)bidx << PAGE_SHIFT);
> +       }
> +
>         if (!check_valid_map(F2FS_I_SB(inode), segno, off)) {
>                 err = -ENOENT;
>                 goto out;
> @@ -1524,6 +1531,8 @@ static int move_data_page(struct inode *inode, block_t bidx, int gc_type,
>                         err = -EAGAIN;
>                         goto out;
>                 }
> +               if (large)
> +                       f2fs_ffs_mark_subrange_dirty(folio, foff, PAGE_SIZE);
>                 folio_mark_dirty(folio);
>                 folio_set_f2fs_gcing(folio);
>         } else {
> @@ -1536,32 +1545,49 @@ static int move_data_page(struct inode *inode, block_t bidx, int gc_type,
>                         .op_flags = REQ_SYNC,
>                         .old_blkaddr = NULL_ADDR,
>                         .folio = folio,
> +                       .folio_offset = bidx - folio->index,
> +                       .folio_blkcnt = 1,
>                         .encrypted_page = NULL,
>                         .need_lock = LOCK_REQ,
>                         .io_type = FS_GC_DATA_IO,
>                 };
> -               bool is_dirty = folio_test_dirty(folio);
> +               struct f2fs_folio_state *ffs = NULL;
> +               bool is_dirty = f2fs_ffs_test_blk_dirty(folio, bidx);
>
>  retry:
>                 f2fs_folio_wait_writeback(folio, DATA, true, true);
>
> +               if (large) {
> +                       ffs = folio->private;
> +                       f2fs_ffs_mark_subrange_dirty(folio, foff, PAGE_SIZE);
> +               }
>                 folio_mark_dirty(folio);
>                 if (folio_clear_dirty_for_io(folio)) {
>                         inode_dec_dirty_pages(inode);
>                         f2fs_remove_dirty_inode(inode);
> +                       if (large &&
> +                           f2fs_ffs_clear_subrange_dirty(folio, foff, PAGE_SIZE))
> +                               folio_mark_dirty(folio);
>                 }
>
> +               if (large)
> +                       atomic_inc(&ffs->write_pages_pending);

Under Patch 08, a cleanly read large folio may NOT have
f2fs_folio_state allocated
(folio->private == NULL), right?

When foreground/background GC tries to migrate a 4KB block in such a folio,
`ffs` is NULL, and `atomic_inc(&ffs->write_pages_pending)` will instantly crash
with a NULL pointer dereference!

Fix: Call `ffs = f2fs_ffs_find_or_alloc(folio);` before accessing `ffs`.

Thanks,

>                 folio_set_f2fs_gcing(folio);
>
>                 err = f2fs_do_write_data_page(&fio);
>                 if (err) {
>                         folio_clear_f2fs_gcing(folio);
> +                       if (large)
> +                               atomic_dec(&ffs->write_pages_pending);
>                         if (err == -ENOMEM) {
>                                 memalloc_retry_wait(GFP_NOFS);
>                                 goto retry;
>                         }
> -                       if (is_dirty)
> +                       if (is_dirty) {
> +                               if (large)
> +                                       f2fs_ffs_mark_subrange_dirty(folio, foff, PAGE_SIZE);
>                                 folio_mark_dirty(folio);
> +                       }
>                 }
>         }
>  out:
> --
> 2.43.0
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel