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

[Daeho Jeong]

On Tue, Aug 4, 2026 at 6:51 PM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 8/5/26 01:35, Daeho Jeong wrote:
> > On Tue, Aug 4, 2026 at 4:24 AM Chao Yu <chao@xxxxxxxxxx> wrote:
> >>
> >> On 8/4/26 02:42, Daeho Jeong wrote:
> >>> 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.
> >>
> >> So why foreground thread like ckpt thread or flush thread won't suffer the
> >> same issue like gc or discard thread? because foreground thread will prevent
> >> UFS from running into suspend state? I may missed something here. :)
> >>
> >
> > Foreground threads (ckpt/flush) issue I/O on-demand for dirty data sync.
> > If suspend aborts due to active I/O, it is legitimate and expected
> > behavior rather than an issue, as it is a necessary filesystem
> > operation under a non-idle workload.
>
> Okay, so if ckpt/flush are active, that means there are userspace applications are
> waiting for checkpoint/flush completion, so freeze_processes() form system suspend
> still didn't completion, and it won't enter phase 2 (device suspend & freezing pm_wq).
>
> Let me know if I understand it correctly.

Yes, your understanding is correct.

>
> > In contrast, f2fs_gc and f2fs_discard are autonomous background
> > threads waking up even on an idle system with UFS in Runtime PM
> > suspend, which is the main cause of UFS deadlock when entering the
> > suspend.
> > With ZUFS, GC runs much more frequently, causing frequent suspend aborts.
> >
> >>>
> >>>>
> >>>>> 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.
> >>
> >> What I mean is:
> >>
> >> if (is_sbi_flag_set(sbi, SBI_IS_SUSPENDING) || unlikely(freezing(current))) {
> >>
> >> otherwise, gc thread or discard thread won't detect freeze state, and will
> >> continue to trigger IO in background even there is a system freeze request
> >> from device snapshot or cgroup freezing, right?
> >>
> >
> > The hang in system suspend happens because Runtime PM workers (pm_wq)
> > are frozen, so UFS cannot be resumed from submit_bio().
> > During cgroup freeze or snapshot, I believe pm_wq is alive, so UFS
> > resumes in a few ms and I/O finishes without deadlock. Also,
> > wait_event_freezable_timeout() will freeze the threads when they
> > sleep.
>
> Here, we need to detect the freezing state and stop issuing any new I/O
> immediately because non-PM freezing mechanisms (such as dm-snapshot or
> cgroup freezer) require the underlying filesystem/device to reach a
> quiescent (static) state. It's not limited strictly to the PM suspend
> state.
>
> >
> > If you prefer keeping freezing(current) as a fast path for non-PM
> > freezing, I can change it to:
> > if (is_sbi_flag_set(sbi, SBI_IS_SUSPENDING) || unlikely(freezing(current)))
> > Do you still think it is required? If so, plz, let me know.
>
> Yes, please.

I will update the patch. Thanks,

>
> Thanks,
>
> >
> > Thanks.
> >
> >> Thanks,
> >>
> >>>
> >>> 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;
> >>>>>
> >>>>
> >>
>