<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <meta name="Generator" content="Microsoft Exchange Server"> <style></style> </head> <body> <meta content="text/html; charset=UTF-8"> <style type="text/css" style="">  </style> <div dir="ltr"> <div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Helvetica,sans-serif"> >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. I suddenly found that the parser->job is really a wild pointer: <div style="color:#d4d4d4; background-color:#1e1e1e; font-family:'Droid Sans Mono','Courier New',monospace,'Droid Sans Fallback'; font-weight:normal; font-size:14px; line-height:19px; white-space:pre"> <div> amdgpu_cs_parser_fini(p, 0, true);</div> <div> trace_amdgpu_cs_ioctl(job);</div> <div> amd_sched_entity_push_job(&job->base);</div> </div> so "cs_parser_fini" cannot be called before "entity_push_job", that part is correct but how to understand 1) what do you mean "The CS can still be blocked because of amdgpu_ctx_add_fence()." 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 ... BR Monk </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Christian König <deathsimple@vodafone.de> Sent: Tuesday, September 5, 2017 9:14:23 PM To: amd-gfx@lists.freedesktop.org; Zhou, David(ChunMing) Subject: [PATCH] drm/amdgpu: revert "fix deadlock of reservation between cs and gpu reset v2" <div> </div> </div> </div> <div class="PlainText">From: Christian König <christian.koenig@amd.com> 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 <christian.koenig@amd.com> --- 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 amd-gfx@lists.freedesktop.org <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </div> </body> </html>