Re: [PATCH v3 03/37] drm/i915/region: support basic eviction

Chris Wilson
2019-08-10T10:18:31Z

Quoting Matthew Auld (2019-08-09 23:26:09)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6ff01a404346..8735dea74809 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1105,6 +1105,23 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
> !i915_gem_object_has_pages(obj))
> i915_gem_object_truncate(obj);
>
> + if (obj->mm.region) {
> + mutex_lock(&obj->mm.region->obj_lock);
> +
> + switch (obj->mm.madv) {
> + case I915_MADV_WILLNEED:
> + list_move(&obj->mm.region_link,
> + &obj->mm.region->objects);
> + break;
> + default:
> + list_move(&obj->mm.region_link,
> + &obj->mm.region->purgeable);
> + break;
> + }
> +
> + mutex_unlock(&obj->mm.region->obj_lock);
> + }
> +
> args->retained = obj->mm.madv != __I915_MADV_PURGED;

Little bit of an impedance mismatch, I hope this turns out fine when
everything is a memory region.

> mutex_unlock(&obj->mm.lock);
>
> diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
> index ef12e462acb8..3a3caaadea1f 100644
> --- a/drivers/gpu/drm/i915/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/intel_memory_region.c
> @@ -12,6 +12,51 @@ const u32 intel_region_map[] = {
> [INTEL_MEMORY_STOLEN] = BIT(INTEL_STOLEN + INTEL_MEMORY_TYPE_SHIFT) | BIT(0),
> };
>
> +static int
> +intel_memory_region_evict(struct intel_memory_region *mem,
> + resource_size_t target,
> + unsigned int flags)
> +{
> + struct drm_i915_gem_object *obj;
> + resource_size_t found;
> + int err;
> +
> + err = 0;
> + found = 0;
> +
> + mutex_lock(&mem->obj_lock);
> + list_for_each_entry(obj, &mem->purgeable, mm.region_link) {
> + if (!i915_gem_object_has_pages(obj))
> + continue;
> +
> + if (READ_ONCE(obj->pin_global))
> + continue;
> +
> + if (atomic_read(&obj->bind_count))
> + continue;
> +
> + mutex_unlock(&mem->obj_lock);
> +
> + __i915_gem_object_put_pages(obj, I915_MM_SHRINKER);

So we don't really care about the object being bound then? As all we
care about is the page's pin_count.

So instead of obj->pin_global, obj->bind_bound, you just want

if (atomic_read(&obj->pages.pin_count))
continue;

as the quick check to see if it is worth preceding.

> + mutex_lock_nested(&obj->mm.lock, I915_MM_SHRINKER);
> + if (!i915_gem_object_has_pages(obj)) {
> + obj->mm.madv = __I915_MADV_PURGED;
> + found += obj->base.size;
> + }
> + mutex_unlock(&obj->mm.lock);

The locking here accomplishes what? You just want a boolean from
put_pages().

> +
> + if (found >= target)
> + return 0;
> +
> + mutex_lock(&mem->obj_lock);
> + }
> +
> + err = -ENOSPC;
> + mutex_unlock(&mem->obj_lock);
> + return err;
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org