On Wed, Oct 05, 2016 at 02:40:09PM +0100, Chris Wilson wrote: > On Wed, Oct 05, 2016 at 03:11:00PM +0200, Daniel Vetter wrote: > > On Fri, Sep 30, 2016 at 02:59:59PM +0100, Chris Wilson wrote: > > > In order to keep the dmabuf alive whilst the mmap is, we need to hold a > > > reference to the dmabuf and not the backing object. This is important as > > > the dmabuf not only keeps the object alive, but also the device so that > > > > > > dmabuf = vgem_create_dmabuf(); > > > ptr = mmap(... dmabuf ...); > > > close(dmabuf); > > > > > > persists across module-unload as well as device closure. > > > > I don't see where we grab the ref to the dma-buf here instead of the > > backing storage. And doesn't the exact same issue happen when you use dumb > > mmap? Or maybe I'm just a bit confused about what's going on here ... > > The reference to the dmabuf is in vma->vm_file, which was used to keep > the module alive. So this might be overzealous, in that there shouldn't > be a way to get back from the shmemfs object to the module. However, > that does make me aware of a failure path we have in patches that do add > callbacks from shmemfs to the driver... > > At least using the ptr after closing the module is ok. So this patch is > not required. I think if we entirely redirect the mmap to shmem, and not forget to also update vm_file, the driver could get unloaded without ill effects. I'll leave this one out for now, picked up 1&2 from v4 to -misc meanwhile. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel