<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body bgcolor="white" lang="ZH-CN" link="blue" vlink="purple"> <div class="WordSection1"> Reviewed-by: Monk Liu <a href="mailto:monk.liu@amd.com">monk.liu@amd.com</a><o:p></o:p> <o:p> </o:p> I’m thinking of directly call “ttm_eu_fence_buffer_object” before ctx_ad_fence(), and right after “fence_get(&job->base.s_fence->finished);”<o:p></o:p> Since that time we already have parser->ticket/validated/fence, <o:p></o:p> <o:p> </o:p> BR Monk<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: Christian König [mailto:deathsimple@vodafone.de] Sent: 2017年9月6日 17:33 To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org; Zhou, David(ChunMing) <David1.Zhou@amd.com> Subject: Re: [PATCH] drm/amdgpu: revert "fix deadlock of reservation between cs and gpu reset v2"<o:p></o:p> </div> </div> <o:p> </o:p> <div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> What’s you plan ? <o:p></o:p> </blockquote> Not 100% sure yet. I need to move the fencing around to fix userptrs anyway. When I'm done with that and when the UVD/VCE stuff is fixed then I'm going to tackle this next. Regards, Christian. Am 06.09.2017 um 11:25 schrieb Liu, Monk:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> Yeah, you are right, although it has 32 slots (compared with entit_push_job which only waits for two slots in gpu scheduler) but still have chance to wait and meanwhile one job could under processing by gpu reset<o:p></o:p> <o:p></o:p> What’s you plan ? <o:p></o:p> <o:p></o:p> Revert this patch is correct since it have potential dirty reference, but how we need another patch to walk around this PD reservation dead lock <o:p></o:p> <o:p></o:p> BR Monk<o:p></o:p> <o:p></o:p> <o:p></o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: Christian König [<a href="mailto:deathsimple@vodafone.de">mailto:deathsimple@vodafone.de</a>] Sent: 2017年9月6日 16:20 To: Liu, Monk <a href="mailto:Monk.Liu@amd.com"><Monk.Liu@amd.com></a>; <a href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a>; Zhou, David(ChunMing) <a href="mailto:David1.Zhou@amd.com"> <David1.Zhou@amd.com></a> Subject: Re: [PATCH] drm/amdgpu: revert "fix deadlock of reservation between cs and gpu reset v2"<o:p></o:p> </div> </div> <o:p></o:p> <div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> but how to understand 1) what do you mean "The CS can still be blocked because of amdgpu_ctx_add_fence()."<o:p></o:p> </blockquote> See amdgpu_ctx_add_fence(), it can block for previous command submissions just like entity_push_job(). So only moving entity_push_job() out of locking the PD doesn't help at all. <o:p></o:p> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> for 2)The order of submission isn't correct any more. I think since the pointer "job" is already a dirty pointer, meaningless that we talking about it if the order is correct ...<o:p></o:p> </blockquote> The problem isn't parser->job, but rather that the job is referencing the entity which is part of the context and we already called amdgpu_ctx_put() on that one. Regards, Christian. Am 06.09.2017 um 10:04 schrieb Liu, Monk:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <div> <div id="x_divtagdefaultwrapper"> >The patch doesn't work at all: 1. The CS can still be blocked because of amdgpu_ctx_add_fence(). 2. The order of submission isn't correct any more. 3. We could end up using freed up memory because we now drop the ctx reference to early.<o:p></o:p> <o:p></o:p> I suddenly found that the parser->job is really a wild pointer:<o:p></o:p> <div> <o:p></o:p> <div> amdgpu_cs_parser_fini(p,<o:p></o:p> 0, true);<o:p></o:p> </div> <o:p></o:p> <o:p></o:p> <o:p></o:p> <div> trace_amdgpu_cs_ioctl(job);<o:p></o:p> </div> <o:p></o:p> <div> amd_sched_entity_push_job(&job->base);<o:p></o:p> </div> <o:p></o:p> </div> so "cs_parser_fini" cannot be called before "entity_push_job", that part is correct<o:p></o:p> <o:p></o:p> but how to understand 1)<o:p></o:p> what do you mean "The CS can still be blocked because of amdgpu_ctx_add_fence()."<o:p></o:p> <o:p></o:p> for 2)The order of submission isn't correct any more.<o:p></o:p> I think since the pointer "job" is already a dirty pointer, meaningless that we talking about it if the order is correct ... <o:p></o:p> <o:p></o:p> BR Monk<o:p></o:p> <o:p></o:p> <o:p></o:p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="3" width="98%" align="center"> </div> <div id="x_divRplyFwdMsg"> From: amd-gfx <a href="mailto:amd-gfx-bounces@lists.freedesktop.org"><amd-gfx-bounces@lists.freedesktop.org></a> on behalf of Christian König <a href="mailto:deathsimple@vodafone.de"><deathsimple@vodafone.de></a> Sent: Tuesday, September 5, 2017 9:14:23 PM To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; Zhou, David(ChunMing) Subject: [PATCH] drm/amdgpu: revert "fix deadlock of reservation between cs and gpu reset v2" <o:p></o:p> <div> <o:p></o:p> </div> </div> </div> <div> From: Christian König <a href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> This reverts commit 10e709cb296c98424c03408d23e3addeddcd4088. The patch doesn't work at all: 1. The CS can still be blocked because of amdgpu_ctx_add_fence(). 2. The order of submission isn't correct any more. 3. We could end up using freed up memory because we now drop the ctx reference to early. This needs to be fixed cleanly by doing the context handling after the BO handling, but this is a larger task just avoid the obvious crashes for now. Signed-off-by: Christian König <a href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index b96776c..2db4010 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1061,7 +1061,6 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence); job->uf_sequence = cs->out.handle; amdgpu_job_free_resources(job); - amdgpu_cs_parser_fini(p, 0, true); trace_amdgpu_cs_ioctl(job); amd_sched_entity_push_job(&job->base); @@ -1120,10 +1119,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) goto out; r = amdgpu_cs_submit(&parser, cs); - if (r) - goto out; - return 0; out: amdgpu_cs_parser_fini(&parser, r, reserved_buffers); return r; -- 2.7.4 _______________________________________________ amd-gfx mailing list <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><o:p></o:p> </div> <o:p></o:p> <pre>_______________________________________________<o:p></o:p></pre> <pre>amd-gfx mailing list<o:p></o:p></pre> <pre><a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><o:p></o:p></pre> <pre><a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><o:p></o:p></pre> </blockquote> <o:p></o:p> <o:p></o:p> <pre>_______________________________________________<o:p></o:p></pre> <pre>amd-gfx mailing list<o:p></o:p></pre> <pre><a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><o:p></o:p></pre> <pre><a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><o:p></o:p></pre> </blockquote> <o:p> </o:p> </div> </body> </html>