On Fri, Jul 19, 2013 at 08:52:39AM +0000, Zhang, Xiong Y wrote: > It just influence one page. > During my test, I found one read slow path info when enable prefault. So > I add this patch. Why don't move fault_in_multipages_writeable() from > i915_gem_shmem_pread() to the beginning of i915_gem_pread_ioctl() ? So > the pread_ioctl() is like pwrite_ioctl(). Is the cost of > fault_in_multipages_writeable() lager than > fault_in_multipages_readable() ? See the attachment.
It's a slight matter of correctness fixed in: commit 96d79b52701758404cf8701986891afc99ce810b Author: Daniel Vetter <[email protected]> Date: Sun Mar 25 19:47:36 2012 +0200 drm/i915: don't clobber userspace memory before commiting to the pread The issue is that we can't prefault before we know that we'll do the write and prefaulting earlier will write garbage into userspace's memory. Hence the tricky ordering. I guess we should add a comment why this is so to the code. -Daniel > > thanks > -----Original Message----- > From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter > Sent: Friday, July 19, 2013 3:29 PM > To: Zhang, Xiong Y > Cc: [email protected]; Chris Wilson > Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: run shmem_pread_fast() after > page fault > > On Fri, Jul 19, 2013 at 01:51:26PM +0800, Xiong Zhang wrote: > > fault_in_multipages_writeable() will load fault page into physical > > memory, then pread can run fast path, no need to run slow path > > > > Signed-off-by: Xiong Zhang <[email protected]> > > Hm, this avoids going through the slowpath for the first page. Does this have > an impact on micro-benchmarks (we have a few) or is this just something > you've spotted? I'm a bit vary of making the already complicated shmem > pread/pwrite paths even more complicated. Chris? > -Daniel > > > --- > > drivers/gpu/drm/i915/i915_gem.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c index f194d7e..982df1e 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -459,6 +459,7 @@ i915_gem_shmem_pread(struct drm_device *dev, > > page_do_bit17_swizzling = obj_do_bit17_swizzling && > > (page_to_phys(page) & (1 << 17)) != 0; > > > > +read_again: > > ret = shmem_pread_fast(page, shmem_page_offset, page_length, > > user_data, page_do_bit17_swizzling, > > needs_clflush); > > @@ -475,6 +476,8 @@ i915_gem_shmem_pread(struct drm_device *dev, > > * and just continue. */ > > (void)ret; > > prefaulted = 1; > > + mutex_lock(&dev->struct_mutex); > > + goto read_again; > > } > > > > ret = shmem_pread_slow(page, shmem_page_offset, page_length, > > -- > > 1.8.3.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
