drm/i915: Track pages pinned due to swizzling quirk

If we have a tiled object and an unknown CPU swizzle pattern, we pin the
pages to prevent the object from being swapped out (and us corrupting
the contents as we do not know the access pattern and so cannot convert
it to linear and back to tiled on reuse). This requires us to remember
to drop the extra pinning when freeing the object, or else we trigger
warnings about the pin leak. In commit fbbd37b36fa5 ("drm/i915: Move
object release to a freelist + worker"), the object free path was
deferred to a worker, but the unpinning of the quirk, along with marking
the object as reclaimable, was left on the immediate path (so that if
required we could reclaim the pages under memory pressure as early as
possible). However, this split introduced a bug where the pages were no
longer being unpinned if they were marked as unneeded.

[  231.800401] WARNING: CPU: 1 PID: 90 at drivers/gpu/drm/i915/i915_gem.c:4275 __i915_gem_free_objects+0x326/0x3c0 [i915]
[  231.800403] WARN_ON(i915_gem_object_has_pinned_pages(obj))
[  231.800405] Modules linked in:
[  231.800406]  snd_hda_intel i915 snd_hda_codec_generic mei_me snd_hda_codec coretemp snd_hwdep mei lpc_ich snd_hda_core snd_pcm e1000e ptp pps_core [last unloaded: i915]
[  231.800426] CPU: 1 PID: 90 Comm: kworker/1:4 Tainted: G     U          4.9.0-rc2-CI-CI_DRM_1780+ #1
[  231.800428] Hardware name: LENOVO 7465CTO/7465CTO, BIOS 6DET44WW (2.08 ) 04/22/2009
[  231.800456] Workqueue: events __i915_gem_free_work [i915]
[  231.800459]  ffffc9000034fc80 ffffffff8142dd65 ffffc9000034fcd0 0000000000000000
[  231.800465]  ffffc9000034fcc0 ffffffff8107e4e6 000010b300000001 0000000000001000
[  231.800469]  ffff88011d3db740 ffff880130ef0000 0000000000000000 ffff880130ef5ea0
[  231.800474] Call Trace:
[  231.800479]  [<ffffffff8142dd65>] dump_stack+0x67/0x92
[  231.800484]  [<ffffffff8107e4e6>] __warn+0xc6/0xe0
[  231.800487]  [<ffffffff8107e54a>] warn_slowpath_fmt+0x4a/0x50
[  231.800491]  [<ffffffff811d12ac>] ? kmem_cache_free+0x2dc/0x340
[  231.800520]  [<ffffffffa009ef36>] __i915_gem_free_objects+0x326/0x3c0 [i915]
[  231.800548]  [<ffffffffa009effe>] __i915_gem_free_work+0x2e/0x50 [i915]
[  231.800552]  [<ffffffff8109c27c>] process_one_work+0x1ec/0x6b0
[  231.800555]  [<ffffffff8109c1f6>] ? process_one_work+0x166/0x6b0
[  231.800558]  [<ffffffff8109c789>] worker_thread+0x49/0x490
[  231.800561]  [<ffffffff8109c740>] ? process_one_work+0x6b0/0x6b0
[  231.800563]  [<ffffffff8109c740>] ? process_one_work+0x6b0/0x6b0
[  231.800566]  [<ffffffff810a2aab>] kthread+0xeb/0x110
[  231.800569]  [<ffffffff810a29c0>] ? kthread_park+0x60/0x60
[  231.800573]  [<ffffffff818164a7>] ret_from_fork+0x27/0x40

Moving to a separate flag for tracking the quirked pin is overkill for
the bug (since we only have to interchange the two tests in
i915_gem_free_object) but it does reduce a complicated test on all
objects and provide a sanitycheck for uncommon code paths.

Fixes: fbbd37b36fa5 ("drm/i915: Move object release to a freelist + worker")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20161101100317.11129-2-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 51360d199263a6b23dcff570f96bc12a89466f56..539106a9c1affcd83761d49ce6d81d8fa7d96556 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2304,6 +2304,12 @@ struct drm_i915_gem_object {
                 * pages were last acquired.
                 */
                bool dirty:1;
+
+               /**
+                * This is set if the object has been pinned due to unknown
+                * swizzling.
+                */
+               bool quirked:1;
        } mm;
 
        /** Breadcrumb of last rendering to the buffer.

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6c51b21565d6e498e6f8487cf5ff7845664fe6be..c9e52f75e1cbeb4ce6ad93a2c9675517a4c3633a 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2324,8 +2324,10 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
                i915_gem_object_do_bit_17_swizzle(obj, st);
 
        if (i915_gem_object_is_tiled(obj) &&
-           dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES)
+           dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
                __i915_gem_object_pin_pages(obj);
+               obj->mm.quirked = true;
+       }
 
        return st;
 
@@ -4091,10 +4093,15 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
        if (obj->mm.pages &&
            i915_gem_object_is_tiled(obj) &&
            dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
-               if (obj->mm.madv == I915_MADV_WILLNEED)
+               if (obj->mm.madv == I915_MADV_WILLNEED) {
+                       GEM_BUG_ON(!obj->mm.quirked);
                        __i915_gem_object_unpin_pages(obj);
-               if (args->madv == I915_MADV_WILLNEED)
+                       obj->mm.quirked = false;
+               }
+               if (args->madv == I915_MADV_WILLNEED) {
                        __i915_gem_object_pin_pages(obj);
+                       obj->mm.quirked = true;
+               }
        }
 
        if (obj->mm.madv != __I915_MADV_PURGED)
@@ -4335,14 +4342,12 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
 {
        struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
 
+       if (obj->mm.quirked)
+               __i915_gem_object_unpin_pages(obj);
+
        if (discard_backing_storage(obj))
                obj->mm.madv = I915_MADV_DONTNEED;
 
-       if (obj->mm.pages && obj->mm.madv == I915_MADV_WILLNEED &&
-           to_i915(obj->base.dev)->quirks & QUIRK_PIN_SWIZZLED_PAGES &&
-           i915_gem_object_is_tiled(obj))
-               __i915_gem_object_unpin_pages(obj);
-
        /* Before we free the object, make sure any pure RCU-only
         * read-side critical sections are complete, e.g.
         * i915_gem_busy_ioctl(). For the corresponding synchronized

diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 6395e62bd9e49655a231bd696e28fdd032b80d7c..1577e7810cd610555ee4faffc0c83664f0869b4e 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -263,10 +263,15 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
                        if (obj->mm.pages &&
                            obj->mm.madv == I915_MADV_WILLNEED &&
                            dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
-                               if (args->tiling_mode == I915_TILING_NONE)
+                               if (args->tiling_mode == I915_TILING_NONE) {
+                                       GEM_BUG_ON(!obj->mm.quirked);
                                        __i915_gem_object_unpin_pages(obj);
-                               if (!i915_gem_object_is_tiled(obj))
+                                       obj->mm.quirked = false;
+                               }
+                               if (!i915_gem_object_is_tiled(obj)) {
                                        __i915_gem_object_pin_pages(obj);
+                                       obj->mm.quirked = true;
+                               }
                        }
                        mutex_unlock(&obj->mm.lock);