On Wed, May 13, 2020 at 9:02 AM Christian König <ckoenig.leichtzumerken@xxxxxxxxx> wrote: > > Am 12.05.20 um 10:59 schrieb Daniel Vetter: > > This is a bit tricky, since ->notifier_lock is held while calling > > dma_fence_wait we must ensure that also the read side (i.e. > > dma_fence_begin_signalling) is on the same side. If we mix this up > > lockdep complaints, and that's again why we want to have these > > annotations. > > > > A nice side effect of this is that because of the fs_reclaim priming > > for dma_fence_enable lockdep now automatically checks for us that > > nothing in here allocates memory, without even running any userptr > > workloads. > > > > Cc: linux-media@xxxxxxxxxxxxxxx > > Cc: linaro-mm-sig@xxxxxxxxxxxxxxxx > > Cc: linux-rdma@xxxxxxxxxxxxxxx > > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > Cc: Christian König <christian.koenig@xxxxxxx> > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > index 7653f62b1b2d..6db3f3c629b0 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > @@ -1213,6 +1213,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, > > struct amdgpu_job *job; > > uint64_t seq; > > int r; > > + bool fence_cookie; > > > > job = p->job; > > p->job = NULL; > > @@ -1227,6 +1228,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, > > */ > > mutex_lock(&p->adev->notifier_lock); > > > > + fence_cookie = dma_fence_begin_signalling(); > > + > > /* If userptr are invalidated after amdgpu_cs_parser_bos(), return > > * -EAGAIN, drmIoctl in libdrm will restart the amdgpu_cs_ioctl. > > */ > > @@ -1264,12 +1267,14 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, > > amdgpu_vm_move_to_lru_tail(p->adev, &fpriv->vm); > > > > ttm_eu_fence_buffer_objects(&p->ticket, &p->validated, p->fence); > > + dma_fence_end_signalling(fence_cookie); > > Mhm, this could come earlier in theory. E.g. after pushing the job to > the scheduler. Yeah, I have not much clue about how amdgpu works :-) In practice it doesn't matter much, since the enclosing adev->notifier_lock is a lot more strict about what it allows than the dma_fence signalling fake lock. -Daniel > > Christian. > > > mutex_unlock(&p->adev->notifier_lock); > > > > return 0; > > > > error_abort: > > drm_sched_job_cleanup(&job->base); > > + dma_fence_end_signalling(fence_cookie); > > mutex_unlock(&p->adev->notifier_lock); > > > > error_unlock: > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
