Evict VRAM after processes are frozen
Subsystems (5)
cover Evict VRAM after processes are frozen
From: Mario Limonciello <[email protected]>
As part of the suspend and hibernate sequences devices supported by amdgpu will have VRAM evicted. This has been littered with problems in the past, and now it is called strategically at multiple times during power management sequences.
Most recently there were a number of issues reported that were root caused to physical memory pressure at suspend time where VRAM couldn't be evicted. A new call was introduced using a PM notifier before swap was turned off so that even if there are physical memory pressures VRAM could still be evicted into swap.
This helped, but users using older systemd noted that this could lead to a deadlock. This is because user processes aren't frozen by older versions of systemd and thus if a game is running it could potentially be creating BOs during the eviction thus preventing eviction from ever completing.
This isn't an issue with newer systemd because it uses cgroup freezing to freeze user processes. Nonetheless this isn't a good behavior to rely upon for userspace, so this series aims to solve the problem in another way.
Introduce new PM notifier calls after "prepare" notifier is done AND after tasks have been frozen. amdgpu listens to these PM notifier calls and will run the first eviction at this time.
Mario Limonciello (2):
PM: Add suspend and hibernate notifications for after freeze drm/amd: Use suspend and hibernate post freeze notifications
Documentation/driver-api/pm/notifiers.rst | 19 ++++++++++++++++--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- include/linux/suspend.h | 14 ++++++++------ kernel/power/hibernate.c | 9 +++++++-- kernel/power/suspend.c | 13 +++++++++---- 5 files changed, 42 insertions(+), 17 deletions(-)
Signature · 1 line
2.43.0
RFC 1/2 PM: Add suspend and hibernate notifications for after freeze
From: Mario Limonciello <[email protected]>
Suspend and hibernate notifications are available specifically when the sequence starts and finishes. However there are no notifications during the process when tasks have been frozen.
Introduce two new events PM_SUSPEND_POST_FREEZE and PM_HIBERNATE_POST_FREEZE that drivers can subscribe to and take different actions specifically knowing userspace is frozen.
--- Documentation/driver-api/pm/notifiers.rst | 19 ++++++++++++++++--- include/linux/suspend.h | 14 ++++++++------ kernel/power/hibernate.c | 9 +++++++-- kernel/power/suspend.c | 13 +++++++++---- 4 files changed, 40 insertions(+), 15 deletions(-)
Documentation/driver-api/pm/notifiers.rst +16 -3
@@ -32,6 +32,18 @@ will be called upon the following events by the PM core: additional work is done between the notifiers and the invocation of PM callbacks for the "freeze" transition. +``PM_HIBERNATION_POST_FREEZE`` + The system is going to hibernate and tasks have just been frozen. + +``PM_SUSPEND_PREPARE`` + The system is going to suspend, tasks will be frozen immediately. This + is different from ``PM_HIBERNATION_PREPARE`` above, because in this case + additional work is done between the notifiers and the invocation of PM + callbacks for the "freeze" transition. + +``PM_SUSPEND_POST_FREEZE`` + The system is going to suspend and tasks have just been frozen. + ``PM_POST_HIBERNATION`` The system memory state has been restored from a hibernation image or an error occurred during hibernation. Device restore callbacks have been
@@ -54,9 +66,10 @@ will be called upon the following events by the PM core: resume callbacks have been executed and tasks have been thawed. It is generally assumed that whatever the notifiers do for -``PM_HIBERNATION_PREPARE``, should be undone for ``PM_POST_HIBERNATION``. -Analogously, operations carried out for ``PM_SUSPEND_PREPARE`` should be -reversed for ``PM_POST_SUSPEND``. +``PM_HIBERNATION_PREPARE`` and ``PM_HIBERNATION_POST_FREEZE``, should be undone +for ``PM_POST_HIBERNATION``. +Analogously, operations carried out for ``PM_SUSPEND_PREPARE`` and ``PM_SUSPEND_POST_FREEZE`` +should be reversed for ``PM_POST_SUSPEND``. Moreover, if one of the notifiers fails for the ``PM_HIBERNATION_PREPARE`` or ``PM_SUSPEND_PREPARE`` event, the notifiers that have already succeeded for that
include/linux/suspend.h +8 -6
@@ -422,12 +422,14 @@ static inline int is_hibernate_resume_dev(dev_t dev) { return 0; }#endif /* Hibernation and suspend events */ -#define PM_HIBERNATION_PREPARE 0x0001/* Going to hibernate */ -#define PM_POST_HIBERNATION 0x0002/* Hibernation finished */ -#define PM_SUSPEND_PREPARE 0x0003/* Going to suspend the system */ -#define PM_POST_SUSPEND 0x0004/* Suspend finished */ -#define PM_RESTORE_PREPARE 0x0005/* Going to restore a saved image */ -#define PM_POST_RESTORE 0x0006/* Restore failed */ +#define PM_HIBERNATION_PREPARE 0x0001/* Going to hibernate */ +#define PM_HIBERNATION_POST_FREEZE 0x0002/* Prepared for hibernation and tasks have been frozen */ +#define PM_POST_HIBERNATION 0x0003/* Hibernation finished */ +#define PM_SUSPEND_PREPARE 0x0004/* Going to suspend the system */ +#define PM_SUSPEND_POST_FREEZE 0x0005/* Prepared and tasks have been frozen */ +#define PM_POST_SUSPEND 0x0006/* Suspend finished */ +#define PM_RESTORE_PREPARE 0x0007/* Going to restore a saved image */ +#define PM_POST_RESTORE 0x0008/* Restore failed */ extern struct mutex system_transition_mutex;
kernel/power/hibernate.c +7 -2
@@ -783,11 +783,15 @@ int hibernate(void)if (error) goto Exit; + error =pm_notifier_call_chain_robust (PM_HIBERNATION_POST_FREEZE ,PM_POST_HIBERNATION ); +if (error) + goto Thaw; +lock_device_hotplug ();/* Allocate memory management structures */ error =create_basic_memory_bitmaps ();if (error) - goto Thaw; + goto Unlock_hotplug; error =hibernation_snapshot (hibernation_mode ==HIBERNATION_PLATFORM );if (error || freezer_test_done)
@@ -833,8 +837,9 @@ int hibernate(void) Free_bitmaps:free_basic_memory_bitmaps (); - Thaw: + Unlock_hotplug:unlock_device_hotplug (); + Thaw:if (snapshot_test) {pm_pr_dbg ("Checking hibernation image\n" ); error =swsusp_check (false);
kernel/power/suspend.c +9 -4
@@ -377,13 +377,18 @@ static int suspend_prepare(suspend_state_t state)trace_suspend_resume (TPS ("freeze_processes" ),0 , true); error =suspend_freeze_processes ();trace_suspend_resume (TPS ("freeze_processes" ),0 , false); -if (!error) -return 0 ; +if (error) + goto Restore; + error =pm_notifier_call_chain_robust (PM_SUSPEND_POST_FREEZE ,PM_POST_SUSPEND ); +if (error) + goto Thaw; -dpm_save_failed_step (SUSPEND_FREEZE ); -pm_notifier_call_chain (PM_POST_SUSPEND ); +return 0 ; + Thaw: +suspend_thaw_processes (); Restore:pm_restore_console (); +dpm_save_failed_step (SUSPEND_FREEZE );return error; }
Signature · 1 line
2.43.0
Re: [RFC 1/2] PM: Add suspend and hibernate notifications for after freeze
Quoted 25 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
Suspend and hibernate notifications are available specifically when the sequence starts and finishes. However there are no notifications during the process when tasks have been frozen.
Introduce two new events
PM_SUSPEND_POST_FREEZEandPM_HIBERNATE_POST_FREEZEthat drivers can subscribe to and take different actions specifically knowing userspace is frozen.Suggested-by Alex Deucher <[email protected]>Signed-off-by Mario Limonciello <[email protected]>--- Documentation/driver-api/pm/notifiers.rst | 19 ++++++++++++++++--- include/linux/suspend.h | 14 ++++++++------ kernel/power/hibernate.c | 9 +++++++-- kernel/power/suspend.c | 13 +++++++++---- 4 files changed, 40 insertions(+), 15 deletions(-)Documentation/driver-api/pm/notifiers.rst +0
-0@@ -32,6 +32,18 @@ will be called upon the following events by the PM core: additional work is done between the notifiers and the invocation of PM
Documentation/driver-api/pm/notifiers.rst will be called upon the following events by the PM core: · line 42
Quoted 10 lines
Documentation/driver-api/pm/notifiers.rst +8
-0callbacks for the "freeze" transition. +``PM_HIBERNATION_POST_FREEZE`` + The system is going to hibernate and tasks have just been frozen. + +``PM_SUSPEND_PREPARE`` + The system is going to suspend, tasks will be frozen immediately. This + is different from ``PM_HIBERNATION_PREPARE`` above, because in this case + additional work is done between the notifiers and the invocation of PM + callbacks for the "freeze" transition.
I accidentally created this section ``PM_SUSPEND_PREPARE`` (there already is one for PM_SUSPEND_PREPARE that is totally fine).
If conceptually the rest of the patch makes sense I will drop this section.
Documentation/driver-api/pm/notifiers.rst will be called upon the following events by the PM core: · line 45
Quoted 3 lines
Documentation/driver-api/pm/notifiers.rst +3
-0+ +``PM_SUSPEND_POST_FREEZE`` + The system is going to suspend and tasks have just been frozen.
Looking again, I think this is better put between (the correct) PM_SUSPEND_PREPARE and PM_POST_SUSPEND.
If conceptually the rest of the patch makes sense I will move it to that section.
Quoted 102 lines
Documentation/driver-api/pm/notifiers.rst +5
-3+ ``PM_POST_HIBERNATION`` The system memory state has been restored from a hibernation image or an error occurred during hibernation. Device restore callbacks have been@@ -54,9 +66,10 @@ will be called upon the following events by the PM core: resume callbacks have been executed and tasks have been thawed. It is generally assumed that whatever the notifiers do for -``PM_HIBERNATION_PREPARE``, should be undone for ``PM_POST_HIBERNATION``. -Analogously, operations carried out for ``PM_SUSPEND_PREPARE`` should be -reversed for ``PM_POST_SUSPEND``. +``PM_HIBERNATION_PREPARE`` and ``PM_HIBERNATION_POST_FREEZE``, should be undone +for ``PM_POST_HIBERNATION``. +Analogously, operations carried out for ``PM_SUSPEND_PREPARE`` and ``PM_SUSPEND_POST_FREEZE`` +should be reversed for ``PM_POST_SUSPEND``. Moreover, if one of the notifiers fails for the ``PM_HIBERNATION_PREPARE`` or ``PM_SUSPEND_PREPARE`` event, the notifiers that have already succeeded for thatinclude/linux/suspend.h +8
-6@@ -422,12 +422,14 @@ static inline int is_hibernate_resume_dev(dev_t dev) { return 0; }#endif /* Hibernation and suspend events */ -#define PM_HIBERNATION_PREPARE 0x0001/* Going to hibernate */ -#define PM_POST_HIBERNATION 0x0002/* Hibernation finished */ -#define PM_SUSPEND_PREPARE 0x0003/* Going to suspend the system */ -#define PM_POST_SUSPEND 0x0004/* Suspend finished */ -#define PM_RESTORE_PREPARE 0x0005/* Going to restore a saved image */ -#define PM_POST_RESTORE 0x0006/* Restore failed */ +#define PM_HIBERNATION_PREPARE 0x0001/* Going to hibernate */ +#define PM_HIBERNATION_POST_FREEZE 0x0002/* Prepared for hibernation and tasks have been frozen */ +#define PM_POST_HIBERNATION 0x0003/* Hibernation finished */ +#define PM_SUSPEND_PREPARE 0x0004/* Going to suspend the system */ +#define PM_SUSPEND_POST_FREEZE 0x0005/* Prepared and tasks have been frozen */ +#define PM_POST_SUSPEND 0x0006/* Suspend finished */ +#define PM_RESTORE_PREPARE 0x0007/* Going to restore a saved image */ +#define PM_POST_RESTORE 0x0008/* Restore failed */ extern struct mutex system_transition_mutex;kernel/power/hibernate.c +7
-2@@ -783,11 +783,15 @@ int hibernate(void)if (error) goto Exit; + error =pm_notifier_call_chain_robust (PM_HIBERNATION_POST_FREEZE ,PM_POST_HIBERNATION ); +if (error) + goto Thaw; +lock_device_hotplug ();/* Allocate memory management structures */ error =create_basic_memory_bitmaps ();if (error) - goto Thaw; + goto Unlock_hotplug; error =hibernation_snapshot (hibernation_mode ==HIBERNATION_PLATFORM );if (error || freezer_test_done)@@ -833,8 +837,9 @@ int hibernate(void) Free_bitmaps:free_basic_memory_bitmaps (); - Thaw: + Unlock_hotplug:unlock_device_hotplug (); + Thaw:if (snapshot_test) {pm_pr_dbg ("Checking hibernation image\n" ); error =swsusp_check (false);kernel/power/suspend.c +9
-4@@ -377,13 +377,18 @@ static int suspend_prepare(suspend_state_t state)trace_suspend_resume (TPS ("freeze_processes" ),0 , true); error =suspend_freeze_processes ();trace_suspend_resume (TPS ("freeze_processes" ),0 , false); -if (!error) -return 0 ; +if (error) + goto Restore; + error =pm_notifier_call_chain_robust (PM_SUSPEND_POST_FREEZE ,PM_POST_SUSPEND ); +if (error) + goto Thaw; -dpm_save_failed_step (SUSPEND_FREEZE ); -pm_notifier_call_chain (PM_POST_SUSPEND ); +return 0 ; + Thaw: +suspend_thaw_processes (); Restore:pm_restore_console (); +dpm_save_failed_step (SUSPEND_FREEZE );return error; }
RFC 2/2 drm/amd: Use suspend and hibernate post freeze notifications
In reply to Mario LimoncielloFrom: Mario Limonciello <[email protected]>
commit 2965e6355dcd ("drm/amd: Add Suspend/Hibernate notification callback support") introduced a VRAM eviction earlier in the PM sequences when swap was still available for evicting to. This helped to fix a number of memory pressure related bugs but also exposed a new one.
If a userspace process is actively using the GPU when suspend starts then a deadlock could occur.
Instead of going off the prepare notifier, use the PM notifiers that occur after processes have been frozen to do evictions.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2 -2
@@ -4917,10 +4917,10 @@ static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long moint r;switch (mode) { -case PM_HIBERNATION_PREPARE : +case PM_HIBERNATION_POST_FREEZE : adev->in_s4 = true; fallthrough; -case PM_SUSPEND_PREPARE : +case PM_SUSPEND_POST_FREEZE : r =amdgpu_device_evict_resources (adev);/* * This is considered non-fatal at this time because
Signature · 1 line
2.43.0
Rafael J. Wysocki
maintainerRe: [RFC 2/2] drm/amd: Use suspend and hibernate post freeze notifications
Quoted 30 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
commit
2965e6355dcd("drm/amd: Add Suspend/Hibernate notification callback support") introduced a VRAM eviction earlier in the PM sequences when swap was still available for evicting to. This helped to fix a number of memory pressure related bugs but also exposed a new one.If a userspace process is actively using the GPU when suspend starts then a deadlock could occur.
Instead of going off the prepare notifier, use the PM notifiers that occur after processes have been frozen to do evictions.
Fixes 2965e6355dcd ("drm/amd: Add Suspend/Hibernate notification callback support")Signed-off-by Mario Limonciello <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -4917,10 +4917,10 @@ static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long moint r;switch (mode) {
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long mo
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-3-case PM_HIBERNATION_PREPARE : +case PM_HIBERNATION_POST_FREEZE : adev->in_s4 = true; fallthrough; -case PM_SUSPEND_PREPARE : +case PM_SUSPEND_POST_FREEZE : r =amdgpu_device_evict_resources (adev);/* * This is considered non-fatal at this time because --
Why do you need a notifier for this?
It looks like this could be done from amdgpu_device_prepare(), but if there is a reason why it cannot be done from there, it should be mentioned in the changelog.
Re: [RFC 2/2] drm/amd: Use suspend and hibernate post freeze notifications
Quoted 44 lines · Rafael J. Wysocki
Quoted 30 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
commit
2965e6355dcd("drm/amd: Add Suspend/Hibernate notification callback support") introduced a VRAM eviction earlier in the PM sequences when swap was still available for evicting to. This helped to fix a number of memory pressure related bugs but also exposed a new one.If a userspace process is actively using the GPU when suspend starts then a deadlock could occur.
Instead of going off the prepare notifier, use the PM notifiers that occur after processes have been frozen to do evictions.
Fixes 2965e6355dcd ("drm/amd: Add Suspend/Hibernate notification callback support")Signed-off-by Mario Limonciello <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -4917,10 +4917,10 @@ static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long moint r;switch (mode) {drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long mo
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-3-case PM_HIBERNATION_PREPARE : +case PM_HIBERNATION_POST_FREEZE : adev->in_s4 = true; fallthrough; -case PM_SUSPEND_PREPARE : +case PM_SUSPEND_POST_FREEZE : r =amdgpu_device_evict_resources (adev);/* * This is considered non-fatal at this time because -- Why do you need a notifier for this?
Quoted 3 lines
It looks like this could be done from
amdgpu_device_prepare(), but if there is a reason why it cannot be done from there, it should be mentioned in the changelog.
It's actually done in amdgpu_device_prepare() "as well" already, but the reason that it's being done earlier is because swap still needs to be available, especially with heavy memory fragmentation.
I'll add more detail about this to the commit for the next spin if you're relatively happy with the new notifier from the first patch.
Rafael J. Wysocki
maintainerRe: [RFC 2/2] drm/amd: Use suspend and hibernate post freeze notifications
Quoted 46 lines · Mario Limonciello
Quoted 44 lines · Rafael J. Wysocki
Quoted 30 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
commit
2965e6355dcd("drm/amd: Add Suspend/Hibernate notification callback support") introduced a VRAM eviction earlier in the PM sequences when swap was still available for evicting to. This helped to fix a number of memory pressure related bugs but also exposed a new one.If a userspace process is actively using the GPU when suspend starts then a deadlock could occur.
Instead of going off the prepare notifier, use the PM notifiers that occur after processes have been frozen to do evictions.
Fixes 2965e6355dcd ("drm/amd: Add Suspend/Hibernate notification callback support")Signed-off-by Mario Limonciello <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -4917,10 +4917,10 @@ static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long moint r;switch (mode) {drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long mo
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-3-case PM_HIBERNATION_PREPARE : +case PM_HIBERNATION_POST_FREEZE : adev->in_s4 = true; fallthrough; -case PM_SUSPEND_PREPARE : +case PM_SUSPEND_POST_FREEZE : r =amdgpu_device_evict_resources (adev);/* * This is considered non-fatal at this time because -- Why do you need a notifier for this?
Quoted 7 lines
Quoted 3 lines
It looks like this could be done from
amdgpu_device_prepare(), but if there is a reason why it cannot be done from there, it should be mentioned in the changelog.It's actually done in
amdgpu_device_prepare()"as well" already, but the reason that it's being done earlier is because swap still needs to be available, especially with heavy memory fragmentation.
Swap should be still available when amdgpu_device_prepare() runs.
Quoted 2 lines
I'll add more detail about this to the commit for the next spin if you're relatively happy with the new notifier from the first patch.
I need to have a look at it, but adding it for just one user seems a bit over the top. I'd prefer to avoid doing this.
Re: [RFC 2/2] drm/amd: Use suspend and hibernate post freeze notifications
Quoted 52 lines · Rafael J. Wysocki
Quoted 50 lines · Mario Limonciello
Quoted 47 lines · Rafael J. Wysocki
Quoted 30 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
commit
2965e6355dcd("drm/amd: Add Suspend/Hibernate notification callback support") introduced a VRAM eviction earlier in the PM sequences when swap was still available for evicting to. This helped to fix a number of memory pressure related bugs but also exposed a new one.If a userspace process is actively using the GPU when suspend starts then a deadlock could occur.
Instead of going off the prepare notifier, use the PM notifiers that occur after processes have been frozen to do evictions.
Fixes 2965e6355dcd ("drm/amd: Add Suspend/Hibernate notification callback support")Signed-off-by Mario Limonciello <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -4917,10 +4917,10 @@ static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long moint r;switch (mode) {drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long mo
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-3-case PM_HIBERNATION_PREPARE : +case PM_HIBERNATION_POST_FREEZE : adev->in_s4 = true; fallthrough; -case PM_SUSPEND_PREPARE : +case PM_SUSPEND_POST_FREEZE : r =amdgpu_device_evict_resources (adev);/* * This is considered non-fatal at this time because -- Why do you need a notifier for this?
It looks like this could be done from
amdgpu_device_prepare(), but if there is a reason why it cannot be done from there, it should be mentioned in the changelog.
Quoted 5 lines
Quoted 3 lines
It's actually done in
amdgpu_device_prepare()"as well" already, but the reason that it's being done earlier is because swap still needs to be available, especially with heavy memory fragmentation.Swap should be still available when
amdgpu_device_prepare()runs.
We need to do this after processes are frozen, otherwise applications keep running and keep allocating VRAM so by the time processes are frozen, swap is disabled and we don't have anywhere to store the contents of VRAM if memory is full or fragmented.
Alex
Quoted 6 lines
Quoted 3 lines
I'll add more detail about this to the commit for the next spin if you're relatively happy with the new notifier from the first patch.
I need to have a look at it, but adding it for just one user seems a bit over the top. I'd prefer to avoid doing this.
Re: [RFC 2/2] drm/amd: Use suspend and hibernate post freeze notifications
In reply to Rafael J. WysockiQuoted 51 lines · Rafael J. Wysocki
Quoted 50 lines · Mario Limonciello
Quoted 47 lines · Rafael J. Wysocki
Quoted 30 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
commit
2965e6355dcd("drm/amd: Add Suspend/Hibernate notification callback support") introduced a VRAM eviction earlier in the PM sequences when swap was still available for evicting to. This helped to fix a number of memory pressure related bugs but also exposed a new one.If a userspace process is actively using the GPU when suspend starts then a deadlock could occur.
Instead of going off the prepare notifier, use the PM notifiers that occur after processes have been frozen to do evictions.
Fixes 2965e6355dcd ("drm/amd: Add Suspend/Hibernate notification callback support")Signed-off-by Mario Limonciello <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -4917,10 +4917,10 @@ static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long moint r;switch (mode) {drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long mo
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-3-case PM_HIBERNATION_PREPARE : +case PM_HIBERNATION_POST_FREEZE : adev->in_s4 = true; fallthrough; -case PM_SUSPEND_PREPARE : +case PM_SUSPEND_POST_FREEZE : r =amdgpu_device_evict_resources (adev);/* * This is considered non-fatal at this time because -- Why do you need a notifier for this?
It looks like this could be done from
amdgpu_device_prepare(), but if there is a reason why it cannot be done from there, it should be mentioned in the changelog.
Quoted 5 lines
Quoted 3 lines
It's actually done in
amdgpu_device_prepare()"as well" already, but the reason that it's being done earlier is because swap still needs to be available, especially with heavy memory fragmentation.Swap should be still available when
amdgpu_device_prepare()runs.
No; it's not. The basic call trace (for suspend) looks like this:
enter_state(state) {
suspend_prepare(state);
...
pm_restrict_gfp_mask(); // disable swap
suspend_devices_and_enter(state) → dpm_suspend_start() {
dpm_prepare() {
amdgpu_pmops_prepare();
}
dpm_suspend() {
amdgpu_pmops_suspend();
}
}
}If the intention was for it to be available, it would be better to move the pm_restrict_gfp_mask() call "into" suspend_devices_and_enter() between dpm_prepare() and dpm_suspend() calls.
Quoted 6 lines
Quoted 3 lines
I'll add more detail about this to the commit for the next spin if you're relatively happy with the new notifier from the first patch.
I need to have a look at it, but adding it for just one user seems a bit over the top. I'd prefer to avoid doing this.
Rafael J. Wysocki
maintainerRe: [RFC 2/2] drm/amd: Use suspend and hibernate post freeze notifications
Quoted 57 lines · Mario Limonciello
Quoted 55 lines · Rafael J. Wysocki
Quoted 53 lines · Mario Limonciello
Quoted 44 lines · Rafael J. Wysocki
Quoted 30 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
commit
2965e6355dcd("drm/amd: Add Suspend/Hibernate notification callback support") introduced a VRAM eviction earlier in the PM sequences when swap was still available for evicting to. This helped to fix a number of memory pressure related bugs but also exposed a new one.If a userspace process is actively using the GPU when suspend starts then a deadlock could occur.
Instead of going off the prepare notifier, use the PM notifiers that occur after processes have been frozen to do evictions.
Fixes 2965e6355dcd ("drm/amd: Add Suspend/Hibernate notification callback support")Signed-off-by Mario Limonciello <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -4917,10 +4917,10 @@ static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long moint r;switch (mode) {drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_device_pm_notifier(struct notifier_block *nb, unsigned long mo
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-3-case PM_HIBERNATION_PREPARE : +case PM_HIBERNATION_POST_FREEZE : adev->in_s4 = true; fallthrough; -case PM_SUSPEND_PREPARE : +case PM_SUSPEND_POST_FREEZE : r =amdgpu_device_evict_resources (adev);/* * This is considered non-fatal at this time because -- Why do you need a notifier for this?
Quoted 3 lines
It looks like this could be done from
amdgpu_device_prepare(), but if there is a reason why it cannot be done from there, it should be mentioned in the changelog.It's actually done in
amdgpu_device_prepare()"as well" already, but the reason that it's being done earlier is because swap still needs to be available, especially with heavy memory fragmentation.
Quoted 8 lines
Quoted 1 line
Swap should be still available when
amdgpu_device_prepare()runs.No; it's not. The basic call trace (for suspend) looks like this:
enter_state(state) { suspend_prepare(state); ... pm_restrict_gfp_mask(); // disable swap
Ah, OK
Quoted 10 lines
suspend_devices_and_enter(state) → dpm_suspend_start() {
dpm_prepare() {
amdgpu_pmops_prepare();
}
dpm_suspend() {
amdgpu_pmops_suspend();
}
}
}Quoted 3 lines
If the intention was for it to be available, it would be better to move the
pm_restrict_gfp_mask()call "into"suspend_devices_and_enter()betweendpm_prepare()anddpm_suspend()calls.
It was not the intention, but then the "prepare" phase is not expected to do anything that would prevent swap from working, so it might as well be disabled later.
Then, it would need to be re-enabled before the "complete" phase which again shouldn't break anything in principle.
Christian König
maintainerQuoted 18 lines · Mario Limonciello
From: Mario Limonciello <[email protected]>
As part of the suspend and hibernate sequences devices supported by amdgpu will have VRAM evicted. This has been littered with problems in the past, and now it is called strategically at multiple times during power management sequences.
Most recently there were a number of issues reported that were root caused to physical memory pressure at suspend time where VRAM couldn't be evicted. A new call was introduced using a PM notifier before swap was turned off so that even if there are physical memory pressures VRAM could still be evicted into swap.
This helped, but users using older systemd noted that this could lead to a deadlock. This is because user processes aren't frozen by older versions of systemd and thus if a game is running it could potentially be creating BOs during the eviction thus preventing eviction from ever completing.
Quoted 7 lines
This isn't an issue with newer systemd because it uses cgroup freezing to freeze user processes. Nonetheless this isn't a good behavior to rely upon for userspace, so this series aims to solve the problem in another way.
Introduce new PM notifier calls after "prepare" notifier is done AND after tasks have been frozen. amdgpu listens to these PM notifier calls and will run the first eviction at this time.
Sounds totally sane to me, but I'm really not deep enough in the PM code to fully judge the implementation.
Feel free to add Acked-by: Christian König <[email protected]> to the patches.
Regards,
Christian.
Quoted 12 lines
Mario Limonciello (2):
PM: Add suspend and hibernate notifications for after freeze drm/amd: Use suspend and hibernate post freeze notificationsDocumentation/driver-api/pm/notifiers.rst | 19 ++++++++++++++++--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- include/linux/suspend.h | 14 ++++++++------ kernel/power/hibernate.c | 9 +++++++-- kernel/power/suspend.c | 13 +++++++++---- 5 files changed, 42 insertions(+), 17 deletions(-)