Re: 回复: [PATCH] drm/amdgpu: Ignore first evction failure during suspend
Felix Kuehling
felix.kuehling at amd.com
Thu Sep 14 13:37:54 UTC 2023
Userptr and SVM restore work is scheduled to the system WQ with
schedule_delayed_work. See amdgpu_amdkfd_evict_userptr and
svm_range_evict. This would need to use queue_delayed_work with the
system_freezable_wq.
BO restoration is scheduled with queue_delayed_work on our own
kfd_restore_wq that was allocated with alloc_ordered_workqueue. This
would need to add the WQ_FREEZABLE flag when we create the wq in
kfd_process_create_wq.
There is also evict_process_worker scheduled with schedule_delayed_work,
which handles stopping of user mode queues, signaling of eviction fences
and scheduling of restore work when BOs are evicted. I think that should
not be freezable because it's needed to signal the eviction fences to
allow suspend to evict BOs.
To make sure I'm not misunderstanding, I assume that freezing a
freezable workqueue flushes work items in progress and prevents
execution of more work until it is unfrozen. I assume work items are not
frozen in the middle of execution, because that would not solve the problem.
Regards,
Felix
On 2023-09-14 2:23, Christian König wrote:
> [putting Harry on BCC, sorry for the noise]
>
> Yeah, that is clearly a bug in the KFD.
>
> During the second eviction the hw should already be disabled, so we
> don't have any SDMA or similar to evict BOs any more and can only copy
> them with the CPU.
>
> @Felix what workqueue do you guys use for the restore work? I've just
> double checked and on the system workqueues you explicitly need to
> specify that stuff is freezable. E.g. use system_freezable_wq instead
> of system_wq.
>
> Alternatively as Xinhui mentioned it might be necessary to flush all
> restore work before the first eviction phase or we have the chance
> that BOs are moved back into VRAM again.
>
> Regards,
> Christian.
>
> Am 14.09.23 um 03:54 schrieb Pan, Xinhui:
>>
>> [AMD Official Use Only - General]
>>
>>
>> I just make one debug patch to show busy BO’s alloc-trace when the
>> eviction fails in suspend.
>>
>> And dmesg log attached.
>>
>> Looks like they are just kfd user Bos and locked by evict/restore work.
>>
>> So in kfd suspend callback, it really need to flush the evict/restore
>> work before HW fini as it do now.
>>
>> That is why the first very early eviction fails and the second
>> eviction succeed.
>>
>> Thanks
>>
>> xinhui
>>
>> *From:* Pan, Xinhui
>> *Sent:* Thursday, September 14, 2023 8:02 AM
>> *To:* Koenig, Christian <Christian.Koenig at amd.com>; Kuehling, Felix
>> <Felix.Kuehling at amd.com>; Christian König
>> <ckoenig.leichtzumerken at gmail.com>; amd-gfx at lists.freedesktop.org;
>> Wentland, Harry <Harry.Wentland at amd.com>
>> *Cc:* Deucher, Alexander <Alexander.Deucher at amd.com>; Fan, Shikang
>> <Shikang.Fan at amd.com>
>> *Subject:* RE: 回复: [PATCH] drm/amdgpu: Ignore first evction failure
>> during suspend
>>
>> Chris,
>>
>> I can dump these busy BOs with their alloc/free stack later today.
>>
>> BTW, the two evictions and the kfd suspend are all called before
>> hw_fini. IOW, between phase 1 and phase 2. SDMA is turned only in
>> phase2. So current code works fine maybe.
>>
>> *From:* Koenig, Christian <Christian.Koenig at amd.com>
>> *Sent:* Wednesday, September 13, 2023 10:29 PM
>> *To:* Kuehling, Felix <Felix.Kuehling at amd.com>; Christian König
>> <ckoenig.leichtzumerken at gmail.com>; Pan, Xinhui <Xinhui.Pan at amd.com>;
>> amd-gfx at lists.freedesktop.org; Wentland, Harry <Harry.Wentland at amd.com>
>> *Cc:* Deucher, Alexander <Alexander.Deucher at amd.com>; Fan, Shikang
>> <Shikang.Fan at amd.com>
>> *Subject:* Re: 回复: [PATCH] drm/amdgpu: Ignore first evction failure
>> during suspend
>>
>> [+Harry]
>>
>> Am 13.09.23 um 15:54 schrieb Felix Kuehling:
>>
>> On 2023-09-13 4:07, Christian König wrote:
>>
>> [+Fleix]
>>
>> Well that looks like quite a serious bug.
>>
>> If I'm not completely mistaken the KFD work item tries to
>> restore the process by moving BOs into memory even after the
>> suspend freeze. Normally work items are frozen together with
>> the user space processes unless explicitly marked as not
>> freezable.
>>
>> That this causes problem during the first eviction phase is
>> just the tip of the iceberg here. If a BO is moved into
>> invisible memory during this we wouldn't be able to get it
>> out of that in the second phase because SDMA and hw is
>> already turned off.
>>
>> @Felix any idea how that can happen? Have you guys marked a
>> work item / work queue as not freezable?
>>
>> We don't set anything to non-freezable in KFD.
>>
>> Regards,
>> Felix
>>
>> Or maybe the display guys?
>>
>>
>> Do you guys in the display do any delayed update in a work item which
>> is marked as not-freezable?
>>
>> Otherwise I have absolutely no idea what's going on here.
>>
>> Thanks,
>> Christian.
>>
>>
>> @Xinhui please investigate what work item that is and where
>> that is coming from. Something like "if (adev->in_suspend)
>> dump_stack();" in the right place should probably do it.
>>
>> Thanks,
>> Christian.
>>
>> Am 13.09.23 um 07:13 schrieb Pan, Xinhui:
>>
>> [AMD Official Use Only - General]
>>
>> I notice that only user space process are frozen on my
>> side. kthread and workqueue keeps running. Maybe some
>> kernel configs are not enabled.
>>
>> I made one module which just prints something like i++
>> with mutex lock both in workqueue and kthread. I paste
>> some logs below.
>>
>> [438619.696196] XH: 14 from workqueue
>>
>> [438619.700193] XH: 15 from kthread
>>
>> [438620.394335] PM: suspend entry (deep)
>>
>> [438620.399619] Filesystems sync: 0.001 seconds
>>
>> [438620.403887] PM: Preparing system for sleep (deep)
>>
>> [438620.409299] Freezing user space processes
>>
>> [438620.414862] Freezing user space processes completed
>> (elapsed 0.001 seconds)
>>
>> [438620.421881] OOM killer disabled.
>>
>> [438620.425197] Freezing remaining freezable tasks
>>
>> [438620.430890] Freezing remaining freezable tasks
>> completed (elapsed 0.001 seconds)
>>
>> [438620.438348] PM: Suspending system (deep)
>>
>> .....
>>
>> [438623.746038] PM: suspend of devices complete after
>> 3303.137 msecs
>>
>> [438623.752125] PM: start suspend of devices complete
>> after 3309.713 msecs
>>
>> [438623.758722] PM: suspend debug: Waiting for 5 second(s).
>>
>> [438623.792166] XH: 22 from kthread
>>
>> [438623.824140] XH: 23 from workqueue
>>
>> So BOs definitely can be in use during suspend.
>>
>> Even if kthread or workqueue can be stopped with one
>> special kernel config. I think suspend can only stop the
>> workqueue with its callback finish.
>>
>> otherwise something like below makes things crazy.
>>
>> LOCK BO
>>
>> do something
>>
>> -> schedule or wait, anycode might sleep. Stopped by
>> suspend now? no, i think.
>>
>> UNLOCK BO
>>
>> I do tests with cmds below.
>>
>> echo devices > /sys/power/pm_test
>>
>> echo 0 > /sys/power/pm_async
>>
>> echo 1 > /sys/power/pm_print_times
>>
>> echo 1 > /sys/power/pm_debug_messages
>>
>> echo 1 > /sys/module/amdgpu/parameters/debug_evictions
>>
>> ./kfd.sh --gtest_filter=KFDEvictTest.BasicTest
>>
>> pm-suspend
>>
>> thanks
>>
>> xinhui
>>
>> ------------------------------------------------------------------------
>>
>> *发件人:*Christian König <ckoenig.leichtzumerken at gmail.com>
>> <mailto:ckoenig.leichtzumerken at gmail.com>
>> *发送时间:*2023年9月12日17:01
>> *收件人:*Pan, Xinhui <Xinhui.Pan at amd.com>
>> <mailto:Xinhui.Pan at amd.com>;
>> amd-gfx at lists.freedesktop.org
>> <amd-gfx at lists.freedesktop.org>
>> <mailto:amd-gfx at lists.freedesktop.org>
>> *抄送:*Deucher, Alexander <Alexander.Deucher at amd.com>
>> <mailto:Alexander.Deucher at amd.com>; Koenig, Christian
>> <Christian.Koenig at amd.com>
>> <mailto:Christian.Koenig at amd.com>; Fan, Shikang
>> <Shikang.Fan at amd.com> <mailto:Shikang.Fan at amd.com>
>> *主题:*Re: [PATCH] drm/amdgpu: Ignore first evction failure
>> during suspend
>>
>> When amdgpu_device_suspend() is called processes should
>> be frozen
>> already. In other words KFD queues etc... should already
>> be idle.
>>
>> So when the eviction fails here we missed something
>> previously and that
>> in turn can cause tons amount of problems.
>>
>> So ignoring those errors is most likely not a good idea
>> at all.
>>
>> Regards,
>> Christian.
>>
>> Am 12.09.23 um 02:21 schrieb Pan, Xinhui:
>> > [AMD Official Use Only - General]
>> >
>> > Oh yep, Pinned BO is moved to other LRU list, So
>> eviction fails because of other reason.
>> > I will change the comments in the patch.
>> > The problem is eviction fails as many reasons, say, BO
>> is locked.
>> > ASAIK, kfd will stop the queues and flush some
>> evict/restore work in its suspend callback. SO the first
>> eviction before kfd callback likely fails.
>> >
>> > -----Original Message-----
>> > From: Christian König
>> <ckoenig.leichtzumerken at gmail.com>
>> <mailto:ckoenig.leichtzumerken at gmail.com>
>> > Sent: Friday, September 8, 2023 2:49 PM
>> > To: Pan, Xinhui <Xinhui.Pan at amd.com>
>> <mailto:Xinhui.Pan at amd.com>; amd-gfx at lists.freedesktop.org
>> > Cc: Deucher, Alexander <Alexander.Deucher at amd.com>
>> <mailto:Alexander.Deucher at amd.com>; Koenig, Christian
>> <Christian.Koenig at amd.com>
>> <mailto:Christian.Koenig at amd.com>; Fan, Shikang
>> <Shikang.Fan at amd.com> <mailto:Shikang.Fan at amd.com>
>> > Subject: Re: [PATCH] drm/amdgpu: Ignore first evction
>> failure during suspend
>> >
>> > Am 08.09.23 um 05:39 schrieb xinhui pan:
>> >> Some BOs might be pinned. So the first eviction's
>> failure will abort
>> >> the suspend sequence. These pinned BOs will be unpined
>> afterwards
>> >> during suspend.
>> > That doesn't make much sense since pinned BOs don't
>> cause eviction failure here.
>> >
>> > What exactly is the error code you see?
>> >
>> > Christian.
>> >
>> >> Actaully it has evicted most BOs, so that should stil
>> work fine in
>> >> sriov full access mode.
>> >>
>> >> Fixes: 47ea20762bb7 ("drm/amdgpu: Add an extra
>> evict_resource call
>> >> during device_suspend.")
>> >> Signed-off-by: xinhui pan <xinhui.pan at amd.com>
>> <mailto:xinhui.pan at amd.com>
>> >> ---
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++----
>> >> 1 file changed, 5 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> index 5c0e2b766026..39af526cdbbe 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> @@ -4148,10 +4148,11 @@ int
>> amdgpu_device_suspend(struct drm_device
>> >> *dev, bool fbcon)
>> >>
>> >> adev->in_suspend = true;
>> >>
>> >> - /* Evict the majority of BOs before grabbing the
>> full access */
>> >> - r = amdgpu_device_evict_resources(adev);
>> >> - if (r)
>> >> - return r;
>> >> + /* Try to evict the majority of BOs before
>> grabbing the full access
>> >> + * Ignore the ret val at first place as we will
>> unpin some BOs if any
>> >> + * afterwards.
>> >> + */
>> >> + (void)amdgpu_device_evict_resources(adev);
>> >>
>> >> if (amdgpu_sriov_vf(adev)) {
>> >> amdgpu_virt_fini_data_exchange(adev);
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20230914/82f176cc/attachment-0001.htm>
More information about the amd-gfx
mailing list