Re: [Linaro-mm-sig] [PATCH 04/18] dma-fence: prime lockdep annotations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/4/20 10:12 AM, Daniel Vetter wrote:
Two in one go:
- it is allowed to call dma_fence_wait() while holding a
   dma_resv_lock(). This is fundamental to how eviction works with ttm,
   so required.

- it is allowed to call dma_fence_wait() from memory reclaim contexts,
   specifically from shrinker callbacks (which i915 does), and from mmu
   notifier callbacks (which amdgpu does, and which i915 sometimes also
   does, and probably always should, but that's kinda a debate). Also
   for stuff like HMM we really need to be able to do this, or things
   get real dicey.

Consequence is that any critical path necessary to get to a
dma_fence_signal for a fence must never a) call dma_resv_lock nor b)
allocate memory with GFP_KERNEL. Also by implication of
dma_resv_lock(), no userspace faulting allowed. That's some supremely
obnoxious limitations, which is why we need to sprinkle the right
annotations to all relevant paths.

The one big locking context we're leaving out here is mmu notifiers,
added in

commit 23b68395c7c78a764e8963fc15a7cfd318bf187f
Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
Date:   Mon Aug 26 22:14:21 2019 +0200

     mm/mmu_notifiers: add a lockdep map for invalidate_range_start/end

that one covers a lot of other callsites, and it's also allowed to
wait on dma-fences from mmu notifiers. But there's no ready-made
functions exposed to prime this, so I've left it out for now.

v2: Also track against mmu notifier context.

v3: kerneldoc to spec the cross-driver contract. Note that currently
i915 throws in a hard-coded 10s timeout on foreign fences (not sure
why that was done, but it's there), which is why that rule is worded
with SHOULD instead of MUST.

Also some of the mmu_notifier/shrinker rules might surprise SoC
drivers, I haven't fully audited them all. Which is infeasible anyway,
we'll need to run them with lockdep and dma-fence annotations and see
what goes boom.

v4: A spelling fix from Mika

Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Cc: Thomas Hellstrom <thomas.hellstrom@xxxxxxxxx>
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>
---
  Documentation/driver-api/dma-buf.rst |  6 ++++
  drivers/dma-buf/dma-fence.c          | 41 ++++++++++++++++++++++++++++
  drivers/dma-buf/dma-resv.c           |  4 +++
  include/linux/dma-fence.h            |  1 +
  4 files changed, 52 insertions(+)

I still have my doubts about allowing fence waiting from within shrinkers. IMO ideally they should use a trywait approach, in order to allow memory allocation during command submission for drivers that publish fences before command submission. (Since early reservation object release requires that).

But since drivers are already waiting from within shrinkers and I take your word for HMM requiring this,

Reviewed-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxx>


_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux