| From b32061b9d46fa6aacdff6686524e5e7bfbba08ba Mon Sep 17 00:00:00 2001 |
| From: Chris Wilson <chris@chris-wilson.co.uk> |
| Date: Mon, 11 Nov 2019 13:32:03 +0000 |
| Subject: [PATCH] drm/i915/userptr: Try to acquire the page lock around |
| set_page_dirty() |
| |
| commit 2d691aeca4aecbb8d0414a777a46981a8e142b05 upstream. |
| |
| set_page_dirty says: |
| |
| For pages with a mapping this should be done under the page lock |
| for the benefit of asynchronous memory errors who prefer a |
| consistent dirty state. This rule can be broken in some special |
| cases, but should be better not to. |
| |
| Under those rules, it is only safe for us to use the plain set_page_dirty |
| calls for shmemfs/anonymous memory. Userptr may be used with real |
| mappings and so needs to use the locked version (set_page_dirty_lock). |
| |
| However, following a try_to_unmap() we may want to remove the userptr and |
| so call put_pages(). However, try_to_unmap() acquires the page lock and |
| so we must avoid recursively locking the pages ourselves -- which means |
| that we cannot safely acquire the lock around set_page_dirty(). Since we |
| can't be sure of the lock, we have to risk skip dirtying the page, or |
| else risk calling set_page_dirty() without a lock and so risk fs |
| corruption. |
| |
| Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203317 |
| Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112012 |
| Fixes: 5cc9ed4b9a7a ("drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl") |
| References: cb6d7c7dc7ff ("drm/i915/userptr: Acquire the page lock around set_page_dirty()") |
| References: 505a8ec7e11a ("Revert "drm/i915/userptr: Acquire the page lock around set_page_dirty()"") |
| References: 6dcc693bc57f ("ext4: warn when page is dirtied without buffers") |
| Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> |
| Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> |
| Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> |
| Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> |
| Cc: stable@vger.kernel.org |
| Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> |
| Link: https://patchwork.freedesktop.org/patch/msgid/20191111133205.11590-1-chris@chris-wilson.co.uk |
| (cherry picked from commit 0d4bbe3d407f79438dc4f87943db21f7134cfc65) |
| Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> |
| (cherry picked from commit cee7fb437edcdb2f9f8affa959e274997f5dca4d) |
| Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c |
| index 8079ea3af103..c91ae67a39c3 100644 |
| --- a/drivers/gpu/drm/i915/i915_gem_userptr.c |
| +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c |
| @@ -677,8 +677,28 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj, |
| i915_gem_gtt_finish_pages(obj, pages); |
| |
| for_each_sgt_page(page, sgt_iter, pages) { |
| - if (obj->mm.dirty) |
| + if (obj->mm.dirty && trylock_page(page)) { |
| + /* |
| + * As this may not be anonymous memory (e.g. shmem) |
| + * but exist on a real mapping, we have to lock |
| + * the page in order to dirty it -- holding |
| + * the page reference is not sufficient to |
| + * prevent the inode from being truncated. |
| + * Play safe and take the lock. |
| + * |
| + * However...! |
| + * |
| + * The mmu-notifier can be invalidated for a |
| + * migrate_page, that is alreadying holding the lock |
| + * on the page. Such a try_to_unmap() will result |
| + * in us calling put_pages() and so recursively try |
| + * to lock the page. We avoid that deadlock with |
| + * a trylock_page() and in exchange we risk missing |
| + * some page dirtying. |
| + */ |
| set_page_dirty(page); |
| + unlock_page(page); |
| + } |
| |
| mark_page_accessed(page); |
| put_page(page); |
| -- |
| 2.7.4 |
| |