[PATCH] drm/vma-manager: Don't unmap COW'd pages when zapping bo ptes

[Daniel Vetter]

On Thu, Nov 21, 2013 at 09:29:26AM +0100, Thomas Hellstrom wrote:
> On 11/21/2013 09:18 AM, Daniel Vetter wrote:
> >On Wed, Nov 20, 2013 at 11:35:31PM +0100, Thomas Hellstrom wrote:
> >>On 11/20/2013 03:24 PM, Daniel Vetter wrote:
> >>>On Wed, Nov 20, 2013 at 01:55:49AM -0800, Thomas Hellstrom wrote:
> >>>>Not sure if there are any user-space users of private bo mappings, but
> >>>>if there are, or will be, zapping the COW'd pages when, for example,
> >>>>moving a bo would confuse the user immensely since the net effect for the
> >>>>user would be that pages written to would lose their contents.
> >>>>
> >>>>Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
> >>>Presuming I'm not horribly confused about that all the vm slang in the
> >>>kerneldoc means this changes is
> >>>
> >>>Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >>>
> >>>Now I still hold that userspace creating anynomous bo mappings is rather
> >>>crazy, but meh ;-) I guess the real question is whether we have anyone
> >>>relying on this out there (or planing to), in which case we either need to
> >>>funnel this through stable kernels or whack a drm feature flag onto
> >>>drivers/kernels with this fixed. But I seriously hope the answer is no.
> >>>
> >>>Cheers, Daniel
> >>>
> >>Thanks for reviewing. I don't think there's a need to take this
> >>through stable, since I don't know of anyone using private VMAs,
> >>
> >>Actually I'll need to hold off on this for a while since on some
> >>archs this may cause
> >>ptes of shared pages to not be zapped. If the arch doesn't have
> >>PTE_SPECIAL, shared pages on MIXEDMAP vmas will come through as
> >>vm_normal_page, and since page->mapping is usually (un)set to NULL
> >>by our drivers, this will result in false positives for COW'ed
> >>pages.
> >>
> >>So that test is buggy, or us not setting page->mapping to the
> >>mapping we use is buggy. It's too late in the day to decide which.
> >On second thought we also use this helper when zapping a bo when
> >deleting it or purging it's backing storage. And there I'd say it would be
> >consistent with normal semantics to also zap cow pages.
> 
> Agreed. That's consistent with truncation. Perhaps this was what
> David meant.
> Out of interest, why do you delete a bo or purge backing store
> before all vmas are gone?

I don't think we actually delete it before all the vmas are gone, but
purging can happen. Userspace (libdrm) aggressively caches both bos and
their memory mappings, but sets them to purgeable. Hence when the kernel
is low on memory and we preferentially remove those objects we can end up
purging backing storage for which userspace still has a cached mmaping.

Of course userspace first needs to adjust the madvise flag to willneed
before it can use that mmap region again. And if the kernel tells it that
the object is unfortunately gone userspace can only do an munmap and close
its gem handle.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch