Re: [Intel-gfx] [PATCH] drm/i915: Mark contents as dirty on a write fault

2019-09-24 Thread Matthew Auld
On Fri, 20 Sep 2019 at 13:18, Chris Wilson  wrote:
>
> Since dropping the set-to-gtt-domain in commit a679f58d0510 ("drm/i915:
> Flush pages on acquisition"), we no longer mark the contents as dirty on
> a write fault. This has the issue of us then not marking the pages as
> dirty on releasing the buffer, which means the contents are not written
> out to the swap device (should we ever pick that buffer as a victim).
> Notably, this is visible in the dumb buffer interface used for cursors.
> Having updated the cursor contents via mmap, and swapped away, if the
> shrinker should evict the old cursor, upon next reuse, the cursor would
> be invisible.
>
> E.g. echo 80 > /proc/sys/kernel/sysrq ; echo f > /proc/sysrq-trigger
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111541
> Fixes: a679f58d0510 ("drm/i915: Flush pages on acquisition")
> Signed-off-by: Chris Wilson 
> Cc: Matthew Auld 
> Cc: Ville Syrjälä 
> Cc:  # v5.2+
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Mark contents as dirty on a write fault

2019-09-20 Thread Chris Wilson
Quoting Chris Wilson (2019-09-20 13:22:13)
> Quoting Chris Wilson (2019-09-20 13:18:21)
> > Since dropping the set-to-gtt-domain in commit a679f58d0510 ("drm/i915:
> > Flush pages on acquisition"), we no longer mark the contents as dirty on
> > a write fault. This has the issue of us then not marking the pages as
> > dirty on releasing the buffer, which means the contents are not written
> > out to the swap device (should we ever pick that buffer as a victim).
> > Notably, this is visible in the dumb buffer interface used for cursors.
> > Having updated the cursor contents via mmap, and swapped away, if the
> > shrinker should evict the old cursor, upon next reuse, the cursor would
> > be invisible.
> 
> Hmm, I think the dumb interface may be missing a few steps around the
> place to ensure the contents are flushed.

No, it's fine. We do the flush in pinning pages, the only thing that was
dropped was then marking the content as dirty.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Mark contents as dirty on a write fault

2019-09-20 Thread Chris Wilson
Quoting Chris Wilson (2019-09-20 13:18:21)
> Since dropping the set-to-gtt-domain in commit a679f58d0510 ("drm/i915:
> Flush pages on acquisition"), we no longer mark the contents as dirty on
> a write fault. This has the issue of us then not marking the pages as
> dirty on releasing the buffer, which means the contents are not written
> out to the swap device (should we ever pick that buffer as a victim).
> Notably, this is visible in the dumb buffer interface used for cursors.
> Having updated the cursor contents via mmap, and swapped away, if the
> shrinker should evict the old cursor, upon next reuse, the cursor would
> be invisible.

Hmm, I think the dumb interface may be missing a few steps around the
place to ensure the contents are flushed.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Mark contents as dirty on a write fault

2019-09-20 Thread Chris Wilson
Since dropping the set-to-gtt-domain in commit a679f58d0510 ("drm/i915:
Flush pages on acquisition"), we no longer mark the contents as dirty on
a write fault. This has the issue of us then not marking the pages as
dirty on releasing the buffer, which means the contents are not written
out to the swap device (should we ever pick that buffer as a victim).
Notably, this is visible in the dumb buffer interface used for cursors.
Having updated the cursor contents via mmap, and swapped away, if the
shrinker should evict the old cursor, upon next reuse, the cursor would
be invisible.

E.g. echo 80 > /proc/sys/kernel/sysrq ; echo f > /proc/sysrq-trigger

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111541
Fixes: a679f58d0510 ("drm/i915: Flush pages on acquisition")
Signed-off-by: Chris Wilson 
Cc: Matthew Auld 
Cc: Ville Syrjälä 
Cc:  # v5.2+
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c 
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 1748e63156a2..860b751c51f1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -319,7 +319,11 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
intel_wakeref_auto(>ggtt.userfault_wakeref,
   
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
 
-   i915_vma_set_ggtt_write(vma);
+   if (write) {
+   GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
+   i915_vma_set_ggtt_write(vma);
+   obj->mm.dirty = true;
+   }
 
 err_fence:
i915_vma_unpin_fence(vma);
-- 
2.23.0

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