All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Yu <chao@kernel.org>
To: Daeho Jeong <daeho43@gmail.com>
Cc: chao@kernel.org, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com,
	Daeho Jeong <daehojeong@google.com>
Subject: Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier
Date: Wed, 5 Aug 2026 09:51:03 +0800	[thread overview]
Message-ID: <a5139ae2-79fa-497f-8c7e-624703df8428@kernel.org> (raw)
In-Reply-To: <CACOAw_y_SnSqcB6D6Yw5z7nkrFFr4aAHU6wj2KW4U2N7TeFMNQ@mail.gmail.com>

On 8/5/26 01:35, Daeho Jeong wrote:
> On Tue, Aug 4, 2026 at 4:24 AM Chao Yu <chao@kernel.org> wrote:
>>
>> On 8/4/26 02:42, Daeho Jeong wrote:
>>> On Mon, Aug 3, 2026 at 2:08 AM Chao Yu <chao@kernel.org> wrote:
>>>>
>>>> On 7/30/26 03:23, Daeho Jeong wrote:
>>>>> From: Daeho Jeong <daehojeong@google.com>
>>>>>
>>>>> 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.

> 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@google.com>
>>>>> ---
>>>>>   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.

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


WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
To: Daeho Jeong <daeho43@gmail.com>
Cc: Daeho Jeong <daehojeong@google.com>,
	kernel-team@android.com, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier
Date: Wed, 5 Aug 2026 09:51:03 +0800	[thread overview]
Message-ID: <a5139ae2-79fa-497f-8c7e-624703df8428@kernel.org> (raw)
In-Reply-To: <CACOAw_y_SnSqcB6D6Yw5z7nkrFFr4aAHU6wj2KW4U2N7TeFMNQ@mail.gmail.com>

On 8/5/26 01:35, Daeho Jeong wrote:
> On Tue, Aug 4, 2026 at 4:24 AM Chao Yu <chao@kernel.org> wrote:
>>
>> On 8/4/26 02:42, Daeho Jeong wrote:
>>> On Mon, Aug 3, 2026 at 2:08 AM Chao Yu <chao@kernel.org> wrote:
>>>>
>>>> On 7/30/26 03:23, Daeho Jeong wrote:
>>>>> From: Daeho Jeong <daehojeong@google.com>
>>>>>
>>>>> 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.

> 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@google.com>
>>>>> ---
>>>>>   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.

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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2026-08-05  1:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 19:23 [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier Daeho Jeong
2026-07-29 19:23 ` Daeho Jeong
2026-08-03  9:08 ` [f2fs-dev] " Chao Yu
2026-08-03  9:08   ` Chao Yu via Linux-f2fs-devel
2026-08-03 18:42   ` Daeho Jeong
2026-08-03 18:42     ` Daeho Jeong
2026-08-04 11:24     ` Chao Yu via Linux-f2fs-devel
2026-08-04 11:24       ` Chao Yu
2026-08-04 17:35       ` Daeho Jeong
2026-08-04 17:35         ` Daeho Jeong
2026-08-05  1:51         ` Chao Yu [this message]
2026-08-05  1:51           ` Chao Yu via Linux-f2fs-devel
2026-08-05 16:45           ` Daeho Jeong
2026-08-05 16:45             ` Daeho Jeong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5139ae2-79fa-497f-8c7e-624703df8428@kernel.org \
    --to=chao@kernel.org \
    --cc=daeho43@gmail.com \
    --cc=daehojeong@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.