From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH] drm/vma-manager: Don't unmap COW'd pages when zapping bo ptes Date: Thu, 21 Nov 2013 09:29:26 +0100 Message-ID: <528DC466.2060507@vmware.com> References: <1384941349-3550-1-git-send-email-thellstrom@vmware.com> <20131120142445.GB27344@phenom.ffwll.local> <528D3933.2090906@vmware.com> <20131121081821.GI27344@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-outbound-1.vmware.com (smtp-outbound-1.vmware.com [208.91.2.12]) by gabe.freedesktop.org (Postfix) with ESMTP id 5D48FFD3AE for ; Thu, 21 Nov 2013 00:29:29 -0800 (PST) In-Reply-To: <20131121081821.GI27344@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Daniel Vetter Cc: linux-graphics-maintainer@vmware.com, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 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 >>> Presuming I'm not horribly confused about that all the vm slang in the >>> kerneldoc means this changes is >>> >>> Reviewed-by: Daniel Vetter >>> >>> 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? > 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. I'll leave it as is. If I happen to sort out the non-cow zapping, I'll post a new patch. /Thomas > -Daniel