> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf > Of Chris Wilson > Sent: Saturday, August 10, 2019 3:19 AM > To: Auld, Matthew <matthew.auld@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v3 03/37] drm/i915/region: support basic > eviction > > 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(). I have the same question. But looked the i915_gem_shrink() function, it has similar code. Do we prevent any race condition here? I want to use this function for swapping so hope to understand more. --CQ > > > + > > + 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
