New: [bisected] s2disk / hibernate blocks on "Saving 506031 image data pages () ..."

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

2) /sys/kernel/debug/bdi/<dev>/stats

They are also in [1] - however the major/minors of my sdbX didn't match with the /sys/.../bdi/<dev>'s. So I just displayed them all.

3) What is the estimated bandwith?

It's an Samsung SSD 840 PRO, in this system: Read: 237 MB/s, Write 265 MB/s - see [2] (the faster writing is maybe due caching?)

Just by curiosity:

Can you also reproduce it ? ... since the test is quite simple. Or is it something specific in my system here ?

[1] Attached session.log.s2disk.20140505_2238.bz2
- 18MiB uncompressed function-trace output + others
- The bdi outputs are also in there

[2] Rough bandwidth tests
Read:
---

gamix64:~# swapon -s 
Filename                                Type            Size    Used    Priority
/dev/sdb7                               partition       4193276 0       -1
gamix64:~# dd if=/dev/sdb7 bs=1024 count=$[1024*1024*4] |pv >/dev/null
   4GB 0:00:18 [ 226MB/s] [                                                     <=>                                                                                        ]4193280+0 records in
4193280+0 records out

4293918720 bytes (4.3 GB) copied, 18.1509 s, 237 MB/s
---

Write:
---

gamix64:~# dd if=/dev/zero bs=1024 count=$[1024*1024*4] |pv >/root/Test/test1.bin
4194304+0 records inMB/s] [                                          <=>                                                                                                   ]
4194304+0 records out
4294967296 bytes (4.3 GB) copied, 16.2039 s, 265 MB/s
   4GB 0:00:15 [ 256MB/s] [                                             <=>                                                                                                ]
---

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : try_to_grab_pending <-mod_delayed_work_on
[  249.148009]   s2disk-3327    2d... 48550414us : del_timer <-try_to_grab_pending
[  249.148009]   s2disk-3327    2d... 48550415us : get_work_pool <-try_to_grab_pending
[  249.148009]   s2disk-3327    2d... 48550415us : _raw_spin_lock <-try_to_grab_pending
[  249.148009]   s2disk-3327    2d... 48550415us : get_work_pwq <-try_to_grab_pending
[  249.148009]   s2disk-3327    2d... 48550415us : pwq_activate_delayed_work <-try_to_grab_pending
[  249.148009]   s2disk-3327    2d... 48550415us : get_work_pwq <-pwq_activate_delayed_work
[  249.148009]   s2disk-3327    2d... 48550415us : move_linked_works <-pwq_activate_delayed_work
[  249.148009]   s2disk-3327    2d... 48550415us : get_work_pwq <-try_to_grab_pending
[  249.148009]   s2disk-3327    2d... 48550416us : pwq_dec_nr_in_flight <-try_to_grab_pending
[  249.148009]   s2disk-3327    2d... 48550416us : __queue_delayed_work <-mod_delayed_work_on
[  249.148009]   s2disk-3327    2d... 48550416us : __queue_work <-mod_delayed_work_on
[  249.148009]   s2disk-3327    2d... 48550416us : get_work_pool <-__queue_work
[  249.148009]   s2disk-3327    2d... 48550416us : _raw_spin_lock <-__queue_work
[  249.148009]   s2disk-3327    2d... 48550416us : insert_work <-__queue_work
[  249.148009]   s2disk-3327    2d... 48550417us : get_pwq.isra.20 <-insert_work
[  249.148009]   s2disk-3327    2d... 48550417us : wake_up_worker <-__queue_work
[  249.148009]   s2disk-3327    2d... 48550417us : wake_up_process <-__queue_work
[  249.148009]   s2disk-3327    2d... 48550417us : try_to_wake_up <-__queue_work
[  249.148009]   s2disk-3327    2d... 48550417us : _raw_spin_lock_irqsave <-try_to_wake_up
[  249.148009]   s2disk-3327    2d... 48550417us : task_waking_fair <-try_to_wake_up
[  249.148009]   s2disk-3327    2d... 48550418us : select_task_rq_fair <-select_task_rq
[  249.148009]   s2disk-3327    2d... 48550418us : idle_cpu <-select_task_rq_fair
[  249.148009]   s2disk-3327    2d... 48550418us : idle_cpu <-select_task_rq_fair
[  249.148009]   s2disk-3327    2d... 48550418us : cpus_share_cache <-try_to_wake_up
[  249.148009]   s2disk-3327    2d... 48550418us : _raw_spin_lock <-try_to_wake_up
[  249.148009]   s2disk-3327    2d... 48550419us : ttwu_do_activate.constprop.100 <-try_to_wake_up
[  249.148009]   s2disk-3327    2d... 48550419us : activate_task <-ttwu_do_activate.constprop.100
[  249.148009]   s2disk-3327    2d... 48550419us : enqueue_task <-ttwu_do_activate.constprop.100
[  249.148009]   s2disk-3327    2d... 48550419us : update_rq_clock <-enqueue_task
[  249.148009]   s2disk-3327    2d... 48550419us : enqueue_task_fair <-ttwu_do_activate.constprop.100
[  249.148009]   s2disk-3327    2d... 48550419us : update_curr <-enqueue_task_fair
[  249.148009]   s2disk-3327    2d... 48550420us : update_min_vruntime <-update_curr
[  249.148009]   s2disk-3327    2d... 48550420us : __compute_runnable_contrib.part.55 <-update_entity_load_avg
[  249.148009]   s2disk-3327    2d... 48550420us : update_cfs_rq_blocked_load <-enqueue_task_fair
[  249.148009]   s2disk-3327    2d... 48550420us : account_entity_enqueue <-enqueue_task_fair
[  249.148009]   s2disk-3327    2d... 48550420us : update_cfs_shares <-enqueue_task_fair
[  249.148009]   s2disk-3327    2d... 48550420us : __enqueue_entity <-enqueue_task_fair
[  249.148009]   s2disk-3327    2d... 48550421us : hrtick_update <-ttwu_do_activate.constprop.100
[  249.148009]   s2disk-3327    2d... 48550421us : wq_worker_waking_up <-ttwu_do_activate.constprop.100
[  249.148009]   s2disk-3327    2d... 48550421us : kthread_data <-wq_worker_waking_up
[  249.148009]   s2disk-3327    2d... 48550421us : ttwu_do_wakeup <-try_to_wake_up
[  249.148009]   s2disk-3327    2d... 48550421us : check_preempt_curr <-ttwu_do_wakeup
[  249.148009]   s2disk-3327    2d... 48550421us : check_preempt_wakeup <-check_preempt_curr
[  249.148009]   s2disk-3327    2d... 48550422us : update_curr <-check_preempt_wakeup
[  249.148009]   s2disk-3327    2d... 48550422us : wakeup_preempt_entity.isra.53 <-check_preempt_wakeup
[  249.148009]   s2disk-3327    2d... 48550422us : _raw_spin_unlock_irqrestore <-try_to_wake_up
[  249.148009]   s2disk-3327    2.... 48550423us : bdi_dirty_limit <-bdi_dirty_limits
[  249.148009]   s2disk-3327    2d... 48550423us : _raw_spin_lock_irqsave <-__percpu_counter_sum
[  249.148009]   s2disk-3327    2d... 48550423us : _raw_spin_unlock_irqrestore <-__percpu_counter_sum
[  249.148009]   s2disk-3327    2d... 48550423us : _raw_spin_lock_irqsave <-__percpu_counter_sum
[  249.148009]   s2disk-3327    2d... 48550424us : _raw_spin_unlock_irqrestore <-__percpu_counter_sum
[  249.148009]   s2disk-3327    2.... 48550424us : bdi_position_ratio <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550424us : io_schedule_timeout <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550424us : __delayacct_blkio_start <-io_schedule_timeout
[  249.148009]   s2disk-3327    2.... 48550424us : ktime_get_ts <-io_schedule_timeout
[  249.148009]   s2disk-3327    2.... 48550424us : blk_flush_plug_list <-io_schedule_timeout
[  249.148009]   s2disk-3327    2.... 48550425us : schedule_timeout <-io_schedule_timeout
[  249.148009]   s2disk-3327    2.... 48550425us : lock_timer_base.isra.35 <-__mod_timer
[  249.148009]   s2disk-3327    2.... 48550425us : _raw_spin_lock_irqsave <-lock_timer_base.isra.35
[  249.148009]   s2disk-3327    2d... 48550425us : detach_if_pending <-__mod_timer
[  249.148009]   s2disk-3327    2d... 48550425us : idle_cpu <-__mod_timer
[  249.148009]   s2disk-3327    2d... 48550425us : internal_add_timer <-__mod_timer
[  249.148009]   s2disk-3327    2d... 48550425us : __internal_add_timer <-internal_add_timer
[  249.148009]   s2disk-3327    2d... 48550426us : _raw_spin_unlock_irqrestore <-__mod_timer
[  249.148009]   s2disk-3327    2.... 48550426us : schedule <-schedule_timeout
[  249.148009]   s2disk-3327    2.... 48550426us : __schedule <-schedule_timeout
[  249.148009]   s2disk-3327    2.... 48550426us : rcu_note_context_switch <-__schedule
[  249.148009]   s2disk-3327    2.... 48550426us : rcu_sched_qs <-rcu_note_context_switch
[  249.148009]   s2disk-3327    2.... 48550426us : _raw_spin_lock_irq <-__schedule
[  249.148009]   s2disk-3327    2d... 48550427us : deactivate_task <-__schedule
[  249.148009]   s2disk-3327    2d... 48550427us : dequeue_task <-__schedule
[  249.148009]   s2disk-3327    2d... 48550427us : update_rq_clock <-dequeue_task
[  249.148009]   s2disk-3327    2d... 48550427us : dequeue_task_fair <-__schedule
[  249.148009]   s2disk-3327    2d... 48550427us : update_curr <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550427us : update_min_vruntime <-update_curr
[  249.148009]   s2disk-3327    2d... 48550427us : cpuacct_charge <-update_curr
[  249.148009]   s2disk-3327    2d... 48550428us : update_cfs_rq_blocked_load <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550428us : clear_buddies <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550428us : account_entity_dequeue <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550428us : update_min_vruntime <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550428us : update_cfs_shares <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550428us : update_curr <-update_cfs_shares
[  249.148009]   s2disk-3327    2d... 48550429us : update_min_vruntime <-update_curr
[  249.148009]   s2disk-3327    2d... 48550429us : account_entity_dequeue <-update_cfs_shares
[  249.148009]   s2disk-3327    2d... 48550429us : account_entity_enqueue <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550429us : update_curr <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550429us : update_cfs_rq_blocked_load <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550429us : clear_buddies <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550429us : account_entity_dequeue <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550430us : update_min_vruntime <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550430us : update_cfs_shares <-dequeue_task_fair
[  249.148009]   s2disk-3327    2d... 48550430us : hrtick_update <-__schedule
[  249.148009]   s2disk-3327    2d... 48550430us : put_prev_task_fair <-__schedule
[  249.148009]   s2disk-3327    2d... 48550430us : pick_next_task_fair <-pick_next_task
[  249.148009]   s2disk-3327    2d... 48550430us : clear_buddies <-pick_next_task_fair
[  249.148009]   s2disk-3327    2d... 48550431us : __dequeue_entity <-pick_next_task_fair

but the worker wakeup doesn't actually do anything:

[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Does this make sense? Rafael? Tejun?

Quoted 35 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited

but the worker wakeup doesn't actually do anything:

[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Can we patch suspend-utils as follows?

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

---
From 73d6546d5e264130e3d108c97d8317f86dc11149 Mon Sep 17 00:00:00 2001
From: Johannes Weiner <[email protected]>
Date: Thu, 12 Jun 2014 17:43:05 -0400
Subject: [patch] s2disk: fix buffered IO throttling deadlock in frozen state

s2disk uses buffered IO when writing the snapshot image to disk. If it runs into the dirty limits, the kernel forces it to wait until the flusher threads clean some of the dirty pages. However, at this point s2disk already froze the system, including the flusher infrastructure, and the whole operation deadlocks.

Open the resume device with O_SYNC to force flushing any dirty pages directly from the write() context before they accumulate and engage dirty throttling.

Signed-off-by Johannes Weiner <[email protected]>

---
  suspend.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

suspend.c +0 -0

index 479ce58555f7..1b9bed81f58a 100644

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 8 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

Quoted 3 lines

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 11 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki" <…

Quoted 10 lines · Johannes Weiner

Quoted 9 lines · Rafael J. Wysocki

Quoted 8 lines · Johannes Weiner

Quoted 7 lines · Rafael J. Wysocki

Quoted 6 lines · Johannes Weiner

Hi Oliver,

Quoted 3 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see

[1]

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in

detail

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 1 line

Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us :

global_dirty_limits <-balance_dirty_pages_ratelimited

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009]   s2disk-3327    2.... 48550414us :

global_dirtyable_memory <-global_dirty_limits

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009]   s2disk-3327    2.... 48550414us :

writeback_in_progress <-balance_dirty_pages_ratelimited

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009]   s2disk-3327    2.... 48550414us :

bdi_start_background_writeback <-balance_dirty_pages_ratelimited

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009]   s2disk-3327    2.... 48550414us :

mod_delayed_work_on <-balance_dirty_pages_ratelimited

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

but the worker wakeup doesn't actually do anything:

[  249.148009] kworker/-3466    2d... 48550431us :

finish_task_switch <-__schedule

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009] kworker/-3466    2.... 48550431us :

_raw_spin_lock_irq <-worker_thread

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009] kworker/-3466    2d... 48550431us :

need_to_create_worker <-worker_thread

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009] kworker/-3466    2d... 48550432us :

worker_enter_idle <-worker_thread

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009] kworker/-3466    2d... 48550432us :

too_many_workers <-worker_enter_idle

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009] kworker/-3466    2.... 48550432us : schedule

<-worker_thread

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

[  249.148009] kworker/-3466    2.... 48550432us : __schedule

<-worker_thread

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

My suspicion is that this fails because the bdi_wq is frozen at

this

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

point and so the flush work never runs until resume, whereas

before my

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

patch the effective dirty limit was high enough so that image

could be

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

written in one go without being throttled; followed by an fsync()

that

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 3 lines

then writes the pages in the context of the unfrozen s2disk.

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model

and

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 2 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore

them

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 1 line

post-resume.

That (and the patch too) doesn't seem to address the problem with

existing

Quoted 10 lines

Quoted 6 lines

Quoted 2 lines

suspend-utils
binaries, however.

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

Wow! Here I am to revive this topic in 2019! I have exactly the same problem, on ubuntu 18.04.2 with basically all kernels since 4.15.0-42 up to 5, which was all I tested, currently on 4.18.0-17-generic... I guess this has nothing to do with the kernel anyway.

It was working fine before, even with proprietary nvidia drivers which would generally cause a bug on the resume and not while saving the ram snapshot. I've been trying to tell this to the ubuntu guys and you can see my whole story with this problem right here:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1819915

Shortly, I tried with or without nvidia modules enabled (or intel or using nouveau), many different kernels, disabled i915, and this is all get in all those different combinations:
https://launchpadlibrarian.net/417327528/i915.jpg

The event is pretty random and seems to be more likely to happen after 2 or 4 gb of ram is ever used (I have 16 in total), and nothing changes if later I reduce the ram usage later. But is random, I successfully hibernated with 11gb in use yesterday, just resumed and hibernated 5 seconds later without doing nothing else than running hibernate, and got freeze there.

This also happens randomly if there's just 3 or 2 gb in use, likely on the second attempt of after more than 5 minutes after the computer is on. What can be wrong here?

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 63 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki"…

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 8 lines

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

Quoted 82 lines

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 63 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki"…

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 8 lines

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Quoted 7 lines

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

I've refreshed my memory wrt this bug and I believe the bug is really on the side of suspend-utils (uswsusp or however it is called). They are low level system tools, they ask the kernel to freeze all processes (SNAPSHOT_FREEZE ioctl), and then they rely on buffered writeback (which is relatively heavyweight infrastructure) to work. That is wrong in my opinion.

I can see Johanness was suggesting in comment 11 to use O_SYNC in suspend-utils which worked but was too slow. Indeed O_SYNC is rather big hammer but using O_DIRECT should be what they need and get better performance - no additional buffering in the kernel, no dirty throttling, etc. They only need their buffer & device offsets sector aligned - they seem to be even page aligned in suspend-utils so they should be fine. And if the performance still sucks (currently they appear to do mostly random 4k writes so it probably would for rotating disks), they could use AIO DIO to get multiple pages in flight (as many as they dare to allocate buffers) and then the IO scheduler will reorder things as good as it can and they should get reasonable performance.

Is there someone who works on suspend-utils these days? Because the repo I've found on kernel.org seems to be long dead (last commit in 2012).

Honza

Am 03.04.19 um 11:34 schrieb Jan Kara:

Quoted 110 lines · Andrew Morton

Quoted 82 lines

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 63 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki"…

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 8 lines

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Quoted 7 lines

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

I've refreshed my memory wrt this bug and I believe the bug is really on the side of suspend-utils (uswsusp or however it is called). They are low level system tools, they ask the kernel to freeze all processes (SNAPSHOT_FREEZE ioctl), and then they rely on buffered writeback (which is relatively heavyweight infrastructure) to work. That is wrong in my opinion.

I can see Johanness was suggesting in comment 11 to use O_SYNC in suspend-utils which worked but was too slow. Indeed O_SYNC is rather big hammer but using O_DIRECT should be what they need and get better performance - no additional buffering in the kernel, no dirty throttling, etc. They only need their buffer & device offsets sector aligned - they seem to be even page aligned in suspend-utils so they should be fine. And if the performance still sucks (currently they appear to do mostly random 4k writes so it probably would for rotating disks), they could use AIO DIO to get multiple pages in flight (as many as they dare to allocate buffers) and then the IO scheduler will reorder things as good as it can and they should get reasonable performance.

Quoted 5 lines

Is there someone who works on suspend-utils these days? Because the repo I've found on kernel.org seems to be long dead (last commit in 2012).

Honza

Whether it's suspend-utils (or uswsusp) or not could be answered quickly by de-installing this package and using the kernel-methods instead.

Yes I can sorta confirm the bug is in uswsusp. I removed the package and pm-utils

/sys/power/state" to hibernate. It seems to succeed and shuts down, I am just not able to resume from it, which seems to be a classical problem solved just by setting the resume swap file/partition on grub. (which i tried and didn't work even with nvidia disabled)

Anyway uswsusp is still necessary because the default kernel hibernation doesn't work with the proprietary nvidia drivers as long as I know and tested.

Is there anyway I could get any workaround to this bug on my current OS by the way?

Quoted 122 lines

Am 03.04.19 um 11:34 schrieb Jan Kara:

Quoted 110 lines · Andrew Morton

Quoted 82 lines

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 63 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki"…

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 8 lines

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Quoted 7 lines

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

I've refreshed my memory wrt this bug and I believe the bug is really on the side of suspend-utils (uswsusp or however it is called). They are low level system tools, they ask the kernel to freeze all processes (SNAPSHOT_FREEZE ioctl), and then they rely on buffered writeback (which is relatively heavyweight infrastructure) to work. That is wrong in my opinion.

I can see Johanness was suggesting in comment 11 to use O_SYNC in suspend-utils which worked but was too slow. Indeed O_SYNC is rather big hammer but using O_DIRECT should be what they need and get better performance - no additional buffering in the kernel, no dirty throttling, etc. They only need their buffer & device offsets sector aligned - they seem to be even page aligned in suspend-utils so they should be fine. And if the performance still sucks (currently they appear to do mostly random 4k writes so it probably would for rotating disks), they could use AIO DIO to get multiple pages in flight (as many as they dare to allocate buffers) and then the IO scheduler will reorder things as good as it can and they should get reasonable performance.

Quoted 5 lines

Is there someone who works on suspend-utils these days? Because the repo I've found on kernel.org seems to be long dead (last commit in 2012).

Honza

Whether it's suspend-utils (or uswsusp) or not could be answered quickly by de-installing this package and using the kernel-methods instead.

What doesn't work: hibernating or resuming?
And /var/log/pm-suspend.log might give you a clue what causes the problem.

Am 03.04.19 um 18:59 schrieb Matheus Fillipe:

Quoted 2 lines

Yes I can sorta confirm the bug is in uswsusp. I removed the package and pm-utils

Matheus,

there is no need to uninstall pm-utils. You actually need this to have comfortable suspend/hibernate.

The only additional option you will get from uswsusp is true s2both (which is nice, imo).

pm-utils provides something similar called "suspend-hybrid" which means that the computer suspends and after a configurable time wakes up again to go into hibernation.

and used both "systemctl hibernate" and "echo disk >>

Quoted 8 lines

/sys/power/state" to hibernate. It seems to succeed and shuts down, I am just not able to resume from it, which seems to be a classical problem solved just by setting the resume swap file/partition on grub. (which i tried and didn't work even with nvidia disabled)

Anyway uswsusp is still necessary because the default kernel hibernation doesn't work with the proprietary nvidia drivers as long as I know and tested.

What doesn't work: hibernating or resuming?
And /var/log/pm-suspend.log might give you a clue what causes the problem.

Quoted 3 lines

Is there anyway I could get any workaround to this bug on my current OS by the way?

*I* don't know, I don't use Ubuntu. But what I would do now is re-install pm-utils *without* uswsusp and make sure that you have got the swap-partition/file right in grub.cfg or menu.lst (grub legacy).

Then do a few pm-hibernate/resume and tell us what happened.

So long!

Quoted 124 lines · Rainer Fiebig

Quoted 122 lines

Am 03.04.19 um 11:34 schrieb Jan Kara:

Quoted 110 lines · Andrew Morton

Quoted 82 lines

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 63 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki"…

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 8 lines

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Quoted 7 lines

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

I've refreshed my memory wrt this bug and I believe the bug is really on the side of suspend-utils (uswsusp or however it is called). They are low level system tools, they ask the kernel to freeze all processes (SNAPSHOT_FREEZE ioctl), and then they rely on buffered writeback (which is relatively heavyweight infrastructure) to work. That is wrong in my opinion.

I can see Johanness was suggesting in comment 11 to use O_SYNC in suspend-utils which worked but was too slow. Indeed O_SYNC is rather big hammer but using O_DIRECT should be what they need and get better performance - no additional buffering in the kernel, no dirty throttling, etc. They only need their buffer & device offsets sector aligned - they seem to be even page aligned in suspend-utils so they should be fine. And if the performance still sucks (currently they appear to do mostly random 4k writes so it probably would for rotating disks), they could use AIO DIO to get multiple pages in flight (as many as they dare to allocate buffers) and then the IO scheduler will reorder things as good as it can and they should get reasonable performance.

Quoted 5 lines

Is there someone who works on suspend-utils these days? Because the repo I've found on kernel.org seems to be long dead (last commit in 2012).

Honza

Whether it's suspend-utils (or uswsusp) or not could be answered quickly by de-installing this package and using the kernel-methods instead.

Okay I found a way to get it working and there was also a huge mistake on my last boot-config, the resume was commented :P
I basically followed this: https://askubuntu.com/a/1064114
but changed to:
resume=/dev/disk/by-uuid/70d967e6-ad52-4c21-baf0-01a813ccc6ac (just the uuid wouldnt work) and this is probably the most important thing to do.it worked!
I also set the resume variable in initramfs to my swap partition but this might nor be so important anyway since it's automatically detected.

I tested both systemctl hibernate and pm-hibernate, i guess they call the same thing anyway. I attached a screenshot. Seems to be working fine without uswsusp and with nvidia proprietary drivers!

Quoted 168 lines · Rainer Fiebig

Am 03.04.19 um 18:59 schrieb Matheus Fillipe:

Quoted 2 lines

Yes I can sorta confirm the bug is in uswsusp. I removed the package and pm-utils

Matheus,

there is no need to uninstall pm-utils. You actually need this to have comfortable suspend/hibernate.

The only additional option you will get from uswsusp is true s2both (which is nice, imo).

pm-utils provides something similar called "suspend-hybrid" which means that the computer suspends and after a configurable time wakes up again to go into hibernation.

and used both "systemctl hibernate" and "echo disk >>

Quoted 8 lines

/sys/power/state" to hibernate. It seems to succeed and shuts down, I am just not able to resume from it, which seems to be a classical problem solved just by setting the resume swap file/partition on grub. (which i tried and didn't work even with nvidia disabled)

Anyway uswsusp is still necessary because the default kernel hibernation doesn't work with the proprietary nvidia drivers as long as I know and tested.

What doesn't work: hibernating or resuming?
And /var/log/pm-suspend.log might give you a clue what causes the problem.

Quoted 3 lines

Is there anyway I could get any workaround to this bug on my current OS by the way?

*I* don't know, I don't use Ubuntu. But what I would do now is re-install pm-utils *without* uswsusp and make sure that you have got the swap-partition/file right in grub.cfg or menu.lst (grub legacy).

Then do a few pm-hibernate/resume and tell us what happened.

So long!

Quoted 124 lines · Rainer Fiebig

Quoted 122 lines

Am 03.04.19 um 11:34 schrieb Jan Kara:

Quoted 110 lines · Andrew Morton

Quoted 82 lines

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 63 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki"…

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1]
attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail
Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 8 lines

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Quoted 7 lines

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

I've refreshed my memory wrt this bug and I believe the bug is really on the side of suspend-utils (uswsusp or however it is called). They are low level system tools, they ask the kernel to freeze all processes (SNAPSHOT_FREEZE ioctl), and then they rely on buffered writeback (which is relatively heavyweight infrastructure) to work. That is wrong in my opinion.

I can see Johanness was suggesting in comment 11 to use O_SYNC in suspend-utils which worked but was too slow. Indeed O_SYNC is rather big hammer but using O_DIRECT should be what they need and get better performance - no additional buffering in the kernel, no dirty throttling, etc. They only need their buffer & device offsets sector aligned - they seem to be even page aligned in suspend-utils so they should be fine. And if the performance still sucks (currently they appear to do mostly random 4k writes so it probably would for rotating disks), they could use AIO DIO to get multiple pages in flight (as many as they dare to allocate buffers) and then the IO scheduler will reorder things as good as it can and they should get reasonable performance.

Quoted 5 lines

Is there someone who works on suspend-utils these days? Because the repo I've found on kernel.org seems to be long dead (last commit in 2012).

Honza

Whether it's suspend-utils (or uswsusp) or not could be answered quickly by de-installing this package and using the kernel-methods instead.

Anyway uswsusp is still necessary because the default kernel hibernation doesn't work with the proprietary nvidia drivers as long as I know and tested

So you got hibernate working now with pm-utils*and* the prop. Nvidia drivers. That's good - although a bit contrary to what you said in Comment 29:

till puzzles me is that while others are having problems, suspend-utils/uswsusp work for me almost 100 % of the time, except for a few extreme test-cases in the past. You also said that it worked "flawlessly" for yo

So I'm wondering whether used-up swap space might play a role in this matter, too. At least for the cases that I've seen on my system, I can't rule this out. And when I look at the screenshot you provided in Comment 27 (https://launchpadlibrarian.net/417327528/i915.jpg), sparse swap-space could have been a factor in that case as well. Because roughly 3.5 GB free swap-space doesn't seem much for a 16-GB-RAM box.

Quoted 82 lines · Andrew Morton

I cc'ed a bunch of people from bugzilla.

Folks, please please please remember to reply via emailed reply-to-all. Don't use the bugzilla interface!

Quoted 63 lines · On Mon, 16 Jun 2014 18:29:26 +0200 "Rafael J. Wysocki"…

Quoted 62 lines · Johannes Weiner

Quoted 36 lines · Rafael J. Wysocki

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rafael J. Wysocki

Quoted 33 lines · Johannes Weiner

Hi Oliver,

Quoted 7 lines · Oliver Winker

Hello,

1) Attached a full function-trace log + other SysRq outputs, see [1] attached.

I saw bdi_...() calls in the s2disk paths, but didn't check in detail Probably more efficient when one of you guys looks directly.

Thanks, this looks interesting. balance_dirty_pages() wakes up the bdi_wq workqueue as it should:

[  249.148009]   s2disk-3327    2.... 48550413us : global_dirty_limits <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : global_dirtyable_memory <-global_dirty_limits
[  249.148009]   s2disk-3327    2.... 48550414us : writeback_in_progress <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : bdi_start_background_writeback <-balance_dirty_pages_ratelimited
[  249.148009]   s2disk-3327    2.... 48550414us : mod_delayed_work_on <-balance_dirty_pages_ratelimited
but the worker wakeup doesn't actually do anything:
[  249.148009] kworker/-3466    2d... 48550431us : finish_task_switch <-__schedule
[  249.148009] kworker/-3466    2.... 48550431us : _raw_spin_lock_irq <-worker_thread
[  249.148009] kworker/-3466    2d... 48550431us : need_to_create_worker <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : worker_enter_idle <-worker_thread
[  249.148009] kworker/-3466    2d... 48550432us : too_many_workers <-worker_enter_idle
[  249.148009] kworker/-3466    2.... 48550432us : schedule <-worker_thread
[  249.148009] kworker/-3466    2.... 48550432us : __schedule <-worker_thread

My suspicion is that this fails because the bdi_wq is frozen at this point and so the flush work never runs until resume, whereas before my patch the effective dirty limit was high enough so that image could be written in one go without being throttled; followed by an fsync() that then writes the pages in the context of the unfrozen s2disk.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

Does this make sense? Rafael? Tejun?

Well, it does seem to make sense to me.

From what I see, this is a deadlock in the userspace suspend model and
just happened to work by chance in the past.

Well, it had been working for quite a while, so it was a rather large opportunity
window it seems. :-)

No doubt about that, and I feel bad that it broke. But it's still a deadlock that can't reasonably be accommodated from dirty throttling.

It can't just put the flushers to sleep and then issue a large amount of buffered IO, hoping it doesn't hit the dirty limits. Don't shoot the messenger, this bug needs to be addressed, not get papered over.

Quoted 11 lines

Quoted 1 line

Can we patch suspend-utils as follows?

Perhaps we can. Let's ask the new maintainer.

Rodolfo, do you think you can apply the patch below to suspend-utils?

Quoted 3 lines

Alternatively, suspend-utils
could clear the dirty limits before it starts writing and restore them post-resume.

That (and the patch too) doesn't seem to address the problem with existing suspend-utils
binaries, however.

Quoted 8 lines

Quoted 4 lines

It's userspace that freezes the system before issuing buffered IO, so my conclusion was that the bug is in there. This is arguable. I also wouldn't be opposed to a patch that sets the dirty limits to infinity from the ioctl that freezes the system or creates the image.

OK, that sounds like a workable plan.

How do I set those limits to infinity?

Five years have passed and people are still hitting this.

Quoted 7 lines

Killian described the workaround in comment 14 at
https://bugzilla.kernel.org/show_bug.cgi?id=75101.

People can use this workaround manually by hand or in scripts. But we really should find a proper solution. Maybe special-case the freezing of the flusher threads until all the writeout has completed. Or something else.

I've refreshed my memory wrt this bug and I believe the bug is really on the side of suspend-utils (uswsusp or however it is called). They are low level system tools, they ask the kernel to freeze all processes (SNAPSHOT_FREEZE ioctl), and then they rely on buffered writeback (which is relatively heavyweight infrastructure) to work. That is wrong in my opinion.

I can see Johanness was suggesting in comment 11 to use O_SYNC in suspend-utils which worked but was too slow. Indeed O_SYNC is rather big hammer but using O_DIRECT should be what they need and get better performance - no additional buffering in the kernel, no dirty throttling, etc. They only need their buffer & device offsets sector aligned - they seem to be even page aligned in suspend-utils so they should be fine. And if the performance still sucks (currently they appear to do mostly random 4k writes so it probably would for rotating disks), they could use AIO DIO to get multiple pages in flight (as many as they dare to allocate buffers) and then the IO scheduler will reorder things as good as it can and they should get reasonable performance.

Is there someone who works on suspend-utils these days?

Because the repo I've found on kernel.org seems to be long dead (last commit in 2012).