lkml.org 
[lkml]   [2009]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: Gem GTT mmaps..
Date
On Wednesday, February 4, 2009 3:42 pm Eric Anholt wrote:
> On Wed, 2009-02-04 at 15:02 -0800, Jesse Barnes wrote:
> > On Wednesday, February 4, 2009 2:32 pm Thomas Hellström wrote:
> > > Jesse,
> > >
> > > I have some concerns about the GEM GTT mmap functionality.
> >
> > Thanks for looking it over again; you would know since some of this code
> > came from you in the first place. :)
> >
> > > First, a gem object pointer is copied to map->offset and then to the
> > > vma->vm_private_data without proper reference counting. This pointer is
> > > used in i915_gem_fault() to access the gem object. However if the gem
> > > object is destroyed and a process then tries to access data in a vma
> > > mapping the (now destroyed) object, it would dereference a stale
> > > pointer into kernel space? Shouldn't those pointers be reference
> > > counted, and to account for fork(), a vm open and close would be needed
> > > to reference count corresponding pointers of newly created and
> > > destroyed vmas?
> >
> > Yeah looks like we don't protect against vm_private_data pointing at a
> > freed or other object. But rather than refcounting the pointers I wonder
> > if we could make the private data use the GEM object name instead, then
> > do the lookup in the fault handler?
>
> The object doesn't necessarily have a public name. You do need to
> refcount the objects.

So if we leave the lookup reference around from the GTT mapping ioctl, that
would take care of new mappings. And if we added/removed references at VM
open/close time, we should be covered for fork. But is it ok to add a new
unref in the finish ioctl for GTT mapped objects? I don't think so, because
we don't know for sure if the caller was the one that created the new fake
offset (which would be one way of detecting whether it was GTT mapped).
Seems like we need a new unmap ioctl? Or we could put the mapping ref/unref
in libdrm, where it would be tracked on a per-process basis...

--
Jesse Barnes, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-02-05 19:39    [W:0.559 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site