[Intel-gfx] [PATCH] drm/i915: Prefault before locking pages in shmem_pwrite

[Matthew Auld]

On Mon, 1 Apr 2019 at 14:39, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>
> If the user passes in a pointer to a GGTT mmaping of the same buffer
> being written to, we can hit a deadlock in acquiring the shmemfs page
> (once as the write destination and then as the read source).

And also shmem_fault, so cpu mmaping?

>
> [<0>] io_schedule+0xd/0x30
> [<0>] __lock_page+0x105/0x1b0
> [<0>] find_lock_entry+0x55/0x90
> [<0>] shmem_getpage_gfp+0xbb/0x800
> [<0>] shmem_read_mapping_page_gfp+0x2d/0x50
> [<0>] shmem_get_pages+0x158/0x5d0 [i915]
> [<0>] ____i915_gem_object_get_pages+0x17/0x90 [i915]
> [<0>] __i915_gem_object_get_pages+0x57/0x70 [i915]
> [<0>] i915_gem_fault+0x1b4/0x5c0 [i915]
> [<0>] __do_fault+0x2d/0x80
> [<0>] __handle_mm_fault+0xad4/0xfb0
> [<0>] handle_mm_fault+0xe6/0x1f0
> [<0>] __do_page_fault+0x18f/0x3f0
> [<0>] page_fault+0x1b/0x20
> [<0>] copy_user_enhanced_fast_string+0x7/0x10
> [<0>] _copy_from_user+0x37/0x60
> [<0>] shmem_pwrite+0xf0/0x160 [i915]
> [<0>] i915_gem_pwrite_ioctl+0x14e/0x520 [i915]
> [<0>] drm_ioctl_kernel+0x81/0xd0
> [<0>] drm_ioctl+0x1a7/0x310
> [<0>] do_vfs_ioctl+0x88/0x5d0
> [<0>] ksys_ioctl+0x35/0x70
> [<0>] __x64_sys_ioctl+0x11/0x20
> [<0>] do_syscall_64+0x39/0xe0
> [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> We can reduce (but not eliminate!) the chance of this happening by
> faulting the user_data before we take the page lock in
> pagecache_write_begin(). One way to eliminate the potential recursion
> here is by disabling pagefaults for the copy, and handling the fallback
> to use an alternative method -- so convert to use kmap_atomic (which
> should disable preemption and pagefaulting for the copy) and report
> ENODEV instead of EFAULT so that our caller tries again with a different
> copy mechanism -- we already check that the page should have been
> faultable so a false negative should be rare.
>
> Testcase: igt/gem_pwrite/self
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld at gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld at gmail.com>