* [PATCH] f2fs: quiesce background threads during system suspend using PM notifier @ 2026-07-29 19:23 Daeho Jeong 2026-08-03 9:08 ` [f2fs-dev] " Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Daeho Jeong @ 2026-07-29 19:23 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong 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 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))) { + 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))) + 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))) { + 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; -- 2.55.0.571.g244d577d93-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier 2026-07-29 19:23 [PATCH] f2fs: quiesce background threads during system suspend using PM notifier Daeho Jeong @ 2026-08-03 9:08 ` Chao Yu 2026-08-03 18:42 ` Daeho Jeong 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2026-08-03 9:08 UTC (permalink / raw) To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team Cc: chao, Daeho Jeong 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? > 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? > + 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; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier 2026-08-03 9:08 ` [f2fs-dev] " Chao Yu @ 2026-08-03 18:42 ` Daeho Jeong 2026-08-04 11:24 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Daeho Jeong @ 2026-08-03 18:42 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong 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. > > > 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. 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; > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier 2026-08-03 18:42 ` Daeho Jeong @ 2026-08-04 11:24 ` Chao Yu 2026-08-04 17:35 ` Daeho Jeong 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2026-08-04 11:24 UTC (permalink / raw) To: Daeho Jeong Cc: chao, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong 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. :) > >> >>> 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? 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; >>> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier 2026-08-04 11:24 ` Chao Yu @ 2026-08-04 17:35 ` Daeho Jeong 2026-08-05 1:51 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Daeho Jeong @ 2026-08-04 17:35 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong 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. 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. 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. 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; > >>> > >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier 2026-08-04 17:35 ` Daeho Jeong @ 2026-08-05 1:51 ` Chao Yu 2026-08-05 16:45 ` Daeho Jeong 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2026-08-05 1:51 UTC (permalink / raw) To: Daeho Jeong Cc: chao, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong 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; >>>>> >>>> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: quiesce background threads during system suspend using PM notifier 2026-08-05 1:51 ` Chao Yu @ 2026-08-05 16:45 ` Daeho Jeong 0 siblings, 0 replies; 7+ messages in thread From: Daeho Jeong @ 2026-08-05 16:45 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong On Tue, Aug 4, 2026 at 6:51 PM Chao Yu <chao@kernel.org> wrote: > > 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. 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@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. 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; > >>>>> > >>>> > >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-05 16:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-29 19:23 [PATCH] f2fs: quiesce background threads during system suspend using PM notifier Daeho Jeong 2026-08-03 9:08 ` [f2fs-dev] " Chao Yu 2026-08-03 18:42 ` Daeho Jeong 2026-08-04 11:24 ` Chao Yu 2026-08-04 17:35 ` Daeho Jeong 2026-08-05 1:51 ` Chao Yu 2026-08-05 16:45 ` Daeho Jeong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox