Jesse Barnes wrote: > On Wednesday, September 17, 2008 5:48 am Thomas Hellström wrote: > >> Jesse Barnes wrote: >> >>> Ok, here's an updated patch (note: only compile tested!) which I think >>> addresses all of the comments I received. I switched over to using >>> dev_mapping for unmapping and tried to cleanup the mm mmap & private >>> fields a bit. >>> >>> How does it look? >>> >>> Thanks, >>> Jesse >>> >> Oops, sorry for the previous spam, I hit "send" too soon. >> >> This looks better, only the >> unmap_mapping_range() <-> fault() >> race remains. Should be easily fixable with a mutex, though. >> > > I think airlied is almost ready to integrate this, so I took a look at the > locking requirements. I can't figure out what I'm protecting against though. > > It looks like in the vm_insert_pfn case we'll already have the mmap_sem > protecting us from other vma updates, but maybe unmap_mapping_range doesn't > honor that? Other callers of vm_insert_pfn don't seem to do anything special > (though I could only find one), and neither do callers of > unmap_mapping_range(). > > As long as the core isn't broken I think the race is harmless; we'll get an > extra page fault if vm_insert_pfn goes first, and avoid one if > unmap_mapping_range ends up first (again assuming the core does the right > thing here). > > Can either of you shed some light on this? > > Thanks, > Jesse > Jesse, The mmap_sem() is taken in read mode only, this means that there can be any number of threads simultaneously in fault() accessing the same buffer object, and I don't think unmap_mapping_range takes any driver-visible locks at all, except there used to be a sequence increment that made old nopage() back off if it raced with unmap_mapping_range.
So we have this sequence in fault: 1a) bind_to_gtt 1b) insert_pfn() And this sequence in mapping teardown: 2a) unmap_mapping_range() 2b) unbind_from_gtt(). So 1b) may well happen while unmap_mapping_range() is running, and even between 2a) and 2b) leaving you with ptes pointing to bogus data. You may even have the sequence 2a) 1a) 2b) 1b). /Thomas ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ -- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
