Message-ID: <CAKMK7uE-kWA==Cko5uenMrcnopEjq42HxoDTDywzBAbHqsN13g@mail.gmail.com>
Date:   Fri, 19 Jun 2020 17:06:04 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Jason Gunthorpe <jgg@...pe.ca>
Cc:     Thomas Hellström (Intel) 
        <thomas_os@...pmail.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        linux-rdma <linux-rdma@...r.kernel.org>,
        Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        amd-gfx list <amd-gfx@...ts.freedesktop.org>,
        "moderated list:DMA BUFFER SHARING FRAMEWORK" 
        <linaro-mm-sig@...ts.linaro.org>,
        Thomas Hellstrom <thomas.hellstrom@...el.com>,
        Daniel Vetter <daniel.vetter@...el.com>,
        "open list:DMA BUFFER SHARING FRAMEWORK" 
        <linux-media@...r.kernel.org>,
        Christian König <christian.koenig@....com>,
        Mika Kuoppala <mika.kuoppala@...el.com>
Subject: Re: [Linaro-mm-sig] [PATCH 04/18] dma-fence: prime lockdep annotations

On Fri, Jun 19, 2020 at 1:39 PM Jason Gunthorpe <jgg@...pe.ca> wrote:
>
> On Fri, Jun 19, 2020 at 09:22:09AM +0200, Daniel Vetter wrote:
> > > As I've understood GPU that means you need to show that the commands
> > > associated with the buffer have completed. This is all local stuff
> > > within the driver, right? Why use fence (other than it already exists)
> >
> > Because that's the end-of-dma thing. And it's cross-driver for the
> > above reasons, e.g.
> > - device A renders some stuff. Userspace gets dma_fence A out of that
> > (well sync_file or one of the other uapi interfaces, but you get the
> > idea)
> > - userspace (across process or just different driver) issues more
> > rendering for device B, which depends upon the rendering done on
> > device A. So dma_fence A is an dependency and will block this dma
> > operation. Userspace (and the kernel) gets dma_fence B out of this
> > - because unfortunate reasons, the same rendering on device B also
> > needs a userptr buffer, which means that dma_fence B is also the one
> > that the mmu_range_notifier needs to wait on before it can tell core
> > mm that it can go ahead and release those pages
>
> I was afraid you'd say this - this is complete madness for other DMA
> devices to borrow the notifier hook of the first device!

The first device might not even have a notifier. This is the 2nd
device, waiting on a dma_fence of its own, but which happens to be
queued up as a dma operation behind something else.

> What if the first device is a page faulting device and doesn't call
> dma_fence??

Not sure what you mean with this ... even if it does page-faulting for
some other reasons, it'll emit a dma_fence which the 2nd device can
consume as a dependency.

> It you are going to treat things this way then the mmu notifier really
> needs to be part of the some core DMA buf, and not randomly sprinkled
> in drivers

So maybe again unclear, we don't allow such userptr dma-buf to even be
shared. They're just for slurping in stuff in the local device
(general from file io or something the cpu has done or similar). There
have been attempts to use it as the general backing storage, but that
didn't go down too well because way too many complications.

Generally most memory the gpu operates on isn't stuff that's
mmu_notifier'ed. And also, the device with userptr support only waits
for its own dma_fence (because well you can't share this stuff, we
disallow that).

The problem is that there's piles of other dependencies for a dma job.
GPU doesn't just consume a single buffer each time, it consumes entire
lists of buffers and mixes them all up in funny ways. Some of these
buffers are userptr, entirely local to the device. Other buffers are
just normal device driver allocations (and managed with some shrinker
to keep them in check). And then there's the actually shared dma-buf
with other devices. The trouble is that they're all bundled up
together.

Now we probably should have some helper code for userptr so that all
drivers do this roughly the same, but that's just not there yet. But
it can't be a dma-buf exporter behind the dma-buf interfaces, because
even just pinned get_user_pages would be too different semantics
compared to normal shared dma-buf objects, that's all very tightly
tied into the specific driver.

> But really this is what page pinning is supposed to be used for, the
> MM behavior when it blocks on a pinned page is less invasive than if
> it stalls inside a mmu notifier.
>
> You can mix it, use mmu notififers to keep track if the buffer is
> still live, but when you want to trigger DMA then pin the pages and
> keep them pinned until DMA is done. The pin protects things (well,
> fork is still a problem)

Hm I thought amdgpu had that (or drm/radeon as the previous
incarnation of that stack), and was unhappy about the issues. Would
need Christian König to chime in.

> Do not need to wait on dma_fence in notifiers.

Maybe :-) The goal of this series is more to document current rules
and make them more consistent. Fixing them if we don't like them might
be a follow-up task, but that would likely be a pile more work. First
we need to know what the exact shape of the problem even is.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Powered by blists - more mailing lists