Re: [f2fs-dev] [PATCH v5] f2fs: use post-decrement count for cp_wait wakeup

Wenjie Qi <[email protected]>
Newsgroups net.sourceforge.lists.linux-f2fs-devel,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <CAGFpFsR9DDYXvad-Q7+ZinqQRPb5KKGP2xTm2a6HbdcrJY+b_g@mail.gmail.com>
Yes. I described the race with a calltrace-style timeline in v7. The
code is
  unchanged from v6.

https://lore.kernel.org/linux-f2fs-devel/[email protected]/T/#u

On Thu, Jun 18, 2026 at 5:10 PM Chao Yu <[email protected]> wrote:
>
> On 6/18/26 11:38, Wenjie Qi wrote:
> >    The race is between dec_page_count() and the later get_pages() check:
> >    another CP-data writeback can be submitted after the counter reaches zero
> >    but before get_pages() observes it, so the zero transition may miss the
> >    cp_wait wakeup.
>
> Can you describe race condition like below calltrace? which will be easier to
> understand?
>
>      loop device                             umount
>      - worker_thread
>       - loop_process_work
>        - do_req_filebacked
>         - lo_rw_aio
>          - lo_rw_aio_complete
>           - blk_mq_end_request
>            - blk_update_request
>             - f2fs_write_end_io
>              - dec_page_count
>              - folio_end_writeback
>                                              - kill_f2fs_super
>                                               - kill_block_super
>                                                - f2fs_put_super
>                                               : free(sbi)
>             : get_pages(, F2FS_WB_CP_DATA)
>               accessed sbi which is freed
>
> Thanks,
>
> >
> >    v6 also adds dec_page_count_return() and uses it instead of accessing
> >    nr_pages directly.  The wakeup logic is unchanged from v5.
> >
> > https://lore.kernel.org/linux-f2fs-devel/[email protected]/T/#u
> >
> > On Thu, Jun 18, 2026 at 2:09 AM Jaegeuk Kim <[email protected]> wrote:
> >>
> >> On 06/16, Wenjie Qi wrote:
> >>> f2fs_write_end_io() decrements the writeback page counter and then
> >>> reads it again with get_pages() to decide whether the last
> >>> F2FS_WB_CP_DATA completion should wake cp_wait.
> >>>
> >>> Use atomic_dec_return() for F2FS_WB_CP_DATA completions so the wakeup
> >>> decision is made from the value produced by the decrement itself. Keep
> >>> the existing dec_page_count() path for other writeback counters.
> >>
> >> Is there a race condition to do this? If so, can you describe? And, I think
> >> we need a wrapper function instead of calling nr_pages directly.
> >>
> >>>
> >>> Fixes: e234088758fc ("f2fs: avoid wait if IO end up when do_checkpoint for better performance")
> >>> Fixes: ce2739e482bc ("f2fs: fix to avoid UAF in f2fs_write_end_io()")
> >>> Cc: [email protected]
> >>> Signed-off-by: Wenjie Qi <[email protected]>
> >>> ---
> >>>   fs/f2fs/data.c | 12 +++++++-----
> >>>   1 file changed, 7 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >>> index d83a21998ec2..58d23eb74ec2 100644
> >>> --- a/fs/f2fs/data.c
> >>> +++ b/fs/f2fs/data.c
> >>> @@ -392,15 +392,17 @@ static void f2fs_write_end_io(struct bio *bio)
> >>>                if (f2fs_in_warm_node_list(folio))
> >>>                        f2fs_del_fsync_node_entry(sbi, folio);
> >>>
> >>> -             dec_page_count(sbi, type);
> >>> -
> >>>                /*
> >>>                 * we should access sbi before folio_end_writeback() to
> >>>                 * avoid racing w/ kill_f2fs_super()
> >>>                 */
> >>> -             if (type == F2FS_WB_CP_DATA && !get_pages(sbi, type) &&
> >>> -                             wq_has_sleeper(&sbi->cp_wait))
> >>> -                     wake_up(&sbi->cp_wait);
> >>> +             if (type == F2FS_WB_CP_DATA) {
> >>> +                     if (!atomic_dec_return(&sbi->nr_pages[type]) &&
> >>> +                         wq_has_sleeper(&sbi->cp_wait))
> >>> +                             wake_up(&sbi->cp_wait);
> >>> +             } else {
> >>> +                     dec_page_count(sbi, type);
> >>> +             }
> >>>
> >>>                folio_clear_f2fs_gcing(folio);
> >>>                folio_end_writeback(folio);
> >>>
> >>> base-commit: c0b65f6129c7fbb526e921dd60261650f1b2bef9
> >>> --
> >>> 2.43.0
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> Linux-f2fs-devel mailing list
> >>> [email protected]
> >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.