[Intel-gfx] [PATCH AUTOSEL 5.2 13/23] drm/i915/userptr: Acquire the page lock around set_page_dirty()

2019-09-03 Thread Sasha Levin 
From: Chris Wilson 

[ Upstream commit aa56a292ce623734ddd30f52d73f527d1f3529b5 ]

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).

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203317
Fixes: 5cc9ed4b9a7a ("drm/i915: Introduce mapping of user pages into video 
memory (userptr) ioctl")
References: 6dcc693bc57f ("ext4: warn when page is dirtied without buffers")
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: sta...@vger.kernel.org
Reviewed-by: Tvrtko Ursulin 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190708140327.26825-1-ch...@chris-wilson.co.uk
(cherry picked from commit cb6d7c7dc7ff8cace666ddec66334117a6068ce2)
Signed-off-by: Jani Nikula 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/i915/i915_gem_userptr.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 8079ea3af1039..b1fc15c7f5997 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -678,7 +678,15 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
 
for_each_sgt_page(page, sgt_iter, pages) {
if (obj->mm.dirty)
-   set_page_dirty(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.
+*/
+   set_page_dirty_lock(page);
 
mark_page_accessed(page);
put_page(page);
-- 
2.20.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH AUTOSEL 5.2 13/23] drm/i915/userptr: Acquire the page lock around set_page_dirty()

2019-09-12 Thread Sasha Levin 

On Thu, Sep 12, 2019 at 11:51:33PM +0300, Thomas Backlund wrote:

Den 03-09-2019 kl. 19:24, skrev Sasha Levin:

From: Chris Wilson 

[ Upstream commit aa56a292ce623734ddd30f52d73f527d1f3529b5 ]

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).

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203317
Fixes: 5cc9ed4b9a7a ("drm/i915: Introduce mapping of user pages into video memory 
(userptr) ioctl")
References: 6dcc693bc57f ("ext4: warn when page is dirtied without buffers")
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: sta...@vger.kernel.org
Reviewed-by: Tvrtko Ursulin 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190708140327.26825-1-ch...@chris-wilson.co.uk
(cherry picked from commit cb6d7c7dc7ff8cace666ddec66334117a6068ce2)
Signed-off-by: Jani Nikula 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/i915/i915_gem_userptr.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 8079ea3af1039..b1fc15c7f5997 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -678,7 +678,15 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
for_each_sgt_page(page, sgt_iter, pages) {
if (obj->mm.dirty)
-   set_page_dirty(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.
+*/
+   set_page_dirty_lock(page);
mark_page_accessed(page);
put_page(page);




Please drop this one from all 5.2 and 4.19 stable queues

It has now been reverted in Linus tree:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=505a8ec7e11ae5236c4a154a1e24ef49a8349600


Now dropped, thank you.

--
Thanks,
Sasha
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH AUTOSEL 5.2 13/23] drm/i915/userptr: Acquire the page lock around set_page_dirty()

2019-09-16 Thread Thomas Backlund 

Den 03-09-2019 kl. 19:24, skrev Sasha Levin:

From: Chris Wilson 

[ Upstream commit aa56a292ce623734ddd30f52d73f527d1f3529b5 ]

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).

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203317
Fixes: 5cc9ed4b9a7a ("drm/i915: Introduce mapping of user pages into video memory 
(userptr) ioctl")
References: 6dcc693bc57f ("ext4: warn when page is dirtied without buffers")
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: sta...@vger.kernel.org
Reviewed-by: Tvrtko Ursulin 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190708140327.26825-1-ch...@chris-wilson.co.uk
(cherry picked from commit cb6d7c7dc7ff8cace666ddec66334117a6068ce2)
Signed-off-by: Jani Nikula 
Signed-off-by: Sasha Levin 
---
  drivers/gpu/drm/i915/i915_gem_userptr.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 8079ea3af1039..b1fc15c7f5997 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -678,7 +678,15 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
  
  	for_each_sgt_page(page, sgt_iter, pages) {

if (obj->mm.dirty)
-   set_page_dirty(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.
+*/
+   set_page_dirty_lock(page);
  
  		mark_page_accessed(page);

put_page(page);




Please drop this one from all 5.2 and 4.19 stable queues

It has now been reverted in Linus tree:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=505a8ec7e11ae5236c4a154a1e24ef49a8349600

--
Thomas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx