From: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
"Christian König" <christian.koenig@amd.com>,
dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
linux-pci@vger.kernel.org, daniel.vetter@ffwll.ch,
Harry.Wentland@amd.com
Cc: Alexander.Deucher@amd.com, gregkh@linuxfoundation.org,
helgaas@kernel.org, Felix.Kuehling@amd.com
Subject: Re: [PATCH v5 15/27] drm/scheduler: Fix hang when sched_entity released
Date: Wed, 5 May 2021 09:57:03 -0400 [thread overview]
Message-ID: <aa42d956-7bcb-9c90-ba5f-12ab701548dd@amd.com> (raw)
In-Reply-To: <a704880d-8e27-3cca-f42b-1320d39ac503@amd.com>
Ping
Andrey
On 2021-04-30 12:10 p.m., Andrey Grodzovsky wrote:
>
>
> On 2021-04-30 2:47 a.m., Christian König wrote:
>>
>>
>> Am 29.04.21 um 19:06 schrieb Andrey Grodzovsky:
>>>
>>>
>>> On 2021-04-29 3:18 a.m., Christian König wrote:
>>>> I need to take another look at this part when I don't have a massive
>>>> headache any more.
>>>>
>>>> Maybe split the patch set up into different parts, something like:
>>>> 1. Adding general infrastructure.
>>>> 2. Making sure all memory is unpolated.
>>>> 3. Job and fence handling
>>>
>>> I am not sure you mean this patch here, maybe another one ?
>>> Also note you already RBed it.
>>
>> No what I meant was to send out the patches before this one as #1 and #2.
>>
>> That is the easier stuff which can easily go into the drm-misc-next or
>> amd-staging-drm-next branch.
>>
>> The scheduler stuff certainly need to go into drm-misc-next.
>>
>> Christian.
>
> Got you. I am fine with it. What we have here is a working hot-unplug
> code but, one with potential use after free MMIO ranges frpom the zombie
> device. The followup patches after this patch are all about preventing
> this and so the patch-set up until this patch including, is functional
> on it's own. While it's necessary to solve the above issue, it's has
> complications as can be seen from the discussion with Daniel on later
> patch in this series. Still, in my opinion it's better to rollout some
> initial support to hot-unplug without use after free protection then
> having no support for hot-unplug at all. It will also make the merge
> work easier as I need to constantly rebase the patches on top latest
> kernel and solve new regressions.
>
> Daniel - given the arguments above can you sound your opinion on this
> approach ?
>
> Andrey
>>
>>>
>>> Andrey
>>>
>>>>
>>>> Christian.
>>>>
>>>> Am 28.04.21 um 17:11 schrieb Andrey Grodzovsky:
>>>>> Problem: If scheduler is already stopped by the time sched_entity
>>>>> is released and entity's job_queue not empty I encountred
>>>>> a hang in drm_sched_entity_flush. This is because
>>>>> drm_sched_entity_is_idle
>>>>> never becomes false.
>>>>>
>>>>> Fix: In drm_sched_fini detach all sched_entities from the
>>>>> scheduler's run queues. This will satisfy drm_sched_entity_is_idle.
>>>>> Also wakeup all those processes stuck in sched_entity flushing
>>>>> as the scheduler main thread which wakes them up is stopped by now.
>>>>>
>>>>> v2:
>>>>> Reverse order of drm_sched_rq_remove_entity and marking
>>>>> s_entity as stopped to prevent reinserion back to rq due
>>>>> to race.
>>>>>
>>>>> v3:
>>>>> Drop drm_sched_rq_remove_entity, only modify entity->stopped
>>>>> and check for it in drm_sched_entity_is_idle
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/scheduler/sched_entity.c | 3 ++-
>>>>> drivers/gpu/drm/scheduler/sched_main.c | 24
>>>>> ++++++++++++++++++++++++
>>>>> 2 files changed, 26 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>>>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>>>> index f0790e9471d1..cb58f692dad9 100644
>>>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>>>> @@ -116,7 +116,8 @@ static bool drm_sched_entity_is_idle(struct
>>>>> drm_sched_entity *entity)
>>>>> rmb(); /* for list_empty to work without lock */
>>>>> if (list_empty(&entity->list) ||
>>>>> - spsc_queue_count(&entity->job_queue) == 0)
>>>>> + spsc_queue_count(&entity->job_queue) == 0 ||
>>>>> + entity->stopped)
>>>>> return true;
>>>>> return false;
>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
>>>>> b/drivers/gpu/drm/scheduler/sched_main.c
>>>>> index 908b0b56032d..ba087354d0a8 100644
>>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>>>> @@ -897,9 +897,33 @@ EXPORT_SYMBOL(drm_sched_init);
>>>>> */
>>>>> void drm_sched_fini(struct drm_gpu_scheduler *sched)
>>>>> {
>>>>> + struct drm_sched_entity *s_entity;
>>>>> + int i;
>>>>> +
>>>>> if (sched->thread)
>>>>> kthread_stop(sched->thread);
>>>>> + for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >=
>>>>> DRM_SCHED_PRIORITY_MIN; i--) {
>>>>> + struct drm_sched_rq *rq = &sched->sched_rq[i];
>>>>> +
>>>>> + if (!rq)
>>>>> + continue;
>>>>> +
>>>>> + spin_lock(&rq->lock);
>>>>> + list_for_each_entry(s_entity, &rq->entities, list)
>>>>> + /*
>>>>> + * Prevents reinsertion and marks job_queue as idle,
>>>>> + * it will removed from rq in drm_sched_entity_fini
>>>>> + * eventually
>>>>> + */
>>>>> + s_entity->stopped = true;
>>>>> + spin_unlock(&rq->lock);
>>>>> +
>>>>> + }
>>>>> +
>>>>> + /* Wakeup everyone stuck in drm_sched_entity_flush for this
>>>>> scheduler */
>>>>> + wake_up_all(&sched->job_scheduled);
>>>>> +
>>>>> /* Confirm no work left behind accessing device structures */
>>>>> cancel_delayed_work_sync(&sched->work_tdr);
>>>>
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-05-05 13:57 UTC|newest]
Thread overview: 82+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-28 15:11 [PATCH v5 00/27] RFC Support hot device unplug in amdgpu Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 01/27] drm/ttm: Remap all page faults to per process dummy page Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 02/27] drm/ttm: Expose ttm_tt_unpopulate for driver use Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 03/27] drm/amdgpu: Split amdgpu_device_fini into early and late Andrey Grodzovsky
2021-04-29 7:04 ` Christian König
2021-04-30 3:10 ` Alex Deucher
2021-04-30 5:19 ` Lazar, Lijo
2021-04-30 5:39 ` Andrey Grodzovsky
2021-04-30 5:49 ` Lazar, Lijo
2021-04-28 15:11 ` [PATCH v5 04/27] drm/amdkfd: Split kfd suspend from devie exit Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 05/27] drm/amdgpu: Add early fini callback Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 06/27] drm/amdgpu: Handle IOMMU enabled case Andrey Grodzovsky
2021-04-29 7:08 ` Christian König
2021-05-03 20:43 ` Andrey Grodzovsky
2021-05-04 7:03 ` Christian König
2021-05-04 15:43 ` Andrey Grodzovsky
2021-05-05 14:51 ` Andrey Grodzovsky
2021-04-30 3:13 ` Alex Deucher
2021-05-03 18:00 ` Andrey Grodzovsky
2021-05-04 17:05 ` Felix Kuehling
2021-05-05 14:05 ` Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 07/27] drm/amdgpu: Remap all page faults to per process dummy page Andrey Grodzovsky
2021-04-29 7:09 ` Christian König
2021-04-28 15:11 ` [PATCH v5 08/27] PCI: add support for dev_groups to struct pci_device_driver Andrey Grodzovsky
2021-04-28 16:53 ` Bjorn Helgaas
2021-04-29 16:53 ` Andrey Grodzovsky
2021-04-29 19:23 ` Bjorn Helgaas
2021-04-29 20:36 ` Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 09/27] dmr/amdgpu: Move some sysfs attrs creation to default_attr Andrey Grodzovsky
2021-04-28 17:23 ` Bjorn Helgaas
2021-04-29 7:11 ` Christian König
2021-04-28 15:11 ` [PATCH v5 10/27] drm/amdgpu: Guard against write accesses after device removal Andrey Grodzovsky
2021-04-29 7:14 ` Christian König
2021-04-28 15:11 ` [PATCH v5 11/27] drm/sched: Make timeout timer rearm conditional Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 12/27] drm/amdgpu: Prevent any job recoveries after device is unplugged Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 13/27] drm/amdgpu: When filizing the fence driver. stop scheduler first Andrey Grodzovsky
2021-04-29 7:15 ` Christian König
2021-04-29 17:12 ` Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 14/27] drm/amdgpu: Fix hang on device removal Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 15/27] drm/scheduler: Fix hang when sched_entity released Andrey Grodzovsky
2021-04-29 7:18 ` Christian König
2021-04-29 17:06 ` Andrey Grodzovsky
2021-04-30 6:47 ` Christian König
2021-04-30 16:10 ` Andrey Grodzovsky
2021-05-05 13:57 ` Andrey Grodzovsky [this message]
2021-05-07 16:29 ` Daniel Vetter
2021-05-07 16:32 ` Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 16/27] drm/amdgpu: Unmap all MMIO mappings Andrey Grodzovsky
2021-04-29 7:19 ` Christian König
2021-04-29 16:55 ` Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 17/27] drm/amdgpu: Add rw_sem to pushing job into sched queue Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 18/27] drm/sched: Expose drm_sched_entity_kill_jobs Andrey Grodzovsky
2021-04-28 15:11 ` [PATCH v5 19/27] drm/amdgpu: Finilise device fences on device remove Andrey Grodzovsky
2021-04-28 15:20 ` Andrey Grodzovsky
2021-04-28 15:12 ` [PATCH v5 20/27] drm: Scope all DRM IOCTLs with drm_dev_enter/exit Andrey Grodzovsky
2021-04-29 11:23 ` Daniel Vetter
2021-04-29 11:32 ` Daniel Vetter
2021-04-29 16:04 ` Andrey Grodzovsky
2021-04-29 16:15 ` Felix Kuehling
2021-04-29 16:21 ` Andrey Grodzovsky
2021-04-29 16:29 ` Felix Kuehling
2021-04-29 16:33 ` Andrey Grodzovsky
2021-04-29 19:05 ` Daniel Vetter
2021-04-29 20:34 ` Andrey Grodzovsky
2021-04-30 10:25 ` Daniel Vetter
2021-04-30 17:27 ` Andrey Grodzovsky
2021-05-05 13:57 ` Andrey Grodzovsky
2021-05-06 9:40 ` Daniel Vetter
2021-05-06 16:25 ` Andrey Grodzovsky
2021-05-07 9:11 ` Daniel Vetter
2021-05-07 15:39 ` Andrey Grodzovsky
2021-05-07 16:24 ` Daniel Vetter
2021-05-07 18:00 ` Andrey Grodzovsky
2021-05-10 15:46 ` Daniel Vetter
2021-04-28 15:12 ` [PATCH v5 21/27] drm/amdgpu: Add support for hot-unplug feature at DRM level Andrey Grodzovsky
2021-04-28 15:12 ` [PATCH v5 22/27] drm/amd/display: Scope all DM queued work with drm_dev_enter/exit Andrey Grodzovsky
2021-04-28 15:12 ` [PATCH v5 23/27] drm/amd/powerplay: Scope all PM " Andrey Grodzovsky
2021-04-28 15:12 ` [PATCH v5 24/27] drm/amdkfd: Scope all KFD " Andrey Grodzovsky
2021-04-28 15:12 ` [PATCH v5 25/27] drm/amdgpu: Scope all amdgpu " Andrey Grodzovsky
2021-04-28 15:12 ` [PATCH v5 26/27] drm/amd/display: Remove superflous drm_mode_config_cleanup Andrey Grodzovsky
2021-04-28 15:12 ` [PATCH v5 27/27] drm/amdgpu: Verify DMA opearations from device are done Andrey Grodzovsky
2021-04-28 17:07 ` [PATCH v5 00/27] RFC Support hot device unplug in amdgpu Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aa42d956-7bcb-9c90-ba5f-12ab701548dd@amd.com \
--to=andrey.grodzovsky@amd.com \
--cc=Alexander.Deucher@amd.com \
--cc=Felix.Kuehling@amd.com \
--cc=Harry.Wentland@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=helgaas@kernel.org \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).