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

[Daniel Vetter]

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. So I'm no longer
sure this is the right approach in general. Maybe we should instead wire
up the switch, but not sure whether that's indeed worth the effort.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch