Re: [PATCH] drm/i915: Migrate stolen objects before hibernation

Chris Wilson
2015-06-30T11:20:28Z

On Tue, Jun 30, 2015 at 12:54:02PM +0200, Daniel Vetter wrote:
> > + list_add(&obj->stolen_link, &to_i915(dev)->mm.stolen_list);
> > +
> > + /* By default, treat the contexts of stolen as volatile. If the object
> > + * must be saved across hibernation, then the caller must take
> > + * action and flag it as WILLNEED.
> > + */
> > + obj->madv = I915_MADV_DONTNEED;
>
> Won't this interfere with autoreclaim of stolen objects (to make room for
> users which really need it like fbc) which are still in use by userspace?
> I think we need a new madv flag for "REAP_ON_HIBERNATE" or something
> similar. Otherwise this is way too surprising for userspace.

I didn't like this much either, and even in the hibernate loop, we
confuse userspace madv with internal objects. I went with a new

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ec10f389886e..c8ea71713ab8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2064,6 +2064,12 @@ struct drm_i915_gem_object {
* Advice: are the backing pages purgeable?
*/
unsigned int madv:2;
+ /**
+ * Whereas madv is for userspace, there are certain situations
+ * where we want I915_MADV_DONTNEED behaviour on internal objects
+ * without conflating the userspace setting.
+ */
+ unsigned int nonvolatile:1;

(I don't like the double negative, but it's better than int _volatile:1;)

So that we could easily distinguish the internal objects from userspace.