[Intel-gfx] [PATCH] drm/i915: Document locking guidelines

[Dave Airlie]

Acked-by: Dave Airlie <airlied at redhat.com>

On Tue, 5 Nov 2019 at 21:06, Joonas Lahtinen
<joonas.lahtinen at linux.intel.com> wrote:
>
> Dave, ping for Acked-by here so we can merge? You already gave an
> early ack in IRC while travelling.
>
> Regards, Joonas
>
> Quoting Joonas Lahtinen (2019-08-30 13:50:53)
> > To ensure cross-driver locking compatibility, document the expected
> > guidelines for implementing the GEM locking in i915. Note that this
> > is a description of how things should end up after being reworked,
> > and does not reflect the current state of things.
> >
> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Dave Airlie <airlied at redhat.com>
> > Cc: Matthew Auld <matthew.auld at intel.com>
> > Cc: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> > Cc: CQ Tang <cq.tang at intel.com>
> > ---
> >  Documentation/gpu/i915.rst | 45 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> > index e249ea7b0ec7..63a72d10f2c7 100644
> > --- a/Documentation/gpu/i915.rst
> > +++ b/Documentation/gpu/i915.rst
> > @@ -320,6 +320,51 @@ for execution also include a list of all locations within buffers that
> >  refer to GPU-addresses so that the kernel can edit the buffer correctly.
> >  This process is dubbed relocation.
> >
> > +Locking Guidelines
> > +------------------
> > +
> > +**NOTE:** This is a description of how the locking should be after
> > +refactoring is done. Does not necessarily reflect what the locking
> > +looks like while WIP.
> > +
> > +#. All locking rules and interface contracts with cross-driver interfaces
> > +   (dma-buf, dma_fence) need to be followed.
> > +
> > +#. No struct_mutex anywhere in the code
> > +
> > +#. dma_resv will be the outermost lock (when needed) and ww_acquire_ctx
> > +   is to be hoisted at highest level and passed down within i915_gem_ctx
> > +   in the call chain
> > +
> > +#. While holding lru/memory manager (buddy, drm_mm, whatever) locks
> > +   system memory allocations are not allowed
> > +
> > +       * Enforce this by priming lockdep (with fs_reclaim). If we
> > +         allocate memory while holding these looks we get a rehash
> > +         of the shrinker vs. struct_mutex saga, and that would be
> > +         real bad.
> > +
> > +#. Do not nest different lru/memory manager locks within each other.
> > +   Take them in turn to update memory allocations, relying on the object’s
> > +   dma_resv ww_mutex to serialize against other operations.
> > +
> > +#. The suggestion for lru/memory managers locks is that they are small
> > +   enough to be spinlocks.
> > +
> > +#. All features need to come with exhaustive kernel selftests and/or
> > +   IGT tests when appropriate
> > +
> > +#. All LMEM uAPI paths need to be fully restartable (_interruptible()
> > +   for all locks/waits/sleeps)
> > +
> > +       * Error handling validation through signal injection.
> > +         Still the best strategy we have for validating GEM uAPI
> > +          corner cases.
> > +         Must be excessively used in the IGT, and we need to check
> > +         that we really have full path coverage of all error cases.
> > +
> > +       * -EDEADLK handling with ww_mutex
> > +
> >  GEM BO Management Implementation Details
> >  ----------------------------------------
> >
> > --
> > 2.20.1
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx