Quoting Joonas Lahtinen (2017-08-10 17:22:49)
> On ke, 2017-07-26 at 14:26 +0100, Chris Wilson wrote:
> > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > @@ -92,9 +92,6 @@ static bool swap_available(void)
> >
> > static bool can_release_pages(struct drm_i915_gem_object *obj)
> > {
> > - if (!i915_gem_object_has_pages(obj))
> > - return false;
>
> So you think the inaccuracies we get for being lockless don't matter
> compared to better forward progress?
Actually, looking at this again, it is all improvement. We no longer
have a scan over the inactive/active list here, just the bound/unbound
list, so the locking is equivalent. And we gain better accounting going
in (no longer do we have an untracked phase for objects that have pages
prior to ever seeing the struct_mutex and being placed on the unbound_list.
We still have an unpleasant walk over many 10s of thousands of objects.
Hmm, I wonder if sticking a cond_resched_lock() in there is sensible.
Probably.
> Maybe the cons should be documented in the commit message to help in
> bisecting.
>
> > @@ -149,8 +149,6 @@ static int igt_overcommit(void *arg)
> > goto cleanup;
> > }
> >
> > - list_move(&obj->global_link, &i915->mm.unbound_list);
>
> Accidentally dropped line?
No, a big change was that in dropping the struct_mutex requirement for
the unbound_list, we could do the list_add in i915_gem_object_init() and
so not need this.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
