[Intel-gfx] [PATCH 01/46] drm/i915: Return immediately if trylock fails for direct-reclaim

[Tvrtko Ursulin]

On 07/01/2019 14:07, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-07 14:00:25)
>>
>> On 07/01/2019 11:54, Chris Wilson wrote:
>>> Ignore trying to shrink from i915 if we fail to acquire the struct_mutex
>>> in the shrinker while performing direct-reclaim. The trade-off being
>>> (much) lower latency for non-i915 clients at an increased risk of being
>>> unable to obtain a page from direct-reclaim without hitting the
>>> oom-notifier. The proviso being that we still keep trying to hard
>>> obtain the lock for kswapd so that we can reap under heavy memory
>>> pressure.
>>>
>>> v2: Taint all mutexes taken within the shrinker with the struct_mutex
>>> subclass as an early warning system, and drop I915_SHRINK_ACTIVE from
>>> vmap to reduce the number of dangerous paths. We also have to drop
>>> I915_SHRINK_ACTIVE from oom-notifier to be able to make the same claim
>>> that ACTIVE is only used from outside context, which fits in with a
>>> longer strategy of avoiding stalls due to scanning active during
>>> shrinking.
>>
>> Oom notifier is not always the outside context?
> 
> oom-notifier is the final death throes of direct reclaim.
> 
>>> @@ -2255,8 +2256,7 @@ int i915_gem_gtt_prepare_pages(struct drm_i915_gem_object *obj,
>>>        } while (i915_gem_shrink(to_i915(obj->base.dev),
>>>                                 obj->base.size >> PAGE_SHIFT, NULL,
>>>                                 I915_SHRINK_BOUND |
>>> -                              I915_SHRINK_UNBOUND |
>>> -                              I915_SHRINK_ACTIVE));
>>> +                              I915_SHRINK_UNBOUND));
>>>    
>>
>> Why this change?
> 
> gtt_prepare_pages is called at the end of obj->ops->get_pages() which is
> not guaranteed to be outside of struct_mutex (yet). So I dropped the
> ACTIVE here as part of the claim that ACTIVE is only used from kswapd (or
> at known idle times such as i915_gem_freeze where I can handwave that
> ACTIVE is meaningless).

Okay,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko