[Mesa-dev] [PATCH v3 8/8] anv: Do relocations in userspace before execbuf ioctl

[Jason Ekstrand]

On Thu, Nov 3, 2016 at 3:00 PM, Chris Wilson <chris at chris-wilson.co.uk>
wrote:

> On Thu, Nov 03, 2016 at 02:19:01PM -0700, Jason Ekstrand wrote:
> >    On Thu, Nov 3, 2016 at 1:53 AM, Chris Wilson <[1]
> chris at chris-wilson.co.uk>
> >    wrote:
> >
> >      Something to consider is just randomly assigning an address and
> using
> >      it. The kernel will relocate if it wants to, but in future the
> kernel
> >      will use your address as a hint and try to bind the object there (if
> >      that space is available). Like softpinning, but without failing with
> >      -EINVAL if the space is not available. Or you may of course do full
> >      client side address allocation under full-ppgtt (the hybrid scheme
> is
> >      still useful elsewhere). The advantage is avoiding the stall on
> first
> >      state use. The disadvantage is that until that patch lands, you
> walk the
> >      relocations for no gain (otoh, since the kernel will take its own
> sweet
> >      time doing the same, the inefficiency here will hopefully be
> >      negligible.)
> >
> >    Does that really work?  My reading of the kernel sources indicates
> that
> >    NO_RELOC means the kernel will just assume that they match what it
> gave
> >    you in the last execbuf2.  It never checks that the offsets match
> unless
> >    it had to move something in the GTT.  If, on the other hand, you mark
> the
> >    object as PINNED and the offsets don't match, it will get flagged in
> >    eb_vma_misplaced, remapped, and the relocations *will* trigger.
> >
> >
> >      /* The requirement for using NO_RELOC is:
> >       *
> >       *      The addresses written in the objects must match the
> >      corresponding
> >       *      reloc.presumed_offset which in turn must match the
> corresponding
> >       *      execobject.offset.
> >       *
> >       *      To avoid stalling, execobject.offset should match the
> current
> >       *      address of that object within the active context.
> >
> >    Actually, from my reading of the i915 sources, the kernel doesn't
> check
> >    this.  It's your responsibility to ensure that they match the
> addresses
> >    returned by the previous execbuf2 ioctl.
>
> As it stands today, using NO_RELOC without PINNED, the kernel will
> arbitrarily assign an address to each buffer. (The kernel doesn't move
> objects unless it has to, so that address is likely to match the last
> execbuf, and indeed guaranteed if it is still active.) If all the
> addresses the kernel choose match the addresses you pass in
> execobject[].offset, it will skip all the relocations. So it works best
> when passing back the offset from the previous execbuf.
>

Are you sure it actually checks execobject[].offset?  I haven't been able
to find that check anywhere in the code except for one check that's guarded
by PINNED.  Also, I've gotten hangs (presumably because the kernel didn't
relocate) when I passed NO_RELOC with offsets that didn't match the last
execbuf2.


> The loop in reservation is:
>
>         for each bound object:
>                 if address not suitable:
>                         unbind
>                 else
>                         pin
>         for each unbound object:
>                 bind
>                 pin
>
> where the pin does the check of kernel address against
> execobject.offset, and marking up if it need_relocs.
>
> The patch I've been trying to land is for execbuf to first try to pin
> the object at the address given in the NO_RELOC execobj.offset,
> because then it can skip all the work. (Under normal scenarios, the
> object is already at that address. But it helps fix up the issues with
> relocations being shared in userspace between different contexts, bad
> libdrm_intel, bad.) Obviously, we don't want to kick anything already
> at that address (it's a balance between doing a relocation now, or
> causing ping-pong).
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161103/24b5d3c1/attachment-0001.html>