[Intel-gfx] [PATCH 3/3] drm/i915: run shmem_pread_fast() after page fault

[Zhang, Xiong Y]

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.

thanks
-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
Sent: Friday, July 19, 2013 3:29 PM
To: Zhang, Xiong Y
Cc: intel-gfx at lists.freedesktop.org; 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 <xiong.y.zhang at intel.com>

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
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-i915-move-page-fault-handle-to-i915_gem_pread_io.patch
Type: application/octet-stream
Size: 1781 bytes
Desc: 0001-drm-i915-move-page-fault-handle-to-i915_gem_pread_io.patch
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20130719/f8ce98d7/attachment.obj>