From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 0/5] Fixup pwrite/pread with gtt mmapped user addresses Date: Sat, 17 Sep 2011 16:07:30 -0700 Message-ID: <20110917160730.065043a7@bwidawsk.net> References: <1316285749-30130-1-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 cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id F0C219E80D for ; Sat, 17 Sep 2011 16:07:16 -0700 (PDT) In-Reply-To: <1316285749-30130-1-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: Daniel Vetter Cc: intel-gfx List-Id: intel-gfx@lists.freedesktop.org On Sat, 17 Sep 2011 20:55:44 +0200 Daniel Vetter wrote: > Hi all, > > Original bugreport: https://bugs.freedesktop.org/show_bug.cgi?id=38115 > > This bug can e.g. be hit with glBufferData(dst, glBufferMap(src)) > > Testcase is gem_mmap_gtt. > > Neglecting the trivial cleanup patch 2, patch 1 converts the possible > deadlock in an errornous -EFAULT. I've already submitted this, but > include it here again for completeness. Patches 3-5 then fix this up. > Imo there's a bit more work left to do (like properly prefaulting, > which would have papered over this bug). But I think this is the > minimal set required to fix the bug and I've wanted to get early > feedback on the approach and on possible race issues. > > I think patch 3-5 are not cc:stable material because they're too > invasive (maybe even stuff for -next). The prefaulting patch will > be much simpler, less invasive and it essentially reduces the issue to > a tiny race, so I suggest that for backporting. > > So review, comments highly appreciated. > > Yours, Daniel > > Daniel Vetter (5): > io-mapping: ensure io_mapping_map_atomic _is_ atomic > drm/i915: drop KM_USER0 argument to k(un)map_atomic > drm/i915: fall through pwrite_gtt_slow to the shmem slow path > drm/i915: rewrite shmem_pwrite_slow to use copy_from_user > drm/i915: rewrite shmem_pread_slow to use copy_to_user > > drivers/gpu/drm/i915/i915_gem.c | 324 > ++++++++++++++------------------- > drivers/gpu/drm/i915/i915_gem_debug.c | 6 +- > include/linux/io-mapping.h | 2 + 3 files changed, 140 > insertions(+), 192 deletions(-) > Still in process of review, but: Tested-by: Ben Widawsky