Skip to content

Commit 2d691ae

Browse files
icklerodrigovivi
authored andcommitted
drm/i915/userptr: Try to acquire the page lock around set_page_dirty()
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: 5cc9ed4 ("drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl") References: cb6d7c7 ("drm/i915/userptr: Acquire the page lock around set_page_dirty()") References: 505a8ec ("Revert "drm/i915/userptr: Acquire the page lock around set_page_dirty()"") References: 6dcc693 ("ext4: warn when page is dirtied without buffers") Signed-off-by: Chris Wilson <[email protected]> Cc: Lionel Landwerlin <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: Joonas Lahtinen <[email protected]> Cc: [email protected] Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 0d4bbe3) Signed-off-by: Joonas Lahtinen <[email protected]> (cherry picked from commit cee7fb4) Signed-off-by: Rodrigo Vivi <[email protected]>
1 parent add3eee commit 2d691ae

File tree

1 file changed

+21
-1
lines changed

1 file changed

+21
-1
lines changed

drivers/gpu/drm/i915/gem/i915_gem_userptr.c

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,8 +671,28 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
671671
obj->mm.dirty = false;
672672

673673
for_each_sgt_page(page, sgt_iter, pages) {
674-
if (obj->mm.dirty)
674+
if (obj->mm.dirty && trylock_page(page)) {
675+
/*
676+
* As this may not be anonymous memory (e.g. shmem)
677+
* but exist on a real mapping, we have to lock
678+
* the page in order to dirty it -- holding
679+
* the page reference is not sufficient to
680+
* prevent the inode from being truncated.
681+
* Play safe and take the lock.
682+
*
683+
* However...!
684+
*
685+
* The mmu-notifier can be invalidated for a
686+
* migrate_page, that is alreadying holding the lock
687+
* on the page. Such a try_to_unmap() will result
688+
* in us calling put_pages() and so recursively try
689+
* to lock the page. We avoid that deadlock with
690+
* a trylock_page() and in exchange we risk missing
691+
* some page dirtying.
692+
*/
675693
set_page_dirty(page);
694+
unlock_page(page);
695+
}
676696

677697
mark_page_accessed(page);
678698
put_page(page);

0 commit comments

Comments
 (0)