Re: [PATCH] f2fs: account compressed writeback by raw folio type

[Wenjie Qi]

  I rechecked the earlier discussion and the current code.
  https://lore.kernel.org/linux-f2fs-devel/CAGFpFsRQmvqTY=W9_ofF93hzQAi7rF9tS3zUiLWixfSiYd4WBw@xxxxxxxxxxxxxx/T/#u

  I see the issue now. I had been treating this as a
writeback-accounting problem and assumed
  the type should follow the raw folio. Your explanation makes it
clear
  that the current F2FS_WB_CP_DATA classification for compressed
writeback
  was intentional, so that fsync/sync keeps the compressed cluster
under
  the checkpoint-guaranteed path for POR safety.

  With that in mind, the reasoning for my patch is not sound, since I
did
  not show that the same guarantee is preserved after changing the
type.

  Please ignore this patch.

On Mon, Aug 3, 2026 at 10:33 AM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 6/16/26 20:52, Wenjie Qi wrote:
> > Compressed writeback is always charged as F2FS_WB_CP_DATA today because
> > WB_DATA_TYPE() is called with compressed_page set.  That is wrong for
> > normal compressed file writeback; the CP guarantee belongs to the raw
> > pagecache folio, not the compressed bounce page.
>
> IIUC, I changed compressed writeback guaranteed by checkpoint to avoid
> compressed cluster corruption when encountering power off recovery after
> fsync/sync.
>
> I didn't get why it's wrong to set F2FS_WB_CP_DATA for compressed page,
> can you please explain more?
>
> Thanks,
>
> >
> > Use the raw folio for the writeback count type.  Also keep one raw page
> > under writeback until the compressed end_io callback is done using sbi,
> > as normal compressed writeback is no longer protected by the CP-data
> > counter.
>  > > Signed-off-by: Wenjie Qi <qiwenjie@xxxxxxxxxx>
> > ---
> > QEMU/KASAN tested with compress_mode=fs and compress_mode=user, using
> > concurrent 1MiB compressed writes and a background sync loop.  The user
> > mode case also ran F2FS_IOC_COMPRESS_FILE and fsync.  No dmesg splat was
> > reported and fsck.f2fs passed after unmount.
> >
> >   fs/f2fs/compress.c | 34 +++++++++++++++++++++++++++-------
> >   fs/f2fs/data.c     |  2 +-
> >   2 files changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index 91855d91bbdd..be9483b9a401 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -1487,9 +1487,19 @@ void f2fs_compress_write_end_io(struct bio *bio, struct folio *folio)
> >       struct page *page = &folio->page;
> >       struct f2fs_sb_info *sbi = bio->bi_private;
> >       struct compress_io_ctx *cic = folio->private;
> > -     enum count_type type = WB_DATA_TYPE(folio, true);
> > +     unsigned int offset = folio->index & (cic->nr_rpages - 1);
> > +     struct page *last_page = NULL;
> > +     enum count_type type;
> >       int i;
> >
> > +     if (unlikely(!offset || offset >= cic->nr_rpages ||
> > +                  !cic->rpages[offset])) {
> > +             f2fs_bug_on(sbi, 1);
> > +             type = F2FS_WB_CP_DATA;
> > +     } else {
> > +             type = WB_DATA_TYPE(page_folio(cic->rpages[offset]), false);
> > +     }
> > +
> >       if (unlikely(bio->bi_status != BLK_STS_OK))
> >               mapping_set_error(cic->inode->i_mapping, -EIO);
> >
> > @@ -1501,21 +1511,31 @@ void f2fs_compress_write_end_io(struct bio *bio, struct folio *folio)
> >       }
> >
> >       for (i = 0; i < cic->nr_rpages; i++) {
> > -             WARN_ON(!cic->rpages[i]);
> > +             if (WARN_ON(!cic->rpages[i]))
> > +                     continue;
> > +             last_page = cic->rpages[i];
> > +     }
> > +
> > +     for (i = 0; i < cic->nr_rpages; i++) {
> > +             if (!cic->rpages[i])
> > +                     continue;
> >               clear_page_private_gcing(cic->rpages[i]);
> > -             end_page_writeback(cic->rpages[i]);
> > +
> > +             if (cic->rpages[i] != last_page)
> > +                     end_page_writeback(cic->rpages[i]);
> >       }
> >
> >       page_array_free(sbi, cic->rpages, cic->nr_rpages);
> >       kmem_cache_free(cic_entry_slab, cic);
> >
> >       /*
> > -      * Make sure dec_page_count() is the last access to sbi.
> > -      * Once it drops the F2FS_WB_CP_DATA counter to zero, the
> > -      * unmount thread can proceed to destroy sbi and
> > -      * sbi->page_array_slab.
> > +      * Keep all sbi accesses before the last raw page writeback is
> > +      * released, so an unmount thread cannot free sbi while this callback
> > +      * is still using it.
> >        */
> >       dec_page_count(sbi, type);
> > +     if (last_page)
> > +             end_page_writeback(last_page);
> >   }
> >
> >   static int f2fs_write_raw_pages(struct compress_ctx *cc,
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index a765fda71536..1466a7825dc5 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1092,7 +1092,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> >       /* set submitted = true as a return value */
> >       fio->submitted = 1;
> >
> > -     type = WB_DATA_TYPE(bio_folio, fio->compressed_page);
> > +     type = WB_DATA_TYPE(fio->compressed_page ? fio->folio : bio_folio, false);
> >       inc_page_count(sbi, type);
> >
> >       if (io->bio &&
> >
> > base-commit: 7cc48ead1a8a96f2dd1eabd87c2f22947dc84cdf
>