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

[Chao Yu]

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. :)

> 
>>
>>> 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?

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;
>>>
>>