Am 24.11.21 um 09:31 schrieb Pekka Paalanen:
On Tue, 23 Nov 2021 15:20:45 +0100
"Christian König" <ckoenig.leichtzumerken@xxxxxxxxx> wrote:
Hi guys,
as discussed before this set of patches completely rework the dma_resv semantic
and spreads the new handling over all the existing drivers and users.
First of all this drops the DAG approach because it requires that every single
driver implements those relatively complicated rules correctly and any
violation of that immediately leads to either corruption of freed memory or
even more severe security problems.
Instead we just keep all fences around all the time until they are signaled.
Only fences with the same context are assumed to be signaled in the correct
order since this is exercised elsewhere as well. Replacing fences is now only
supported for hardware mechanism like VM page table updates where the hardware
can guarantee that the resource can't be accessed any more.
Then the concept of a single exclusive fence and multiple shared fences is
dropped as well.
Instead the dma_resv object is now just a container for dma_fence objects where
each fence has associated usage flags. Those use flags describe how the
operation represented by the dma_fence object is using the resource protected
by the dma_resv object. This allows us to add multiple fences for each usage
type.
Additionally to the existing WRITE/READ usages this patch set also adds the new
KERNEL and OTHER usages. The KERNEL usages is used in cases where the kernel
needs to do some operation with the resource protected by the dma_resv object,
like copies or clears. Those are mandatory to wait for when dynamic memory
management is used.
The OTHER usage is for cases where we don't want that the operation represented
by the dma_fence object participate in any implicit sync but needs to be
respected by the kernel memory management. Examples for those are VM page table
updates and preemption fences.
Hi,
reading just the cover letter I wonder if KERNEL and OTHER could have
better names based on what you describe how they are used. WRITE and
READ immediately give an idea of semantics, KERNEL and OTHER not so
much.
Some suggestions coming to my mind:
KERNEL -> PREPARE or INITIALIZE or SANITIZE
OTHER -> BOOKKEEP
Yes, I was entertaining similar thoughts for quite a while as well.
I think KERNEL fits better than the suggestions because that is really
something only the kernel should use and we should not encourage anybody
to use that for userspace submissions.
But using BOOKKEEP instead of OTHER sounds like a really good idea to
me. Going to keep that in mind and if nobody has any better idea making
the change for the next revision.
Thanks,
Christian.
Thanks,
pq
While doing this the new implementation cleans up existing workarounds all over
the place, but especially amdgpu and TTM. Surprisingly I also found two use
cases for the KERNEL/OTHER usage in i915 and Nouveau, those might need more
thoughts.
In general the existing functionality should been preserved, the only downside
is that we now always need to reserve a slot before adding a fence. The newly
added call to the reservation function can probably use some more cleanup.
TODOs: Testing, testing, testing, doublechecking the newly added
kerneldoc for any typos.
Please review and/or comment,
Christian.
