<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 8, 2018 at 4:58 PM Huang Rui <<a href="mailto:ray.huang@amd.com">ray.huang@amd.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Aug 08, 2018 at 03:10:07PM +0800, Koenig, Christian wrote:<br>
> Yeah that is a known issue, but this solution is not correct either.<br>
> <br>
> See the scheduler where the job is execute on is simply not determined <br>
> yet when we want to trace it.<br>
> <br>
> So using the scheduler name from the entity is wrong as well.<br>
> <br>
> We should probably move the reschedule from drm_sched_entity_push_job() <br>
> to drm_sched_job_init() to fix that.<br>
<br>
Could you please explain why move reschedule along can fix the issue.<br>
Seemingly, only s_fence's sched is written to entity rq's sched, it can<br>
avoid the issue.<br>
<br>
sched_job->s_fence->sched = entity->rq->sched<br>
<br></blockquote><div>Because entity->rq->sched may not necessarily be the scheduler on which this job will get scheduled. And assigning a wrong scheduler could lead to wrong dependency optimizations. Hence it was assigned NULL initially we don't know scheduler it will be scheduled on to avoid any wrong optimizations.<br><br></div><div>Cheers,<br></div><div>Nayan<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks,<br>
Ray<br>
<br>
> <br>
> I will prepare a patch for that today,<br>
> Christian.<br>
> <br>
> Am 08.08.2018 um 09:05 schrieb Huang Rui:<br>
> > We won't initialize fence scheduler in drm_sched_fence_create() anymore, so it<br>
> > will refer null fence scheduler if open trace event to get the timeline name.<br>
> > Actually, it is the scheduler name from the entity, so add a macro to replace<br>
> > legacy getting timeline name by job.<br>
> ><br>
> > [  212.844281] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018<br>
> > [  212.852401] PGD 8000000427c13067 P4D 8000000427c13067 PUD 4235fc067 PMD 0<br>
> > [  212.859419] Oops: 0000 [#1] SMP PTI<br>
> > [  212.862981] CPU: 4 PID: 1520 Comm: amdgpu_test Tainted: G           OE     4.18.0-rc1-custom #1<br>
> > [  212.872194] Hardware name: Gigabyte Technology Co., Ltd. Z170XP-SLI/Z170XP-SLI-CF, BIOS F20 11/04/2016<br>
> > [  212.881704] RIP: 0010:drm_sched_fence_get_timeline_name+0x2b/0x30 [gpu_sched]<br>
> > [  212.888948] Code: 1f 44 00 00 48 8b 47 08 48 3d c0 b1 4f c0 74 13 48 83 ef 60 48 3d 60 b1 4f c0 b8 00 00 00 00 48 0f 45 f8 48 8b 87 e0 00 00 00 <48> 8b 40 18 c3 0f 1f 44 00 00 b8 01 00 00 00 c3 0f 1f 44 00 00 0f<br>
> > [  212.908162] RSP: 0018:ffffa3ed81f27af0 EFLAGS: 00010246<br>
> > [  212.913483] RAX: 0000000000000000 RBX: 0000000000070034 RCX: ffffa3ed81f27da8<br>
> > [  212.920735] RDX: ffff8f24ebfb5460 RSI: ffff8f24e40d3c00 RDI: ffff8f24ebfb5400<br>
> > [  212.928008] RBP: ffff8f24e40d3c00 R08: 0000000000000000 R09: ffffffffae4deafc<br>
> > [  212.935263] R10: ffffffffada000ed R11: 0000000000000001 R12: ffff8f24e891f898<br>
> > [  212.942558] R13: 0000000000000000 R14: ffff8f24ebc46000 R15: ffff8f24e3de97a8<br>
> > [  212.949796] FS:  00007ffff7fd2700(0000) GS:ffff8f24fed00000(0000) knlGS:0000000000000000<br>
> > [  212.958047] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033<br>
> > [  212.963921] CR2: 0000000000000018 CR3: 0000000423422003 CR4: 00000000003606e0<br>
> > [  212.971201] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000<br>
> > [  212.978482] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400<br>
> > [  212.985720] Call Trace:<br>
> > [  212.988236]  trace_event_raw_event_amdgpu_cs_ioctl+0x4c/0x170 [amdgpu]<br>
> > [  212.994904]  ? amdgpu_ctx_add_fence+0xa9/0x110 [amdgpu]<br>
> > [  213.000246]  ? amdgpu_job_free_resources+0x4b/0x70 [amdgpu]<br>
> > [  213.005944]  amdgpu_cs_ioctl+0x16d1/0x1b50 [amdgpu]<br>
> > [  213.010920]  ? amdgpu_cs_find_mapping+0xf0/0xf0 [amdgpu]<br>
> > [  213.016354]  drm_ioctl_kernel+0x8a/0xd0 [drm]<br>
> > [  213.020794]  ? recalc_sigpending+0x17/0x50<br>
> > [  213.024965]  drm_ioctl+0x2d7/0x390 [drm]<br>
> > [  213.028979]  ? amdgpu_cs_find_mapping+0xf0/0xf0 [amdgpu]<br>
> > [  213.034366]  ? do_signal+0x36/0x700<br>
> > [  213.037928]  ? signal_wake_up_state+0x15/0x30<br>
> > [  213.042375]  amdgpu_drm_ioctl+0x46/0x80 [amdgpu]<br>
> ><br>
> > Signed-off-by: Huang Rui <<a href="mailto:ray.huang@amd.com">ray.huang@amd.com</a>><br>
> > ---<br>
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c    |  2 +-<br>
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 10 ++++++----<br>
> >   2 files changed, 7 insertions(+), 5 deletions(-)<br>
> ><br>
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c<br>
> > index e12871d..be01e1b 100644<br>
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c<br>
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c<br>
> > @@ -1247,7 +1247,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,<br>
> >   <br>
> >     amdgpu_job_free_resources(job);<br>
> >   <br>
> > -   trace_amdgpu_cs_ioctl(job);<br>
> > +   trace_amdgpu_cs_ioctl(job, entity);<br>
> >     amdgpu_vm_bo_trace_cs(&fpriv->vm, &p->ticket);<br>
> >     priority = job->base.s_priority;<br>
> >     drm_sched_entity_push_job(&job->base, entity);<br>
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h<br>
> > index 8c2dab2..25cdcb7 100644<br>
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h<br>
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h<br>
> > @@ -36,6 +36,8 @@<br>
> >   <br>
> >   #define AMDGPU_JOB_GET_TIMELINE_NAME(job) \<br>
> >      job->base.s_fence->finished.ops->get_timeline_name(&job->base.s_fence->finished)<br>
> > +#define AMDGPU_GET_SCHED_NAME(entity) \<br>
> > +    (entity->rq->sched->name)<br>
> >   <br>
> >   TRACE_EVENT(amdgpu_mm_rreg,<br>
> >         TP_PROTO(unsigned did, uint32_t reg, uint32_t value),<br>
> > @@ -161,11 +163,11 @@ TRACE_EVENT(amdgpu_cs,<br>
> >   );<br>
> >   <br>
> >   TRACE_EVENT(amdgpu_cs_ioctl,<br>
> > -       TP_PROTO(struct amdgpu_job *job),<br>
> > -       TP_ARGS(job),<br>
> > +       TP_PROTO(struct amdgpu_job *job, struct drm_sched_entity *entity),<br>
> > +       TP_ARGS(job, entity),<br>
> >         TP_STRUCT__entry(<br>
> >                          __field(uint64_t, sched_job_id)<br>
> > -                        __string(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job))<br>
> > +                        __string(timeline, AMDGPU_GET_SCHED_NAME(entity))<br>
> >                          __field(unsigned int, context)<br>
> >                          __field(unsigned int, seqno)<br>
> >                          __field(struct dma_fence *, fence)<br>
> > @@ -175,7 +177,7 @@ TRACE_EVENT(amdgpu_cs_ioctl,<br>
> >   <br>
> >         TP_fast_assign(<br>
> >                        __entry->sched_job_id = job-><a href="http://base.id" rel="noreferrer">base.id</a>;<br>
> > -                      __assign_str(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job))<br>
> > +                      __assign_str(timeline, AMDGPU_GET_SCHED_NAME(entity))<br>
> >                        __entry->context = job->base.s_fence->finished.context;<br>
> >                        __entry->seqno = job->base.s_fence->finished.seqno;<br>
> >                        __entry->ring_name = to_amdgpu_ring(job->base.sched)->name;<br>
> <br>
</blockquote></div></div>