Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier

[Daeho Jeong]

On Mon, Aug 3, 2026 at 2:08 AM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 7/30/26 03:23, Daeho Jeong wrote:
> > From: Daeho Jeong <daehojeong@xxxxxxxxxx>
> >
> > During system suspend, a race condition can cause f2fs_gc and f2fs_discard
> > threads to call submit_bio() while the underlying block device (e.g., UFS)
> > is in Runtime PM suspend. Because Runtime PM worker threads are already
> > frozen during task freezing, the threads become trapped in
> > __bio_queue_enter() waiting on mq_freeze_wq, leading to a PM freezer
> > timeout.
> >
> > To prevent this deadlock, register a PM notifier to set SBI_IS_SUSPENDING
> > during PM_SUSPEND_PREPARE. Background GC and discard threads check this
>
> Should we cover issue_flush_thread and issue_checkpoint_thread as well in
> where we will submit bio?

Unlike GC and discard, which are background optimization tasks and can
be safely paused, other operations like checkpoint are critical for
data consistency.
It is better not to interrupt them in the middle of their progress. If
they are running and take too long, it is safer to just let the system
suspend temporarily fail rather than forcibly breaking their
operations.

>
> > flag and immediately stop issuing new bios, allowing them to enter a
> > freezable sleep state cleanly before process freezing begins.
> >
> > Signed-off-by: Daeho Jeong <daehojeong@xxxxxxxxxx>
> > ---
> >   fs/f2fs/f2fs.h    |  3 +++
> >   fs/f2fs/gc.c      | 13 ++++++++-----
> >   fs/f2fs/segment.c | 13 +++++++++----
> >   fs/f2fs/super.c   | 25 +++++++++++++++++++++++++
> >   4 files changed, 45 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index f24e30bb5c3d..c46bf4df9412 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -25,6 +25,7 @@
> >   #include <linux/quotaops.h>
> >   #include <linux/part_stat.h>
> >   #include <linux/rw_hint.h>
> > +#include <linux/suspend.h>
> >
> >   #include <linux/fscrypt.h>
> >   #include <linux/fsverity.h>
> > @@ -1494,6 +1495,7 @@ enum {
> >       SBI_IS_FREEZING,                        /* freezefs is in process */
> >       SBI_IS_WRITABLE,                        /* remove ro mountoption transiently */
> >       SBI_ENABLE_CHECKPOINT,                  /* indicate it's during f2fs_enable_checkpoint() */
> > +     SBI_IS_SUSPENDING,                      /* system suspend is in progress */
> >       MAX_SBI_FLAG,
> >   };
> >
> > @@ -1757,6 +1759,7 @@ struct f2fs_sb_info {
> >       struct f2fs_rwsem sb_lock;              /* lock for raw super block */
> >       int valid_super_block;                  /* valid super block no */
> >       unsigned long s_flag;                           /* flags for sbi */
> > +     struct notifier_block pm_nb;            /* for PM notifier */
> >       struct mutex writepages;                /* mutex for writepages() */
> >
> >   #ifdef CONFIG_BLK_DEV_ZONED
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index 93bcb35a5b5d..86b2b29402a5 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -71,7 +71,8 @@ static int gc_thread_func(void *data)
> >               if (kthread_should_stop())
> >                       break;
> >
> > -             if (sbi->sb->s_writers.frozen >= SB_FREEZE_WRITE) {
> > +             if (sbi->sb->s_writers.frozen >= SB_FREEZE_WRITE ||
> > +                     is_sbi_flag_set(sbi, SBI_IS_SUSPENDING)) {
> >                       increase_sleep_time(gc_th, &wait_ms);
> >                       stat_other_skip_bggc_count(sbi);
> >                       continue;
> > @@ -1064,8 +1065,9 @@ static int gc_node_segment(struct f2fs_sb_info *sbi,
> >               struct node_info ni;
> >               int err;
> >
> > -             /* stop BG_GC if there is not enough free sections. */
> > -             if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0))
> > +             /* stop BG_GC if there is not enough free sections or suspending. */
> > +             if (gc_type == BG_GC && (has_not_enough_free_secs(sbi, 0, 0) ||
> > +                                     is_sbi_flag_set(sbi, SBI_IS_SUSPENDING)))
> >                       return submitted;
> >
> >               if (check_valid_map(sbi, segno, off) == 0)
> > @@ -1611,7 +1613,8 @@ static int gc_data_segment(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
> >                * Or, stop GC if the segment becomes fully valid caused by
> >                * race condition along with SSR block allocation.
> >                */
> > -             if ((gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0)) ||
> > +             if ((gc_type == BG_GC && (has_not_enough_free_secs(sbi, 0, 0) ||
> > +                                     is_sbi_flag_set(sbi, SBI_IS_SUSPENDING))) ||
> >                       (!force_migrate && get_valid_blocks(sbi, segno, true) ==
> >                                                       CAP_BLKS_PER_SEC(sbi)))
> >                       return submitted;
> > @@ -2015,7 +2018,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control)
> >               goto stop;
> >       }
> >   retry:
> > -     if (unlikely(freezing(current))) {
>
> Shouldn't we keep original freezing logic? in case filesystem are frozen
> when low device snapshot is triggered?

Since the runtime PM suspend/resume workers are only frozen during
system-wide PM transitions (suspend/hibernation), the PM notifier
approach with SBI_IS_SUSPENDING sufficiently prevents the deadlock.

Thanks,

>
> > +     if (is_sbi_flag_set(sbi, SBI_IS_SUSPENDING)) {
> >               ret = 0;
> >               goto stop;
> >       }
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index d70dc5ef3de4..e27197953356 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1300,7 +1300,8 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> >       if (dc->state != D_PREP)
> >               return 0;
> >
> > -     if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > +     if (is_sbi_flag_set(sbi, SBI_NEED_FSCK) ||
> > +             is_sbi_flag_set(sbi, SBI_IS_SUSPENDING))
> >               return 0;
> >
> >   #ifdef CONFIG_BLK_DEV_ZONED
> > @@ -1341,6 +1342,9 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> >               unsigned long flags;
> >               bool last = true;
> >
> > +             if (is_sbi_flag_set(sbi, SBI_IS_SUSPENDING))
> > +                     break;
> > +
> >               if (len > max_discard_blocks) {
> >                       len = max_discard_blocks;
> >                       last = false;
> > @@ -1615,7 +1619,7 @@ static void __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
> >               if (dc->state != D_PREP)
> >                       goto next;
> >
> > -             if (*issued > 0 && unlikely(freezing(current)))
>
> Ditto,
>
> > +             if (is_sbi_flag_set(sbi, SBI_IS_SUSPENDING))
> >                       break;
> >
> >               if (dpolicy->io_aware && !is_idle(sbi, DISCARD_TIME)) {
> > @@ -1688,7 +1692,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
> >               list_for_each_entry_safe(dc, tmp, pend_list, list) {
> >                       f2fs_bug_on(sbi, dc->state != D_PREP);
> >
> > -                     if (issued > 0 && unlikely(freezing(current))) {
>
> Ditto,
>
> Thanks,
>
> > +                     if (is_sbi_flag_set(sbi, SBI_IS_SUSPENDING)) {
> >                               suspended = true;
> >                               break;
> >                       }
> > @@ -1955,7 +1959,8 @@ static int issue_discard_thread(void *data)
> >                       continue;
> >               if (kthread_should_stop())
> >                       return 0;
> > -             if (is_sbi_flag_set(sbi, SBI_NEED_FSCK) ||
> > +             if (is_sbi_flag_set(sbi, SBI_IS_SUSPENDING) ||
> > +                     is_sbi_flag_set(sbi, SBI_NEED_FSCK) ||
> >                       !atomic_read(&dcc->discard_cmd_cnt)) {
> >                       wait_ms = dpolicy.max_interval;
> >                       continue;
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index d5dc83e613e2..536f3ffe5354 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -1979,6 +1979,26 @@ static void destroy_device_list(struct f2fs_sb_info *sbi)
> >       kvfree(sbi->devs);
> >   }
> >
> > +static int f2fs_pm_notifier(struct notifier_block *nb,
> > +                         unsigned long action, void *ptr)
> > +{
> > +     struct f2fs_sb_info *sbi = container_of(nb, struct f2fs_sb_info, pm_nb);
> > +
> > +     switch (action) {
> > +     case PM_HIBERNATION_PREPARE:
> > +     case PM_SUSPEND_PREPARE:
> > +     case PM_RESTORE_PREPARE:
> > +             set_sbi_flag(sbi, SBI_IS_SUSPENDING);
> > +             break;
> > +     case PM_POST_SUSPEND:
> > +     case PM_POST_HIBERNATION:
> > +     case PM_POST_RESTORE:
> > +             clear_sbi_flag(sbi, SBI_IS_SUSPENDING);
> > +             break;
> > +     }
> > +     return NOTIFY_OK;
> > +}
> > +
> >   static void f2fs_put_super(struct super_block *sb)
> >   {
> >       struct f2fs_sb_info *sbi = F2FS_SB(sb);
> > @@ -1986,6 +2006,8 @@ static void f2fs_put_super(struct super_block *sb)
> >       int err = 0;
> >       bool done;
> >
> > +     unregister_pm_notifier(&sbi->pm_nb);
> > +
> >       /* unregister procfs/sysfs entries in advance to avoid race case */
> >       f2fs_unregister_sysfs(sbi);
> >
> > @@ -5472,6 +5494,9 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
> >
> >       f2fs_restore_device_alias(sbi);
> >
> > +     sbi->pm_nb.notifier_call = f2fs_pm_notifier;
> > +     register_pm_notifier(&sbi->pm_nb);
> > +
> >       sbi->umount_lock_holder = NULL;
> >       return 0;
> >
>