From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 2/2] drm/i915: properly prefault for pread/pwrite Date: Wed, 28 Sep 2011 12:24:04 +0100 Message-ID: References: <1317203844-2930-1-git-send-email-daniel.vetter@ffwll.ch> <1317203844-2930-2-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id C8A049EF3B for ; Wed, 28 Sep 2011 04:24:08 -0700 (PDT) In-Reply-To: <1317203844-2930-2-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: intel-gfx Cc: Daniel Vetter , stable@kernel.org List-Id: intel-gfx@lists.freedesktop.org On Wed, 28 Sep 2011 11:57:24 +0200, Daniel Vetter wrote: > The helper functions used are designed for pagecache io and splice, > i.e. they prefault at most PAGE_SIZE bytes spanning at most 2 pages. > > pread/pwrite want to write/read much more to avoid dropping the > struct_mutex lock in between. So write our helper function to prefault. > We're the only user of these pagemap.h helpers that want this behaviour, > so keep these new helpers private. > > Based on a patch by Chris Wilson. In addition to his approach this > alos tries to prefault the last page in case the user address range > crosses a page boundary at the end (and hence might sit on n+1 pages > for at most n*PAGE_SIZE of date). > > As a nice side-effect this rather reliably papers over the current > code's inability to handle non-struct page-backed user memory in the > slow paths. Because the real fix is grossly invasive, I think this > patch is the right thing for backporting. > > Cc: stable@kernel.org > Cc: Chris Wilson > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38115 > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_gem.c | 59 ++++++++++++++++++++++++++++++++++++-- > 1 files changed, 55 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 5f0f46e..42dc922 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -503,6 +503,32 @@ out: > return ret; > } > > +static inline int __prefault_writeable(char __user *uaddr, int size) > +{ > + int ret; > + char __user *end = uaddr + size - 1; > + > + if (unlikely(size == 0)) > + return 0; > + > + /* > + * Writing zeroes into userspace here is OK, because we know that if > + * the zero gets there, we'll be overwriting it. > + */ > + while (uaddr <= end) { > + ret = __put_user(0, uaddr); > + if (ret != 0) > + return ret; > + uaddr += PAGE_SIZE; > + } > + if (ret == 0) { > + if (((unsigned long)uaddr & PAGE_MASK) != > + ((unsigned long)end & PAGE_MASK)) This is a little ugly. Perhaps, #define page_align(addr) ((unsigned long)addr & PAGE_MASK) if (page_align(uaddr) != page_align(end)) Otherwise, both are Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre